From d6fadc57a728eef0348212d83ff30792b802243a Mon Sep 17 00:00:00 2001 From: Mattie Fu Date: Fri, 30 Oct 2026 19:41:58 +0000 Subject: [PATCH] fix(bigtable): check family_name field presence in merge_rows ReadRowsResponse.CellChunk.family_name can be set to an empty string as a valid sentinel column family name. Check c.HasField("family_name") instead of string truthiness so empty family names neither raise InvalidChunk nor merge into the previous family. --- .../cloud/bigtable/data/_async/_read_rows.py | 4 +- .../bigtable/data/_sync_autogen/_read_rows.py | 4 +- .../data/_async/test_read_rows_acceptance.py | 44 +++++++++++++++++++ .../test_read_rows_acceptance.py | 42 ++++++++++++++++++ 4 files changed, 90 insertions(+), 4 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 3e6c2b533b1e..7648c610d798 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 @@ -260,11 +260,11 @@ async def merge_rows( if c.reset_row: raise _ResetRow(c) k = c.row_key - f = c.family_name.value + f = c.family_name.value if c.HasField("family_name") else None q = c.qualifier.value if c.HasField("qualifier") else None if k and k != row_key: raise InvalidChunk("unexpected new row key") - if f: + if f is not None: family = f if q is not None: qualifier = q 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 ce29226cac53..346c201ff045 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 @@ -219,11 +219,11 @@ def merge_rows( if c.reset_row: raise _ResetRow(c) k = c.row_key - f = c.family_name.value + f = c.family_name.value if c.HasField("family_name") else None q = c.qualifier.value if c.HasField("qualifier") else None if k and k != row_key: raise InvalidChunk("unexpected new row key") - if f: + if f is not None: family = f if q is not None: qualifier = q diff --git a/packages/google-cloud-bigtable/tests/unit/data/_async/test_read_rows_acceptance.py b/packages/google-cloud-bigtable/tests/unit/data/_async/test_read_rows_acceptance.py index 53689c9c33f7..f03dc6dbcf98 100644 --- a/packages/google-cloud-bigtable/tests/unit/data/_async/test_read_rows_acceptance.py +++ b/packages/google-cloud-bigtable/tests/unit/data/_async/test_read_rows_acceptance.py @@ -263,6 +263,50 @@ async def test_missing_family(self): ) ) + @CrossSync.pytest + async def test_empty_family_name_first_chunk(self): + rows = await self._process_chunks( + ReadRowsResponse.CellChunk( + row_key=b"a", + family_name="", + qualifier=b"q", + timestamp_micros=1000, + value=b"v", + commit_row=True, + ) + ) + assert len(rows) == 1 + cells = rows[0].get_cells(family="", qualifier=b"q") + assert len(cells) == 1 + assert cells[0].value == b"v" + + @CrossSync.pytest + async def test_empty_family_name_after_nonempty_family(self): + rows = await self._process_chunks( + ReadRowsResponse.CellChunk( + row_key=b"a", + family_name="cf1", + qualifier=b"q", + timestamp_micros=2000, + value=b"v1", + commit_row=False, + ), + ReadRowsResponse.CellChunk( + family_name="", + qualifier=b"q", + timestamp_micros=1000, + value=b"v2", + commit_row=True, + ), + ) + assert len(rows) == 1 + cf1_cells = rows[0].get_cells(family="cf1", qualifier=b"q") + empty_cells = rows[0].get_cells(family="", qualifier=b"q") + assert len(cf1_cells) == 1 + assert cf1_cells[0].value == b"v1" + assert len(empty_cells) == 1 + assert empty_cells[0].value == b"v2" + @CrossSync.pytest async def test_mid_cell_row_key_change(self): with pytest.raises(InvalidChunk): diff --git a/packages/google-cloud-bigtable/tests/unit/data/_sync_autogen/test_read_rows_acceptance.py b/packages/google-cloud-bigtable/tests/unit/data/_sync_autogen/test_read_rows_acceptance.py index 77c55ce0183b..8af2c1017344 100644 --- a/packages/google-cloud-bigtable/tests/unit/data/_sync_autogen/test_read_rows_acceptance.py +++ b/packages/google-cloud-bigtable/tests/unit/data/_sync_autogen/test_read_rows_acceptance.py @@ -250,6 +250,48 @@ def test_missing_family(self): ) ) + def test_empty_family_name_first_chunk(self): + rows = self._process_chunks( + ReadRowsResponse.CellChunk( + row_key=b"a", + family_name="", + qualifier=b"q", + timestamp_micros=1000, + value=b"v", + commit_row=True, + ) + ) + assert len(rows) == 1 + cells = rows[0].get_cells(family="", qualifier=b"q") + assert len(cells) == 1 + assert cells[0].value == b"v" + + def test_empty_family_name_after_nonempty_family(self): + rows = self._process_chunks( + ReadRowsResponse.CellChunk( + row_key=b"a", + family_name="cf1", + qualifier=b"q", + timestamp_micros=2000, + value=b"v1", + commit_row=False, + ), + ReadRowsResponse.CellChunk( + family_name="", + qualifier=b"q", + timestamp_micros=1000, + value=b"v2", + commit_row=True, + ), + ) + assert len(rows) == 1 + cf1_cells = rows[0].get_cells(family="cf1", qualifier=b"q") + empty_cells = rows[0].get_cells(family="", qualifier=b"q") + assert len(cf1_cells) == 1 + assert cf1_cells[0].value == b"v1" + assert len(empty_cells) == 1 + assert empty_cells[0].value == b"v2" + def test_mid_cell_row_key_change(self): with pytest.raises(InvalidChunk): self._process_chunks(