Skip to content

fix(bigtable): encode str qualifier in DeleteRangeFromColumn and allow -2**63 in int mutations - #18881

Open
mutianf wants to merge 2 commits into
mainfrom
fix-mutation-validation
Open

mutianf wants to merge 2 commits into
mainfrom
fix-mutation-validation

Conversation

@mutianf

@mutianf mutianf commented Oct 28, 2026

Copy link
Copy Markdown
Contributor

Summary

  • DeleteRangeFromColumn qualifier validation and encoding: DeleteRangeFromColumn did not encode str qualifiers to bytes in __post_init__, causing proto-plus to treat string qualifiers as base64 during _to_pb() serialization and either silently decode into the wrong column bytes or fail with binascii.Error. __post_init__ now encodes str qualifiers to UTF-8 bytes and raises TypeError for non-bytes/str inputs, matching SetCell and AddToCell.
  • Allow -2**63 (INT64_MIN) in integer mutations and increment rules: SetCell, AddToCell, and IncrementRule checked abs(val) > (1 << 63) - 1, which rejected the valid two's-complement minimum 64-bit signed integer -2**63 (-9223372036854775808) with ValueError. Replaced the abs() check with _MIN_INCREMENT_VALUE <= val <= _MAX_INCREMENT_VALUE.

Test plan

  • Added unit tests in tests/unit/data/test_mutations.py and tests/unit/data/test_read_modify_write_rules.py covering -2**63 and 2**63 - 1 bounds and DeleteRangeFromColumn str / invalid qualifier handling.

@mutianf
mutianf requested a review from a team as a code owner October 28, 2026 15:22

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates integer boundary checks across mutations and read-modify-write rules to correctly validate 64-bit signed integers using both minimum and maximum bounds, and adds support for string qualifiers in DeleteRangeFromColumn. The review feedback correctly points out that several error messages incorrectly state the upper limit as 263 instead of the exclusive 263 - 1, and suggests updating these messages to prevent user confusion.

Comment on lines +178 to 181
if not (_MIN_INCREMENT_VALUE <= new_value <= _MAX_INCREMENT_VALUE):
raise ValueError(
"int values must be between -2**63 and 2**63 (64-bit signed int)"
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The error message states that integer values must be between -2**63 and 2**63. However, the upper bound 2**63 is exclusive (the maximum valid 64-bit signed integer is 2**63 - 1). To avoid confusion for users hitting the upper boundary, the error message should be updated to specify 2**63 - 1 as the upper limit.

Suggested change
if not (_MIN_INCREMENT_VALUE <= new_value <= _MAX_INCREMENT_VALUE):
raise ValueError(
"int values must be between -2**63 and 2**63 (64-bit signed int)"
)
if not (_MIN_INCREMENT_VALUE <= new_value <= _MAX_INCREMENT_VALUE):
raise ValueError(
"int values must be between -2**63 and 2**63 - 1 (64-bit signed int)"
)

Comment on lines +335 to 338
if not (_MIN_INCREMENT_VALUE <= value <= _MAX_INCREMENT_VALUE):
raise ValueError(
"int values must be between -2**63 and 2**63 (64-bit signed int)"
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The error message states that integer values must be between -2**63 and 2**63. However, the upper bound 2**63 is exclusive (the maximum valid 64-bit signed integer is 2**63 - 1). To avoid confusion for users hitting the upper boundary, the error message should be updated to specify 2**63 - 1 as the upper limit.

Suggested change
if not (_MIN_INCREMENT_VALUE <= value <= _MAX_INCREMENT_VALUE):
raise ValueError(
"int values must be between -2**63 and 2**63 (64-bit signed int)"
)
if not (_MIN_INCREMENT_VALUE <= value <= _MAX_INCREMENT_VALUE):
raise ValueError(
"int values must be between -2**63 and 2**63 - 1 (64-bit signed int)"
)

Comment on lines +67 to 70
if not (_MIN_INCREMENT_VALUE <= increment_amount <= _MAX_INCREMENT_VALUE):
raise ValueError(
"increment_amount must be between -2**63 and 2**63 (64-bit signed int)"
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The error message states that the increment amount must be between -2**63 and 2**63. However, the upper bound 2**63 is exclusive (the maximum valid 64-bit signed integer is 2**63 - 1). To avoid confusion for users hitting the upper boundary, the error message should be updated to specify 2**63 - 1 as the upper limit.

Suggested change
if not (_MIN_INCREMENT_VALUE <= increment_amount <= _MAX_INCREMENT_VALUE):
raise ValueError(
"increment_amount must be between -2**63 and 2**63 (64-bit signed int)"
)
if not (_MIN_INCREMENT_VALUE <= increment_amount <= _MAX_INCREMENT_VALUE):
raise ValueError(
"increment_amount must be between -2**63 and 2**63 - 1 (64-bit signed int)"
)

@mutianf
mutianf force-pushed the fix-mutation-validation branch from e251888 to a8364ea Compare October 28, 2026 19:30

This branch has not been deployed

No deployments
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.

1 participant