This is an automated email from the ASF dual-hosted git repository.
xiangfu0 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pinot.git
The following commit(s) were added to refs/heads/master by this push:
new 66fca21494e Reuse fixed-row buffers for DataTable result construction
(#19533)
66fca21494e is described below
commit 66fca21494e2070fad854e10bf8a98e383e53174
Author: Xiang Fu <[email protected]>
AuthorDate: Mon Sep 14 00:22:12 2026 -0700
Reuse fixed-row buffers for DataTable result construction (#19533)
* Reuse fixed-row buffers for DataTable result construction
Building multi-row DataTables creates a short-lived fixed-row buffer for
every row. Reuse the builder-owned buffer while resetting its contents and
state so partially written rows retain their existing wire representation.
* Remove redundant DataTable row buffer resets
Allocate the row scratch buffer once in the constructor after verifying all
production callers populate every column. Document the complete-row contract
and cover reverse-order writes, empty values, and explicit nulls in serde.
Reproduction:
Construct a multi-row V4 DataTable. The previous startRow implementation
cleared the entire scratch buffer and reset its state even though every
setter seeks to and overwrites a complete column slot.
Validation:
83 focused serde, selection, and GROUP BY tests passed on JDK 25.
Spotless, Checkstyle, and license checks passed. Scoped lint compilation
passed with no new-line warnings; full warning-enabled reactor compilation
remains blocked by an unchanged missing JetBrains NotNull dependency.
* Make DataTable row size a constructor-local variable
The reused row buffer only needs the computed row size while it is
allocated.
Remove the unused instance field and keep rowSizeInBytes local to the
constructor, addressing the remaining review comment.
Validation: 10 DataTableSerDeTest cases passed on JDK 25.
---
.../common/datatable/BaseDataTableBuilder.java | 9 +--
.../core/common/datatable/DataTableBuilder.java | 1 +
.../core/common/datatable/DataTableSerDeTest.java | 66 ++++++++++++++++++++++
3 files changed, 72 insertions(+), 4 deletions(-)
diff --git
a/pinot-core/src/main/java/org/apache/pinot/core/common/datatable/BaseDataTableBuilder.java
b/pinot-core/src/main/java/org/apache/pinot/core/common/datatable/BaseDataTableBuilder.java
index eba77e37300..441a6e4b70b 100644
---
a/pinot-core/src/main/java/org/apache/pinot/core/common/datatable/BaseDataTableBuilder.java
+++
b/pinot-core/src/main/java/org/apache/pinot/core/common/datatable/BaseDataTableBuilder.java
@@ -64,7 +64,7 @@ public abstract class BaseDataTableBuilder implements
DataTableBuilder {
protected final DataSchema _dataSchema;
protected final int _version;
protected final int[] _columnOffsets;
- protected final int _rowSizeInBytes;
+ protected final ByteBuffer _currentRowDataByteBuffer;
protected final ByteArrayOutputStream _fixedSizeDataByteArrayOutputStream =
new ByteArrayOutputStream();
protected final DataOutputStream _fixedSizeDataOutputStream =
new DataOutputStream(_fixedSizeDataByteArrayOutputStream);
@@ -81,13 +81,15 @@ public abstract class BaseDataTableBuilder implements
DataTableBuilder {
private int _nullRowIdsColId;
protected int _numRows;
- protected ByteBuffer _currentRowDataByteBuffer;
public BaseDataTableBuilder(DataSchema dataSchema, int version) {
_dataSchema = dataSchema;
_version = version;
_columnOffsets = new int[dataSchema.size()];
- _rowSizeInBytes = DataTableUtils.computeColumnOffsets(dataSchema,
_columnOffsets, _version);
+ int rowSizeInBytes = DataTableUtils.computeColumnOffsets(dataSchema,
_columnOffsets, _version);
+ // Every column is populated for each row, and each setter positions the
buffer at its column offset.
+ // finishRow() copies the bytes, so the same buffer can be reused without
clearing or resetting it.
+ _currentRowDataByteBuffer = ByteBuffer.allocate(rowSizeInBytes);
_storedColumnDataTypes = dataSchema.getStoredColumnDataTypes();
_nullBitmaps = new RoaringBitmap[dataSchema.size()];
}
@@ -95,7 +97,6 @@ public abstract class BaseDataTableBuilder implements
DataTableBuilder {
@Override
public void startRow() {
_numRows++;
- _currentRowDataByteBuffer = ByteBuffer.allocate(_rowSizeInBytes);
}
@Override
diff --git
a/pinot-core/src/main/java/org/apache/pinot/core/common/datatable/DataTableBuilder.java
b/pinot-core/src/main/java/org/apache/pinot/core/common/datatable/DataTableBuilder.java
index 7682b225147..6b0b54e8bc3 100644
---
a/pinot-core/src/main/java/org/apache/pinot/core/common/datatable/DataTableBuilder.java
+++
b/pinot-core/src/main/java/org/apache/pinot/core/common/datatable/DataTableBuilder.java
@@ -47,6 +47,7 @@ import org.roaringbitmap.RoaringBitmap;
@InterfaceStability.Evolving
public interface DataTableBuilder {
+ /// Starts a row. Callers must set every column, using [#setNull] for null
values, before calling [#finishRow].
void startRow();
void setColumn(int colId, int value);
diff --git
a/pinot-core/src/test/java/org/apache/pinot/core/common/datatable/DataTableSerDeTest.java
b/pinot-core/src/test/java/org/apache/pinot/core/common/datatable/DataTableSerDeTest.java
index 91e46c537c9..521d2082916 100644
---
a/pinot-core/src/test/java/org/apache/pinot/core/common/datatable/DataTableSerDeTest.java
+++
b/pinot-core/src/test/java/org/apache/pinot/core/common/datatable/DataTableSerDeTest.java
@@ -32,6 +32,7 @@ import org.apache.pinot.common.datatable.DataTable;
import org.apache.pinot.common.datatable.DataTable.MetadataKey;
import org.apache.pinot.common.datatable.DataTableFactory;
import org.apache.pinot.common.utils.DataSchema;
+import org.apache.pinot.common.utils.DataSchema.ColumnDataType;
import org.apache.pinot.spi.accounting.ThreadResourceUsageProvider;
import org.apache.pinot.spi.exception.QueryErrorCode;
import org.apache.pinot.spi.utils.ByteArray;
@@ -75,6 +76,71 @@ public class DataTableSerDeTest {
private static final BigDecimal[][] BIG_DECIMAL_ARRAYS = new
BigDecimal[NUM_ROWS][];
private static final Map<String, Object>[] MAPS = new Map[NUM_ROWS];
+ @Test
+ public void testRowBufferReuse()
+ throws IOException {
+ DataSchema schema = new DataSchema(new String[]{"int", "long", "float",
"double", "string", "bytes", "array"},
+ new ColumnDataType[]{ColumnDataType.INT, ColumnDataType.LONG,
ColumnDataType.FLOAT, ColumnDataType.DOUBLE,
+ ColumnDataType.STRING, ColumnDataType.BYTES,
ColumnDataType.INT_ARRAY});
+ DataTableBuilder builder = new DataTableBuilderV4(schema);
+ builder.startRow();
+ builder.setColumn(0, Integer.MIN_VALUE);
+ builder.setColumn(1, Long.MAX_VALUE);
+ builder.setColumn(2, Float.intBitsToFloat(0x7fc00042));
+ builder.setColumn(3, -0.0d);
+ builder.setColumn(4, "first");
+ builder.setColumn(5, new ByteArray(new byte[]{1, 2, 3}));
+ builder.setColumn(6, new int[]{17, -23});
+ builder.finishRow();
+
+ // Populate every column in reverse order, overwriting both fixed values
and variable-data offset/length slots.
+ builder.startRow();
+ builder.setColumn(6, new int[0]);
+ builder.setColumn(5, new ByteArray(new byte[0]));
+ builder.setColumn(4, "second");
+ builder.setColumn(3, 0.0d);
+ builder.setColumn(2, 0.0f);
+ builder.setColumn(1, 0L);
+ builder.setColumn(0, 0);
+ builder.finishRow();
+
+ builder.startRow();
+ builder.setNull(0);
+ builder.setColumn(1, -11L);
+ builder.setColumn(2, -0.0f);
+ builder.setColumn(3, Double.longBitsToDouble(0x7ff8000000000042L));
+ builder.setNull(4);
+ builder.setColumn(5, new ByteArray(new byte[]{4}));
+ builder.setColumn(6, new int[]{5});
+ builder.finishRow();
+
+ DataTable result =
DataTableFactory.getDataTable(builder.build().toBytes());
+ assertEquals(result.getNumberOfRows(), 3);
+ assertEquals(result.getInt(0, 0), Integer.MIN_VALUE);
+ assertEquals(result.getLong(0, 1), Long.MAX_VALUE);
+ assertEquals(Float.floatToRawIntBits(result.getFloat(0, 2)), 0x7fc00042);
+ assertEquals(Double.doubleToRawLongBits(result.getDouble(0, 3)),
Double.doubleToRawLongBits(-0.0d));
+ assertEquals(result.getString(0, 4), "first");
+ assertEquals(result.getBytes(0, 5), new ByteArray(new byte[]{1, 2, 3}));
+ assertEquals(result.getIntArray(0, 6), new int[]{17, -23});
+
+ assertEquals(result.getInt(1, 0), 0);
+ assertEquals(result.getLong(1, 1), 0L);
+ assertEquals(Float.floatToRawIntBits(result.getFloat(1, 2)), 0);
+ assertEquals(Double.doubleToRawLongBits(result.getDouble(1, 3)), 0L);
+ assertEquals(result.getString(1, 4), "second");
+ assertEquals(result.getBytes(1, 5).getBytes(), new byte[0]);
+ assertEquals(result.getIntArray(1, 6), new int[0]);
+
+ assertEquals(result.getNullRowIds(0), RoaringBitmap.bitmapOf(2));
+ assertEquals(result.getLong(2, 1), -11L);
+ assertEquals(Float.floatToRawIntBits(result.getFloat(2, 2)),
Float.floatToRawIntBits(-0.0f));
+ assertEquals(Double.doubleToRawLongBits(result.getDouble(2, 3)),
0x7ff8000000000042L);
+ assertEquals(result.getNullRowIds(4), RoaringBitmap.bitmapOf(2));
+ assertEquals(result.getBytes(2, 5), new ByteArray(new byte[]{4}));
+ assertEquals(result.getIntArray(2, 6), new int[]{5});
+ }
+
@Test(dataProvider = "versionProvider")
public void testException(int dataTableVersion)
throws IOException {
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
