fix: type untyped binds by Ruby class instead of defaulting to INT64 - #8
Draft
zhaitz wants to merge 1 commit into
Draft
fix: type untyped binds by Ruby class instead of defaulting to INT64#8zhaitz wants to merge 1 commit into
zhaitz wants to merge 1 commit into
Conversation
Binds that come from raw SQL placeholders (`Arel.sql("name = ?", "abc")` on
7.1+, or `where("name = ?", "abc")` on Rails 8.1+) arrive as bare Ruby values
with no attached ActiveModel type. `to_types` only recognised query attributes,
Symbols and booleans, and declared everything else INT64, so a String bind was
rejected by Spanner with "Expected INT64". `to_params` always serialised such
binds through the Integer type as well.
Add `untyped_bind_type`, which maps String, true/false, Float, BigDecimal,
Time/DateTime and Date to the matching ActiveModel type, and use it from both
`to_types` and `to_params` so the declared type and serialised value agree.
Unrecognised values keep the previous behaviour (INT64, value sent as-is).
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
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.
Problem
When ActiveRecord hands the adapter a bind that came from a raw SQL placeholder (
where("name = ?", "abc")on Rails 8.1, orArel.sql("name = ?", "abc")on any Rails since 7.1), the value arrives as a bare Ruby object with no attached column type.to_typesonly knew how to type query attributes, Symbols, and booleans; everything else was declaredINT64. A String bind was therefore sent to Spanner labelled as an integer and rejected withInvalid value for bind parameter p1: Expected INT64.Fix
untyped_bind_type(value)maps a Ruby value's class to an ActiveModel type: String, true/false, Float, BigDecimal, Time/DateTime (via the adapter's own Spanner timestamp type), and Date.to_typesuses it in its else branch, so the declared Spanner type matches the value.to_paramsuses the same method to pick the serializer, so the encoded value matches the declared type. Previously it always serialised unknown binds through the Integer type.nil, which preserves the historical behaviour exactly: declaredINT64, value sent as-is. (An Integer type instance was deliberately not used as the fallback, since it would silently coerce e.g. Arrays toNULL.)ActiveModel::Type::Boolean; wire result is unchanged.Tests
test_untyped_binds_from_arel_sql_are_typed_by_ruby_classsends eight binds throughArel.sql(the seven typed kinds plus an Integer to guard the fallback) and asserts both the type code and encoded value the mock server receives. Passes on ActiveRecord 7.1 and 8.1; fails on both without the fix.test_where_with_positional_string_placeholderis the production shape,Singer.where("first_name = ?", "Alice"). Passes on 8.1, skips below 8.1 where Rails still inlines the literal, and fails on 8.1 without the fix.Full suite has the same result set with and without this change on both versions (the only other failures are pre-existing local
json3.x incompatibilities in the JSON tests).Net effect
Raw
?placeholders against Spanner work for strings, floats, decimals, timestamps, and dates. Theconnection.quoteandCAST(? AS STRING)workarounds in the api-backend become unnecessary once the fork revision is bumped.🤖 Generated with Claude Code