Quote column names in DuplicatedField update fragments#4237
Closed
MarkVDD wants to merge 2 commits intoJasperFx:masterfrom
Closed
Quote column names in DuplicatedField update fragments#4237MarkVDD wants to merge 2 commits intoJasperFx:masterfrom
MarkVDD wants to merge 2 commits intoJasperFx:masterfrom
Conversation
3 tasks
Member
|
@MarkVDD I've got the Weasel changes in. I'm working on the regression failures up above. Thanks for taking this on! |
jeremydmiller
added a commit
that referenced
this pull request
Apr 8, 2026
…agment Update all test expectations to include double-quoted column names, matching the change in DuplicatedField.UpdateSqlFragment() from PR #4237 that unconditionally quotes column names to handle PostgreSQL reserved keywords (e.g., "offset"). 26/26 DuplicatedFieldTests passing. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 task
Contributor
Author
All good. Just let me know if I can help ;) |
jeremydmiller
added a commit
that referenced
this pull request
Apr 8, 2026
…agment Update all test expectations to include double-quoted column names, matching the change in DuplicatedField.UpdateSqlFragment() from PR #4237 that unconditionally quotes column names to handle PostgreSQL reserved keywords (e.g., "offset"). 26/26 DuplicatedFieldTests passing. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Member
|
@MarkVDD I pulled your work in through a separate pull request w/ some annoying test fixes to go with your real fixes. Thank you! The Weasel release is coming too |
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
2 participants
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.
Description
The Problem
When adding a new
DuplicatedField(mapping a JSONB property to a dedicated column) where the property name is a PostgreSQL reserved keyword (e.g.,Offset), Marten fails during the auto-migration phase. While theALTER TABLEmight succeed, the subsequent bulk update to populate the new column fails:Failing SQL:
update public.mt_doc_foo set offset = CAST(data ->> 'Offset' as bigint); -- Syntax errorThe Fix
Modified
DuplicatedField.UpdateSqlFragment()to unconditionally quote theColumnName. Since PostgreSQL is case-insensitive for unquoted identifiers but case-sensitive for quoted ones, and Marten already ensures column names are lowercase, always quoting is the safest and most performant way to handle all potential keywords.Changes
Marten.Schema.DuplicatedField.UpdateSqlFragment()to wrap the column name in double quotes.Dependency
This PR is complementary to the fix in Weasel (JasperFx/weasel#242) which handles the quoting within the generated Upsert functions. Together, these changes provide full support for reserved keywords in document properties.
Verification
ToFunctionUpdatenow produces correctly quoted identifiers for reserved keywords.DuplicatedFieldnow generates quoted column identifiers for bulk data migration, matching the schema requirements for reserved keywords.