From eacfeaf3c7912cc4cad4ca4fa6562267a29c01a4 Mon Sep 17 00:00:00 2001 From: gibber9809 Date: Mon, 2 Mar 2026 18:34:47 +0000 Subject: [PATCH 01/15] Update clp version to pull in faster timestamp marshalling and some bug fixes. --- CMake/resolve_dependency_modules/clp.cmake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMake/resolve_dependency_modules/clp.cmake b/CMake/resolve_dependency_modules/clp.cmake index c9579fbc314..779c5239910 100644 --- a/CMake/resolve_dependency_modules/clp.cmake +++ b/CMake/resolve_dependency_modules/clp.cmake @@ -16,7 +16,7 @@ include_guard(GLOBAL) FetchContent_Declare( clp GIT_REPOSITORY https://github.com/y-scope/clp.git - GIT_TAG f82e6114160a6addd4727259906bcf621ac9912c + GIT_TAG a91e5f71f0715d7d6b3ea7c177e1b39b3e6a24a6 ) set(CLP_BUILD_CLP_REGEX_UTILS OFF CACHE BOOL "Build CLP regex utils") From 96594f32c9fbad6259126b810c31dcfeeb361ebb Mon Sep 17 00:00:00 2001 From: gibber9809 Date: Thu, 29 Jan 2026 11:35:09 -0500 Subject: [PATCH 02/15] Add utility for converting nanosecond epoch timestamps to velox timestamps. --- .../clp/search_lib/ClpTimestampsUtils.h | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/velox/connectors/clp/search_lib/ClpTimestampsUtils.h b/velox/connectors/clp/search_lib/ClpTimestampsUtils.h index 4e49af389a0..7837f768d2c 100644 --- a/velox/connectors/clp/search_lib/ClpTimestampsUtils.h +++ b/velox/connectors/clp/search_lib/ClpTimestampsUtils.h @@ -16,6 +16,7 @@ #pragma once +#include "clp_s/Defs.hpp" #include "velox/type/Timestamp.h" namespace facebook::velox::connector::clp::search_lib { @@ -121,4 +122,18 @@ inline auto convertToVeloxTimestamp(int64_t timestamp) -> Timestamp { return Timestamp(seconds, static_cast(nanoseconds)); } +/// Converts a nanosecond precision epochtime_t into a Velox timestamp. +/// +/// @param timestamp the input timestamp as an integer +/// @return the corresponding Velox timestamp +inline auto convertNanosecondEpochToVeloxTimestamp(clp_s::epochtime_t timestamp) -> Timestamp { + int64_t seconds{timestamp / Timestamp::kNanosInSecond}; + int64_t nanoseconds{timestamp % Timestamp::kNanosInSecond}; + if (nanoseconds < 0) { + seconds -= 1; + nanoseconds += Timestamp::kNanosInSecond; + } + return Timestamp(seconds, static_cast(nanoseconds)); +} + } // namespace facebook::velox::connector::clp::search_lib From a291254a0ffb2ffdb791e47e58cd1e93ff3b13cc Mon Sep 17 00:00:00 2001 From: gibber9809 Date: Mon, 2 Mar 2026 18:41:02 +0000 Subject: [PATCH 03/15] Handle timestamp literal precision special cases for IR and archive search. --- .../clp/search_lib/archive/ClpArchiveCursor.cpp | 11 +++++++++++ velox/connectors/clp/search_lib/ir/ClpIrCursor.cpp | 6 ++++++ 2 files changed, 17 insertions(+) diff --git a/velox/connectors/clp/search_lib/archive/ClpArchiveCursor.cpp b/velox/connectors/clp/search_lib/archive/ClpArchiveCursor.cpp index 4953c585f37..27b9320956b 100644 --- a/velox/connectors/clp/search_lib/archive/ClpArchiveCursor.cpp +++ b/velox/connectors/clp/search_lib/archive/ClpArchiveCursor.cpp @@ -17,9 +17,12 @@ #include #include "clp_s/ArchiveReader.hpp" +#include "clp_s/SingleFileArchiveDefs.hpp" #include "clp_s/search/EvaluateTimestampIndex.hpp" #include "clp_s/search/ast/EmptyExpr.hpp" #include "clp_s/search/ast/SearchUtils.hpp" +#include "clp_s/search/ast/SetTimestampLiteralPrecision.hpp" +#include "clp_s/search/ast/TimestampLiteral.hpp" #include "velox/connectors/clp/ClpColumnHandle.h" #include "velox/connectors/clp/search_lib/archive/ClpArchiveCursor.h" #include "velox/connectors/clp/search_lib/archive/ClpArchiveJsonStringVectorLoader.h" @@ -136,6 +139,14 @@ ErrorCode ClpArchiveCursor::loadSplit() { auto schemaTree = archiveReader_->get_schema_tree(); auto schemaMap = archiveReader_->get_schema_map(); + auto const defaultTimestampPrecision{ + archiveReader_->has_deprecated_timestamp_format() + ? TimestampLiteral::Precision::Milliseconds + : TimestampLiteral::Precision::Nanoseconds}; + SetTimestampLiteralPrecision timestampPrecisionPass{ + defaultTimestampPrecision}; + expr_ = timestampPrecisionPass.run(expr_); + EvaluateTimestampIndex timestampIndex(timestampDict); if (clp_s::EvaluatedValue::False == timestampIndex.run(expr_)) { VLOG(2) << "No matching timestamp ranges for query '" << query_ << "'"; diff --git a/velox/connectors/clp/search_lib/ir/ClpIrCursor.cpp b/velox/connectors/clp/search_lib/ir/ClpIrCursor.cpp index 78ecee7fde2..6363b904f49 100644 --- a/velox/connectors/clp/search_lib/ir/ClpIrCursor.cpp +++ b/velox/connectors/clp/search_lib/ir/ClpIrCursor.cpp @@ -16,6 +16,8 @@ #include "clp_s/ColumnReader.hpp" #include "clp_s/InputConfig.hpp" +#include "clp_s/search/ast/SetTimestampLiteralPrecision.hpp" +#include "clp_s/search/ast/TimestampLiteral.hpp" #include "ffi/ir_stream/search/QueryHandler.hpp" #include "velox/connectors/clp/ClpColumnHandle.h" @@ -75,6 +77,10 @@ ErrorCode ClpIrCursor::loadSplit() { ? NetworkAuthOption{.method = AuthMethod::None} : NetworkAuthOption{.method = AuthMethod::S3PresignedUrlV4}; + search::ast::SetTimestampLiteralPrecision timestampPrecisionPass{ + ast::TimestampLiteral::Precision::Milliseconds}; + expr_ = timestampPrecisionPass.run(expr_); + auto projections = splitFieldsToNamesAndTypes(); auto queryHandlerResult{QueryHandlerType::create( projectionResolutionCallback_, From ec360b80c1dd2cccdd12bbd2aec19a31a53bc670 Mon Sep 17 00:00:00 2001 From: gibber9809 Date: Mon, 2 Mar 2026 18:45:49 +0000 Subject: [PATCH 04/15] Minor syntax fixes. --- .../clp/search_lib/archive/ClpArchiveVectorLoader.cpp | 8 ++++++++ velox/connectors/clp/search_lib/ir/ClpIrCursor.cpp | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/velox/connectors/clp/search_lib/archive/ClpArchiveVectorLoader.cpp b/velox/connectors/clp/search_lib/archive/ClpArchiveVectorLoader.cpp index eb2434726ae..89b92afb51e 100644 --- a/velox/connectors/clp/search_lib/archive/ClpArchiveVectorLoader.cpp +++ b/velox/connectors/clp/search_lib/archive/ClpArchiveVectorLoader.cpp @@ -260,6 +260,14 @@ ClpArchiveVectorLoader::populateTimestampData( RowSet rows, FlatVector* vector); template void +ClpArchiveVectorLoader::populateTimestampData( + RowSet rows, + FlatVector* vector); +template void +ClpArchiveVectorLoader::populateTimestampData( + RowSet rows, + FlatVector* vector); +template void ClpArchiveVectorLoader::populateTimestampData( RowSet rows, FlatVector* vector); diff --git a/velox/connectors/clp/search_lib/ir/ClpIrCursor.cpp b/velox/connectors/clp/search_lib/ir/ClpIrCursor.cpp index 6363b904f49..ece5c435cec 100644 --- a/velox/connectors/clp/search_lib/ir/ClpIrCursor.cpp +++ b/velox/connectors/clp/search_lib/ir/ClpIrCursor.cpp @@ -78,7 +78,7 @@ ErrorCode ClpIrCursor::loadSplit() { : NetworkAuthOption{.method = AuthMethod::S3PresignedUrlV4}; search::ast::SetTimestampLiteralPrecision timestampPrecisionPass{ - ast::TimestampLiteral::Precision::Milliseconds}; + search::ast::TimestampLiteral::Precision::Milliseconds}; expr_ = timestampPrecisionPass.run(expr_); auto projections = splitFieldsToNamesAndTypes(); From 42ee0252c274447935562ef0ce8d09399b96023d Mon Sep 17 00:00:00 2001 From: gibber9809 Date: Wed, 18 Feb 2026 15:11:02 -0500 Subject: [PATCH 05/15] Update existing tests to use timestamp literal in pushdown. --- velox/connectors/clp/tests/ClpConnectorTest.cpp | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/velox/connectors/clp/tests/ClpConnectorTest.cpp b/velox/connectors/clp/tests/ClpConnectorTest.cpp index 114d5e50cbd..2c925e5045a 100644 --- a/velox/connectors/clp/tests/ClpConnectorTest.cpp +++ b/velox/connectors/clp/tests/ClpConnectorTest.cpp @@ -580,8 +580,8 @@ TEST_F(ClpConnectorTest, test4IrTimestampNoPushdown) { TEST_F(ClpConnectorTest, test4IrTimestampPushdown) { // Only the second event meet the condition, the first event is a date string // which is not supported yet so the value will be NULL. - const std::shared_ptr kqlQuery = - std::make_shared("(timestamp < 1756003005000000)"); + const std::shared_ptr kqlQuery = std::make_shared( + R"(timestamp < timestamp("1756003005000000", "\L"))"); auto plan = PlanBuilder(pool_.get()) .startTableScan() @@ -661,11 +661,9 @@ TEST_F(ClpConnectorTest, test5FloatTimestampNoPushdown) { TEST_F(ClpConnectorTest, test5FloatTimestampPushdown) { // Test filtering rows with a timestamp parsed from a date string and floats - // in various formats. Because KQL doesn’t automatically interpret the unit of - // the timestamp, the returned result differs slightly from the one without - // pushdown. + // in various formats. const std::shared_ptr kqlQuery = std::make_shared( - "(timestamp < 1746003005.127 and timestamp >= 1746003005.124)"); + R"(timestamp < timestamp("1746003005.127") and timestamp >= timestamp("1746003005.124")"); auto plan = PlanBuilder(pool_.get()) .startTableScan() From 2ba6894699400c8a21dfbd418380e3e7b28d6fd3 Mon Sep 17 00:00:00 2001 From: gibber9809 Date: Wed, 18 Feb 2026 20:25:07 +0000 Subject: [PATCH 06/15] Update out of date query. --- velox/connectors/clp/tests/ClpConnectorTest.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/velox/connectors/clp/tests/ClpConnectorTest.cpp b/velox/connectors/clp/tests/ClpConnectorTest.cpp index 2c925e5045a..e0ee0bd2ff1 100644 --- a/velox/connectors/clp/tests/ClpConnectorTest.cpp +++ b/velox/connectors/clp/tests/ClpConnectorTest.cpp @@ -663,7 +663,7 @@ TEST_F(ClpConnectorTest, test5FloatTimestampPushdown) { // Test filtering rows with a timestamp parsed from a date string and floats // in various formats. const std::shared_ptr kqlQuery = std::make_shared( - R"(timestamp < timestamp("1746003005.127") and timestamp >= timestamp("1746003005.124")"); + R"(timestamp < timestamp("1746003070000", "\L") and timestamp >= timestamp("1746003005124", "\L")"); auto plan = PlanBuilder(pool_.get()) .startTableScan() From 2204ef5150260d8525c9e60b1ccc840a88bf8230 Mon Sep 17 00:00:00 2001 From: gibber9809 Date: Wed, 18 Feb 2026 20:31:54 +0000 Subject: [PATCH 07/15] Add timestamp filtering and marshalling test for v0.5.0 archives. --- .../connectors/clp/tests/ClpConnectorTest.cpp | 48 ++++++++++++++++++ .../clp/tests/examples/test_5.v0.5.0.clps | Bin 0 -> 945 bytes 2 files changed, 48 insertions(+) create mode 100644 velox/connectors/clp/tests/examples/test_5.v0.5.0.clps diff --git a/velox/connectors/clp/tests/ClpConnectorTest.cpp b/velox/connectors/clp/tests/ClpConnectorTest.cpp index e0ee0bd2ff1..d9d5641569a 100644 --- a/velox/connectors/clp/tests/ClpConnectorTest.cpp +++ b/velox/connectors/clp/tests/ClpConnectorTest.cpp @@ -702,6 +702,54 @@ TEST_F(ClpConnectorTest, test5FloatTimestampPushdown) { test::assertEqualVectors(expected, output); } +TEST_F(ClpConnectorTest, test5FloatTimestampPushdown) { + // Test filtering rows with a timestamp parsed from a date string and floats + // in various formats. + const std::shared_ptr kqlQuery = std::make_shared( + R"(timestamp < timestamp("1746003070000", "\L") and timestamp >= timestamp("1746003005124", "\L")"); + auto plan = + PlanBuilder(pool_.get()) + .startTableScan() + .outputType(ROW({"timestamp", "floatValue"}, {TIMESTAMP(), DOUBLE()})) + .tableHandle( + std::make_shared(kClpConnectorId, "test_5")) + .assignments( + {{"timestamp", + std::make_shared( + "timestamp", "timestamp", TIMESTAMP())}, + {"floatValue", + std::make_shared( + "floatValue", "floatValue", DOUBLE())}}) + .endTableScan() + .orderBy({"\"timestamp\" ASC"}, false) + .planNode(); + + auto output = getResults( + plan, + {makeClpSplit( + getExampleFilePath("test_5.v0.5.0.clps"), + ClpConnectorSplit::SplitType::kArchive, + kqlQuery)}); + auto expected = makeRowVector({// timestamp + makeFlatVector( + {Timestamp(1746003005, 124000000), + Timestamp(1746003005, 124100000), + Timestamp(1746003005, 125000000), + Timestamp(1746003005, 126000000), + Timestamp(1746003005, 127000000), + Timestamp(1746003060, 0), + Timestamp(1746003065, 0)}), + makeFlatVector( + {1.2345678912345E9, + 1E16, + 1.234567891234567E9, + 1.234567891234567E9, + -1.234567891234567E-9, + 1234567891.234567, + -1234567891.234567})}); + test::assertEqualVectors(expected, output); +} + TEST_F(ClpConnectorTest, test5FormattedFloatNoPushdown) { // Test floats of only FormattedFloat type const std::shared_ptr kqlQuery = nullptr; diff --git a/velox/connectors/clp/tests/examples/test_5.v0.5.0.clps b/velox/connectors/clp/tests/examples/test_5.v0.5.0.clps new file mode 100644 index 0000000000000000000000000000000000000000..169d042dba2bea2f5133b78b87d5b43ec77add2b GIT binary patch literal 945 zcmeyXf7F11ft5jyivbKaGDB!WWF;d+g~nF>zYGzr+ziX6`dI~dUS)H5T|8YnRym|m z!*-3*0}=K!iPo=nAGH(OX|L$5nfg7*$Ee^;c**RD%LUOqf;{Ejo6p>h^y4f`ulsI)jsM#HBX?ZN(s@2zuC)EJquR_j^7i^! z=hqjldGYui|2>XAMn)O7hQk3CjF$qh21-6}$VkX&I8Yl^@W82wi)W3~-2HxU`RsO9 zx45!hx;ydNfp=$|7&{DgGWA*=c10i8RFo)-I>DCjwB?`C#3QGc?()z$xuk1>%2VwS zmbeM4H1Z>rgikM4d44$j@bQne>3K?R{Y)=z_1{-ueP@>xpvJk~;zUbv{s-52+xO0F z*fQbO)8!GZ^YSB~Uzd!Ry?;yEc6#0NL=&^>&8DWCwyrO~tmPHIkk_(qb|A@k(2;9H!IZmGUwcR%o9tmAb!UV7OVfv34vS6l zJ(ZuyK?9zN;RrJu!+#)P0LLINLr#8rd}>*0UP*jrN(DD76EL1QOEPm)i%Sx73;00X zw4D6JlCZ>_(o`N^b_Odi(HRvJM7;`G+ZZGH8Mq+^A7x+&U}9iqWMO6n>R@F!aH4zX z)H7v~IWPsPj0_tX84j^9Ffk}HB!N-`Eak8=#HTWykhmcs1qH35)4Lq28M=g3=5_$?Bo8$(9D>@!@vzR2?aoOnsP9-06oaaAo^alv6{zP;H5T~#RzovMH|C|XMNsV@-Us>(vW(|k)h#%dF5e8XI18{ zPuARDZz{U);K9QO4<7tr-_W2Czktc<|NHue2aFF6G8}nu@xk2(Zyz{5IQZaU!U2W@ z2|&njfZ+|porE_DcM>inJVmk zE^g~hXY>*@xC~R*gkGB1Z2Wa={mPGALRR*MJ`9I*RjxF;>rIJZ%1gM?_|oY98N1UD rcxK-zn$hCkR6HXs%V;*|HS+{<71#Pnnd^4gyB{cf Date: Wed, 18 Feb 2026 21:00:01 +0000 Subject: [PATCH 08/15] Address comment about test name. --- velox/connectors/clp/tests/ClpConnectorTest.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/velox/connectors/clp/tests/ClpConnectorTest.cpp b/velox/connectors/clp/tests/ClpConnectorTest.cpp index d9d5641569a..9f9a36c325c 100644 --- a/velox/connectors/clp/tests/ClpConnectorTest.cpp +++ b/velox/connectors/clp/tests/ClpConnectorTest.cpp @@ -702,7 +702,7 @@ TEST_F(ClpConnectorTest, test5FloatTimestampPushdown) { test::assertEqualVectors(expected, output); } -TEST_F(ClpConnectorTest, test5FloatTimestampPushdown) { +TEST_F(ClpConnectorTest, test5v050FloatTimestampPushdown) { // Test filtering rows with a timestamp parsed from a date string and floats // in various formats. const std::shared_ptr kqlQuery = std::make_shared( From 781bbc153346f38781b42d501148ce076b63e217 Mon Sep 17 00:00:00 2001 From: gibber9809 Date: Mon, 2 Mar 2026 19:04:26 +0000 Subject: [PATCH 09/15] Fix tests. --- .../connectors/clp/tests/ClpConnectorTest.cpp | 49 +++++++++++-------- 1 file changed, 28 insertions(+), 21 deletions(-) diff --git a/velox/connectors/clp/tests/ClpConnectorTest.cpp b/velox/connectors/clp/tests/ClpConnectorTest.cpp index 9f9a36c325c..49e5aa5414e 100644 --- a/velox/connectors/clp/tests/ClpConnectorTest.cpp +++ b/velox/connectors/clp/tests/ClpConnectorTest.cpp @@ -693,16 +693,22 @@ TEST_F(ClpConnectorTest, test5FloatTimestampPushdown) { {Timestamp(1746003005, 124000000), Timestamp(1746003005, 124100000), Timestamp(1746003005, 125000000), - Timestamp(1746003005, 126000000)}), + Timestamp(1746003005, 126000000), + Timestamp(1746003005, 127000000), + Timestamp(1746003060, 0), + Timestamp(1746003065, 0)}), makeFlatVector( - {1.234567891234500E9, + {1.2345678912345E9, 1E16, 1.234567891234567E9, - 1.234567891234567E9})}); + 1.234567891234567E9, + -1.234567891234567E-9, + 1234567891.234567, + -1234567891.234567})}); test::assertEqualVectors(expected, output); } -TEST_F(ClpConnectorTest, test5v050FloatTimestampPushdown) { +TEST_F(ClpConnectorTest, test5NewTimestampFormatFloatTimestampPushdown) { // Test filtering rows with a timestamp parsed from a date string and floats // in various formats. const std::shared_ptr kqlQuery = std::make_shared( @@ -730,23 +736,24 @@ TEST_F(ClpConnectorTest, test5v050FloatTimestampPushdown) { getExampleFilePath("test_5.v0.5.0.clps"), ClpConnectorSplit::SplitType::kArchive, kqlQuery)}); - auto expected = makeRowVector({// timestamp - makeFlatVector( - {Timestamp(1746003005, 124000000), - Timestamp(1746003005, 124100000), - Timestamp(1746003005, 125000000), - Timestamp(1746003005, 126000000), - Timestamp(1746003005, 127000000), - Timestamp(1746003060, 0), - Timestamp(1746003065, 0)}), - makeFlatVector( - {1.2345678912345E9, - 1E16, - 1.234567891234567E9, - 1.234567891234567E9, - -1.234567891234567E-9, - 1234567891.234567, - -1234567891.234567})}); + auto expected = makeRowVector( + {// timestamp + makeFlatVector( + {Timestamp(1746003005, 124000000), + Timestamp(1746003005, 124100000), + Timestamp(1746003005, 125000000), + Timestamp(1746003005, 126000000), + Timestamp(1746003005, 127000000), + Timestamp(1746003060, 0), + Timestamp(1746003065, 0)}), + makeFlatVector( + {1.2345678912345E9, + 1E16, + 1.234567891234567E9, + 1.234567891234567E9, + -1.234567891234567E-9, + 1234567891.234567, + -1234567891.234567})}); test::assertEqualVectors(expected, output); } From 90661823ec8e33af757009736c6d5d1098de2758 Mon Sep 17 00:00:00 2001 From: gibber9809 Date: Mon, 2 Mar 2026 19:10:03 +0000 Subject: [PATCH 10/15] Add marshalling code for clp_s::NodeType::Timestamp. --- .../archive/ClpArchiveVectorLoader.cpp | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/velox/connectors/clp/search_lib/archive/ClpArchiveVectorLoader.cpp b/velox/connectors/clp/search_lib/archive/ClpArchiveVectorLoader.cpp index 89b92afb51e..f233b53cb9e 100644 --- a/velox/connectors/clp/search_lib/archive/ClpArchiveVectorLoader.cpp +++ b/velox/connectors/clp/search_lib/archive/ClpArchiveVectorLoader.cpp @@ -73,6 +73,7 @@ void ClpArchiveVectorLoader::populateTimestampData( case clp_s::NodeType::DictionaryFloat: case clp_s::NodeType::Integer: case clp_s::NodeType::DeprecatedDateString: + case clp_s::NodeType::Timestamp: supportedNodeType = true; break; default: @@ -88,7 +89,12 @@ void ClpArchiveVectorLoader::populateTimestampData( for (int vectorIndex : rows) { auto messageIndex = filteredRowIndices_->at(vectorIndex); - if (clp_s::NodeType::Float == Type) { + if (clp_s::NodeType::Timestamp == Type) { + auto reader{static_cast(columnReader_)}; + vector->set( + vectorIndex, + convertNanosecondEpochToVeloxTimestamp(reader->get_encoded_time(message_index))); + } else if (clp_s::NodeType::Float == Type) { auto reader = static_cast(columnReader_); vector->set( vectorIndex, @@ -211,7 +217,10 @@ void ClpArchiveVectorLoader::loadInternal( } case ColumnType::Timestamp: { auto timestampVector = vector->asFlatVector(); - if (nullptr != dynamic_cast(columnReader_)) { + if (nullptr != dynamic_cast(columnReader_)) { + populateTimestampData( + rows, timestampVector); + } else if (nullptr != dynamic_cast(columnReader_)) { populateTimestampData(rows, timestampVector); } else if ( nullptr != @@ -256,6 +265,10 @@ template void ClpArchiveVectorLoader::populateData( RowSet rows, FlatVector* vector); template void +ClpArchiveVectorLoader::populateTimestampData( + RowSet rows, + FlatVector* vector); +template void ClpArchiveVectorLoader::populateTimestampData( RowSet rows, FlatVector* vector); From 8bb62b39be413aef68e555e7882f00d983c85b80 Mon Sep 17 00:00:00 2001 From: gibber9809 Date: Mon, 2 Mar 2026 19:14:28 +0000 Subject: [PATCH 11/15] Lint fix --- .../connectors/clp/search_lib/ClpTimestampsUtils.h | 3 ++- .../search_lib/archive/ClpArchiveVectorLoader.cpp | 13 ++++++++----- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/velox/connectors/clp/search_lib/ClpTimestampsUtils.h b/velox/connectors/clp/search_lib/ClpTimestampsUtils.h index 7837f768d2c..06359fb9a74 100644 --- a/velox/connectors/clp/search_lib/ClpTimestampsUtils.h +++ b/velox/connectors/clp/search_lib/ClpTimestampsUtils.h @@ -126,7 +126,8 @@ inline auto convertToVeloxTimestamp(int64_t timestamp) -> Timestamp { /// /// @param timestamp the input timestamp as an integer /// @return the corresponding Velox timestamp -inline auto convertNanosecondEpochToVeloxTimestamp(clp_s::epochtime_t timestamp) -> Timestamp { +inline auto convertNanosecondEpochToVeloxTimestamp(clp_s::epochtime_t timestamp) + -> Timestamp { int64_t seconds{timestamp / Timestamp::kNanosInSecond}; int64_t nanoseconds{timestamp % Timestamp::kNanosInSecond}; if (nanoseconds < 0) { diff --git a/velox/connectors/clp/search_lib/archive/ClpArchiveVectorLoader.cpp b/velox/connectors/clp/search_lib/archive/ClpArchiveVectorLoader.cpp index f233b53cb9e..03ebb9f6ceb 100644 --- a/velox/connectors/clp/search_lib/archive/ClpArchiveVectorLoader.cpp +++ b/velox/connectors/clp/search_lib/archive/ClpArchiveVectorLoader.cpp @@ -92,8 +92,9 @@ void ClpArchiveVectorLoader::populateTimestampData( if (clp_s::NodeType::Timestamp == Type) { auto reader{static_cast(columnReader_)}; vector->set( - vectorIndex, - convertNanosecondEpochToVeloxTimestamp(reader->get_encoded_time(message_index))); + vectorIndex, + convertNanosecondEpochToVeloxTimestamp( + reader->get_encoded_time(message_index))); } else if (clp_s::NodeType::Float == Type) { auto reader = static_cast(columnReader_); vector->set( @@ -217,10 +218,12 @@ void ClpArchiveVectorLoader::loadInternal( } case ColumnType::Timestamp: { auto timestampVector = vector->asFlatVector(); - if (nullptr != dynamic_cast(columnReader_)) { + if (nullptr != + dynamic_cast(columnReader_)) { populateTimestampData( - rows, timestampVector); - } else if (nullptr != dynamic_cast(columnReader_)) { + rows, timestampVector); + } else if ( + nullptr != dynamic_cast(columnReader_)) { populateTimestampData(rows, timestampVector); } else if ( nullptr != From 021d8eb363437a9524aa0b5dfebbcf830a4bd8eb Mon Sep 17 00:00:00 2001 From: gibber9809 Date: Mon, 2 Mar 2026 14:26:02 -0500 Subject: [PATCH 12/15] Fix typo --- .../clp/search_lib/archive/ClpArchiveVectorLoader.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/velox/connectors/clp/search_lib/archive/ClpArchiveVectorLoader.cpp b/velox/connectors/clp/search_lib/archive/ClpArchiveVectorLoader.cpp index 03ebb9f6ceb..7e291dea83a 100644 --- a/velox/connectors/clp/search_lib/archive/ClpArchiveVectorLoader.cpp +++ b/velox/connectors/clp/search_lib/archive/ClpArchiveVectorLoader.cpp @@ -94,7 +94,7 @@ void ClpArchiveVectorLoader::populateTimestampData( vector->set( vectorIndex, convertNanosecondEpochToVeloxTimestamp( - reader->get_encoded_time(message_index))); + reader->get_encoded_time(messageIndex))); } else if (clp_s::NodeType::Float == Type) { auto reader = static_cast(columnReader_); vector->set( From 92153751084bef6595fe455b6660d4d575b4f971 Mon Sep 17 00:00:00 2001 From: gibber9809 Date: Mon, 2 Mar 2026 15:04:31 -0500 Subject: [PATCH 13/15] Fix typos in tests. --- velox/connectors/clp/tests/ClpConnectorTest.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/velox/connectors/clp/tests/ClpConnectorTest.cpp b/velox/connectors/clp/tests/ClpConnectorTest.cpp index 49e5aa5414e..3195d95ed4e 100644 --- a/velox/connectors/clp/tests/ClpConnectorTest.cpp +++ b/velox/connectors/clp/tests/ClpConnectorTest.cpp @@ -581,7 +581,7 @@ TEST_F(ClpConnectorTest, test4IrTimestampPushdown) { // Only the second event meet the condition, the first event is a date string // which is not supported yet so the value will be NULL. const std::shared_ptr kqlQuery = std::make_shared( - R"(timestamp < timestamp("1756003005000000", "\L"))"); + R"(timestamp < timestamp("1756003005000", "\L"))"); auto plan = PlanBuilder(pool_.get()) .startTableScan() @@ -663,7 +663,7 @@ TEST_F(ClpConnectorTest, test5FloatTimestampPushdown) { // Test filtering rows with a timestamp parsed from a date string and floats // in various formats. const std::shared_ptr kqlQuery = std::make_shared( - R"(timestamp < timestamp("1746003070000", "\L") and timestamp >= timestamp("1746003005124", "\L")"); + R"(timestamp < timestamp("1746003070000", "\L") and timestamp >= timestamp("1746003005124", "\L"))"); auto plan = PlanBuilder(pool_.get()) .startTableScan() @@ -712,7 +712,7 @@ TEST_F(ClpConnectorTest, test5NewTimestampFormatFloatTimestampPushdown) { // Test filtering rows with a timestamp parsed from a date string and floats // in various formats. const std::shared_ptr kqlQuery = std::make_shared( - R"(timestamp < timestamp("1746003070000", "\L") and timestamp >= timestamp("1746003005124", "\L")"); + R"(timestamp < timestamp("1746003070000", "\L") and timestamp >= timestamp("1746003005124", "\L"))"); auto plan = PlanBuilder(pool_.get()) .startTableScan() From 5445fb7aa507a9508775d8fe3da5ac54ae2d2174 Mon Sep 17 00:00:00 2001 From: gibber9809 Date: Mon, 2 Mar 2026 15:52:25 -0500 Subject: [PATCH 14/15] Don't use timestamp() literal in pushdown test 4 for IR, since the timestamps are in microsecond precision. --- velox/connectors/clp/tests/ClpConnectorTest.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/velox/connectors/clp/tests/ClpConnectorTest.cpp b/velox/connectors/clp/tests/ClpConnectorTest.cpp index 3195d95ed4e..5a1de3204f6 100644 --- a/velox/connectors/clp/tests/ClpConnectorTest.cpp +++ b/velox/connectors/clp/tests/ClpConnectorTest.cpp @@ -580,8 +580,12 @@ TEST_F(ClpConnectorTest, test4IrTimestampNoPushdown) { TEST_F(ClpConnectorTest, test4IrTimestampPushdown) { // Only the second event meet the condition, the first event is a date string // which is not supported yet so the value will be NULL. - const std::shared_ptr kqlQuery = std::make_shared( - R"(timestamp < timestamp("1756003005000", "\L"))"); + // This test can not use the `timestamp()` literal, since the integer + // timestamps are in microsecond precision, and we currently assume all IR + // timestamps are millisecond precision when comparing against timestamp + // literals. + const std::shared_ptr kqlQuery = + std::make_shared("(timestamp < 1756003005000000)"); auto plan = PlanBuilder(pool_.get()) .startTableScan() From 8f673c6316c8e87265e8bc65a41e2d554dae6b54 Mon Sep 17 00:00:00 2001 From: gibber9809 Date: Mon, 2 Mar 2026 15:58:39 -0500 Subject: [PATCH 15/15] Update test to account for fact that new timestamp format fully preserves timestamp. --- velox/connectors/clp/tests/ClpConnectorTest.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/velox/connectors/clp/tests/ClpConnectorTest.cpp b/velox/connectors/clp/tests/ClpConnectorTest.cpp index 5a1de3204f6..d11d46502a0 100644 --- a/velox/connectors/clp/tests/ClpConnectorTest.cpp +++ b/velox/connectors/clp/tests/ClpConnectorTest.cpp @@ -747,7 +747,7 @@ TEST_F(ClpConnectorTest, test5NewTimestampFormatFloatTimestampPushdown) { Timestamp(1746003005, 124100000), Timestamp(1746003005, 125000000), Timestamp(1746003005, 126000000), - Timestamp(1746003005, 127000000), + Timestamp(1746003005, 127000001), Timestamp(1746003060, 0), Timestamp(1746003065, 0)}), makeFlatVector(