LEFT JOIN + default orderBy = bug

Hi Rob, 

Thanks for reporting this.

So is the SQL generated by the previous and current versions of CMSB the same?  And if so do you know if the MySQL/MariaDB might have been updated at the same time as you updated CMSB?

I'm thinking either there's been some changes in MySQL/MariaDB that has them be more strict about requiring fully qualified table.column names in the ORDER BY for joins, or maybe some of the more stricter MySQL settings we're using are causing the error.  I've been looking into that and haven't found anything yet though.  

Do you know if you have a server that's using the same version of MySQL/MariaDB but on the older CMSB that's still working?  Or can you replicate the working condition on the same server with a temporary old cmsb install?  

We'd like to try and isolate the issue and/or find a workaround.

Thanks!

Hey Dave

So is the SQL generated by the previous and current versions of CMSB the same?

Correct, which makes me think as well that it's MySQL config related.

And if so do you know if the MySQL/MariaDB might have been updated at the same time as you updated CMSB?

MySQL instance is unchanged. MySQL is running in it's own docker container which is untouched by the CMSB upgrade.

Do you know if you have a server that's using the same version of MySQL/MariaDB but on the older CMSB that's still working?  Or can you replicate the working condition on the same server with a temporary old cmsb install?  

Yes, LEFT JOINS work correctly if I switch back to the old CMSB version. I have the CMSB upgrade on a new branch so easy to switch back, and with all other environmental factors the same:

Old CMSB codebase: LEFT JOINs work

New CMSB codebase: LEFT JOINs result in ambiguous ORDER BY

I'm thinking either there's been some changes in MySQL/MariaDB that has them be more strict about requiring fully qualified table.column names in the ORDER BY for joins, or maybe some of the more stricter MySQL settings we're using are causing the error. 

I agree :-) I think it's more likely related to the stricter MySQL settings the new CMSB is using, as I've had no changes to my actual MySQL instance.

Are you able to replicate? If not, then I wonder if we need to compare MySQL server settings?

I'll do some more digging at my end too, but also happy to run some tests if you can point in whatever direction.

Cheers

Rob

Sorry for the barrage of posts, but this fixes it:

viewer_functions.php, line 568

Was:

  // create query
    $query = "SELECT COUNT(*)\n";
    $query .= "FROM `$TABLE_PREFIX{$options['tableName']}` as `{$options['tableName']}`\n";
    $query .= $LEFT_JOIN;
    $query .= "$where\n";
    $query .= (!empty($options['groupBy'])) ? " GROUP BY {$options['groupBy']}" : '';
    $query .= (!empty($options['having']))  ? " HAVING {$options['having']}" : '';
    $query .= (!empty($options['orderBy'])) ? " ORDER BY {$options['orderBy']}" : '';

Now:

// create query
 $query = "SELECT COUNT(*)\n";
    $query .= "FROM `$TABLE_PREFIX{$options['tableName']}` as `{$options['tableName']}`\n";
    $query .= $LEFT_JOIN;
    $query .= "$where\n";
    $query .= (!empty($options['groupBy'])) ? " GROUP BY {$options['tableName']} . {$options['groupBy']}" : '';
    $query .= (!empty($options['having']))  ? " HAVING {$options['having']}" : '';
    $query .= (!empty($options['orderBy'])) ? " ORDER BY {$options['tableName']} . {$options['orderBy']}" : '';

But this then can cause another error when a custom orderBy is defined already with a fully qualified field.

So I scrapped that idea and then I thought... arguably, SELECT COUNT doesn't need an orderBy as it will make no difference to the results of the COUNT, so just removed it entirely from this SELECT COUNT query.

groupBy can cause this same ambiguous error but removing this from the COUNT does cause different results so this may need a better fix. For now, I have fixed this by fully qualifying it in the getRecords as I only have a few places.

I have not tested having as I have never used that.

Hi Rob, 

Great debugging work!  

Yea, we needed to replace FOUND_ROWS() because it was deprecated in MySQL 8.

I think that given the purpose of the function is solely to get a record count we can safely remove ORDER BY.  Each of the the HAVING or GROUP BY conditions can alter the count but the order shouldn't matter in this case.

Can you try commenting out this line at the bottom of /lib/viewer_functions.php:_getRecords_getCountQuery()

//$query .= (!empty($options['orderBy'])) ? " ORDER BY {$options['orderBy']}" : '';

Let me know if that works for you and we'll include it in the next beta release.

Thanks again for all the info on this.  We were chasing this one for a while.