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

I've done some more digging and I think it's related to the COUNT which is performed as part of getRecords.

This works:

SELECT `primary_table`.*,
`secondary_table`.`num` AS `secondary_table.num`,
`secondary_table`.`createdDate` AS `secondary_table.createdDate`
FROM `primary_table` as `records`
LEFT JOIN `secondary_table` AS `secondary_table` ON (primary_table.num = secondary_table.linked_field)
ORDER BY createdDate

This doesn't work:

SELECT COUNT(*)
FROM `primary_table` as `records`
LEFT JOIN `secondary_table` AS `secondary_table` ON (primary_table.num = secondary_table.linked_field)
ORDER BY createdDate

and results in the error

column createdDate in order clause is ambiguous

And if I remove the COUNT entirely for testing

// comment out for testing
// $totalRecords = mysql_get_query($countQuery, true)[0];

from /lib/viewer_functions.php line 612, I do not get the error :-)

The previous CMS used the following for $totalRecords within function_getRecords_loadResults:

SELECT FOUND_ROWS()

The new CMS uses the following:

SELECT COUNT(*) FROM 

Previously it was querying the found rows from the original MySQL select query to get the count. Now, it's running its own separate COUNT query.

Looking at the full MySQL statements above, in the first SELECT, createdDate for the secondary_table is defined AS secondary_table.createdDate, removing the ambiguity. In the second SELECT COUNT, it is not defining each secondary_table field AS, and this is likely the cause for the ambiguity.

function _getRecords_getQuery includes // add fieldnames to SELECT resulting in the x AS y in the SELECT. (viewer_functions.php L398)

function _getRecords_getCountQuery however doesn't include this full SELECT AS

Has SELECT FOUND_ROWS() been replaced by SELECT COUNT(*) for efficiency reasons?

FYI all line numbers in ^^ are based on CMSB v3.65.

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.