Skip to content

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

Merged
mutianf merged 3 commits into
mainfrom
fix-mutation-validation
Oct 30, 2026
Merged

mutianf merged 3 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.

@mutianf
mutianf force-pushed the fix-mutation-validation branch from e251888 to a8364ea Compare October 28, 2026 19:30
@daniel-sanche daniel-sanche added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 29, 2026
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 29, 2026
@mutianf
mutianf force-pushed the fix-mutation-validation branch from a8364ea to 4aed324 Compare October 30, 2026 02:49
@mutianf
mutianf merged commit 6e80b83 into main Oct 30, 2026
57 checks passed
@mutianf
mutianf deleted the fix-mutation-validation branch October 30, 2026 19:54
daniel-sanche pushed a commit that referenced this pull request Oct 30, 2026
…w -2**63 in int mutations (#18881)

## 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

- [x] 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.
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.

3 participants