Implement double-encoding escaping mechanism for column names with escape sequence prefix#3056
Draft
Copilot wants to merge 4 commits intorelease/1.6from
Draft
Implement double-encoding escaping mechanism for column names with escape sequence prefix#3056Copilot wants to merge 4 commits intorelease/1.6from
Copilot wants to merge 4 commits intorelease/1.6from
Conversation
) ### Why make this change? Serialization and deserialization of metadata currently fail when column names are prefixed with the $ symbol. ### Root cause This issue occurs because we’ve enabled the ReferenceHandler flag in our System.Text.Json serialization settings. When this flag is active, the serializer treats $ as a reserved character used for special metadata (e.g., $id, $ref). As a result, any property name starting with $ is interpreted as metadata and cannot be deserialized properly. ### What is this change? This update introduces custom logic in the converter’s Write and Read methods to handle $-prefixed column names safely. - During serialization, columns beginning with $ are escaped as "_$". - During deserialization, this transformation is reversed to restore the original property names. ### How was this tested - [x] Unit tests --------- Co-authored-by: Aniruddh Munde <anmunde@microsoft.com>
## Why make this change? - To apply correct serialization and deserialization logic for stored procedures. With the previous changes, serialization was not working correctly for the StoredProcedureDefinition type, which extends SourceDefinition. When the value type was passed explicitly for serialization, the parent type was used instead, causing some child-type properties to be omitted. ## What is this change? Instead of manually specifying the value type during serialization, this change allows the library to infer the type automatically and perform the correct serialization. ## How was this tested? - [x] Unit Tests --------- Co-authored-by: Aniruddh Munde <anmunde@microsoft.com>
1 task
…king Co-authored-by: Alekhya-Polavarapu <67075378+Alekhya-Polavarapu@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Address feedback on serialization escaping mechanism improvements
Implement double-encoding escaping mechanism for column names with escape sequence prefix
Jan 14, 2026
1b8a646 to
8726a59
Compare
Base automatically changed from
dev/alpolava/cherrypick_serialization
to
release/1.6
January 15, 2026 19:20
JerryNixon
approved these changes
Jan 21, 2026
JerryNixon
approved these changes
Jan 23, 2026
Contributor
JerryNixon
left a comment
There was a problem hiding this comment.
One
private static void EscapeDollaredColumns(SourceDefinition sourceDef)
{
// Mutates sourceDef.Columns dictionary directly
sourceDef.Columns.Remove(key);
sourceDef.Columns[newKey] = col;
}This modifies the original object's Columns dictionary during serialization. After serialization, the in-memory object will have escaped column names instead of the original names.
Please consider creating a new dictionary with escaped keys rather than modifying in-place, or ensure the unescaping happens reliably after serialization.
Two
ColumnDefinition col = sourceDef.Columns[key]; // Line 108, 118Consider using TryGetValue for defensive programming.
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.
Why make this change?
Addresses review feedback from #3051 requesting a more robust escaping mechanism to handle edge cases where column names already contain the escape sequence "DAB_ESCAPE$".
What is this change?
Double-Encoding Strategy
Enhanced the serialization escaping logic with two-step processing:
DAB_ESCAPE$→DAB_ESCAPE$DAB_ESCAPE$...$→DAB_ESCAPE$...Deserialization reverses this in opposite order to restore original names.
Prevent Double-Escaping
Added HashSet tracking in
Write()method to handle cases whereSourceDefinitionandTableDefinitionproperties reference the same object instance.Example Transformations
$FirstName→DAB_ESCAPE$FirstName(existing behavior, unchanged)DAB_ESCAPE$FirstName→DAB_ESCAPE$DAB_ESCAPE$FirstName(new edge case)RegularColumn→RegularColumn(unchanged)How was this tested?
Added test cases covering:
Note: Existing serialization tests pass, confirming backward compatibility. New edge case tests demonstrate the double-encoding logic but require investigation - the in-place dictionary modifications are not reflecting in serialized JSON output despite working correctly in isolation.
Sample Request(s)
N/A - Internal serialization mechanism change with no API surface modifications.
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.