Copilot commented on code in PR #18969:
URL: https://github.com/apache/pinot/pull/18969#discussion_r3561923292
##########
pinot-core/src/test/java/org/apache/pinot/core/segment/processing/genericrow/GenericRowSerDeTest.java:
##########
@@ -146,6 +150,46 @@ public void testCompare() {
}
}
+ @Test
+ public void testMvBytesAndBigDecimalCompare() {
+ // Exercises the non-zero branch of the MV BYTES / BIG_DECIMAL compare
paths (differing element and differing
+ // cardinality), which the byte-identical rows in testCompare never reach.
+ List<FieldSpec> fieldSpecs = Arrays.asList(new
DimensionFieldSpec("bytesMV", DataType.BYTES, false),
+ new DimensionFieldSpec("bigDecimalMV", DataType.BIG_DECIMAL, false));
+
+ GenericRow low = new GenericRow();
+ low.putValue("bytesMV", new Object[]{new byte[]{1, 2}, new byte[]{3}});
+ low.putValue("bigDecimalMV", new Object[]{new BigDecimal("1.5"), new
BigDecimal("2.5")});
+
+ // Differs from `low` only in the last element of each MV column, with a
larger value.
+ GenericRow high = new GenericRow();
+ high.putValue("bytesMV", new Object[]{new byte[]{1, 2}, new byte[]{4}});
+ high.putValue("bigDecimalMV", new Object[]{new BigDecimal("1.5"), new
BigDecimal("9.9")});
+
+ // Same last element but fewer values, to exercise the
differing-cardinality early return.
+ GenericRow shorter = new GenericRow();
+ shorter.putValue("bytesMV", new Object[]{new byte[]{1, 2}});
+ shorter.putValue("bigDecimalMV", new Object[]{new BigDecimal("1.5")});
+
+ for (int field = 0; field < fieldSpecs.size(); field++) {
+ assertTrue(compareRows(fieldSpecs, low, high, field + 1) < 0);
+ assertTrue(compareRows(fieldSpecs, high, low, field + 1) > 0);
+ assertTrue(compareRows(fieldSpecs, shorter, low, field + 1) < 0);
+ assertTrue(compareRows(fieldSpecs, low, shorter, field + 1) > 0);
+ }
+ }
+
+ private static int compareRows(List<FieldSpec> fieldSpecs, GenericRow row1,
GenericRow row2, int numFieldsToCompare) {
+ GenericRowSerializer serializer = new GenericRowSerializer(fieldSpecs,
false);
+ byte[] bytes1 = serializer.serialize(row1);
+ byte[] bytes2 = serializer.serialize(row2);
+ long numBytes = Math.max(bytes1.length, bytes2.length);
+ PinotDataBuffer dataBuffer = PinotDataBuffer.allocateDirect(numBytes * 2,
PinotDataBuffer.NATIVE_ORDER, null);
+ dataBuffer.readFrom(0L, bytes1);
+ dataBuffer.readFrom(numBytes, bytes2);
+ return new GenericRowDeserializer(dataBuffer, fieldSpecs,
false).compare(0L, numBytes, numFieldsToCompare);
+ }
Review Comment:
`compareRows()` allocates a direct `PinotDataBuffer` but never closes it.
Even though the allocation is small, leaking off-heap buffers across many tests
can increase flakiness and memory pressure; use try-with-resources (or a
finally block) to ensure `close()` is always called.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
