fix: Fix partition filters with timestamp value #12368

kecookier · 2025-02-18T04:02:30Z

When using partition filters with timestamp value, use configuration
to control whether to interpret it as local time or UTC.

Follow-up for #11957

netlify · 2025-02-18T04:02:50Z

✅ Deploy Preview for meta-velox canceled.

Name	Link
🔨 Latest commit	`0f72f61`
🔍 Latest deploy log	https://app.netlify.com/sites/meta-velox/deploys/67e0f7a57e8fc900081e23f4

kecookier · 2025-02-18T04:05:12Z

@rui-mo Please take a look. Thanks.

rui-mo

Hi @kecookier, thanks for your fix! Is this change proposed to align the behaviour with Spark, and could you please provide more details on the issue you have met?

rui-mo · 2025-02-18T08:30:37Z

velox/connectors/hive/HiveConnectorUtil.h

    const std::unordered_map<std::string, std::shared_ptr<HiveColumnHandle>>&
-        partitionKeysHandle);
+        partitionKeysHandle,
+    bool asLocalTime = true);


Do we need the default value?

@rui-mo testFilters is also called by PositionalDeleteFileReader (https://github.com/facebookincubator/velox/blob/main/velox/connectors/hive/iceberg/PositionalDeleteFileReader.cpp#L109). I'm not sure whether this reader also needs this feature, so I set a default value. If this reader also needs it, I'll modify it accordingly.

Gluten with Iceberg may also need to align with Spark, so remove this default value.

rui-mo · 2025-02-18T08:35:08Z

velox/exec/tests/TableScanTest.cpp

-                     VELOX_USER_FAIL("{}", status.message());
-                   });
-      t.toGMT(Timestamp::defaultTimezone());
-      partitionValueStr = "'" + t.toString() + "'";


I'm not entirely sure why this piece needs to be removed.

I'm not entirely sure why this piece needs to be removed.

This code was introduced by https://github.com/facebookincubator/velox/pull/11754/files#diff-c62590f29a2bf22e50e276f2665e0a7666968880de73a6b4db745cbc0cfde6feL202 and is only called by TableScanTest.partitionedTableTimestampKey. To clarify the code, I moved these logics into TableScanTest.partitionedTableTimestampKey from L2043 to L2085.

velox/exec/tests/TableScanTest.cpp

kecookier · 2025-02-18T09:27:50Z

@rui-mo Yes, This PR is attend to align with Spark, when I run all UT of oap/velox:gluten-1.3.0, TableScanTest.partitionedTableTimestampKey will failed. I found this failure is caused by this commit from oap/velox oap-project@bd181bb#diff-5b58b2fe28277afcbd1af560bbe3f237d3571969504fc9ee92b5149c6e132813

After comunicate with jiake offline, I kown PR #11957 will replace this commit. When I check this PR, I found it not contain the modify of HiveConnectorUtil.cpp.

Yes, this PR is intended to align with Spark. When I run all unit tests for oap/velox:gluten-1.3.0, TableScanTest.partitionedTableTimestampKey fails. I found that this failure is caused by this commit in oap/velox: oap-project@bd181bb#diff-5b58b2fe28277afcbd1af560bbe3f237d3571969504fc9ee92b5149c6e132813.

After communicating with Jiake offline, I learned that PR #11957 will replace this commit. However, when I checked this PR, I found that it does not contain modifications to HiveConnectorUtil.cpp.

[ RUN      ] TableScanTest.partitionedTableTimestampKey
I0218 17:14:01.329938 24474 Compression.cpp:678] Initialized zstd compressor with compression level 7
/opt/meituan/zhaokuo03/workspace/velox/velox/exec/tests/utils/QueryAssertions.cpp:1182: Failure
Failed
Expected 10000, got 10000
10000 extra rows, 10000 missing rows
10 of extra rows:
        "2023-10-27T00:12:35.000000000" | null | null
        "2023-10-27T00:12:35.000000000" | null | null
        "2023-10-27T00:12:35.000000000" | null | null
        "2023-10-27T00:12:35.000000000" | null | null
        "2023-10-27T00:12:35.000000000" | null | null
        "2023-10-27T00:12:35.000000000" | null | null
        "2023-10-27T00:12:35.000000000" | null | null
        "2023-10-27T00:12:35.000000000" | null | null
        "2023-10-27T00:12:35.000000000" | null | null
        "2023-10-27T00:12:35.000000000" | null | null

10 of missing rows:
        "2023-10-27T07:12:35.000000000" | null | null
        "2023-10-27T07:12:35.000000000" | null | null
        "2023-10-27T07:12:35.000000000" | null | null
        "2023-10-27T07:12:35.000000000" | null | null
        "2023-10-27T07:12:35.000000000" | null | null
        "2023-10-27T07:12:35.000000000" | null | null
        "2023-10-27T07:12:35.000000000" | null | null
        "2023-10-27T07:12:35.000000000" | null | null
        "2023-10-27T07:12:35.000000000" | null | null
        "2023-10-27T07:12:35.000000000" | null | null

Note: DuckDB only supports timestamps of millisecond precision. If this test involves timestamp inputs, please make sure you use the right precision.
DuckDB query: SELECT '2023-10-27T07:12:35.000000000', * FROM tmp
Note: results are compared without epsilon because values at non-floating-point columns do not form unique keys.
/opt/meituan/zhaokuo03/workspace/velox/velox/exec/tests/utils/QueryAssertions.cpp:1182: Failure
Failed
Expected 10000, got 10000
10000 extra rows, 10000 missing rows
10 of extra rows:
        null | "2023-10-27T00:12:35.000000000" | null
        null | "2023-10-27T00:12:35.000000000" | null
        null | "2023-10-27T00:12:35.000000000" | null
        null | "2023-10-27T00:12:35.000000000" | null
        null | "2023-10-27T00:12:35.000000000" | null
        null | "2023-10-27T00:12:35.000000000" | null
        null | "2023-10-27T00:12:35.000000000" | null
        null | "2023-10-27T00:12:35.000000000" | null
        null | "2023-10-27T00:12:35.000000000" | null
        null | "2023-10-27T00:12:35.000000000" | null

10 of missing rows:
        null | "2023-10-27T07:12:35.000000000" | null
        null | "2023-10-27T07:12:35.000000000" | null
        null | "2023-10-27T07:12:35.000000000" | null
        null | "2023-10-27T07:12:35.000000000" | null
        null | "2023-10-27T07:12:35.000000000" | null
        null | "2023-10-27T07:12:35.000000000" | null
        null | "2023-10-27T07:12:35.000000000" | null
        null | "2023-10-27T07:12:35.000000000" | null
        null | "2023-10-27T07:12:35.000000000" | null
        null | "2023-10-27T07:12:35.000000000" | null

Note: DuckDB only supports timestamps of millisecond precision. If this test involves timestamp inputs, please make sure you use the right precision.
DuckDB query: SELECT c0, '2023-10-27T07:12:35.000000000', c1 FROM tmp
Note: results are compared without epsilon because values at non-floating-point columns do not form unique keys.
/opt/meituan/zhaokuo03/workspace/velox/velox/exec/tests/utils/QueryAssertions.cpp:1182: Failure
Failed
Expected 10000, got 10000
10000 extra rows, 10000 missing rows
10 of extra rows:
        null | null | "2023-10-27T00:12:35.000000000"
        null | null | "2023-10-27T00:12:35.000000000"
        null | null | "2023-10-27T00:12:35.000000000"
        null | null | "2023-10-27T00:12:35.000000000"
        null | null | "2023-10-27T00:12:35.000000000"
        null | null | "2023-10-27T00:12:35.000000000"
        null | null | "2023-10-27T00:12:35.000000000"
        null | null | "2023-10-27T00:12:35.000000000"
        null | null | "2023-10-27T00:12:35.000000000"
        null | null | "2023-10-27T00:12:35.000000000"

10 of missing rows:
        null | null | "2023-10-27T07:12:35.000000000"
        null | null | "2023-10-27T07:12:35.000000000"
        null | null | "2023-10-27T07:12:35.000000000"
        null | null | "2023-10-27T07:12:35.000000000"
        null | null | "2023-10-27T07:12:35.000000000"
        null | null | "2023-10-27T07:12:35.000000000"
        null | null | "2023-10-27T07:12:35.000000000"
        null | null | "2023-10-27T07:12:35.000000000"
        null | null | "2023-10-27T07:12:35.000000000"
        null | null | "2023-10-27T07:12:35.000000000"

Note: DuckDB only supports timestamps of millisecond precision. If this test involves timestamp inputs, please make sure you use the right precision.
DuckDB query: SELECT c0, c1, '2023-10-27T07:12:35.000000000' FROM tmp
Note: results are compared without epsilon because values at non-floating-point columns do not form unique keys.
/opt/meituan/zhaokuo03/workspace/velox/velox/exec/tests/utils/QueryAssertions.cpp:1182: Failure
Failed
Expected 10000, got 10000
10000 extra rows, 10000 missing rows
10 of extra rows:
        "2023-10-27T00:12:35.000000000"
        "2023-10-27T00:12:35.000000000"
        "2023-10-27T00:12:35.000000000"
        "2023-10-27T00:12:35.000000000"
        "2023-10-27T00:12:35.000000000"
        "2023-10-27T00:12:35.000000000"
        "2023-10-27T00:12:35.000000000"
        "2023-10-27T00:12:35.000000000"
        "2023-10-27T00:12:35.000000000"
        "2023-10-27T00:12:35.000000000"

10 of missing rows:
        "2023-10-27T07:12:35.000000000"
        "2023-10-27T07:12:35.000000000"
        "2023-10-27T07:12:35.000000000"
        "2023-10-27T07:12:35.000000000"
        "2023-10-27T07:12:35.000000000"
        "2023-10-27T07:12:35.000000000"
        "2023-10-27T07:12:35.000000000"
        "2023-10-27T07:12:35.000000000"
        "2023-10-27T07:12:35.000000000"
        "2023-10-27T07:12:35.000000000"

Note: DuckDB only supports timestamps of millisecond precision. If this test involves timestamp inputs, please make sure you use the right precision.
DuckDB query: SELECT '2023-10-27T07:12:35.000000000' FROM tmp
/opt/meituan/zhaokuo03/workspace/velox/velox/exec/tests/utils/QueryAssertions.cpp:1182: Failure
Failed
Expected 10000, got 10000
10000 extra rows, 10000 missing rows
10 of extra rows:
        "2023-10-27T00:12:35.000000000" | null | null
        "2023-10-27T00:12:35.000000000" | null | null
        "2023-10-27T00:12:35.000000000" | null | null
        "2023-10-27T00:12:35.000000000" | null | null
        "2023-10-27T00:12:35.000000000" | null | null
        "2023-10-27T00:12:35.000000000" | null | null
        "2023-10-27T00:12:35.000000000" | null | null
        "2023-10-27T00:12:35.000000000" | null | null
        "2023-10-27T00:12:35.000000000" | null | null
        "2023-10-27T00:12:35.000000000" | null | null

10 of missing rows:
        "2023-10-27T07:12:35.000000000" | null | null
        "2023-10-27T07:12:35.000000000" | null | null
        "2023-10-27T07:12:35.000000000" | null | null
        "2023-10-27T07:12:35.000000000" | null | null
        "2023-10-27T07:12:35.000000000" | null | null
        "2023-10-27T07:12:35.000000000" | null | null
        "2023-10-27T07:12:35.000000000" | null | null
        "2023-10-27T07:12:35.000000000" | null | null
        "2023-10-27T07:12:35.000000000" | null | null
        "2023-10-27T07:12:35.000000000" | null | null

Note: DuckDB only supports timestamps of millisecond precision. If this test involves timestamp inputs, please make sure you use the right precision.
DuckDB query: SELECT '2023-10-27T07:12:35.000000000', * FROM tmp
Note: results are compared without epsilon because values at non-floating-point columns do not form unique keys.
[  FAILED  ] TableScanTest.partitionedTableTimestampKey (6115 ms)

kecookier · 2025-02-20T02:03:25Z

@rui-mo All comments done, could you please review it again?

rui-mo

Thanks.

velox/exec/tests/TableScanTest.cpp

rui-mo · 2025-02-21T20:07:26Z

velox/connectors/hive/iceberg/PositionalDeleteFileReader.cpp

-          {})) {
+          {},
+          hiveConfig_->readTimestampPartitionValueAsLocalTime(
+              connectorQueryCtx_->sessionProperties()))) {


It would be nice if we could add test for this change on the iceberg read.

@kecookier are you planning to address this comment?

@rui-mo @majetideepak I am not familiar with Iceberg now, and I need some time to add this test case. This may be done in another PR. Perhaps this PR can be merged first?

kecookier · 2025-02-25T02:18:29Z

@majetideepak Could you help review this?

kecookier · 2025-03-12T03:05:40Z

@majetideepak Can you help merge this PR?

majetideepak · 2025-03-12T08:11:42Z

@kecookier I missed adding the ready-to-merge label which I did now. Meta should import and merge it.

facebook-github-bot · 2025-03-12T17:07:08Z

@kagamiori has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

kagamiori · 2025-03-12T22:45:13Z

Hi @kecookier, could you please rebase this PR onto the latest main? That is needed for merging this PR. Thanks!

kecookier · 2025-03-13T03:07:24Z

Hi @kecookier, could you please rebase this PR onto the latest main? That is needed for merging this PR. Thanks!

Hi @kagamiori, I've rebased the PR onto the latest main. Please let me know if you need anything else. Thanks.

facebook-github-bot · 2025-03-13T18:19:18Z

@kagamiori has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

kagamiori · 2025-03-13T22:08:07Z

Hi @kecookier, there is an internal build failure, possibly due to passing non-constant-literal string to fmt::format in TableScanTest.cpp. Could you take a look?

Error message:
[New Failure] Target failed to build.
Action failed: //velox/exec/tests:velox_exec_test (cfg:linux-x86_64-platform010-clang17-no-san#434e8269c) (cxx_compile TableScanTest.cpp)
Remote command returned non-zero exit code 1
Remote action, reproduce with: `frecli cas download-action 4350a94f32dcf9bc01a524de8f539ffbea42e5502dfc4581c980198052eb63fd:145`
Stdout: <empty>
Stderr:
velox/exec/tests/TableScanTest.cpp:2043:3: error: call to immediate function 'TableScanTest_partitionedTableTimestampKey_Test::TestBody()::(anonymous class)::operator()' is not a constant expression
 2043 |   expect(
      |   ^
velox/exec/tests/TableScanTest.cpp:2044:7: note: non-literal type 'PlanNodePtr' (aka 'shared_ptr<const PlanNode>') cannot be used in a constant expression
 2044 |       PlanBuilder()
      |       ^
velox/exec/tests/TableScanTest.cpp:2054:3: error: call to immediate function 'TableScanTest_partitionedTableTimestampKey_Test::TestBody()::(anonymous class)::operator()' is not a constant expression
 2054 |   expect(
      |   ^
velox/exec/tests/TableScanTest.cpp:2055:7: note: non-literal type 'PlanNodePtr' (aka 'shared_ptr<const PlanNode>') cannot be used in a constant expression
 2055 |       PlanBuilder()
      |       ^
velox/exec/tests/TableScanTest.cpp:2065:3: error: call to immediate function 'TableScanTest_partitionedTableTimestampKey_Test::TestBody()::(anonymous class)::operator()' is not a constant expression
 2065 |   expect(
      |   ^
velox/exec/tests/TableScanTest.cpp:2066:7: note: non-literal type 'PlanNodePtr' (aka 'shared_ptr<const PlanNode>') cannot be used in a constant expression
 2066 |       PlanBuilder()
      |       ^
velox/exec/tests/TableScanTest.cpp:2077:3: error: call to immediate function 'TableScanTest_partitionedTableTimestampKey_Test::TestBody()::(anonymous class)::operator()' is not a constant expression
 2077 |   expect(
      |   ^
velox/exec/tests/TableScanTest.cpp:2078:7: note: non-literal type 'PlanNodePtr' (aka 'shared_ptr<const PlanNode>') cannot be used in a constant expression
 2078 |       PlanBuilder()
      |       ^
velox/exec/tests/TableScanTest.cpp:2128:24: error: call to immediate function 'TableScanTest_partitionedTableTimestampKey_Test::TestBody()::(anonymous class)::operator()' is not a constant expression
 2128 |         .assertResults(sql("SELECT {}, * FROM tmp", true));
      |                        ^
velox/exec/tests/TableScanTest.cpp:2128:28: note: non-literal type 'const std::string' (aka 'const basic_string<char>') cannot be used in a constant expression
 2128 |         .assertResults(sql("SELECT {}, * FROM tmp", true));
      |                            ^
velox/exec/tests/TableScanTest.cpp:2138:24: error: call to immediate function 'TableScanTest_partitionedTableTimestampKey_Test::TestBody()::(anonymous class)::operator()' is not a constant expression
 2138 |         .assertResults(sql("SELECT {}, * FROM tmp", false));
      |                        ^
velox/exec/tests/TableScanTest.cpp:2138:28: note: non-literal type 'const std::string' (aka 'const basic_string<char>') cannot be used in a constant expression
 2138 |         .assertResults(sql("SELECT {}, * FROM tmp", false));
      |                            ^
6 errors generated.

kecookier · 2025-03-14T05:27:00Z

@kagamiori Thanks for the review, I have addressed this issue.

facebook-github-bot · 2025-03-14T19:41:37Z

@kagamiori has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

kagamiori · 2025-03-17T04:56:50Z

Hi @kecookier, I updated the imported code but still see the same build error. I think the problem is that fmt::format() cannot accept a variable at the first argument (but rather expect a constant literal, e.g., "abc{}").

kecookier · 2025-03-17T05:03:14Z

Hi @kecookier, I updated the imported code but still see the same build error. I think the problem is that fmt::format() cannot accept a variable at the first argument (but rather expect a constant literal, e.g., "abc{}").

Hi @kagamiori, what's your build environment? I tested this in Clang 15 on macOS.

$ clang --version
Apple clang version 15.0.0 (clang-1500.3.9.4)
Target: x86_64-apple-darwin23.6.0
Thread model: posix
InstalledDir: /Library/Developer/CommandLineTools/usr/bin

kagamiori · 2025-03-17T23:06:53Z

Hi @kecookier, I updated the imported code but still see the same build error. I think the problem is that fmt::format() cannot accept a variable at the first argument (but rather expect a constant literal, e.g., "abc{}").

Hi @kagamiori, what's your build environment? I tested this in Clang 15 on macOS.
$ clang --version
Apple clang version 15.0.0 (clang-1500.3.9.4)
Target: x86_64-apple-darwin23.6.0
Thread model: posix
InstalledDir: /Library/Developer/CommandLineTools/usr/bin

Hi @kecookier, I think we're using clang-17 on linux internally according to the error message above.

kecookier · 2025-03-19T09:15:33Z

@kagamiori I built with clang-17 in an Ubuntu Docker container, and it succeeded. Although I couldn't reproduce this build error, I also tried to make some fixes based on your suggestions. I deleted the helper function expect(). Could you help trigger your internal build?

{
  "directory": "/velox/_build/release",
  "command": "/usr/bin/clang++-17 -DBOOST_ATOMIC_NO_LIB -DBOOST_ATOMIC_STATIC_LINK -DBOOST_CHRONO_NO_LIB -DBOOST_CHRONO_STATIC_LINK -DBOOST_CONTAINER_NO_LIB -DBOOST_CONTAINER_STATIC_LINK -DBOOST_CONTEXT_EXPORT=\"\" -DBOOST_CONTEXT_NO_LIB=\"\" -DBOOST_CONTEXT_STATIC_LINK=\"\" -DBOOST_DATE_TIME_NO_LIB -DBOOST_DATE_TIME_STATIC_LINK -DBOOST_FILESYSTEM_NO_LIB -DBOOST_FILESYSTEM_STATIC_LINK=1 -DBOOST_PROGRAM_OPTIONS_NO_LIB -DBOOST_PROGRAM_OPTIONS_STATIC_LINK -DBOOST_RANDOM_NO_LIB -DBOOST_RANDOM_STATIC_LINK -DBOOST_THREAD_NO_LIB -DBOOST_THREAD_STATIC_LINK -DBOOST_THREAD_USE_LIB -DCURL_STATICLIB -DGFLAGS_IS_A_DLL=0 -DGLOG_CUSTOM_PREFIX_SUPPORT -DSIMDJSON_THREADS_ENABLED=1 -DVELOX_ENABLE_PARQUET -I/velox/. -I/velox/velox/external/xxhash -I/velox/_build/release/_deps/fmt-src/include -I/velox/_build/release/_deps/boost-src/libs/atomic/include -I/velox/_build/release/_deps/boost-src/libs/assert/include -I/velox/_build/release/_deps/boost-src/libs/config/include -I/velox/_build/release/_deps/boost-src/libs/type_traits/include -I/velox/_build/release/_deps/boost-src/libs/static_assert/include -I/velox/_build/release/_deps/boost-src/libs/context/include -I/velox/_build/release/_deps/boost-src/libs/core/include -I/velox/_build/release/_deps/boost-src/libs/throw_exception/include -I/velox/_build/release/_deps/boost-src/libs/mp11/include -I/velox/_build/release/_deps/boost-src/libs/pool/include -I/velox/_build/release/_deps/boost-src/libs/integer/include -I/velox/_build/release/_deps/boost-src/libs/winapi/include -I/velox/_build/release/_deps/boost-src/libs/predef/include -I/velox/_build/release/_deps/boost-src/libs/smart_ptr/include -I/velox/_build/release/_deps/boost-src/libs/move/include -I/velox/_build/release/_deps/boost-src/libs/date_time/include -I/velox/_build/release/_deps/boost-src/libs/algorithm/include -I/velox/_build/release/_deps/boost-src/libs/array/include -I/velox/_build/release/_deps/boost-src/libs/bind/include -I/velox/_build/release/_deps/boost-src/libs/concept_check/include -I/velox/_build/release/_deps/boost-src/libs/preprocessor/include -I/velox/_build/release/_deps/boost-src/libs/exception/include -I/velox/_build/release/_deps/boost-src/libs/tuple/include -I/velox/_build/release/_deps/boost-src/libs/function/include -I/velox/_build/release/_deps/boost-src/libs/iterator/include -I/velox/_build/release/_deps/boost-src/libs/detail/include -I/velox/_build/release/_deps/boost-src/libs/function_types/include -I/velox/_build/release/_deps/boost-src/libs/mpl/include -I/velox/_build/release/_deps/boost-src/libs/utility/include -I/velox/_build/release/_deps/boost-src/libs/io/include -I/velox/_build/release/_deps/boost-src/libs/fusion/include -I/velox/_build/release/_deps/boost-src/libs/container_hash/include -I/velox/_build/release/_deps/boost-src/libs/describe/include -I/velox/_build/release/_deps/boost-src/libs/typeof/include -I/velox/_build/release/_deps/boost-src/libs/functional/include -I/velox/_build/release/_deps/boost-src/libs/optional/include -I/velox/_build/release/_deps/boost-src/libs/range/include -I/velox/_build/release/_deps/boost-src/libs/conversion/include -I/velox/_build/release/_deps/boost-src/libs/regex/include -I/velox/_build/release/_deps/boost-src/libs/unordered/include -I/velox/_build/release/_deps/boost-src/libs/lexical_cast/include -I/velox/_build/release/_deps/boost-src/libs/container/include -I/velox/_build/release/_deps/boost-src/libs/intrusive/include -I/velox/_build/release/_deps/boost-src/libs/numeric/conversion/include -I/velox/_build/release/_deps/boost-src/libs/tokenizer/include -I/velox/_build/release/_deps/boost-src/libs/filesystem/include -I/velox/_build/release/_deps/boost-src/libs/system/include -I/velox/_build/release/_deps/boost-src/libs/variant2/include -I/velox/_build/release/_deps/boost-src/libs/program_options/include -I/velox/_build/release/_deps/boost-src/libs/any/include -I/velox/_build/release/_deps/boost-src/libs/type_index/include -I/velox/_build/release/_deps/boost-src/libs/thread/include -I/velox/_build/release/_deps/boost-src/libs/chrono/include -I/velox/_build/release/_deps/boost-src/libs/ratio/include -I/velox/_build/release/_deps/boost-src/libs/headers/include -I/velox/_build/release/_deps/boost-src/libs/crc/include -I/velox/_build/release/_deps/boost-src/libs/circular_buffer/include -I/velox/_build/release/_deps/boost-src/libs/math/include -I/velox/_build/release/_deps/boost-src/libs/random/include -I/velox/_build/release/_deps/boost-src/libs/dynamic_bitset/include -I/velox/_build/release/_deps/boost-src/libs/multi_index/include -I/velox/_build/release/_deps/boost-src/libs/multiprecision/include -I/velox/_build/release/_deps/boost-src/libs/uuid/include -I/velox/_build/release/_deps/boost-src/libs/tti/include -I/velox/_build/release/_deps/boost-src/libs/variant/include -I/velox/_build/release/_deps/xsimd-src/include -I/velox/velox/external/date/velox/external/date -I/velox/_build/release/_deps/re2-src -I/velox/_build/release/_deps/simdjson-src/include -I/velox/_build/release/_deps/duckdb-src/src/include -I/velox/_build/release/_deps/duckdb-src/third_party/fsst -I/velox/_build/release/_deps/duckdb-src/third_party/libpg_query/include -I/velox/_build/release/_deps/duckdb-src/third_party/miniz -I/velox/_build/release/_deps/duckdb-src/third_party/hyperloglog -I/velox/_build/release/_deps/duckdb-src/third_party/fastpforlib -I/velox/_build/release -I/velox/_build/release/_deps/cpr-src/include -I/velox/_build/release/_deps/cpr-build/cpr_generated_includes -I/velox/_build/release/_deps/curl-src/include -isystem /velox/_build/release/_deps/protobuf-src/src -isystem /velox/velox -isystem /velox/velox/external -isystem /velox/_build/release/_deps/folly-src -isystem /velox/_build/release/_deps/folly-build -isystem /usr/include/libdwarf -isystem /velox/_build/release/_deps/gflags-build/include -isystem /velox/_build/release/_deps/glog-build -isystem /usr/include/double-conversion -isystem /velox/_build/release/_deps/googletest-src/googletest/include -isystem /velox/_build/release/_deps/googletest-src/googletest -isystem /velox/_build/release/_deps/libstemmer/src/libstemmer/include -isystem /velox/_build/release/CMake/resolve_dependency_modules/arrow/arrow_ep/install/include -isystem /velox/_build/release/_deps/googletest-src/googlemock/include -isystem /velox/_build/release/_deps/googletest-src/googlemock  -mavx2 -mfma -mavx -mf16c -mlzcnt -mbmi2 -D USE_VELOX_COMMON_BASE -D HAS_UNCAUGHT_EXCEPTIONS -Wall -Wextra -Wno-unused        -Wno-unused-parameter        -Wno-sign-compare        -Wno-ignored-qualifiers        -Wno-range-loop-analysis          -Wno-mismatched-tags          -Wno-nullability-completeness -Werror -O3 -DNDEBUG -std=gnu++17 -fPIE -fcolor-diagnostics -o velox/exec/tests/CMakeFiles/velox_exec_test.dir/TableScanTest.cpp.o -c /velox/velox/exec/tests/TableScanTest.cpp",
  "file": "/velox/velox/exec/tests/TableScanTest.cpp",
  "output": "velox/exec/tests/CMakeFiles/velox_exec_test.dir/TableScanTest.cpp.o"
}

facebook-github-bot · 2025-03-21T16:02:37Z

@kagamiori has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

kagamiori · 2025-03-21T22:31:27Z

Hi @kecookier, thank you for updating! The internal build now gets another error with sql() that is similar. I think this is also due to passing a variable as the first argument to fmt::format. Could we avoid that too if possible?

Error message:
[New Failure] Target failed to build.
Action failed: fbcode//velox/exec/tests:velox_exec_test (cfg:linux-x86_64-platform010-clang17-no-san#b645abfe1d1f) (cxx_compile TableScanTest.cpp)
Remote command returned non-zero exit code 1
Remote action, reproduce with: `frecli cas download-action 312fc65395a3c6d5618ae20b41703546aa4a20b1c5c13527ce5141a039017dce:145`
Stdout: <empty>
Stderr:
fbcode/velox/exec/tests/TableScanTest.cpp:2102:24: error: call to immediate function 'TableScanTest_partitionedTableTimestampKey_Test::TestBody()::(anonymous class)::operator()' is not a constant expression
 2102 |         .assertResults(sql(sqlTemplate, true));
      |                        ^
fbcode/velox/exec/tests/TableScanTest.cpp:2102:28: note: read of non-constexpr variable 'sqlTemplate' is not allowed in a constant expression
 2102 |         .assertResults(sql(sqlTemplate, true));
      |                            ^
fbcode/velox/exec/tests/TableScanTest.cpp:2092:17: note: declared here
 2092 |     const char* sqlTemplate = "SELECT {}, * FROM tmp";
      |                 ^
fbcode/velox/exec/tests/TableScanTest.cpp:2112:24: error: call to immediate function 'TableScanTest_partitionedTableTimestampKey_Test::TestBody()::(anonymous class)::operator()' is not a constant expression
 2112 |         .assertResults(sql(sqlTemplate, false));
      |                        ^
fbcode/velox/exec/tests/TableScanTest.cpp:2112:28: note: read of non-constexpr variable 'sqlTemplate' is not allowed in a constant expression
 2112 |         .assertResults(sql(sqlTemplate, false));
      |                            ^
fbcode/velox/exec/tests/TableScanTest.cpp:2092:17: note: declared here
 2092 |     const char* sqlTemplate = "SELECT {}, * FROM tmp";
      |                 ^
fbcode/velox/exec/tests/TableScanTest.cpp:2135:24: error: call to immediate function 'TableScanTest_partitionedTableTimestampKey_Test::TestBody()::(anonymous class)::operator()' is not a constant expression
 2135 |         .assertResults(sql(sqlTemplate, true));
      |                        ^
fbcode/velox/exec/tests/TableScanTest.cpp:2135:28: note: read of non-constexpr variable 'sqlTemplate' is not allowed in a constant expression
 2135 |         .assertResults(sql(sqlTemplate, true));
      |                            ^
fbcode/velox/exec/tests/TableScanTest.cpp:2125:17: note: declared here
 2125 |     const char* sqlTemplate = "SELECT c0, {}, c1 FROM tmp";
      |                 ^
fbcode/velox/exec/tests/TableScanTest.cpp:2145:24: error: call to immediate function 'TableScanTest_partitionedTableTimestampKey_Test::TestBody()::(anonymous class)::operator()' is not a constant expression
 2145 |         .assertResults(sql(sqlTemplate, false));
      |                        ^
fbcode/velox/exec/tests/TableScanTest.cpp:2145:28: note: read of non-constexpr variable 'sqlTemplate' is not allowed in a constant expression
 2145 |         .assertResults(sql(sqlTemplate, false));
      |                            ^
fbcode/velox/exec/tests/TableScanTest.cpp:2125:17: note: declared here
 2125 |     const char* sqlTemplate = "SELECT c0, {}, c1 FROM tmp";
      |                 ^
fbcode/velox/exec/tests/TableScanTest.cpp:2167:24: error: call to immediate function 'TableScanTest_partitionedTableTimestampKey_Test::TestBody()::(anonymous class)::operator()' is not a constant expression
 2167 |         .assertResults(sql(sqlTemplate, true));
      |                        ^
fbcode/velox/exec/tests/TableScanTest.cpp:2167:28: note: read of non-constexpr variable 'sqlTemplate' is not allowed in a constant expression
 2167 |         .assertResults(sql(sqlTemplate, true));
      |                            ^
fbcode/velox/exec/tests/TableScanTest.cpp:2158:17: note: declared here
 2158 |     const char* sqlTemplate = "SELECT c0, c1, {} FROM tmp";
      |                 ^
fbcode/velox/exec/tests/TableScanTest.cpp:2177:24: error: call to immediate function 'TableScanTest_partitionedTableTimestampKey_Test::TestBody()::(anonymous class)::operator()' is not a constant expression
 2177 |         .assertResults(sql(sqlTemplate, false));
      |                        ^
fbcode/velox/exec/tests/TableScanTest.cpp:2177:28: note: read of non-constexpr variable 'sqlTemplate' is not allowed in a constant expression
 2177 |         .assertResults(sql(sqlTemplate, false));
      |                            ^
fbcode/velox/exec/tests/TableScanTest.cpp:2158:17: note: declared here
 2158 |     const char* sqlTemplate = "SELECT c0, c1, {} FROM tmp";
      |                 ^
fbcode/velox/exec/tests/TableScanTest.cpp:2199:24: error: call to immediate function 'TableScanTest_partitionedTableTimestampKey_Test::TestBody()::(anonymous class)::operator()' is not a constant expression
 2199 |         .assertResults(sql(sqlTemplate, true));
      |                        ^
fbcode/velox/exec/tests/TableScanTest.cpp:2199:28: note: read of non-constexpr variable 'sqlTemplate' is not allowed in a constant expression
 2199 |         .assertResults(sql(sqlTemplate, true));
      |                            ^
fbcode/velox/exec/tests/TableScanTest.cpp:2190:17: note: declared here
 2190 |     const char* sqlTemplate = "SELECT {} FROM tmp";
      |                 ^
fbcode/velox/exec/tests/TableScanTest.cpp:2209:24: error: call to immediate function 'TableScanTest_partitionedTableTimestampKey_Test::TestBody()::(anonymous class)::operator()' is not a constant expression
 2209 |         .assertResults(sql(sqlTemplate, false));
      |                        ^
fbcode/velox/exec/tests/TableScanTest.cpp:2209:28: note: read of non-constexpr variable 'sqlTemplate' is not allowed in a constant expression
 2209 |         .assertResults(sql(sqlTemplate, false));
      |                            ^
fbcode/velox/exec/tests/TableScanTest.cpp:2190:17: note: declared here
 2190 |     const char* sqlTemplate = "SELECT {} FROM tmp";
      |                 ^
fbcode/velox/exec/tests/TableScanTest.cpp:2253:24: error: call to immediate function 'TableScanTest_partitionedTableTimestampKey_Test::TestBody()::(anonymous class)::operator()' is not a constant expression
 2253 |         .assertResults(sql("SELECT {}, * FROM tmp", true));
      |                        ^
fbcode/velox/exec/tests/TableScanTest.cpp:2253:24: note: non-constexpr function 'operator()' cannot be used in a constant expression
fbcode/velox/exec/tests/TableScanTest.cpp:2068:14: note: declared here
 2068 |   auto sql = [&](const char* sqlTemplate, bool asLocalTime) {
      |              ^
fbcode/velox/exec/tests/TableScanTest.cpp:2263:24: error: call to immediate function 'TableScanTest_partitionedTableTimestampKey_Test::TestBody()::(anonymous class)::operator()' is not a constant expression
 2263 |         .assertResults(sql("SELECT {}, * FROM tmp", false));
      |                        ^
fbcode/velox/exec/tests/TableScanTest.cpp:2263:24: note: non-constexpr function 'operator()' cannot be used in a constant expression
fbcode/velox/exec/tests/TableScanTest.cpp:2068:14: note: declared here
 2068 |   auto sql = [&](const char* sqlTemplate, bool asLocalTime) {
      |              ^
10 errors generated.

kecookier · 2025-03-24T04:03:11Z

Hi @kagamiori, I've removed the helper function sql() as suggested. Please take a look.

facebook-github-bot · 2025-03-24T17:01:19Z

@kagamiori has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot · 2025-03-25T01:18:31Z

@kagamiori merged this pull request in e8dd265.

) Summary: When using partition filters with timestamp value, use configuration to control whether to interpret it as local time or UTC. Follow-up for facebookincubator#11957 Pull Request resolved: facebookincubator#12368 Reviewed By: kKPulla Differential Revision: D71047729 Pulled By: kagamiori fbshipit-source-id: 471011591f5c8ff70507d49e425b9749a06b02d4

kecookier requested a review from majetideepak as a code owner February 18, 2025 04:02

facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Feb 18, 2025

rui-mo reviewed Feb 18, 2025

View reviewed changes

rui-mo approved these changes Feb 21, 2025

View reviewed changes

majetideepak approved these changes Mar 3, 2025

View reviewed changes

majetideepak added the ready-to-merge PR that have been reviewed and are ready for merging. PRs with this tag notify the Velox Meta oncall label Mar 12, 2025

kecookier force-pushed the fix-timestamp-partition-key branch from a158170 to 94a6862 Compare March 13, 2025 01:39

kecookier added 2 commits March 24, 2025 14:10

fix: Compatible with Spark when partition filters with timestamp value

bc9e9b3

fix compile

bd13a09

kecookier added 11 commits March 24, 2025 14:10

fix comments

e06a639

remove asLocalTime default value

08fcbfb

fix

cf9eb8e

try fix

d4fa061

try delete helper function exepct() to fix build error in meta

3015b9d

fix

82f5bef

fix

b2e1dd3

fix ut

239854f

fix

e45c7ac

fix

acbf15e

fix

0f72f61

kecookier force-pushed the fix-timestamp-partition-key branch from 7b0ee08 to 0f72f61 Compare March 24, 2025 06:11

facebook-github-bot closed this in e8dd265 Mar 25, 2025

facebook-github-bot added the Merged label Mar 25, 2025

fix: Fix partition filters with timestamp value #12368

fix: Fix partition filters with timestamp value #12368

Uh oh!

Conversation

kecookier commented Feb 18, 2025 • edited by majetideepak Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

netlify bot commented Feb 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for meta-velox canceled.

Uh oh!

kecookier commented Feb 18, 2025

Uh oh!

rui-mo left a comment

Choose a reason for hiding this comment

Uh oh!

rui-mo Feb 18, 2025

Choose a reason for hiding this comment

Uh oh!

kecookier Feb 18, 2025

Choose a reason for hiding this comment

Uh oh!

kecookier Feb 20, 2025

Choose a reason for hiding this comment

Uh oh!

rui-mo Feb 18, 2025

Choose a reason for hiding this comment

Uh oh!

kecookier Feb 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

kecookier commented Feb 18, 2025

Uh oh!

kecookier commented Feb 20, 2025

Uh oh!

rui-mo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

rui-mo Feb 21, 2025

Choose a reason for hiding this comment

Uh oh!

majetideepak Mar 3, 2025

Choose a reason for hiding this comment

Uh oh!

kecookier Mar 4, 2025

Choose a reason for hiding this comment

Uh oh!

kecookier commented Feb 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kecookier commented Mar 12, 2025

Uh oh!

majetideepak commented Mar 12, 2025

Uh oh!

facebook-github-bot commented Mar 12, 2025

Uh oh!

kagamiori commented Mar 12, 2025

Uh oh!

kecookier commented Mar 13, 2025

Uh oh!

facebook-github-bot commented Mar 13, 2025

Uh oh!

kagamiori commented Mar 13, 2025

Uh oh!

kecookier commented Mar 14, 2025

Uh oh!

facebook-github-bot commented Mar 14, 2025

Uh oh!

kagamiori commented Mar 17, 2025

Uh oh!

kecookier commented Mar 17, 2025

Uh oh!

kagamiori commented Mar 17, 2025

Uh oh!

kecookier commented Mar 19, 2025

kecookier commented Feb 18, 2025 •

edited by majetideepak

Loading

netlify bot commented Feb 18, 2025 •

edited

Loading

kecookier Feb 18, 2025 •

edited

Loading

kecookier commented Feb 25, 2025 •

edited

Loading