Code review comments. (Development)

by Joe I, (141 days ago) @ Auge
edited by Joe I, ,

Hi Auge,

Some comments here, and still need to dig into the testing.

Line 40/41 Update
Looks very good. ORDER BY Should generally work either with or without the “ft.”, however including "ft.time" Should eliminate any possible issue here, as you have done. See Line 109 for more info.

Line 88 Update
I Think this looks good, but need to test numerous combinations. The original format should not have caused any issue according to SQL standards, however there have been some SQL / MySQL implementations that required any ORDER BY also be in the SELECT. I need to make sure adding ft.sticky and ft.time in the SELECT DISTINCT won't change the # of output rows.

Line 109 Update
Looks good. The issue may stem from unsafe use of the variable name “time” in the SQL SELECT (Line 109), as this is a Reserved word.

Line 113 Update
Looks fine. This could have also been left as “ft.time” but no harm done.

Line 249 Update
Thanks. for cleaning that up.

Some questions as well:

Q1: To avoid potential row result issues arising from the change in Line 88, I'd like to know if you did already try changing Line 40/41 without changing Line 88.

From your upgrade notes:
"Only when I called up the main forum page with the overview of threads, which was also empty at this time, the page displayed a database error. It concerned queries with ORDER-BY clauses in which columns are to be sorted that are not contained in the SELECT."
Q2: Do you know if this error occurred on both Line 88 and 109?
Q3: Can you confirm whether you're using MySQL or MariaDB? Trying to wrap my head around these, as I can't duplicate the issues on my existing system (MariaDB, with data). Still need to go through upgrade using an empty DB, but curious if there may be an additional complicating factor between MariaDB and MySQL.


Testing factors I need to look at (particularly for Line 88 and system not crashing)
Categories (categories disabled, categories enabled but no categories, categories enabled and exist)
Spam (no spam entries, spam entries with view enabled & disabled)
MariaDB vs MySQL. I only have MariaDB 11, so won't be able to test out MySQL 8.


Complete thread:

 RSS Feed of thread