Stop explicitly setting sql_auto_is_null #44739
Conversation
|
Even if the default value of So I think it needs to check the current setting of sql_auto_is_null first as follows. variables["sql_auto_is_null"] = 0 unless query_value('select @@sql_auto_is_null') == 0 |
|
@yahonda my understanding from the issue is that setting the variable at all causes RDS to change its connection behavior regardless of the value it is set to. If you know a way to unset a variable, we can recommend that instead, though I am not sure how valuable keeping this would be even in that case since it is the default value on all rails supported MySQL versions. |
|
Oh, no I understand sorry, you are saying if the variable was set manually we still need to override it if they did, but only if they did. Thanks! I like that idea. I'll add that right away! |
HParker
force-pushed
the
stop-explicitly-setting-sql_auto_is_null
branch
from
9aef40e
to
66cb4a3
Compare
Yes. I mean to say that. I do not know how many users set |
|
Hmm, I am confused by this error. Maybe since we are still setting up the connection we can't query this value yet? |
activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
Show resolved
Hide resolved
| @@ -787,8 +787,8 @@ def supports_rename_column? | |||
| def configure_connection | |||
| variables = @config.fetch(:variables, {}).stringify_keys | |||
|
|
|||
| # By default, MySQL 'where id is null' selects the last inserted id; Turn this off. | |||
| variables["sql_auto_is_null"] = 0 | |||
| # Since MySQL 5.5 sql_auto_is_null is off by default, but if it is set for any reason, we need to turn it off. | |||
There was a problem hiding this comment.
do we really need force it off?
is it to semi-preserve existing behavior?
seems to me appropriate that rails not override an explicit setting by a user saying he wants to get the last item written if he queries for id is null
There was a problem hiding this comment.
Yeah, I believe for most users where id is null returning the last inserted record would be surprising behavior.
HParker
changed the title
Since version 5.5 the default has been off by default, this change explicitly sets the variable if it is already set.
HParker
force-pushed
the
stop-explicitly-setting-sql_auto_is_null
branch
from
a89b2d2
to
9e87228
Compare
|
Updated and rebased back to one commit just in case. |
| # By default, MySQL 'where id is null' selects the last inserted id; Turn this off. | ||
| variables["sql_auto_is_null"] = 0 | ||
| # Since MySQL 5.5 sql_auto_is_null is off by default, but if it is set for any reason, we need to turn it off. | ||
| variables["sql_auto_is_null"] = 0 unless query_value("select @@sql_auto_is_null") == 0 |
There was a problem hiding this comment.
I get that people may have flipped this config, but honestly I think it's unlikely, and for the very rare people who did that, we should probably recommend to add this to their database.yml.
I'd rather make it a bit harder to config for these theoretical people, than add an extra query on connect for everyone.
|
I agree with @byroot that adding a query probably isn't worth it. I also think I don't know enough about this theoretical RDS Proxy user to know what decisions make sense for them. I'll close, but I encourage @xiankaing or anyone who ran into this issue and has more knowledge to open a similar PR. |
|
@HParker I was suggesting to remove |
|
Cool, I am happy with that solution as well. I force pushed so GitHub is requiring me to make a new PR: #45134 |
Fixes: #44667
Since version 5.5 the default has been off which is the minimum supported mysql version.