Skip to content

Fix insert_all/upsert_all for single character, temporary and three part table names - #1403

Closed
Alt70155 wants to merge 1 commit into
rails-sqlserver:mainfrom
Alt70155:fix/merge-into-aliased-target-table-name
Closed

Fix insert_all/upsert_all for single character, temporary and three part table names#1403
Alt70155 wants to merge 1 commit into
rails-sqlserver:mainfrom
Alt70155:fix/merge-into-aliased-target-table-name

Conversation

@Alt70155

@Alt70155 Alt70155 commented Sep 2, 2026

Copy link
Copy Markdown

Fixes #1402.

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 (and insert_all when it takes the MERGE path) raises
NoMethodError when the MERGE table name pattern cannot match the target, and
returns 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, UPDATE and FROM already handle three part names; only MERGE was
limited 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:

MERGE_TARGET_IDENTIFIER = /(?:\[[^\]]+\]|[a-z0-9_-]+)/i # :nodoc:
MERGE_TARGET_TABLE_NAME = /\A\s*MERGE\s+INTO\s+(#{MERGE_TARGET_IDENTIFIER}(?:\s*\.#{MERGE_TARGET_IDENTIFIER}){0,2})\s+(?:AS|WITH|USING)/i # :nodoc:

Notes on the choices:

  • A bracket quoted segment is \[[^\]]+\], so spaces inside brackets are kept
    ([WITH - SPACES]) while a space is no longer allowed in a bare name. That is
    what stops an aliased target being captured as part of the table name.
  • The dot separator is \s*\., not \., so MERGE INTO dbo .products keeps
    working. 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 existing
    MERGE statements block (5 -> 13): bracket quoted and bare names, a single
    character name, a name with spaces inside brackets, two and three part names,
    with and without a WITH (UPDLOCK, HOLDLOCK) hint, and the space before the
    dot case.
  • test/cases/adapter_test_sqlserver.rb — one more assertion in the
    query_requires_identity_insert? test, using the generated MERGE with the
    table hint removed so the aliased target is exercised.

Without the change these fail with 3 failures and 2 errors; with it the file is
green.

schema_test_sqlserver.rb     32 runs, 42 assertions, 0 failures, 0 errors
adapter_test_sqlserver.rb    83 runs, 187 assertions, 0 failures, 0 errors, 1 skip
insert_all_test_sqlserver.rb  1 runs,  1 assertions, 0 failures, 0 errors

bundle exec standardrb is clean for the whole repository.

Branches

The same pattern is present on main, 8-1-stable and 8-0-stable. This PR
targets main. Happy to open backport PRs for the stable branches if you would
like them.

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.
@aidanharan

Copy link
Copy Markdown
Contributor

@Alt70155 Could you create a PR against the 8-1-stable branch instead of main? The tests aren't passing on main at the moment so cannot tell if your changes introduce any regressions. Thanks

@Alt70155

Alt70155 commented Sep 3, 2026

Copy link
Copy Markdown
Author

Thanks for taking a look. Opened #1405 against 8-1-stable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

upsert_all raises NoMethodError when the table name is a single character

2 participants