Copilot commented on code in PR #18969:
URL: https://github.com/apache/pinot/pull/18969#discussion_r3561669168
##########
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);
+ }
+ }
Review Comment:
The BIG_DECIMAL MV compare path is not actually exercised here because
`fieldSpecs` orders `bytesMV` first, and `low`/`high` already differ in
`bytesMV`, so `compare()` returns non-zero before it ever reaches the
BIG_DECIMAL field. As a result, this test only validates the MV BYTES compare
behavior.
To cover BIG_DECIMAL too, add an additional assertion block that compares
the same rows with the field order swapped (BIG_DECIMAL first), so the
BIG_DECIMAL MV branch (including differing-cardinality) is reached.
--
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]
