From b792911950665015ea87d1c0262603541937869f Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Wed, 28 Oct 2026 15:23:19 +0000 Subject: [PATCH 1/3] fix(bigtable): preserve empty row keys in RowRange and fix UTF-8 prefix bounds --- .../cloud/bigtable/data/_async/_read_rows.py | 14 ++++++-- .../bigtable/data/_sync_autogen/_read_rows.py | 14 ++++++-- .../cloud/bigtable/data/read_rows_query.py | 32 ++++++++++++------- .../google/cloud/bigtable/row_set.py | 17 +++++++--- .../tests/unit/data/_async/test__read_rows.py | 7 ++++ .../data/_sync_autogen/test__read_rows.py | 7 ++++ .../tests/unit/data/test_read_rows_query.py | 28 ++++++++++++++++ .../tests/unit/v2_client/test_row_set.py | 27 ++++++++++++++++ 8 files changed, 126 insertions(+), 20 deletions(-) diff --git a/packages/google-cloud-bigtable/google/cloud/bigtable/data/_async/_read_rows.py b/packages/google-cloud-bigtable/google/cloud/bigtable/data/_async/_read_rows.py index d6fcfc73520b..cfff1f46b742 100644 --- a/packages/google-cloud-bigtable/google/cloud/bigtable/data/_async/_read_rows.py +++ b/packages/google-cloud-bigtable/google/cloud/bigtable/data/_async/_read_rows.py @@ -377,11 +377,21 @@ def _revise_request_rowset( # adjust ranges to ignore keys before last seen adjusted_ranges: list[RowRangePB] = [] for row_range in row_set.row_ranges: - end_key = row_range.end_key_closed or row_range.end_key_open or None + if "end_key_closed" in row_range: + end_key: bytes | None = row_range.end_key_closed + elif "end_key_open" in row_range: + end_key = row_range.end_key_open + else: + end_key = None if end_key is None or end_key > last_seen_row_key: # end range is after last seen key new_range = RowRangePB(row_range) - start_key = row_range.start_key_closed or row_range.start_key_open + if "start_key_closed" in row_range: + start_key: bytes | None = row_range.start_key_closed + elif "start_key_open" in row_range: + start_key = row_range.start_key_open + else: + start_key = None if start_key is None or start_key <= last_seen_row_key: # replace start key with last seen new_range.start_key_open = last_seen_row_key diff --git a/packages/google-cloud-bigtable/google/cloud/bigtable/data/_sync_autogen/_read_rows.py b/packages/google-cloud-bigtable/google/cloud/bigtable/data/_sync_autogen/_read_rows.py index f8c2d28b2903..cd2221669afd 100644 --- a/packages/google-cloud-bigtable/google/cloud/bigtable/data/_sync_autogen/_read_rows.py +++ b/packages/google-cloud-bigtable/google/cloud/bigtable/data/_sync_autogen/_read_rows.py @@ -318,10 +318,20 @@ def _revise_request_rowset(row_set: RowSetPB, last_seen_row_key: bytes) -> RowSe ] adjusted_ranges: list[RowRangePB] = [] for row_range in row_set.row_ranges: - end_key = row_range.end_key_closed or row_range.end_key_open or None + if "end_key_closed" in row_range: + end_key: bytes | None = row_range.end_key_closed + elif "end_key_open" in row_range: + end_key = row_range.end_key_open + else: + end_key = None if end_key is None or end_key > last_seen_row_key: new_range = RowRangePB(row_range) - start_key = row_range.start_key_closed or row_range.start_key_open + if "start_key_closed" in row_range: + start_key: bytes | None = row_range.start_key_closed + elif "start_key_open" in row_range: + start_key = row_range.start_key_open + else: + start_key = None if start_key is None or start_key <= last_seen_row_key: new_range.start_key_open = last_seen_row_key adjusted_ranges.append(new_range) diff --git a/packages/google-cloud-bigtable/google/cloud/bigtable/data/read_rows_query.py b/packages/google-cloud-bigtable/google/cloud/bigtable/data/read_rows_query.py index 938303ea5a4c..75a5b7d7c21b 100644 --- a/packages/google-cloud-bigtable/google/cloud/bigtable/data/read_rows_query.py +++ b/packages/google-cloud-bigtable/google/cloud/bigtable/data/read_rows_query.py @@ -52,9 +52,11 @@ def __init__( start_is_inclusive: bool | None = None, end_is_inclusive: bool | None = None, ): - # convert empty key inputs to None for consistency - start_key = None if not start_key else start_key - end_key = None if not end_key else end_key + # convert empty key inputs to None for consistency when using default inclusiveness + if not start_key and start_is_inclusive is not False: + start_key = None + if end_key == "" and end_is_inclusive is None: + end_key = None # check for invalid combinations of arguments if start_is_inclusive is None: start_is_inclusive = True @@ -105,7 +107,11 @@ def start_key(self) -> bytes | None: """ Returns the start key of the range. If None, the range is unbounded on the left. """ - return self._pb.start_key_closed or self._pb.start_key_open or None + if "start_key_closed" in self._pb: + return self._pb.start_key_closed + elif "start_key_open" in self._pb: + return self._pb.start_key_open + return None @property def end_key(self) -> bytes | None: @@ -115,7 +121,11 @@ def end_key(self) -> bytes | None: Returns: bytes | None: The end key of the range, or None if the range is unbounded on the right. """ - return self._pb.end_key_closed or self._pb.end_key_open or None + if "end_key_closed" in self._pb: + return self._pb.end_key_closed + elif "end_key_open" in self._pb: + return self._pb.end_key_open + return None @property def start_is_inclusive(self) -> bool: @@ -127,7 +137,7 @@ def start_is_inclusive(self) -> bool: Returns: bool: Whether the range is inclusive of the start key. """ - return not bool(self._pb.start_key_open) + return "start_key_open" not in self._pb @property def end_is_inclusive(self) -> bool: @@ -139,7 +149,7 @@ def end_is_inclusive(self) -> bool: Returns: bool: Whether the range is inclusive of the end key. """ - return not bool(self._pb.end_key_open) + return "end_key_open" not in self._pb def _to_pb(self) -> RowRangePB: """ @@ -190,10 +200,10 @@ def __bool__(self) -> bool: bool: True if the RowRange is not empty, False otherwise """ return bool( - self._pb.start_key_closed - or self._pb.start_key_open - or self._pb.end_key_closed - or self._pb.end_key_open + "start_key_closed" in self._pb + or "start_key_open" in self._pb + or "end_key_closed" in self._pb + or "end_key_open" in self._pb ) def __eq__(self, other: Any) -> bool: diff --git a/packages/google-cloud-bigtable/google/cloud/bigtable/row_set.py b/packages/google-cloud-bigtable/google/cloud/bigtable/row_set.py index 6e041e3ed7b8..a90f499ac509 100644 --- a/packages/google-cloud-bigtable/google/cloud/bigtable/row_set.py +++ b/packages/google-cloud-bigtable/google/cloud/bigtable/row_set.py @@ -14,6 +14,8 @@ """User-friendly container for Google Cloud Bigtable RowSet""" +from google.cloud._helpers import _to_bytes # type: ignore + from google.cloud.bigtable.data.read_rows_query import ( ReadRowsQuery, ) @@ -121,14 +123,19 @@ def add_row_range_with_prefix(self, row_key_prefix): :start-after: [START bigtable_api_add_row_range_with_prefix] :end-before: [END bigtable_api_add_row_range_with_prefix] - :type row_key_prefix: str + :type row_key_prefix: str | bytes :param row_key_prefix: To retrieve all rows that start with this row key prefix. Prefix cannot be zero length.""" - end_key = row_key_prefix[:-1] + chr(ord(row_key_prefix[-1]) + 1) - self.add_row_range_from_keys( - row_key_prefix.encode("utf-8"), end_key.encode("utf-8") - ) + prefix_bytes = _to_bytes(row_key_prefix, encoding="utf-8") + if not prefix_bytes: + raise ValueError("row_key_prefix cannot be empty.") + stripped = prefix_bytes.rstrip(b"\xff") + if not stripped: + end_key = None + else: + end_key = stripped[:-1] + bytes([stripped[-1] + 1]) + self.add_row_range_from_keys(prefix_bytes, end_key) class RowRange(_MappableAttributesMixin, BaseRowRange): diff --git a/packages/google-cloud-bigtable/tests/unit/data/_async/test__read_rows.py b/packages/google-cloud-bigtable/tests/unit/data/_async/test__read_rows.py index 24b92d2de5ad..96387707244d 100644 --- a/packages/google-cloud-bigtable/tests/unit/data/_async/test__read_rows.py +++ b/packages/google-cloud-bigtable/tests/unit/data/_async/test__read_rows.py @@ -160,6 +160,13 @@ def test_revise_request_rowset_keys_with_range( "a", [{"start_key_open": "a", "end_key_open": "z"}], ), + ([{"end_key_closed": ""}], "", []), + ([{"end_key_open": ""}], "", []), + ( + [{"start_key_open": "", "end_key_closed": "d"}], + "", + [{"start_key_open": "", "end_key_closed": "d"}], + ), ], ) @pytest.mark.parametrize("with_key", [True, False]) diff --git a/packages/google-cloud-bigtable/tests/unit/data/_sync_autogen/test__read_rows.py b/packages/google-cloud-bigtable/tests/unit/data/_sync_autogen/test__read_rows.py index b6cac9cafbc0..511d20794d80 100644 --- a/packages/google-cloud-bigtable/tests/unit/data/_sync_autogen/test__read_rows.py +++ b/packages/google-cloud-bigtable/tests/unit/data/_sync_autogen/test__read_rows.py @@ -155,6 +155,13 @@ def test_revise_request_rowset_keys_with_range( "a", [{"start_key_open": "a", "end_key_open": "z"}], ), + ([{"end_key_closed": ""}], "", []), + ([{"end_key_open": ""}], "", []), + ( + [{"start_key_open": "", "end_key_closed": "d"}], + "", + [{"start_key_open": "", "end_key_closed": "d"}], + ), ], ) @pytest.mark.parametrize("with_key", [True, False]) diff --git a/packages/google-cloud-bigtable/tests/unit/data/test_read_rows_query.py b/packages/google-cloud-bigtable/tests/unit/data/test_read_rows_query.py index cfef2da4b793..8efb4083acf0 100644 --- a/packages/google-cloud-bigtable/tests/unit/data/test_read_rows_query.py +++ b/packages/google-cloud-bigtable/tests/unit/data/test_read_rows_query.py @@ -57,12 +57,40 @@ def test_ctor_empty_strings(self): """ empty strings should be treated as None """ + from google.cloud.bigtable_v2.types import RowRange as RowRangePB + row_range = self._make_one("", "") assert row_range.start_key is None assert row_range.end_key is None assert row_range.start_is_inclusive is True assert row_range.end_is_inclusive is True + rr_open_start = self._make_one(start_key=b"", start_is_inclusive=False) + assert rr_open_start.start_key == b"" + assert rr_open_start.start_is_inclusive is False + assert bool(rr_open_start) is True + assert "start_key_open" in rr_open_start._to_pb() + + rr_closed_end = self._make_one(end_key=b"", end_is_inclusive=True) + assert rr_closed_end.end_key == b"" + assert rr_closed_end.end_is_inclusive is True + assert bool(rr_closed_end) is True + assert "end_key_closed" in rr_closed_end._to_pb() + + rr_open_end = self._make_one(end_key=b"", end_is_inclusive=False) + assert rr_open_end.end_key == b"" + assert rr_open_end.end_is_inclusive is False + assert bool(rr_open_end) is True + assert "end_key_open" in rr_open_end._to_pb() + + rr_from_pb = self._get_target_class()._from_pb( + RowRangePB(start_key_open=b"", end_key_closed=b"z") + ) + assert rr_from_pb.start_key == b"" + assert rr_from_pb.start_is_inclusive is False + assert rr_from_pb.end_key == b"z" + assert rr_from_pb.end_is_inclusive is True + def test_ctor_inclusive_flags(self): row_range = self._make_one("test_row5", "test_row6", False, True) assert row_range.start_key == "test_row5".encode() diff --git a/packages/google-cloud-bigtable/tests/unit/v2_client/test_row_set.py b/packages/google-cloud-bigtable/tests/unit/v2_client/test_row_set.py index e76d1f15135c..5ed3ce223364 100644 --- a/packages/google-cloud-bigtable/tests/unit/v2_client/test_row_set.py +++ b/packages/google-cloud-bigtable/tests/unit/v2_client/test_row_set.py @@ -163,6 +163,33 @@ def test_row_set_add_row_range_with_prefix(): assert row_set.row_ranges[0].end_key == b"rox" +def test_row_set_add_row_range_with_prefix_utf8_boundaries(): + import pytest + + from google.cloud.bigtable.row_set import RowSet + + cases = [ + ("user\x7f", b"user\x7f", b"user\x80"), + ("p\u07ff", b"p\xdf\xbf", b"p\xdf\xc0"), + ("p\uffff", b"p\xef\xbf\xbf", b"p\xef\xbf\xc0"), + ("prefix\ud7ff", b"prefix\xed\x9f\xbf", b"prefix\xed\x9f\xc0"), + ("prefix\U0010ffff", b"prefix\xf4\x8f\xbf\xbf", b"prefix\xf4\x8f\xbf\xc0"), + (b"abc\xff", b"abc\xff", b"abd"), + (b"\xff\xff", b"\xff\xff", None), + ] + for prefix, expected_start, expected_end in cases: + row_set = RowSet() + row_set.add_row_range_with_prefix(prefix) + assert row_set.row_ranges[0].start_key == expected_start + assert row_set.row_ranges[0].end_key == expected_end + + row_set = RowSet() + with pytest.raises(ValueError): + row_set.add_row_range_with_prefix("") + with pytest.raises(ValueError): + row_set.add_row_range_with_prefix(b"") + + def test_row_range_constructor(): from google.cloud.bigtable.row_set import RowRange From b8c452c63723d6a3d9cf9c930802d3527e3f0dc5 Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Wed, 28 Oct 2026 15:43:11 +0000 Subject: [PATCH 2/3] fix(bigtable): use not end_key when end_is_inclusive is None in RowRange --- .../google/cloud/bigtable/data/read_rows_query.py | 5 +++-- .../tests/unit/data/test_read_rows_query.py | 11 ++++++----- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/packages/google-cloud-bigtable/google/cloud/bigtable/data/read_rows_query.py b/packages/google-cloud-bigtable/google/cloud/bigtable/data/read_rows_query.py index 75a5b7d7c21b..2313504fbe7d 100644 --- a/packages/google-cloud-bigtable/google/cloud/bigtable/data/read_rows_query.py +++ b/packages/google-cloud-bigtable/google/cloud/bigtable/data/read_rows_query.py @@ -52,10 +52,11 @@ def __init__( start_is_inclusive: bool | None = None, end_is_inclusive: bool | None = None, ): - # convert empty key inputs to None for consistency when using default inclusiveness + # Normalize default empty inputs (e.g. RowRange("", "")) to unbounded None, + # but preserve explicit empty-key bounds when inclusiveness is specified. if not start_key and start_is_inclusive is not False: start_key = None - if end_key == "" and end_is_inclusive is None: + if not end_key and end_is_inclusive is None: end_key = None # check for invalid combinations of arguments if start_is_inclusive is None: diff --git a/packages/google-cloud-bigtable/tests/unit/data/test_read_rows_query.py b/packages/google-cloud-bigtable/tests/unit/data/test_read_rows_query.py index 8efb4083acf0..28b31e5f0dda 100644 --- a/packages/google-cloud-bigtable/tests/unit/data/test_read_rows_query.py +++ b/packages/google-cloud-bigtable/tests/unit/data/test_read_rows_query.py @@ -59,11 +59,12 @@ def test_ctor_empty_strings(self): """ from google.cloud.bigtable_v2.types import RowRange as RowRangePB - row_range = self._make_one("", "") - assert row_range.start_key is None - assert row_range.end_key is None - assert row_range.start_is_inclusive is True - assert row_range.end_is_inclusive is True + for empty in ("", b""): + row_range = self._make_one(empty, empty) + assert row_range.start_key is None + assert row_range.end_key is None + assert row_range.start_is_inclusive is True + assert row_range.end_is_inclusive is True rr_open_start = self._make_one(start_key=b"", start_is_inclusive=False) assert rr_open_start.start_key == b"" From 6735ff749c5f1539a52453801ba6265e066e7784 Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Wed, 28 Oct 2026 18:40:09 +0000 Subject: [PATCH 3/3] docs(bigtable): clarify empty end_key normalization and prefix 0xFF carry --- .../google/cloud/bigtable/data/read_rows_query.py | 5 +++-- .../google-cloud-bigtable/google/cloud/bigtable/row_set.py | 2 ++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/google-cloud-bigtable/google/cloud/bigtable/data/read_rows_query.py b/packages/google-cloud-bigtable/google/cloud/bigtable/data/read_rows_query.py index 2313504fbe7d..837ee4bfec1d 100644 --- a/packages/google-cloud-bigtable/google/cloud/bigtable/data/read_rows_query.py +++ b/packages/google-cloud-bigtable/google/cloud/bigtable/data/read_rows_query.py @@ -52,8 +52,9 @@ def __init__( start_is_inclusive: bool | None = None, end_is_inclusive: bool | None = None, ): - # Normalize default empty inputs (e.g. RowRange("", "")) to unbounded None, - # but preserve explicit empty-key bounds when inclusiveness is specified. + # Normalize default empty inputs (e.g. RowRange("", "")) to unbounded None. + # [b"", ...) is equivalent to an unbounded start, whereas (..., b""] matches + # only b"", so end_key is only normalized when end_is_inclusive was omitted. if not start_key and start_is_inclusive is not False: start_key = None if not end_key and end_is_inclusive is None: diff --git a/packages/google-cloud-bigtable/google/cloud/bigtable/row_set.py b/packages/google-cloud-bigtable/google/cloud/bigtable/row_set.py index a90f499ac509..b05b13356728 100644 --- a/packages/google-cloud-bigtable/google/cloud/bigtable/row_set.py +++ b/packages/google-cloud-bigtable/google/cloud/bigtable/row_set.py @@ -130,6 +130,8 @@ def add_row_range_with_prefix(self, row_key_prefix): prefix_bytes = _to_bytes(row_key_prefix, encoding="utf-8") if not prefix_bytes: raise ValueError("row_key_prefix cannot be empty.") + # Carry the +1 increment past any trailing 0xFF (max byte) bytes; + # if all bytes are 0xFF, no larger prefix exists so the upper bound is None. stripped = prefix_bytes.rstrip(b"\xff") if not stripped: end_key = None