This is an automated email from the ASF dual-hosted git repository.
yashmayya 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 2103aa27006 Preserve millisecond precision for TIMESTAMP comparisons
in the multi-stage engine (#18883)
2103aa27006 is described below
commit 2103aa27006bcfe63490623063de952cb424c46f
Author: Yash Mayya <[email protected]>
AuthorDate: Mon Jun 29 16:14:23 2026 -0700
Preserve millisecond precision for TIMESTAMP comparisons in the multi-stage
engine (#18883)
---
.../tests/MultiStageEngineIntegrationTest.java | 10 +++----
.../tests/OfflineClusterIntegrationTest.java | 6 ++--
.../integration/tests/custom/TimestampTest.java | 26 ++++++++++++++++++
.../org/apache/pinot/query/type/TypeSystem.java | 15 +++++++++-
.../query/validate/PinotTypeCoercionTest.java | 32 ++++++++++++++++++++--
.../resources/queries/LiteralEvaluationPlans.json | 6 ++--
6 files changed, 81 insertions(+), 14 deletions(-)
diff --git
a/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/MultiStageEngineIntegrationTest.java
b/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/MultiStageEngineIntegrationTest.java
index c7183143d22..479c537d25d 100644
---
a/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/MultiStageEngineIntegrationTest.java
+++
b/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/MultiStageEngineIntegrationTest.java
@@ -1003,9 +1003,9 @@ public class MultiStageEngineIntegrationTest extends
BaseClusterIntegrationTestS
JsonNode results = resultTable.get("rows").get(0);
assertEquals(results.get(0).asInt(), 1);
long nowResult = results.get(1).asLong();
- // Timestamp granularity is seconds
- assertTrue(nowResult >= ((queryStartTimeMs / 1000) * 1000));
- assertTrue(nowResult <= ((queryEndTimeMs / 1000) * 1000));
+ // now() returns millisecond-precision epoch millis, consistent with the
single-stage engine (issue #18881)
+ assertTrue(nowResult >= queryStartTimeMs);
+ assertTrue(nowResult <= queryEndTimeMs);
long oneHourAgoResult = results.get(2).asLong();
assertTrue(oneHourAgoResult >= queryStartTimeMs -
TimeUnit.HOURS.toMillis(1));
assertTrue(oneHourAgoResult <= queryEndTimeMs -
TimeUnit.HOURS.toMillis(1));
@@ -1017,8 +1017,8 @@ public class MultiStageEngineIntegrationTest extends
BaseClusterIntegrationTestS
String dateTimeResult = results.get(4).asText();
assertTrue(dateTimeResult.equals(queryStartTimeDay) ||
dateTimeResult.equals(queryEndTimeDay));
nowResult = results.get(5).asLong();
- assertTrue(nowResult >= ((queryStartTimeMs / 1000) * 1000));
- assertTrue(nowResult <= ((queryEndTimeMs / 1000) * 1000));
+ assertTrue(nowResult >= queryStartTimeMs);
+ assertTrue(nowResult <= queryEndTimeMs);
oneHourAgoResult = results.get(6).asLong();
assertTrue(oneHourAgoResult >= queryStartTimeMs -
TimeUnit.HOURS.toMillis(1));
assertTrue(oneHourAgoResult <= queryEndTimeMs -
TimeUnit.HOURS.toMillis(1));
diff --git
a/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/OfflineClusterIntegrationTest.java
b/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/OfflineClusterIntegrationTest.java
index fc92bdd18a5..f68d9ef01c9 100644
---
a/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/OfflineClusterIntegrationTest.java
+++
b/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/OfflineClusterIntegrationTest.java
@@ -1440,9 +1440,9 @@ public class OfflineClusterIntegrationTest extends
BaseClusterIntegrationTestSet
JsonNode results = resultTable.get("rows").get(0);
assertEquals(results.get(0).asInt(), 1);
long nowResult = results.get(1).asLong();
- // Timestamp granularity is seconds
- assertTrue(nowResult >= ((queryStartTimeMs / 1000) * 1000));
- assertTrue(nowResult <= ((queryEndTimeMs / 1000) * 1000));
+ // now() returns millisecond-precision epoch millis, consistent with the
single-stage engine (issue #18881)
+ assertTrue(nowResult >= queryStartTimeMs);
+ assertTrue(nowResult <= queryEndTimeMs);
long oneHourAgoResult = results.get(2).asLong();
assertTrue(oneHourAgoResult >= queryStartTimeMs -
TimeUnit.HOURS.toMillis(1));
assertTrue(oneHourAgoResult <= queryEndTimeMs -
TimeUnit.HOURS.toMillis(1));
diff --git
a/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/custom/TimestampTest.java
b/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/custom/TimestampTest.java
index f218edf6423..9578e516c48 100644
---
a/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/custom/TimestampTest.java
+++
b/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/custom/TimestampTest.java
@@ -51,6 +51,7 @@ public class TimestampTest extends
CustomDataQueryClusterIntegrationTest {
private static final String TIMESTAMP_ONE_MONTH_AFTER = "tsOneMonthAfter";
// 1 month after TIMESTAMP_BASE
private static final String TIMESTAMP_ONE_QUARTER_AFTER =
"tsOneQuarterAfter"; // 1 quarter after TIMESTAMP_BASE
private static final String TIMESTAMP_ONE_YEAR_AFTER = "tsOneYearAfter"; //
1 year after TIMESTAMP_BASE
+ private static final String TIMESTAMP_SUB_SECOND = "tsSubSecond"; //
TIMESTAMP_BASE + 482 millis (sub-second)
private static final String LONG_BASE = "longBase";
private static final String LONG_HALF_DAY_AFTER = "longHalfDayAfter";
private static final String LONG_ONE_DAY_AFTER = "longOneDayAfter";
@@ -433,6 +434,27 @@ public class TimestampTest extends
CustomDataQueryClusterIntegrationTest {
assertEquals(jsonNode.get("resultTable").get("rows").get(0).get(1).longValue(),
1546300800000L);
}
+ @Test(dataProvider = "useBothQueryEngines")
+ public void testSubSecondTimestampEqualityQueries(boolean
useMultiStageQueryEngine)
+ throws Exception {
+ setUseMultiStageQueryEngine(useMultiStageQueryEngine);
+ // Regression test for issue #18881: equality between a TIMESTAMP column
and a sub-second epoch-millis literal must
+ // match rows. 1546300800482 == 2019-01-01 00:00:00.482 UTC, the
tsSubSecond value of the first ingested row
+ // (tsBase + 482 ms). Before the fix, the multi-stage planner cast the
literal to a precision-0 TIMESTAMP and
+ // constant folding truncated it to whole seconds (1546300800000), so this
query returned 0 rows.
+ String matchQuery =
+ String.format("SELECT COUNT(*) FROM %s WHERE %s = 1546300800482",
getTableName(), TIMESTAMP_SUB_SECOND);
+ JsonNode jsonNode = postQuery(matchQuery);
+
assertEquals(jsonNode.get("resultTable").get("rows").get(0).get(0).asLong(),
1L);
+
+ // The whole-second (truncated) literal must NOT match the sub-second row
- guards against a regression to the old
+ // precision-0 truncation behavior.
+ String truncatedQuery =
+ String.format("SELECT COUNT(*) FROM %s WHERE %s = 1546300800000",
getTableName(), TIMESTAMP_SUB_SECOND);
+ jsonNode = postQuery(truncatedQuery);
+
assertEquals(jsonNode.get("resultTable").get("rows").get(0).get(0).asLong(),
0L);
+ }
+
@Override
public String getTableName() {
return DEFAULT_TABLE_NAME;
@@ -448,6 +470,7 @@ public class TimestampTest extends
CustomDataQueryClusterIntegrationTest {
.addSingleValueDimension(TIMESTAMP_ONE_MONTH_AFTER,
FieldSpec.DataType.TIMESTAMP)
.addSingleValueDimension(TIMESTAMP_ONE_QUARTER_AFTER,
FieldSpec.DataType.TIMESTAMP)
.addSingleValueDimension(TIMESTAMP_ONE_YEAR_AFTER,
FieldSpec.DataType.TIMESTAMP)
+ .addSingleValueDimension(TIMESTAMP_SUB_SECOND,
FieldSpec.DataType.TIMESTAMP)
.addSingleValueDimension(LONG_BASE, FieldSpec.DataType.LONG)
.addSingleValueDimension(LONG_HALF_DAY_AFTER, FieldSpec.DataType.LONG)
.addSingleValueDimension(LONG_ONE_DAY_AFTER, FieldSpec.DataType.LONG)
@@ -472,6 +495,7 @@ public class TimestampTest extends
CustomDataQueryClusterIntegrationTest {
new Field(TIMESTAMP_ONE_MONTH_AFTER, create(Type.LONG), null, null),
new Field(TIMESTAMP_ONE_QUARTER_AFTER, create(Type.LONG), null, null),
new Field(TIMESTAMP_ONE_YEAR_AFTER, create(Type.LONG), null, null),
+ new Field(TIMESTAMP_SUB_SECOND, create(Type.LONG), null, null),
new Field(LONG_BASE, create(Type.LONG), null, null),
new Field(LONG_HALF_DAY_AFTER, create(Type.LONG), null, null),
new Field(LONG_ONE_DAY_AFTER, create(Type.LONG), null, null),
@@ -510,6 +534,8 @@ public class TimestampTest extends
CustomDataQueryClusterIntegrationTest {
record.put(TIMESTAMP_ONE_MONTH_AFTER, tsOneMonthAfter);
record.put(TIMESTAMP_ONE_QUARTER_AFTER, tsOneQuarterAfter);
record.put(TIMESTAMP_ONE_YEAR_AFTER, tsOneYearAfter);
+ // Sub-second value (tsBase + 482 ms) for the issue #18881
millisecond-precision regression test.
+ record.put(TIMESTAMP_SUB_SECOND, tsBaseLong + 482);
record.put(LONG_BASE, tsBaseLong);
record.put(LONG_HALF_DAY_AFTER, tsHalfDayAfter);
record.put(LONG_ONE_DAY_AFTER, tsOneDayAfter);
diff --git
a/pinot-query-planner/src/main/java/org/apache/pinot/query/type/TypeSystem.java
b/pinot-query-planner/src/main/java/org/apache/pinot/query/type/TypeSystem.java
index 814f1b4bf5c..b078ddabfce 100644
---
a/pinot-query-planner/src/main/java/org/apache/pinot/query/type/TypeSystem.java
+++
b/pinot-query-planner/src/main/java/org/apache/pinot/query/type/TypeSystem.java
@@ -34,6 +34,10 @@ public class TypeSystem extends RelDataTypeSystemImpl {
private static final int MAX_DECIMAL_SCALE = 1000;
private static final int MAX_DECIMAL_PRECISION = 2000;
+ // Pinot stores TIMESTAMP as epoch-millis (LONG), i.e. millisecond precision
(3 fractional digits). This is also
+ // Calcite's MAX_DATETIME_PRECISION, so 3 is both the default and the max
for TIMESTAMP and no clamping occurs.
+ private static final int TIMESTAMP_PRECISION_MILLIS = 3;
+
/**
* Default precision for derived arithmetic decimal
types(plus/multiply/divide/mod). We won't allow the return
* precision to be larger than this value majorly due to the following
reasons:
@@ -84,7 +88,16 @@ public class TypeSystem extends RelDataTypeSystemImpl {
@Override
public int getDefaultPrecision(SqlTypeName typeName) {
- return typeName == SqlTypeName.DECIMAL ? MAX_DECIMAL_PRECISION :
super.getDefaultPrecision(typeName);
+ if (typeName == SqlTypeName.DECIMAL) {
+ return MAX_DECIMAL_PRECISION;
+ }
+ if (typeName == SqlTypeName.TIMESTAMP) {
+ // Calcite's default TIMESTAMP precision is 0 (whole seconds), which
truncates sub-second epoch-millis literals
+ // during constant folding (RexBuilder.clean -> TimestampString.round).
Pin it to millisecond precision so folded
+ // TIMESTAMP literals preserve millis and round-trip Pinot's LONG
storage. See issue #18881.
+ return TIMESTAMP_PRECISION_MILLIS;
+ }
+ return super.getDefaultPrecision(typeName);
}
@Override
diff --git
a/pinot-query-planner/src/test/java/org/apache/pinot/query/validate/PinotTypeCoercionTest.java
b/pinot-query-planner/src/test/java/org/apache/pinot/query/validate/PinotTypeCoercionTest.java
index 89285a1a0bc..6a771e266f3 100644
---
a/pinot-query-planner/src/test/java/org/apache/pinot/query/validate/PinotTypeCoercionTest.java
+++
b/pinot-query-planner/src/test/java/org/apache/pinot/query/validate/PinotTypeCoercionTest.java
@@ -120,7 +120,11 @@ public class PinotTypeCoercionTest extends
QueryEnvironmentTestBase {
String plan = explain("SELECT ts_timestamp FROM a WHERE ts_timestamp >
NOW() - 1000");
assertFalse(plan.contains("CAST(" + TS_TIMESTAMP_ORD + ")"),
"TIMESTAMP column should not be wrapped in CAST when right-hand side
is constant. Got:\n" + plan);
- assertTrue(plan.matches("(?s).*>\\(\\" + TS_TIMESTAMP_ORD + ",
\\d{4}-\\d{2}-\\d{2} \\d{2}:\\d{2}:\\d{2}\\).*"),
+ // The fractional part is optional here: NOW()/ago() produce
non-deterministic millis that may land on a whole
+ // second (rendered without a fractional part). The deterministic
millis-preservation guard is in
+ // testTimestampColumnVsSubSecondLiteralPreservesMillis below.
+ assertTrue(
+ plan.matches("(?s).*>\\(\\" + TS_TIMESTAMP_ORD + ",
\\d{4}-\\d{2}-\\d{2} \\d{2}:\\d{2}:\\d{2}(\\.\\d+)?\\).*"),
"Right-hand side should be constant-folded to a TIMESTAMP literal.
Got:\n" + plan);
}
@@ -174,7 +178,11 @@ public class PinotTypeCoercionTest extends
QueryEnvironmentTestBase {
String plan = explain("SELECT ts_timestamp FROM a WHERE ts_timestamp >
ago('PT5M')");
assertFalse(plan.contains("CAST(" + TS_TIMESTAMP_ORD + ")"),
"TIMESTAMP column should not be wrapped in CAST when compared to
ago(...). Got:\n" + plan);
- assertTrue(plan.matches("(?s).*>\\(\\" + TS_TIMESTAMP_ORD + ",
\\d{4}-\\d{2}-\\d{2} \\d{2}:\\d{2}:\\d{2}\\).*"),
+ // The fractional part is optional here: NOW()/ago() produce
non-deterministic millis that may land on a whole
+ // second (rendered without a fractional part). The deterministic
millis-preservation guard is in
+ // testTimestampColumnVsSubSecondLiteralPreservesMillis below.
+ assertTrue(
+ plan.matches("(?s).*>\\(\\" + TS_TIMESTAMP_ORD + ",
\\d{4}-\\d{2}-\\d{2} \\d{2}:\\d{2}:\\d{2}(\\.\\d+)?\\).*"),
"Right-hand side should be constant-folded to a TIMESTAMP literal.
Got:\n" + plan);
}
@@ -190,4 +198,24 @@ public class PinotTypeCoercionTest extends
QueryEnvironmentTestBase {
assertTrue(plan.contains("CAST(" + TS_TIMESTAMP_ORD + "):BIGINT"),
"TIMESTAMP column must be cast to BIGINT for binary arithmetic.
Got:\n" + plan);
}
+
+ /**
+ * Regression test for issue #18881: a TIMESTAMP column compared to a
sub-second epoch-millis literal must preserve
+ * the millisecond component. With Calcite's default TIMESTAMP precision of
0, the implicit literal-to-TIMESTAMP cast
+ * added by {@link PinotTypeCoercion#binaryComparisonCoercion} folded
through {@code RexBuilder.clean ->
+ * TimestampString.round(0)}, truncating the literal to whole seconds — so
{@code ts = 1761667561482} silently became
+ * {@code ts = 1761667561000} and matched no rows. Pinot's {@code
TypeSystem} now pins TIMESTAMP to precision 3, so
+ * the folded literal keeps its millis. The column must also stay unwrapped
(the property #18396 added).
+ */
+ @Test
+ public void testTimestampColumnVsSubSecondLiteralPreservesMillis() {
+ // 1761667561482 ms has a non-zero sub-second component (.482). The
rendered wall-clock date/seconds depend on the
+ // JVM default time zone, but the fractional-second part is time-zone
invariant, so we assert only on ".482".
+ String plan = explain("SELECT ts_timestamp FROM a WHERE ts_timestamp =
1761667561482");
+ assertFalse(plan.contains("CAST(" + TS_TIMESTAMP_ORD + ")"),
+ "TIMESTAMP column should not be wrapped in CAST. Got:\n" + plan);
+ assertTrue(
+ plan.matches("(?s).*=\\(\\" + TS_TIMESTAMP_ORD + ",
\\d{4}-\\d{2}-\\d{2} \\d{2}:\\d{2}:\\d{2}\\.482\\).*"),
+ "Sub-second literal must fold preserving millis (.482), not truncate
to whole seconds. Got:\n" + plan);
+ }
}
diff --git
a/pinot-query-planner/src/test/resources/queries/LiteralEvaluationPlans.json
b/pinot-query-planner/src/test/resources/queries/LiteralEvaluationPlans.json
index e63b4aab8a6..5decbcd0408 100644
--- a/pinot-query-planner/src/test/resources/queries/LiteralEvaluationPlans.json
+++ b/pinot-query-planner/src/test/resources/queries/LiteralEvaluationPlans.json
@@ -25,7 +25,7 @@
"sql": "EXPLAIN PLAN FOR SELECT FROMDATETIME( '1997-02-01 00:00:00',
'yyyy-MM-dd HH:mm:ss') FROM d",
"output": [
"Execution Plan",
- "\nLogicalProject(EXPR$0=[1997-02-01 00:00:00])",
+ "\nLogicalProject(EXPR$0=[1997-02-01 00:00:00:TIMESTAMP(3)])",
"\n PinotLogicalTableScan(table=[[default, d]])",
"\n"
]
@@ -45,7 +45,7 @@
"sql": "EXPLAIN PLAN FOR SELECT timestampDiff(DAY, CAST(ts as
TIMESTAMP), CAST(dateTrunc('MONTH', FROMDATETIME('1997-02-01 00:00:00',
'yyyy-MM-dd HH:mm:ss')) as TIMESTAMP)) FROM d",
"output": [
"Execution Plan",
- "\nLogicalProject(EXPR$0=[TIMESTAMPDIFF(FLAG(DAY),
CAST($7):TIMESTAMP(0) NOT NULL, 1997-02-01 00:00:00)])",
+ "\nLogicalProject(EXPR$0=[TIMESTAMPDIFF(FLAG(DAY),
CAST($7):TIMESTAMP(3) NOT NULL, 1997-02-01 00:00:00:TIMESTAMP(3))])",
"\n PinotLogicalTableScan(table=[[default, d]])",
"\n"
]
@@ -55,7 +55,7 @@
"sql": "EXPLAIN PLAN FOR SELECT * FROM d WHERE CAST(ts AS TIMESTAMP) =
FROMDATETIME('2019-01-01 00:00:00', 'yyyy-MM-dd HH:mm:ss')",
"output": [
"Execution Plan",
- "\nLogicalFilter(condition=[=(CAST($7):TIMESTAMP(0) NOT NULL,
2019-01-01 00:00:00)])",
+ "\nLogicalFilter(condition=[=(CAST($7):TIMESTAMP(3) NOT NULL,
2019-01-01 00:00:00)])",
"\n PinotLogicalTableScan(table=[[default, d]])",
"\n"
]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
