Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make test_select_with_subquery_in_from_uses_original_table_name work with old SQLite3 #35375

Conversation

y-yagi
Copy link
Member

@y-yagi y-yagi commented Feb 23, 2019

It seems that the reason why the test_select_with_subquery_in_from_uses_original_table_name does not pass is that the return value of sqlite3_column_name() is wrong due to subquery flattening.

This seems to have been fixed with 3.20.0(https://sqlite.org/changes.html#version_3_20_0).
But CI uses the old version(maybe 3.11.0), I added DISTINCT to avoid optimization by subquery flattening.

Ref: https://sqlite.org/optoverview.html#flattening

Copy link
Member

@kamipo kamipo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you ❤️

relation = Comment.joins(:post).select(:id).order(:id)
subquery = Comment.from(Comment.all, Comment.quoted_table_name).joins(:post).select(:id).order(:id)
subquery = Comment.from(Comment.all.distinct, Comment.quoted_table_name).joins(:post).select(:id).order(:id)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you note about that like here? We can drop the workaround if older SQLite3 support is dropped in the future.

# Materialize subquery by adding distinct
# to work with MySQL 5.7.6 which sets optimizer_switch='derived_merge=on'
unless has_limit_or_offset_or_orders?(subselect)
core = subselect.cores.last
core.set_quantifier = Arel::Nodes::Distinct.new
end

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Make sense. I fixed.

…k with old SQLite3

It seems that the reason why the `test_select_with_subquery_in_from_uses_original_table_name`
does not pass is that the return value of `sqlite3_column_name()` is
wrong due to subquery flattening.

This seems to have been fixed with SQLite 3.20.0(https://sqlite.org/changes.html#version_3_20_0).
But CI uses the old version(maybe 3.11.0), I added `DISTINCT` to avoid
optimization by subquery flattening.

Ref: https://sqlite.org/optoverview.html#flattening
@y-yagi y-yagi force-pushed the fix_test_select_with_subquery_in_from_uses_original_table_name branch from 29c3588 to 619ae03 Compare February 23, 2019 04:37
@y-yagi
Copy link
Member Author

y-yagi commented Feb 23, 2019

@y-yagi y-yagi merged commit f3641b1 into rails:master Feb 23, 2019
@y-yagi y-yagi deleted the fix_test_select_with_subquery_in_from_uses_original_table_name branch February 23, 2019 05:33
kamipo pushed a commit that referenced this pull request Feb 23, 2019
…n_from_uses_original_table_name

Make `test_select_with_subquery_in_from_uses_original_table_name` work with old SQLite3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants