From 50680e904a3ad99f48b46c4bf8de2821ced66ed0 Mon Sep 17 00:00:00 2001 From: Igor Bernstein Date: Sat, 2 Feb 2019 17:57:12 -0500 Subject: [PATCH] Bigtable: make row & cell ordering more consistent. (#4421) * Bigtable: make row & cell ordering more consistent. * RowCells should always be ordered in lexicographically by family, then qualifier and finally by reverse chronological order * Although rows will always be ordered lexicographically by row key, they should not implement Comparable to avoid confusion when compareTo() == 0 and equals() is false. Instead that ordering was moved to a separate comparator. * Add helpers to filter cells by family & qualifier * tweaks * code style * code style --- .../data/v2/models/DefaultRowAdapter.java | 51 ++++++- .../cloud/bigtable/data/v2/models/Row.java | 143 +++++++++++++++--- .../bigtable/data/v2/models/RowCell.java | 27 ++++ .../v2/stub/ReadModifyWriteRowCallable.java | 29 ++-- .../data/v2/models/DefaultRowAdapterTest.java | 43 ++++++ .../bigtable/data/v2/models/RowCellTest.java | 101 +++++++++++++ .../bigtable/data/v2/models/RowTest.java | 78 +++++++++- .../stub/ReadModifyWriteRowCallableTest.java | 48 ++++++ 8 files changed, 480 insertions(+), 40 deletions(-) create mode 100644 google-cloud-clients/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/models/RowCellTest.java diff --git a/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/models/DefaultRowAdapter.java b/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/models/DefaultRowAdapter.java index 4c9d95ae01f0..efe56dc4188f 100644 --- a/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/models/DefaultRowAdapter.java +++ b/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/models/DefaultRowAdapter.java @@ -18,6 +18,8 @@ import com.google.common.collect.ImmutableList; import com.google.protobuf.ByteString; import java.util.List; +import java.util.Objects; +import java.util.TreeMap; /** * Default implementation of a {@link RowAdapter} that uses {@link Row}s to represent logical rows. @@ -44,13 +46,21 @@ public ByteString getKey(Row row) { /** {@inheritDoc} */ public class DefaultRowBuilder implements RowBuilder { private ByteString currentKey; - private ImmutableList.Builder cells; + private TreeMap> cellsByFamily; + private ImmutableList.Builder currentFamilyCells; + private String previousFamily; + private int totalCellCount; + private String family; private ByteString qualifier; private List labels; private long timestamp; private ByteString value; + public DefaultRowBuilder() { + reset(); + } + /** {@inheritDoc} */ @Override public Row createScanMarkerRow(ByteString key) { @@ -61,7 +71,6 @@ public Row createScanMarkerRow(ByteString key) { @Override public void startRow(ByteString key) { currentKey = key; - cells = ImmutableList.builder(); } /** {@inheritDoc} */ @@ -84,20 +93,52 @@ public void cellValue(ByteString value) { /** {@inheritDoc} */ @Override public void finishCell() { - cells.add(RowCell.create(family, qualifier, timestamp, labels, value)); + if (!Objects.equals(family, previousFamily)) { + previousFamily = family; + currentFamilyCells = ImmutableList.builder(); + cellsByFamily.put(family, currentFamilyCells); + } + + RowCell rowCell = RowCell.create(family, qualifier, timestamp, labels, value); + currentFamilyCells.add(rowCell); + totalCellCount++; } /** {@inheritDoc} */ @Override public Row finishRow() { - return Row.create(currentKey, cells.build()); + final ImmutableList sortedCells; + + // Optimization: If there are no cells, then just return the static empty list. + if (cellsByFamily.size() == 0) { + sortedCells = ImmutableList.of(); + } else if (cellsByFamily.size() == 1) { + // Optimization: If there is a single family, avoid copies and return that one list. + sortedCells = currentFamilyCells.build(); + } else { + // Normal path: concatenate the cells order by family. + ImmutableList.Builder sortedCellsBuilder = + ImmutableList.builderWithExpectedSize(totalCellCount); + + for (ImmutableList.Builder familyCells : cellsByFamily.values()) { + sortedCellsBuilder.addAll(familyCells.build()); + } + sortedCells = sortedCellsBuilder.build(); + } + + return Row.create(currentKey, sortedCells); } /** {@inheritDoc} */ @Override public void reset() { currentKey = null; - cells = null; + + cellsByFamily = new TreeMap<>(); + currentFamilyCells = null; + previousFamily = null; + totalCellCount = 0; + family = null; qualifier = null; labels = null; diff --git a/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/models/Row.java b/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/models/Row.java index a4ea6893548f..83b71bf19e9d 100644 --- a/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/models/Row.java +++ b/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/models/Row.java @@ -18,15 +18,38 @@ import com.google.api.core.InternalApi; import com.google.api.core.InternalExtensionOnly; import com.google.auto.value.AutoValue; +import com.google.cloud.bigtable.data.v2.internal.ByteStringComparator; +import com.google.common.base.Preconditions; +import com.google.common.collect.ImmutableList; import com.google.protobuf.ByteString; import java.io.Serializable; +import java.util.Comparator; import java.util.List; import javax.annotation.Nonnull; +import javax.annotation.Nullable; -/** Default representation of a logical row. */ +/** + * Default representation of a logical row. + * + *

The cells contained within, will be sorted by the native order. Please see {@link + * RowCell#compareByNative()} for details. + */ @InternalExtensionOnly @AutoValue -public abstract class Row implements Comparable, Serializable { +public abstract class Row implements Serializable { + /** + * Returns a comparator that compares two Row objects by comparing the result of {@link + * #getKey()}} for each. + */ + public static Comparator compareByKey() { + return new Comparator() { + @Override + public int compare(Row r1, Row r2) { + return ByteStringComparator.INSTANCE.compare(r1.getKey(), r2.getKey()); + } + }; + } + /** Creates a new instance of the {@link Row}. */ @InternalApi public static Row create(ByteString key, List cells) { @@ -38,30 +61,110 @@ public static Row create(ByteString key, List cells) { public abstract ByteString getKey(); /** - * Returns the list of cells. The cells will be clustered by their family and sorted by their - * qualifier. + * Returns a sorted list of cells. The cells will be sorted natively. + * + * @see RowCell#compareByNative() For details about the ordering. */ public abstract List getCells(); - /** Lexicographically compares this row's key to another row's key. */ - @Override - public int compareTo(@Nonnull Row row) { - int sizeA = getKey().size(); - int sizeB = row.getKey().size(); - int size = Math.min(sizeA, sizeB); - - for (int i = 0; i < size; i++) { - int byteA = getKey().byteAt(i) & 0xff; - int byteB = row.getKey().byteAt(i) & 0xff; - if (byteA == byteB) { - continue; + /** + * Returns a sublist of the cells that belong to the specified family. + * + * @see RowCell#compareByNative() For details about the ordering. + */ + public List getCells(@Nonnull String family) { + Preconditions.checkNotNull(family, "family"); + + int start = getFirst(family, null); + if (start < 0) { + return ImmutableList.of(); + } + + int end = getLast(family, null, start); + + return getCells().subList(start, end + 1); + } + + /** + * Returns a sublist of the cells that belong to the specified family and qualifier. + * + * @see RowCell#compareByNative() For details about the ordering. + */ + public List getCells(@Nonnull String family, @Nonnull String qualifier) { + Preconditions.checkNotNull(family, "family"); + Preconditions.checkNotNull(qualifier, "qualifier"); + + return getCells(family, ByteString.copyFromUtf8(qualifier)); + } + + /** + * Returns a sublist of the cells that belong to the specified family and qualifier. + * + * @see RowCell#compareByNative() For details about the ordering. + */ + public List getCells(@Nonnull String family, @Nonnull ByteString qualifier) { + Preconditions.checkNotNull(family, "family"); + Preconditions.checkNotNull(qualifier, "qualifier"); + + int start = getFirst(family, qualifier); + if (start < 0) { + return ImmutableList.of(); + } + + int end = getLast(family, qualifier, start); + + return getCells().subList(start, end + 1); + } + + private int getFirst(@Nonnull String family, @Nullable ByteString qualifier) { + int low = 0; + int high = getCells().size(); + int index = -1; + + while (low < high) { + int mid = (high + low) / 2; + RowCell midCell = getCells().get(mid); + + int c = midCell.getFamily().compareTo(family); + if (c == 0 && qualifier != null) { + c = ByteStringComparator.INSTANCE.compare(midCell.getQualifier(), qualifier); + } + + if (c < 0) { + low = mid + 1; + } else if (c == 0) { + index = mid; + high = mid; } else { - return byteA < byteB ? -1 : 1; + high = mid; } } - if (sizeA == sizeB) { - return 0; + return index; + } + + private int getLast(@Nonnull String family, @Nullable ByteString qualifier, int startIndex) { + int low = startIndex; + int high = getCells().size(); + int index = -1; + + while (low < high) { + int mid = (high + low) / 2; + RowCell midCell = getCells().get(mid); + + int c = midCell.getFamily().compareTo(family); + if (c == 0 && qualifier != null) { + c = ByteStringComparator.INSTANCE.compare(midCell.getQualifier(), qualifier); + } + + if (c < 0) { + low = mid + 1; + } else if (c == 0) { + index = mid; + low = mid + 1; + } else { + high = mid; + } } - return sizeA < sizeB ? -1 : 1; + return index; } } diff --git a/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/models/RowCell.java b/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/models/RowCell.java index 28f19c59e4b7..473c560b83a5 100644 --- a/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/models/RowCell.java +++ b/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/models/RowCell.java @@ -18,8 +18,11 @@ import com.google.api.core.InternalApi; import com.google.api.core.InternalExtensionOnly; import com.google.auto.value.AutoValue; +import com.google.cloud.bigtable.data.v2.internal.ByteStringComparator; +import com.google.common.collect.ComparisonChain; import com.google.protobuf.ByteString; import java.io.Serializable; +import java.util.Comparator; import java.util.List; import javax.annotation.Nonnull; @@ -27,6 +30,30 @@ @InternalExtensionOnly @AutoValue public abstract class RowCell implements Serializable { + /** + * A comparator that compares the cells by Bigtable native ordering: + * + *

    + *
  • Family lexicographically ascending + *
  • Qualifier lexicographically ascending + *
  • Timestamp in reverse chronological order + *
+ * + *

Labels and values are not included in the comparison. + */ + public static Comparator compareByNative() { + return new Comparator() { + @Override + public int compare(RowCell c1, RowCell c2) { + return ComparisonChain.start() + .compare(c1.getFamily(), c2.getFamily()) + .compare(c1.getQualifier(), c2.getQualifier(), ByteStringComparator.INSTANCE) + .compare(c2.getTimestamp(), c1.getTimestamp()) + .result(); + } + }; + } + /** Creates a new instance of the {@link RowCell}. */ @InternalApi public static RowCell create( diff --git a/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/ReadModifyWriteRowCallable.java b/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/ReadModifyWriteRowCallable.java index 314465a3d170..f456f0202bfa 100644 --- a/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/ReadModifyWriteRowCallable.java +++ b/google-cloud-clients/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/stub/ReadModifyWriteRowCallable.java @@ -26,22 +26,25 @@ import com.google.bigtable.v2.ReadModifyWriteRowRequest; import com.google.bigtable.v2.ReadModifyWriteRowResponse; import com.google.cloud.bigtable.data.v2.internal.RequestContext; +import com.google.cloud.bigtable.data.v2.models.DefaultRowAdapter; import com.google.cloud.bigtable.data.v2.models.ReadModifyWriteRow; import com.google.cloud.bigtable.data.v2.models.Row; -import com.google.cloud.bigtable.data.v2.models.RowCell; -import com.google.common.collect.ImmutableList; +import com.google.cloud.bigtable.data.v2.models.RowAdapter; +import com.google.cloud.bigtable.data.v2.models.RowAdapter.RowBuilder; import com.google.common.util.concurrent.MoreExecutors; /** Simple wrapper for ReadModifyWriteRow to wrap the request and response protobufs. */ class ReadModifyWriteRowCallable extends UnaryCallable { private final UnaryCallable inner; private final RequestContext requestContext; + private final RowAdapter rowAdapter; ReadModifyWriteRowCallable( UnaryCallable inner, RequestContext requestContext) { this.inner = inner; this.requestContext = requestContext; + this.rowAdapter = new DefaultRowAdapter(); } @Override @@ -61,21 +64,25 @@ public Row apply(ReadModifyWriteRowResponse readModifyWriteRowResponse) { } private Row convertResponse(ReadModifyWriteRowResponse response) { - ImmutableList.Builder cells = ImmutableList.builder(); + RowBuilder rowBuilder = rowAdapter.createRowBuilder(); + rowBuilder.startRow(response.getRow().getKey()); for (Family family : response.getRow().getFamiliesList()) { for (Column column : family.getColumnsList()) { for (Cell cell : column.getCellsList()) { - cells.add( - RowCell.create( - family.getName(), - column.getQualifier(), - cell.getTimestampMicros(), - cell.getLabelsList(), - cell.getValue())); + rowBuilder.startCell( + family.getName(), + column.getQualifier(), + cell.getTimestampMicros(), + cell.getLabelsList(), + cell.getValue().size()); + + rowBuilder.cellValue(cell.getValue()); + + rowBuilder.finishCell(); } } } - return Row.create(response.getRow().getKey(), cells.build()); + return rowBuilder.finishRow(); } } diff --git a/google-cloud-clients/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/models/DefaultRowAdapterTest.java b/google-cloud-clients/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/models/DefaultRowAdapterTest.java index a64d66414717..5adbfc40c158 100644 --- a/google-cloud-clients/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/models/DefaultRowAdapterTest.java +++ b/google-cloud-clients/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/models/DefaultRowAdapterTest.java @@ -128,4 +128,47 @@ public void markerRowTest() { assertThat(adapter.isScanMarkerRow(rowBuilder.finishRow())).isFalse(); } + + @Test + public void sortFamiliesAreSorted() { + ByteString col1 = ByteString.copyFromUtf8("col1"); + ByteString col2 = ByteString.copyFromUtf8("col2"); + List labels = ImmutableList.of(); + ByteString value1 = ByteString.copyFromUtf8("my-value1"); + ByteString value2 = ByteString.copyFromUtf8("my-value2"); + ByteString value3 = ByteString.copyFromUtf8("my-value3"); + ByteString value4 = ByteString.copyFromUtf8("my-value4"); + + rowBuilder.startRow(ByteString.copyFromUtf8("key1")); + + // family2 with 2 cells is received before family1 + rowBuilder.startCell("family2", col1, 1000, labels, value1.size()); + rowBuilder.cellValue(value1); + rowBuilder.finishCell(); + + rowBuilder.startCell("family2", col2, 1000, labels, value2.size()); + rowBuilder.cellValue(value2); + rowBuilder.finishCell(); + + // family1 with 2 cells is received after family2 cells + rowBuilder.startCell("family1", col1, 1000, ImmutableList.of(), value3.size()); + rowBuilder.cellValue(value3); + rowBuilder.finishCell(); + + rowBuilder.startCell("family1", col2, 1000, ImmutableList.of(), value4.size()); + rowBuilder.cellValue(value4); + rowBuilder.finishCell(); + + Row actualRow = rowBuilder.finishRow(); + + // The resulting the cells will reorder family1 first + assertThat(actualRow.getCells()) + .containsExactlyElementsIn( + ImmutableList.of( + RowCell.create("family1", col1, 1000, labels, value3), + RowCell.create("family1", col2, 1000, labels, value4), + RowCell.create("family2", col1, 1000, labels, value1), + RowCell.create("family2", col2, 1000, labels, value2))) + .inOrder(); + } } diff --git a/google-cloud-clients/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/models/RowCellTest.java b/google-cloud-clients/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/models/RowCellTest.java new file mode 100644 index 000000000000..96c78c2c33d6 --- /dev/null +++ b/google-cloud-clients/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/models/RowCellTest.java @@ -0,0 +1,101 @@ +/* + * Copyright 2019 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.google.cloud.bigtable.data.v2.models; + +import static com.google.common.truth.Truth.*; + +import com.google.common.collect.ImmutableList; +import com.google.protobuf.ByteString; +import java.util.Comparator; +import java.util.List; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +@RunWith(JUnit4.class) +public class RowCellTest { + + @Test + public void compareTest() { + ByteString col1 = ByteString.copyFromUtf8("col1"); + long timestamp1 = 1_000; + List labels1 = ImmutableList.of("label1"); + ByteString value1 = ByteString.copyFromUtf8("value1"); + + ByteString col2 = ByteString.copyFromUtf8("col2"); + long timestamp2 = 2_000; + List labels2 = ImmutableList.of("label2"); + ByteString value2 = ByteString.copyFromUtf8("value2"); + + Comparator c = RowCell.compareByNative(); + + // equal + assertThat( + c.compare( + RowCell.create("family1", col1, timestamp1, labels1, value1), + RowCell.create("family1", col1, timestamp1, labels1, value1))) + .isEqualTo(0); + + // equal ignores labels & values + assertThat( + c.compare( + RowCell.create("family1", col1, timestamp1, labels1, value1), + RowCell.create("family1", col1, timestamp1, labels2, value2))) + .isEqualTo(0); + + // family lesser then + assertThat( + c.compare( + RowCell.create("family1", col1, timestamp1, labels1, value1), + RowCell.create("family2", col1, timestamp1, labels1, value1))) + .isEqualTo(-1); + + // family greater then + assertThat( + c.compare( + RowCell.create("family2", col1, timestamp1, labels1, value1), + RowCell.create("family1", col1, timestamp1, labels1, value1))) + .isEqualTo(1); + + // col lesser then + assertThat( + c.compare( + RowCell.create("family1", col1, timestamp1, labels1, value1), + RowCell.create("family1", col2, timestamp1, labels1, value1))) + .isEqualTo(-1); + + // col greater then + assertThat( + c.compare( + RowCell.create("family1", col2, timestamp1, labels1, value1), + RowCell.create("family1", col1, timestamp1, labels1, value1))) + .isEqualTo(1); + + // reverse timestamp lesser then + assertThat( + c.compare( + RowCell.create("family1", col1, timestamp2, labels1, value1), + RowCell.create("family1", col1, timestamp1, labels1, value1))) + .isEqualTo(-1); + + // reverse timestamp greater then + assertThat( + c.compare( + RowCell.create("family1", col1, timestamp1, labels1, value1), + RowCell.create("family1", col1, timestamp2, labels1, value1))) + .isEqualTo(1); + } +} diff --git a/google-cloud-clients/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/models/RowTest.java b/google-cloud-clients/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/models/RowTest.java index 19bad91456bc..b361ff2d0a3b 100644 --- a/google-cloud-clients/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/models/RowTest.java +++ b/google-cloud-clients/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/models/RowTest.java @@ -24,6 +24,7 @@ import java.io.IOException; import java.io.ObjectInputStream; import java.io.ObjectOutputStream; +import java.util.List; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -63,12 +64,11 @@ public void compareTest() { ImmutableList.of(), ByteString.copyFromUtf8("value")))); - assertThat(row1).isEqualTo(row1); - assertThat(row1).isLessThan(row2); - assertThat(row2).isGreaterThan(row1); + assertThat(Row.compareByKey().compare(row1, row2)).isEqualTo(-1); + assertThat(Row.compareByKey().compare(row2, row1)).isEqualTo(1); // Comparator only cares about row keys - assertThat(row2).isEquivalentAccordingToCompareTo(row2b); + assertThat(Row.compareByKey().compare(row2, row2b)).isEqualTo(0); } @Test @@ -92,4 +92,74 @@ public void serializationTest() throws IOException, ClassNotFoundException { ObjectInputStream ois = new ObjectInputStream(new ByteArrayInputStream(bos.toByteArray())); assertThat(ois.readObject()).isEqualTo(expected); } + + @Test + public void getFamilyCellsTest() { + ByteString col1 = ByteString.copyFromUtf8("col1"); + ByteString col2 = ByteString.copyFromUtf8("col2"); + + List labels = ImmutableList.of(); + ByteString value = ByteString.EMPTY; + + Row row = + Row.create( + ByteString.copyFromUtf8("ignored-key"), + ImmutableList.of( + RowCell.create("family1", col1, 1_000, labels, value), + RowCell.create("family1", col2, 1_000, labels, value), + RowCell.create("family2", col1, 1_000, labels, value), + RowCell.create("family4", col1, 1_000, labels, value))); + + assertThat(row.getCells("family1")) + .containsExactly( + RowCell.create("family1", col1, 1_000, labels, value), + RowCell.create("family1", col2, 1_000, labels, value)) + .inOrder(); + + assertThat(row.getCells("family2")) + .containsExactly(RowCell.create("family2", col1, 1_000, labels, value)); + + assertThat(row.getCells("family3")).isEmpty(); + + assertThat(row.getCells("family4")) + .containsExactly(RowCell.create("family4", col1, 1_000, labels, value)); + } + + @Test + public void getQualifierCellsTest() { + ByteString col1 = ByteString.copyFromUtf8("col1"); + ByteString col2 = ByteString.copyFromUtf8("col2"); + + List labels = ImmutableList.of(); + ByteString value = ByteString.EMPTY; + + Row row = + Row.create( + ByteString.copyFromUtf8("ignored-key"), + ImmutableList.of( + RowCell.create("family1", col1, 1_000, labels, value), + RowCell.create("family1", col2, 2_000, labels, value), + RowCell.create("family1", col2, 1_000, labels, value), + RowCell.create("family2", col1, 1_000, labels, value), + RowCell.create("family4", col1, 1_000, labels, value))); + + assertThat(row.getCells("family1", col1)) + .containsExactly(RowCell.create("family1", col1, 1_000, labels, value)); + + assertThat(row.getCells("family1", col2)) + .containsExactly( + RowCell.create("family1", col2, 1_000, labels, value), + RowCell.create("family1", col2, 2_000, labels, value)); + + assertThat(row.getCells("family2", col1)) + .containsExactly(RowCell.create("family2", col1, 1_000, labels, value)); + + assertThat(row.getCells("family2", col2)).isEmpty(); + + assertThat(row.getCells("family3", col1)).isEmpty(); + assertThat(row.getCells("family3", col2)).isEmpty(); + + assertThat(row.getCells("family4", col1)) + .containsExactly(RowCell.create("family4", col1, 1_000, labels, value)); + } } diff --git a/google-cloud-clients/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/ReadModifyWriteRowCallableTest.java b/google-cloud-clients/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/ReadModifyWriteRowCallableTest.java index 76dabd159303..4a8f857d0530 100644 --- a/google-cloud-clients/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/ReadModifyWriteRowCallableTest.java +++ b/google-cloud-clients/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/ReadModifyWriteRowCallableTest.java @@ -114,6 +114,54 @@ public void responseCorrectlyTransformed() throws Exception { ByteString.copyFromUtf8("suffix"))))); } + @Test + public void responseSortsFamilies() throws Exception { + ByteString col = ByteString.copyFromUtf8("col1"); + ByteString value1 = ByteString.copyFromUtf8("value1"); + ByteString value2 = ByteString.copyFromUtf8("value2"); + + ApiFuture result = + callable.futureCall( + ReadModifyWriteRow.create("my-table", "my-key").append("my-family", "col", "suffix")); + + inner.response.set( + ReadModifyWriteRowResponse.newBuilder() + .setRow( + com.google.bigtable.v2.Row.newBuilder() + .setKey(ByteString.copyFromUtf8("my-key")) + // family2 is out of order + .addFamilies( + Family.newBuilder() + .setName("family2") + .addColumns( + Column.newBuilder() + .setQualifier(col) + .addCells( + Cell.newBuilder() + .setTimestampMicros(1_000) + .setValue(value2)))) + .addFamilies( + Family.newBuilder() + .setName("family1") + .addColumns( + Column.newBuilder() + .setQualifier(col) + .addCells( + Cell.newBuilder() + .setTimestampMicros(1_000) + .setValue(value1))) + .build())) + .build()); + + assertThat(result.get(1, TimeUnit.SECONDS)) + .isEqualTo( + Row.create( + ByteString.copyFromUtf8("my-key"), + ImmutableList.of( + RowCell.create("family1", col, 1_000, ImmutableList.of(), value1), + RowCell.create("family2", col, 1_000, ImmutableList.of(), value2)))); + } + @Test public void errorIsPropagated() throws Exception { ApiFuture result =