Unify dictionary queries on plain select_value(s) with bind variables#2652
Merged
Conversation
Extends the bind-variable pattern introduced in rsim#2629 to the remaining all_* / user_* dictionary queries that were still routing through the `select_values_forcing_binds` / `select_value_forcing_binds` helpers. After this commit every dictionary query in `OracleEnhancedAdapter` calls plain `select_values(sql, name, binds)` / `select_value(sql, name, binds)` with a `:placeholder` raw SQL and an explicit `binds` array — one consistent shape across the file. As a consequence, the two helpers no longer have any callers and are removed. Methods touched (`oracle_enhanced_adapter.rb`): * identity_primary_key? * trigger_backed_primary_key? * pk_and_sequence_for (×2 callsites — sequence lookup and PK lookup) * primary_keys * temporary_table? The `_forcing_binds` helpers were added in rsim#2125 (2021) to make dictionary queries use bind variables even inside an `unprepared_statement do ... end` block, by temporarily removing the connection from `prepared_statements_disabled_cache` so AR's visitor wouldn't inline the binds. Two recent changes made that mechanism unnecessary: 1. rsim#2629 finished converting the remaining literal-interpolated dictionary queries to raw SQL with `:placeholder` markers and explicit `binds` arrays. AR's visitor isn't involved on the raw-SQL-with-binds path, so there is nothing to inline. 2. rsim#2651 made `OracleEnhanced::DatabaseStatements#perform_query` honor `prepared_statements?` directly. Inside an `unprepared_statement` block, dictionary queries now prepare-bind-exec-close one-shot instead of caching the cursor in `@statements`, but bind values still flow to the server. SQL-text matching keeps the shared cursor reusable in the database's shared pool — the original server-side cursor-sharing goal of rsim#2125 is preserved without the helper. The helpers were defined under `private` and were not part of the documented adapter API.
41fc5ca to
905d11c
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Extends the bind-variable pattern introduced in #2629 to the remaining
all_*/user_*dictionary queries that were still routing through theselect_values_forcing_binds/select_value_forcing_bindshelpers. After this PR every dictionary query inOracleEnhancedAdaptercalls plainselect_values(sql, name, binds)/select_value(sql, name, binds)with a:placeholderraw SQL and an explicitbindsarray — one consistent shape across the file.As a result, the two helpers
select_values_forcing_bindsandselect_value_forcing_bindsno longer have any callers and are removed.identity_primary_key?select_values_forcing_bindsselect_valuestrigger_backed_primary_key?select_values_forcing_bindsselect_valuespk_and_sequence_for(×2)select_values_forcing_bindsselect_valuesprimary_keysselect_values_forcing_bindsselect_valuestemporary_table?select_value_forcing_bindsselect_valueprivate)Diff:
+6 / -22 lines, 1 file.Why the unification is safe now
The
_forcing_bindshelpers were added in #2125 (2021) to make dictionary queries use bind variables even inside anunprepared_statement do ... endblock, by temporarily removing the connection fromprepared_statements_disabled_cacheso AR's visitor wouldn't inline the binds.Two recent changes made that mechanism unnecessary:
:placeholdermarkers and explicitbindsarrays. AR's visitor isn't involved on the raw-SQL-with-binds path, so there is nothing to inline — theprepared_statements_disabled_cachetrick had nothing to defend against on that path.OracleEnhanced::DatabaseStatements#perform_queryhonorprepared_statements?directly. Inside anunprepared_statementblock, dictionary queries now prepare-bind-exec-close one-shot instead of caching the cursor in@statements, but bind values still flow to the server. SQL-text matching keeps the shared cursor reusable in the database's shared pool — the original server-side cursor-sharing goal of Usage of bind variables for volatile filter conditions (3) #2125 is preserved without the helper.The helpers were defined under
privateand were not part of the documented adapter API.Test plan
prepared_statements: trueworkflow)master test_prepared_statements_falseworkflow green after mergeprepared_statements: true→ 613 examples, 0 failures, 7 pendingprepared_statements: false→ 613 examples, 0 failures, 8 pendingRelated
prepared_statements?inperform_query), Respect the database's CURSOR_SHARING setting by default #2626 (respect DB'scursor_sharing).