Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SQL query fails in region.php #328

Closed
jteresco opened this issue Jul 14, 2019 · 13 comments
Closed

SQL query fails in region.php #328

jteresco opened this issue Jul 14, 2019 · 13 comments
Labels
bug SQL Userpages user/{index.php,region.php,system.php,topstats.php}

Comments

@jteresco
Copy link
Contributor

Related to #325, decided to open a new Issue rather than discussing in the closed PR.

Starting at http://tmtest.teresco.org/user/?units=miles&u=terescoj, I click on a region (say, NY), to get to http://tmtest.teresco.org/user/region.php?units=miles&u=terescoj&rg=NY, and I get an SQL error:

Query failed: SELECT traveler, COUNT(cr.route) AS driven, SUM(cr.clinched) AS clinched, ROUND(COUNT(cr.route) / 578 * 100, 2) as drivenPct, ROUND(sum(cr.clinched) / 578 * 100, 2) as clinchedPct, RANK() OVER (ORDER BY COUNT(cr.route) DESC) drivenRank, RANK() OVER (ORDER BY SUM(cr.clinched) DESC) clinchedRank FROM routes AS r LEFT JOIN clinchedRoutes AS cr ON cr.route = r.root LEFT JOIN systems ON r.systemName = systems.systemName WHERE (r.region = 'NY' AND systems.level = 'active') GROUP BY traveler;

Running that query interactively in mysql on noreaster gives this error:

ERROR 1064 (42000): You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'Query failed: SELECT traveler, COUNT(cr.route) AS driven, SUM(cr.clinched) AS cl' at line 1

@jteresco jteresco added bug Userpages user/{index.php,region.php,system.php,topstats.php} SQL labels Jul 14, 2019
@jteresco
Copy link
Contributor Author

I'm thinking the code installed on tmtest is missing some of the recent changes. Let me check it out. @yakra Is what's in GitHub now the same as what's on lab2 (where this works)?

@jteresco
Copy link
Contributor Author

Reinstalled GitHub latest onto tmtest and loaded into a fresh browser in case it was caching some code or something. Same problem. I suspect something important on lab2 didn't make it into GitHub yet.

@jteresco
Copy link
Contributor Author

Further investigation: on lab2, the same query is run but is successful. So it appears that we have some sort of DB difference.

noreaster is at MySQL Server version: 5.7.26-log. What's on lab2?

@jteresco
Copy link
Contributor Author

However, the fact that I pasted in a multiline SQL query from lab2's page source gives an error message with a better indication where noreaster's version becomes unhappy.

ERROR 1064 (42000): You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '(ORDER BY COUNT(cr.route) DESC) drivenRank, RANK() OVER (ORDER BY ' at line 7

@jteresco
Copy link
Contributor Author

And...now I know what's up. The RANK() function was not introduced until MySQL 8. So we either need a solution that doesn't use it, or need to upgrade MySQL on noreaster. I worry about that since it's used for a few other things beyond TM and our forum. But maybe this is the kick that will force me to do that.

@yakra
Copy link
Contributor

yakra commented Jul 14, 2019

A workaround could be to do 2 queries and 2 calls to tm_fetch_user_row_with_rank

@yakra
Copy link
Contributor

yakra commented Jul 15, 2019

MySQL 8 has binary logging enabled by default. This may be what slowed me down the most during my first attempts at ingesting the SQL file. If you do upgrade, this may be something to look out for.

@jteresco
Copy link
Contributor Author

jteresco commented Jul 15, 2019

If the workaround is not too bad, I think it's best to do that than try to do a major version upgrade on the DB.

@yakra
Copy link
Contributor

yakra commented Jul 15, 2019

region.php:
The query in the OP (multiline as above) takes 0.13-0.14 sec on lab2.
Bizarrely, if I remove the two rank() lines, it takes 0.17-0.21 sec.
Ran each query 10 times. I don't get it. Hail Eris!

Among the slower queries I've seen, but not completely awful.
DB changes for #91 may help here too.

system.php
if ($region == ""), system stats across all regions:

SELECT
    ccr.traveler,
    count(ccr.route) as driven,
    sum(ccr.clinched) as clinched,
    RANK() OVER (ORDER BY COUNT(ccr.route) DESC) drivenRank,
    RANK() OVER (ORDER BY SUM(ccr.clinched) DESC) clinchedRank
FROM connectedRoutes as cr
LEFT JOIN clinchedConnectedRoutes as ccr
ON cr.firstRoot = ccr.route
WHERE cr.systemName = 'usany'
GROUP BY traveler;

takes 0.10-0.12 sec.
Here, removing the rank() lines yields 0.09-0.10 sec. HA!

system stats for a single region:

SELECT
    ccr.traveler,
    count(ccr.route) as driven,
    sum(ccr.clinched) as clinched,
    RANK() OVER (ORDER BY COUNT(ccr.route) DESC) drivenRank,
    RANK() OVER (ORDER BY SUM(ccr.clinched) DESC) clinchedRank
FROM routes as cr
LEFT JOIN clinchedRoutes as ccr
ON cr.root = ccr.route
WHERE cr.region = 'NY' AND cr.systemName = 'usany'
GROUP BY ccr.traveler;

takes 0.06-0.08 sec, with no noticeable difference snipping rank().


region.php rounds percentages in the SQL query, while system.php rounds them itself when writing to the HTML table. We've got to round all this stuff anyway, but maybe we could save a little time but not putting all these values into the result table?
Back to the query in the OP, removing the two ROUND lines...
Saves ~0.01 sec. Still slower with RANK()ing. 😕


I googled difference between MySQL 5.7.26 and MySQL 8.
Some results I've not read yet:
https://blog.softhints.com/mysql-5-7-vs-mysql-8-do-you-need-to-upgrade-to-mysql-8/
https://www.eversql.com/mysql-5-7-vs-mysql-8-0-whats-new-in-mysql-8-0/
https://mysqlserverteam.com/upgrading-to-mysql-8-0-here-is-what-you-need-to-know/

@yakra
Copy link
Contributor

yakra commented Jul 17, 2019

@jteresco wrote:

can't easily go live until we have a fix for #328.

Reverted the changes locally. Working on a "Two Four Query" solution.

Near term, that can let us go live with recent changes.
Other enhancements as in #91 can be longer term.

@jteresco
Copy link
Contributor Author

Sounds good, thanks.

@yakra
Copy link
Contributor

yakra commented Jul 18, 2019

Mileage rank is also buggy.

http://travelmapping.net/user/region.php?u=yakra&rg=NY ranks me as # 42 in both the Active and the Active+Preview tables.
Down in the Travelers in Region NY table, I'm on the # 42 line for each. But looks can be deceiving: @dave1693 has 0.27 more active miles but is ranked below me.

On Lab2, This is differently sorted, and I'm on the # 43 line on the Active Systems table. But the rank value up top still has me at # 42, not # 43.

Why?

  • The SQL query is ORDER BY activePercentage DESC, instead of, say, activeClinched.
  • @dave1693 & I have the same activePercentage; we're incorrectly listed as tied. This is because activePercentage comes out of the SQL already rounded. We could not round it in the SQL query, instead having PHP round it when it's time to echo it to HTML.

Fixing either one of these bullet points should be sufficient to squash this bug.

@yakra
Copy link
Contributor

yakra commented Jul 18, 2019

we could save a little time but not putting all these values into the result table?

The result table receives a bunch of values whether rounded or not. Won't change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug SQL Userpages user/{index.php,region.php,system.php,topstats.php}
Projects
None yet
Development

No branches or pull requests

2 participants