search

My-Little-Forum / mylittleforum

A simple PHP and MySQL based internet forum that displays the messages in classical threaded view (tree structure)
GNU General Public License v3.0
124 stars 48 forks source link

Further case of ordering a result by a column, that was not in the select #731

Closed auge8472 closed 3 months ago

auge8472 commented 3 months ago

When ordering a result of a database request, that is selected with DISTINCT, the column by which it should be sorted by must be part of the SELECT. This case comes up, when one wants to reorder the threads on the main overview page.

Expression #2 of ORDER BY clause is not in SELECT list, references column '[database].ft.last_reply' which is not in SELECT list; this is incompatible with DISTINCT
joeiacoponi1 commented 3 months ago

I just tried this on my recent upgrade to 20240729.1, and it worked correctly with no errors. I'm running MariaDB 11.

So, I did some digging. This error is due to a setting in both MySQL and MariaDB called "ONLY_FULL_GROUP_BY". If enabled, the restriction / error you are experiencing is encountered. Up until at least recently, the default is Enabled, meaning the error will occur / be displayed.

My version of MariaDB is a recent (past year) install, and see that my implementation has this Disabled. I did not change any settings, so either MariaDB's default has been changed or it is due to my host provider removing / changing this setting by default. Either way, we clearly need to code SQL as if this restriction is in place.

Let me take a look.

joeiacoponi1 commented 3 months ago

OK, adding ", ft.last_reply" to the same SELECT DISTINCT list (Line 88) should resolve that as well. I tested it on my instance and nothing broke, so that is a start at least.

$display_page_threads = "SELECT DISTINCT ft.tid, ft.sticky, ft.time, ft.last_reply FROM ".$db_settings['forum_table']." AS ft

I also see that I do not have Admin privileges to change this setting / test it out. It is apparently locked down by our provider, so hopefully you can confirm easily enough.

auge8472 commented 3 months ago

OK, adding ", ft.last_reply" to the same SELECT DISTINCT list (Line 88) should resolve that as well. I tested it on my instance and nothing broke, so that is a start at least.

That's what I did (pull request will follow).

I also see that I do not have Admin privileges to change this setting / test it out. It is apparently locked down by our provider, so hopefully you can confirm easily enough.

I suspect this to be the default behaviour on most not self administrated servers/webspaces. As you wrote:

Either way, we clearly need to code SQL as if this restriction is in place.

auge8472 commented 3 months ago

This error is due to a setting in both MySQL and MariaDB called "ONLY_FULL_GROUP_BY". If enabled, the restriction / error you are experiencing is encountered.

It was not clear to me in a first sight, that DISTINCT can have the restrictions of GROUP BY. But it is actually comparable because selecting one row of a identical group of rows is like grouping and melting the lines of the group into one resulting line. Furthermore, a similar restriction (all columns, that get selected in a SELECT with a GROUP BY, have to take part in the GROUP BY) is mandatory in example on a MS SQL Server.

Anyway, the pull request (#732) is ready unless someone else finds another SQL error within a short time.