Fix insert_all/upsert_all for single character, temporary and three part table names - #1405
Conversation
The MERGE branch of get_raw_table_name has two identifier slots and both are `+`, so each needs to consume at least one character. `[products]` matches because the engine can split it across the slots (`produc` + `ts`), but `[x]` cannot be split and the match fails, so `nil[1]` raises NoMethodError. The same happens for any name whose characters are not in `[a-z0-9_ -]`, which covers temporary tables, non-ASCII names, names containing `$`, and three part (cross database) names. Because the character class also allows a space and the second slot is reachable without a preceding dot, an aliased target is swallowed into the captured name: # MERGE INTO [products] AS target USING ... get_raw_table_name(sql) # => "[products] AS target" That case is not reachable from build_sql_for_merge_insert, which always emits `WITH (UPDLOCK, HOLDLOCK) AS target` and so terminates the match early, but it is reachable from hand written MERGE passed to execute. Replace the pattern with an explicit identifier that matches either a bracket quoted or a bare name, joined by up to two dots, so one, two and three part names are all handled. INSERT, UPDATE and FROM already handle three part names; only MERGE was limited to two. `MERGE INTO dbo .products` (a space before the dot) keeps working, since the separator allows surrounding whitespace. Fixes rails-sqlserver#1402.
|
The
With four STI types the bind numbers shift, which matches the failing query exactly: This PR only changes the |
Fixes #1402.
Replaces #1403, which targeted
main.I used AI assistance to narrow down the cause and to check the patch for
regressions, but everything here was verified by running the reproduction script
and the adapter's test suite against a local SQL Server 2019.
upsert_all(andinsert_allwhen it takes theMERGEpath) raisesNoMethodErrorwhen theMERGEtable name pattern cannot match the target, andreturns a wrong name when the target carries an alias. See #1402 for the full
analysis of why the two identifier slots behave that way.
INSERT,UPDATEandFROMalready handle three part names; onlyMERGEwaslimited to two.
Fix
Replace the pattern with an explicit identifier that matches either a bracket
quoted or a bare name, joined by up to two dots:
Notes on the choices:
\[[^\]]+\], so spaces inside brackets are kept(
[WITH - SPACES]) while a space is no longer allowed in a bare name. That iswhat stops an aliased target being captured as part of the table name.
\s*\., not\., soMERGE INTO dbo .productskeepsworking. It parses today and I did not want to change that.
{0,2}allows one, two and three part names, matching the other branches.Tests
test/cases/schema_test_sqlserver.rb— 8 cases added to the existingMERGE statementsblock (5 -> 13): bracket quoted and bare names, a singlecharacter name, a name with spaces inside brackets, two and three part names,
with and without a
WITH (UPDLOCK, HOLDLOCK)hint, and the space before thedot case.
test/cases/adapter_test_sqlserver.rb— one more assertion in thequery_requires_identity_insert?test, using the generatedMERGEwith thetable hint removed so the aliased target is exercised.
Run against Rails 8.1.3.1 and SQL Server 2019.
Without the change:
With the change:
bundle exec standardrbis clean for the whole repository.Branches
Targeting
8-1-stableas requested in #1403.The same pattern is present on
mainand8-0-stable. The change appliescleanly to both, and
test/cases/schema_test_sqlserver.rbis identical acrossthe three branches. Happy to open follow up PRs for either once you are happy
with this one.