sequencescape icon indicating copy to clipboard operation
sequencescape copied to clipboard

DPL-nnn Re-enable ONLY_FULL_GROUP_BY mode for MySQL connections (At least in dev)

Open JamesGlover opened this issue 2 years ago • 0 comments

Describe the Housekeeping We used to connect to MySQL in strict mode in development, which places certain constraints on queries, especially GROUP BY. (The ONLY_FULL_GROUP_BY mode is especially important here) https://dev.mysql.com/doc/refman/5.7/en/group-by-handling.html At some point this got disabled, possibly when the connection mode was switched to TRADITIONAL?.

However this results in the following:

If ONLY_FULL_GROUP_BY is disabled, a MySQL extension to the standard SQL use of GROUP BY permits the select list, HAVING condition, or ORDER BY list to refer to nonaggregated columns even if the columns are not functionally dependent on GROUP BY columns. This causes MySQL to accept the preceding query. In this case, the server is free to choose any value from each group, so unless they are the same, the values chosen are nondeterministic, which is probably not what you want. Furthermore, the selection of values from each group cannot be influenced by adding an ORDER BY clause. Result set sorting occurs after values have been chosen, and ORDER BY does not affect which value within each group the server chooses. Disabling ONLY_FULL_GROUP_BY is useful primarily when you know that, due to some property of the data, all values in each nonaggregated column not named in the GROUP BY are the same for each group.

This could make for some very tricky bugs, and we should probably re-instate strict mode in development at test.

Current connection parameters in devel:

ActiveRecord::Base.connection.execute('SELECT @@SESSION.sql_mode').first
=> ["STRICT_TRANS_TABLES,STRICT_ALL_TABLES,NO_ZERO_IN_DATE,NO_ZERO_DATE,ERROR_FOR_DIVISION_BY_ZERO,TRADITIONAL,NO_ENGINE_SUBSTITUTION"]

Current connection parameters in prod:

ActiveRecord::Base.connection.execute('SELECT @@SESSION.sql_mod
e').first
=> ["NO_AUTO_VALUE_ON_ZERO,STRICT_TRANS_TABLES,STRICT_ALL_TABLES"]

By contrast, the default connection parameters to my local MySQL (albeit 8) are:

ONLY_FULL_GROUP_BY,STRICT_TRANS_TABLES,NO_ZERO_IN_DATE,NO_ZERO_DATE,ERROR_FOR_DIVISION_BY_ZERO,NO_ENGINE_SUBSTITUTION

This is partly based on different server default. I think ideally we should migrate to using strict behaviours in production as well, but we'll need to be a bit cautious as there may be elements without test coverage. It probably makes sense to flip UAT for a while first to build confidence.

Blocking issues Describe any other issues or tickets that may be blocking this change.

Additional context AdapterConfiguration GROUP BY handling SQL modes

JamesGlover avatar Mar 25 '22 11:03 JamesGlover