Fix upserting when Synapse column name contains space or special characters - #1454
Merged
Merged
Conversation
andrewelamb
marked this pull request as draft
September 17, 2026 17:52
andrewelamb
marked this pull request as ready for review
September 17, 2026 18:05
BryanFauble
approved these changes
Sep 18, 2026
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:
upsert_rowsmisreports how many rows need to be updated when a Synapsetable or view has a column name that is not a valid Python identifier, such
as a name with a space (
AMP AIM_SSc-Skin_Access Team) or a hyphen(
col-with-hyphen).(against
syn76143756, a table with several columns whose names containspaces).
_construct_partial_rows_for_upsertinsynapseclient/models/mixins/table_components.pyloops over the freshlyqueried rows with
results.itertuples(index=False).itertuplesbuilds anamedtuple from the column names, but a namedtuple field name must be a
valid Python identifier. When a column name has a space or another
special character, pandas silently swaps in a fallback name for that
field (e.g.
_5), so the real column name is no longer reachable viagetattr/hasattr.getattr(row, column) if hasattr(row, column) else Nonetoread each cell, and
if not hasattr(row, column): values_differ = Trueto decide whether a row changed. Since
hasattrwas alwaysFalseforany column with a space or special character, every row was treated as
changed for that column, on every call, even when nothing changed.
"rows to update," regardless of how many rows actually changed.
([SYNPY-1912] Fix upsert_rows misreporting Table update responses #1446) fixed a separate, downstream issue in how update results were
logged, but did not touch this code path.
Solution:
getattr/hasattron theitertuplesresult) with position-based access, which works regardlessof what pandas named the namedtuple field.
column_positionsmap ({column_name: index}) fromresults.columnsonce per call, and used it to look upROW_ETAG,ROW_ID,id, the primary key columns, and each compared column byposition (
row[column_positions[...]]) instead of by name.itertuplesloopand comparison logic, and only changes how values are read out of each
row.
Testing:
test_construct_partial_rows_for_upsert_with_column_name_containing_special_charactersto
tests/unit/synapseclient/mixins/unit_test_table_components.py. Itexercises columns named
col with spaceandcol-with-hyphenwithidentical values in the queried results and the upsert data, and asserts
that no rows are reported as changed. This test failed before the fix
(
assert 2 == 0) and passes after it.(
tests/unit/synapseclient/mixins/unit_test_table_components.py): all 336tests pass, including the 13 pre-existing tests that call
_construct_partial_rows_for_upsertdirectly, so no regressions.syn76143756) that its column names doin fact contain spaces and special characters (e.g.
AMP AIM_SSc-Skin_Access Team), confirming this fix addresses thereported case.