From 2d4f357319673c012d74407d56708708cfbb6c19 Mon Sep 17 00:00:00 2001 From: Igor Bernstein Date: Wed, 28 Oct 2026 11:30:22 -0400 Subject: [PATCH] fix(bigtable): reject empty split points in RowSetUtil and clarify QueryPaginator Javadoc --- .../bigtable/data/v2/internal/RowSetUtil.java | 2 ++ .../cloud/bigtable/data/v2/models/Query.java | 10 ++++++++-- .../data/v2/internal/RowSetUtilTest.java | 17 +++++++++++++++++ 3 files changed, 27 insertions(+), 2 deletions(-) diff --git a/java-bigtable/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/internal/RowSetUtil.java b/java-bigtable/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/internal/RowSetUtil.java index 2c4ce914bb66..4f44c5fd15da 100644 --- a/java-bigtable/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/internal/RowSetUtil.java +++ b/java-bigtable/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/internal/RowSetUtil.java @@ -51,6 +51,7 @@ private RowSetUtil() {} /** Removes the {@code #excludePoint} rowkey from the {@code RowSet} */ public static RowSet eraseLargeRow(RowSet rowSet, ByteString excludePoint) { + Preconditions.checkArgument(!excludePoint.isEmpty(), "Can't handle empty excludePoint"); RowSet.Builder newRowSet = RowSet.newBuilder(); @@ -89,6 +90,7 @@ public static RowSet eraseLargeRow(RowSet rowSet, ByteString excludePoint) { * removed. The primary usecase is to resume a broken ReadRows stream. */ public static RowSet erase(RowSet rowSet, ByteString splitPoint, boolean fromStart) { + Preconditions.checkArgument(!splitPoint.isEmpty(), "Can't handle empty splitPoint"); RowSet.Builder newRowSet = RowSet.newBuilder(); if (rowSet.getRowKeysList().isEmpty() && rowSet.getRowRangesList().isEmpty()) { diff --git a/java-bigtable/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/models/Query.java b/java-bigtable/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/models/Query.java index d37712c682c2..ce356cacd6a3 100644 --- a/java-bigtable/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/models/Query.java +++ b/java-bigtable/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/models/Query.java @@ -292,13 +292,13 @@ public List shard(SortedSet splitPoints) { * *
{@code
    * Query query = Query.create(...).range("a", "z");
-   * Query.QueryPaginator paginator = query.createQueryPaginator(100);
+   * Query.QueryPaginator paginator = query.createPaginator(100);
    * ByteString lastSeenRowKey = ByteString.EMPTY;
    * do {
    *     List rows = client.readRowsCallable().all().call(paginator.getNextQuery());
    *     for (Row row : rows) {
    *        // do some processing
-   *        lastSeenRow = row;
+   *        lastSeenRowKey = row.getKey();
    *     }
    * } while (paginator.advance(lastSeenRowKey));
    * }
@@ -437,6 +437,12 @@ public Query getNextQuery() { /** * Construct the next query. Return true if there are more queries to return. False if we've * read everything. + * + * @param lastSeenRowKey the key of the last row received across the pagination loop. Initialize + * this to {@link ByteString#EMPTY} before the loop and update it as each row is read (see + * {@link #createPaginator(int)}). If the first page is empty, {@link ByteString#EMPTY} + * signals completion; if a subsequent page is empty, passing the unchanged key from the + * previous page signals completion. */ public boolean advance(@Nonnull ByteString lastSeenRowKey) { Preconditions.checkNotNull( diff --git a/java-bigtable/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/internal/RowSetUtilTest.java b/java-bigtable/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/internal/RowSetUtilTest.java index 26609216e68b..25a0d3f8fd1b 100644 --- a/java-bigtable/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/internal/RowSetUtilTest.java +++ b/java-bigtable/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/internal/RowSetUtilTest.java @@ -16,6 +16,7 @@ package com.google.cloud.bigtable.data.v2.internal; import static com.google.common.truth.Truth.assertThat; +import static org.junit.Assert.assertThrows; import com.google.bigtable.v2.RowRange; import com.google.bigtable.v2.RowSet; @@ -344,4 +345,20 @@ public void eraseLargeRowEmptyRangeTest() { RowSet actual = RowSetUtil.eraseLargeRow(rowSet, key); assertThat(actual).isNull(); } + + @Test + public void testEraseEmptyKey() { + RowSet rowSet = RowSet.getDefaultInstance(); + assertThrows( + IllegalArgumentException.class, () -> RowSetUtil.erase(rowSet, ByteString.EMPTY, true)); + assertThrows( + IllegalArgumentException.class, () -> RowSetUtil.erase(rowSet, ByteString.EMPTY, false)); + } + + @Test + public void testEraseLargeRowEmptyKey() { + RowSet rowSet = RowSet.getDefaultInstance(); + assertThrows( + IllegalArgumentException.class, () -> RowSetUtil.eraseLargeRow(rowSet, ByteString.EMPTY)); + } }