diff --git a/presto-clp/src/main/java/com/facebook/presto/plugin/clp/metadata/ClpSchemaTree.java b/presto-clp/src/main/java/com/facebook/presto/plugin/clp/metadata/ClpSchemaTree.java index b3e0e0777d23f..559e7fa6150e4 100644 --- a/presto-clp/src/main/java/com/facebook/presto/plugin/clp/metadata/ClpSchemaTree.java +++ b/presto-clp/src/main/java/com/facebook/presto/plugin/clp/metadata/ClpSchemaTree.java @@ -124,7 +124,8 @@ private Type mapColumnType(byte type) case VarString: case NullValue: return VARCHAR; - case DateString: + case DeprecatedDateString: + case Timestamp: return TimestampType.TIMESTAMP; case UnstructuredArray: return new ArrayType(VARCHAR); diff --git a/presto-clp/src/main/java/com/facebook/presto/plugin/clp/metadata/ClpSchemaTreeNodeType.java b/presto-clp/src/main/java/com/facebook/presto/plugin/clp/metadata/ClpSchemaTreeNodeType.java index a08eb5ba79bed..d2bce8beefefa 100644 --- a/presto-clp/src/main/java/com/facebook/presto/plugin/clp/metadata/ClpSchemaTreeNodeType.java +++ b/presto-clp/src/main/java/com/facebook/presto/plugin/clp/metadata/ClpSchemaTreeNodeType.java @@ -26,10 +26,11 @@ public enum ClpSchemaTreeNodeType Object((byte) 5), UnstructuredArray((byte) 6), NullValue((byte) 7), - DateString((byte) 8), + DeprecatedDateString((byte) 8), StructuredArray((byte) 9), FormattedFloat((byte) 12), - DictionaryFloat((byte) 13); + DictionaryFloat((byte) 13), + Timestamp((byte) 14); private static final ClpSchemaTreeNodeType[] LOOKUP_TABLE; private final byte type; diff --git a/presto-clp/src/main/java/com/facebook/presto/plugin/clp/optimization/ClpFilterToKqlConverter.java b/presto-clp/src/main/java/com/facebook/presto/plugin/clp/optimization/ClpFilterToKqlConverter.java index b27a61ef0d65a..d659182869a7c 100644 --- a/presto-clp/src/main/java/com/facebook/presto/plugin/clp/optimization/ClpFilterToKqlConverter.java +++ b/presto-clp/src/main/java/com/facebook/presto/plugin/clp/optimization/ClpFilterToKqlConverter.java @@ -260,7 +260,9 @@ private ClpExpression handleBetween(CallExpression node) String variable = variableOpt.get(); String lowerBound = getLiteralString((ConstantExpression) second); String upperBound = getLiteralString((ConstantExpression) third); - String kql = String.format("%s >= %s AND %s <= %s", variable, lowerBound, variable, upperBound); + String kql = String.format("%s >= %s AND %s <= %s", + variable, formatLiteral(second.getType(), lowerBound), + variable, formatLiteral(third.getType(), upperBound)); String metadataSqlQuery = metadataFilterColumns.contains(variable) ? String.format("\"%s\" >= %s AND \"%s\" <= %s", variable, lowerBound, variable, upperBound) : null; @@ -450,7 +452,7 @@ private ClpExpression buildClpExpression( if (metadataFilterColumns.contains(variableName)) { metadataSqlQuery = format("\"%s\" = %s", variableName, literalString); } - return new ClpExpression(format("%s: %s", variableName, literalString), metadataSqlQuery); + return new ClpExpression(format("%s: %s", variableName, formatLiteral(literalType, literalString)), metadataSqlQuery); } } else if (operator.equals(NOT_EQUAL)) { @@ -461,14 +463,14 @@ else if (operator.equals(NOT_EQUAL)) { if (metadataFilterColumns.contains(variableName)) { metadataSqlQuery = format("NOT \"%s\" = %s", variableName, literalString); } - return new ClpExpression(format("NOT %s: %s", variableName, literalString), metadataSqlQuery); + return new ClpExpression(format("NOT %s: %s", variableName, formatLiteral(literalType, literalString)), metadataSqlQuery); } } else if (LOGICAL_BINARY_OPS_FILTER.contains(operator) && !(literalType instanceof VarcharType)) { if (metadataFilterColumns.contains(variableName)) { metadataSqlQuery = format("\"%s\" %s %s", variableName, operator.getOperator(), literalString); } - return new ClpExpression(format("%s %s %s", variableName, operator.getOperator(), literalString), metadataSqlQuery); + return new ClpExpression(format("%s %s %s", variableName, operator.getOperator(), formatLiteral(literalType, literalString)), metadataSqlQuery); } return new ClpExpression(originalNode); } @@ -888,6 +890,23 @@ private ClpExpression handleDereference(RowExpression expression) return new ClpExpression(baseString.getPushDownExpression().get() + "." + fieldName); } + /** + * Wraps a literal string in the KQL timestamp function for TIMESTAMP types. + *

+ * Example: "1672531200000" → timestamp("1672531200000", "\L") + * + * @param literalType the type of the literal + * @param literalString the raw string representation of the literal + * @return the formatted literal string, wrapped if TIMESTAMP type + */ + private static String formatLiteral(Type literalType, String literalString) + { + if (literalType.equals(TIMESTAMP)) { + return format("timestamp(\"%s\", \"\\L\")", literalString); + } + return literalString; + } + /** * See * here diff --git a/presto-clp/src/test/java/com/facebook/presto/plugin/clp/TestClpFilterToKql.java b/presto-clp/src/test/java/com/facebook/presto/plugin/clp/TestClpFilterToKql.java index 413c9cd773a6d..273f8d5b420e7 100644 --- a/presto-clp/src/test/java/com/facebook/presto/plugin/clp/TestClpFilterToKql.java +++ b/presto-clp/src/test/java/com/facebook/presto/plugin/clp/TestClpFilterToKql.java @@ -13,6 +13,7 @@ */ package com.facebook.presto.plugin.clp; +import com.facebook.presto.common.type.TimeZoneKey; import com.facebook.presto.plugin.clp.optimization.ClpFilterToKqlConverter; import com.facebook.presto.spi.ColumnHandle; import com.facebook.presto.spi.relation.RowExpression; @@ -271,6 +272,31 @@ public void testMetadataSqlGeneration() testMetadataFilterColumns); } + @Test + public void testTimestampPushDown() + { + // Use UTC to ensure stable epoch-ms values regardless of local timezone + SessionHolder sessionHolder = new SessionHolder(TimeZoneKey.UTC_KEY); + + // Epoch ms for TIMESTAMP '2023-01-01 00:00:00.000' UTC = 1672531200000 + testPushDown(sessionHolder, "clpTimestamp > TIMESTAMP '2023-01-01 00:00:00.000'", + "clpTimestamp > timestamp(\"1672531200000\", \"\\L\")", null); + testPushDown(sessionHolder, "clpTimestamp >= TIMESTAMP '2023-01-01 00:00:00.000'", + "clpTimestamp >= timestamp(\"1672531200000\", \"\\L\")", null); + testPushDown(sessionHolder, "clpTimestamp < TIMESTAMP '2023-01-01 00:00:00.000'", + "clpTimestamp < timestamp(\"1672531200000\", \"\\L\")", null); + testPushDown(sessionHolder, "clpTimestamp <= TIMESTAMP '2023-01-01 00:00:00.000'", + "clpTimestamp <= timestamp(\"1672531200000\", \"\\L\")", null); + testPushDown(sessionHolder, "clpTimestamp = TIMESTAMP '2023-01-01 00:00:00.000'", + "clpTimestamp: timestamp(\"1672531200000\", \"\\L\")", null); + + // Epoch ms for TIMESTAMP '2023-01-02 00:00:00.000' UTC = 1672617600000 + testPushDown(sessionHolder, + "clpTimestamp BETWEEN TIMESTAMP '2023-01-01 00:00:00.000' AND TIMESTAMP '2023-01-02 00:00:00.000'", + "clpTimestamp >= timestamp(\"1672531200000\", \"\\L\") AND clpTimestamp <= timestamp(\"1672617600000\", \"\\L\")", + null); + } + @Test public void testClpWildcardUdf() { diff --git a/presto-clp/src/test/java/com/facebook/presto/plugin/clp/TestClpQueryBase.java b/presto-clp/src/test/java/com/facebook/presto/plugin/clp/TestClpQueryBase.java index 728da16e15eec..522d8e65c84e4 100644 --- a/presto-clp/src/test/java/com/facebook/presto/plugin/clp/TestClpQueryBase.java +++ b/presto-clp/src/test/java/com/facebook/presto/plugin/clp/TestClpQueryBase.java @@ -17,6 +17,7 @@ import com.facebook.presto.SystemSessionProperties; import com.facebook.presto.common.block.BlockEncodingManager; import com.facebook.presto.common.type.RowType; +import com.facebook.presto.common.type.TimeZoneKey; import com.facebook.presto.common.type.Type; import com.facebook.presto.metadata.AnalyzePropertyManager; import com.facebook.presto.metadata.CatalogManager; @@ -52,6 +53,7 @@ import static com.facebook.presto.common.type.BigintType.BIGINT; import static com.facebook.presto.common.type.BooleanType.BOOLEAN; import static com.facebook.presto.common.type.DoubleType.DOUBLE; +import static com.facebook.presto.common.type.TimestampType.TIMESTAMP; import static com.facebook.presto.common.type.VarcharType.VARCHAR; import static com.facebook.presto.metadata.FunctionAndTypeManager.createTestFunctionAndTypeManager; import static com.facebook.presto.metadata.SessionPropertyManager.createTestingSessionPropertyManager; @@ -91,8 +93,9 @@ public class TestClpQueryBase RowType.field("Name", VARCHAR))))))); protected static final ClpColumnHandle fare = new ClpColumnHandle("fare", DOUBLE); protected static final ClpColumnHandle isHoliday = new ClpColumnHandle("isHoliday", BOOLEAN); + protected static final ClpColumnHandle clpTimestamp = new ClpColumnHandle("clpTimestamp", TIMESTAMP); protected static final Map variableToColumnHandleMap = - Stream.of(city, fare, isHoliday) + Stream.of(city, fare, isHoliday, clpTimestamp) .collect(toMap( ch -> new VariableReferenceExpression(Optional.empty(), ch.getColumnName(), ch.getColumnType()), ch -> ch)); @@ -133,6 +136,21 @@ public SessionHolder() session = TestingSession.testSessionBuilder(createTestingSessionPropertyManager(new SystemSessionProperties().getSessionProperties())).build(); } + /** + * Creates a session with a fixed timezone. Use this when tests assert against hardcoded + * epoch-ms values derived from timestamp literals (e.g., {@code TIMESTAMP '2023-01-01 + * 00:00:00.000'}), since Presto parses timestamp literals relative to the session timezone. + * Without pinning the timezone, the same literal produces different epoch-ms values on + * machines in different timezones, causing the assertions to fail. + */ + public SessionHolder(TimeZoneKey timeZoneKey) + { + connectorSession = SESSION; + session = TestingSession.testSessionBuilder(createTestingSessionPropertyManager(new SystemSessionProperties().getSessionProperties())) + .setTimeZoneKey(timeZoneKey) + .build(); + } + public ConnectorSession getConnectorSession() { return connectorSession; diff --git a/presto-native-execution/src/test/java/com/facebook/presto/nativeworker/TestPrestoNativeClpGeneralQueries.java b/presto-native-execution/src/test/java/com/facebook/presto/nativeworker/TestPrestoNativeClpGeneralQueries.java index b7016c4f51cb0..202f46660a1c1 100644 --- a/presto-native-execution/src/test/java/com/facebook/presto/nativeworker/TestPrestoNativeClpGeneralQueries.java +++ b/presto-native-execution/src/test/java/com/facebook/presto/nativeworker/TestPrestoNativeClpGeneralQueries.java @@ -31,7 +31,7 @@ import static com.facebook.presto.plugin.clp.ClpQueryRunner.DEFAULT_NUM_OF_WORKERS; import static com.facebook.presto.plugin.clp.metadata.ClpSchemaTreeNodeType.Boolean; import static com.facebook.presto.plugin.clp.metadata.ClpSchemaTreeNodeType.ClpString; -import static com.facebook.presto.plugin.clp.metadata.ClpSchemaTreeNodeType.DateString; +import static com.facebook.presto.plugin.clp.metadata.ClpSchemaTreeNodeType.DeprecatedDateString; import static com.facebook.presto.plugin.clp.metadata.ClpSchemaTreeNodeType.Integer; import static com.facebook.presto.plugin.clp.metadata.ClpSchemaTreeNodeType.NullValue; import static com.facebook.presto.plugin.clp.metadata.ClpSchemaTreeNodeType.UnstructuredArray; @@ -88,7 +88,7 @@ protected void createTables() UnstructuredArray, UnstructuredArray, NullValue, - DateString)))); + DeprecatedDateString)))); mockMetadataDatabase.addSplits(ImmutableMap.of(DEFAULT_TABLE_NAME, new ArchivesTableRows( ImmutableList.of("mongodb-processed-single-file-archive"), ImmutableList.of(1679441694576L), diff --git a/presto-native-execution/velox b/presto-native-execution/velox index b7eb104aed53d..739a3aac8f314 160000 --- a/presto-native-execution/velox +++ b/presto-native-execution/velox @@ -1 +1 @@ -Subproject commit b7eb104aed53d5ea6119cfef333d9291da1be1cd +Subproject commit 739a3aac8f3146d178c27e0cc751be4ba689dc65