[GLUTEN-10247][FLINK] Support date_format function by KevinyhZou · Pull Request #10248 · apache/gluten

KevinyhZou · 2025-07-23T03:54:17Z

What changes were proposed in this pull request?

(Please fill in changes proposed in this fix)

(Fixes: #10247)

How was this patch tested?

UT

github-actions · 2025-07-23T03:54:31Z

#10247

PHILO-HE · 2025-07-23T09:26:13Z

...link/ut/src/test/java/org/apache/gluten/table/runtime/stream/custom/ScalarFunctionsTest.java

+        Arrays.asList(Row.of(1, "2024-12-31 12:12:12"), Row.of(2, "2025-02-28 12:12:12"));
+    createSimpleBoundedValuesTable("dateFormatTbl", "a int, b string", rows);
+    String query =
+        "select a, DATE_FORMAT(cast(b as Timestamp(3)), 'yyyy-MM-dd') from dateFormatTbl";


In Spark and Velox, date_format respects the configured session timezone. I assume Flink should also respect it.

The test here cannot expose the issue that local session timezone is not respected. The default timezone (utc) is used in cast string to timestamp. Then, the same default timezone is used for date_format. As long as the timezone is consistent, there is no issue exposed. But the actual timezone configured for Flink is not actually respected.

I think we need to pass Flink's session timezone to Velox's config, as Gluten Spark code does.

Now I have pass the flink session timezone to velox, and add the ut to test this. please take review again. @PHILO-HE

It seems not properly to pass the session timezone config to the velox, as in StatefulTask#initOperators will check the memory reversed, and would throw exceptions if it's not 0.

PHILO-HE · 2025-07-28T10:06:35Z

gluten-flink/runtime/src/main/java/org/apache/gluten/table/runtime/config/VeloxQueryConfig.java

+    Map<String, String> configMap = new HashMap<>();
+    if (config.get(ADJUST_TIMESTMP_TO_SESSION_TIMEZONE)) {
+      String localTimeZone = config.get(TableConfigOptions.LOCAL_TIME_ZONE);
+      configMap.put(keyVeloxAdjustTimestampToSessionTimeZone, "true");


In Spark, we always set this to true to let velox respect configured session timezone. I assume we should also do this for Flink, not depending on the user's setting for ADJUST_TIMESTMP_TO_SESSION_TIMEZONE.

PHILO-HE · 2025-07-28T10:26:45Z

...link/ut/src/test/java/org/apache/gluten/table/runtime/stream/custom/ScalarFunctionsTest.java

+            VeloxQueryConfig.ADJUST_TIMESTMP_TO_SESSION_TIMEZONE.key(),
+            "true",
+            TableConfigOptions.LOCAL_TIME_ZONE.key(),
+            "America/Los_Angeles");


In this designed test, the result cannot reflect the timezone is changed. As I mentioned, both cast & date_format are respecting the timezone. As long as the timezone they are using is same, the result aligns with the input date time string, even though America/Los_Angeles is not passed to velox but the default timezone is in use. So the test here cannot confirm this new timezone is passed to Velox and taking effect.

I haven't come up with a good way. Do you have any idea?

I think we can set some illegal timezone value to velox, but which is correct to flink , e.g : default timezone. And velox would throw a exception with invalid timezone, but flink can pass it. and in contrast, we pass a valid timezone to the test, e.g : America/Los_Angeles, can get the correct result. and it will prove that the timezone is passed to velox. @PHILO-HE

@KevinyhZou, it may not well test the behavior of date_format function on respecting timezone.

Could you confirm if the suggestion below makes sense?

Set timezone to UTC
CREATE TEMPRARY VIEW my_view AS SELECT CAST(b AS TIMESTAMP) as c1 FROM tbl;

Set timezone to "America/Los_Angeles"
SELECT date_format(c1) FROM my_view;
Check the result to see if it matches the expected string (-8 hours offset, compared with the string column b).

Set timezone to "Asia/Shanghai"
SELECT date_format(c1) FROM my_view;
Check the result to see if it matches the expected string (+8 hours offset, compared with the string column b).

It does not work. It always return the same string while time zone set to different(UTC, Asia/Shanghai, America/Los_Angeles).

Do you know the reason? If different timestamp is set in Velox, date_format should return different date/time string for the same timestamp input.

The flink create view is logical view, data is not materialized， so you described above it is the same as select ... date_format(cast(xxx as timestamp), 'yyyy-MM-dd HH:mm:ss'), and this is different from spark.

And we can still test this, by insert the result of 1st step to a table instead of view, and then the 2nd and 3rd step result can be affected by the timezone, but the timestamp with timezone type is not supported, so the 1st step would cause errors, and I have tried this yesterday.

Do you mean the inserting will create TIMESTAMP_LTZ type in the 1st step, not TIMESTAMP type?

Yes, similar to that. cast(xxx as Timestamp(3)) will return a Timestamp vector with timezone by using the arrow's TimeStampMilliTZVector, and now we only map the TimeStampMilliVector to flink's timestamp type, but not do anything about TimeStampMilliTZVector and flink's timestamp type with timezone, like TIMESTAMP_LTZ. maybe we should open another issue for this?

@KevinyhZou, if I understand correctly, we should ensure the result type is Flink's timestamp type for cast(xxx as timestamp). You can submit another PR to fix the issue.

I investigated Flink just now. It seems Flink's timestamp type is corresponding to Spark's timestamp_ntz type (no counterpart in Velox), and Flinks' timestamp_ltz type is corresponding to Spark/Velox's timestamp type. Please confirm this to avoid wrong type mapping.

In Flink, when casting datetime string to timestamp_ltz or outputting string represented result for timestamp_ltz, timezone is respected. But regarding timestamp type, timezone is not respected in such conversions.

Both timestamp and timestamp_ltz are supported for Flink's date_format function. So if timestamp is used, the result is not influenced by session timezone. But if timestamp_ltz is used, the result should be adjusted based on session timezone.

shuai-xu · 2025-07-31T04:14:19Z

gluten-flink/runtime/src/main/java/org/apache/gluten/table/runtime/config/VeloxQueryConfig.java

+    return Config.create(configMap);
+  }
+
+  private static boolean isConfigUsedForTest(Configuration config) {


Why need to add test related code here?

shuai-xu · 2025-08-27T06:55:01Z

...ntime/src/main/java/org/apache/gluten/table/runtime/operators/GlutenSingleInputOperator.java

  public void close() throws Exception {
    inputQueue.close();
-    task.close();
+    if (task != null) {


why need this?

KevinyhZou · 2025-09-12T03:37:46Z

depends on pr: bigo-sg/velox#9

KevinyhZou · 2025-09-12T03:39:02Z

I have consider timestamp timezone in date_format function ut tests by using flink's timestamp_ltz type, please take a review at this, thanks. @PHILO-HE

PHILO-HE

Thank for your updating. The added tests are what I wanted, which ensures timezone is actually respected. I posted only few minor comments.

BTW, ~~are there any other code changes required to map Flink's timestampLtz type to Velox's timestamp type~~? Could you verify the query is offloaded to Gluten with this PR? Thanks.

I note the type mapping already exists. It would be better to add some comments to clarify.
https://github.com/apache/incubator-gluten/blob/4d2a200c45d6a8849124d6eed4d54f1ae48568fd/gluten-flink/runtime/src/main/java/org/apache/gluten/util/LogicalTypeConverter.java#L109-L111

gluten-flink/runtime/src/main/java/org/apache/gluten/table/runtime/config/VeloxQueryConfig.java

PHILO-HE · 2025-09-17T07:57:26Z

...link/ut/src/test/java/org/apache/gluten/table/runtime/stream/custom/ScalarFunctionsTest.java

+        Arrays.asList(
+            Row.of(1, LocalDateTime.parse("2024-12-31 12:12:12", formatter)),
+            Row.of(2, LocalDateTime.parse("2025-02-28 12:12:12", formatter)));
+    createSimpleBoundedValuesTable("dateFormatTbl", "a int, b Timestamp(3)", rows);


Nit:
Use descriptive table name: timestampTable,

PHILO-HE · 2025-09-17T07:57:27Z

...link/ut/src/test/java/org/apache/gluten/table/runtime/stream/custom/ScalarFunctionsTest.java

+            Row.of(
+                2,
+                LocalDateTime.parse("2025-02-28 12:12:12", formatter).toInstant(ZoneOffset.UTC)));
+    createSimpleBoundedValuesTable("dateFormatTblLTZ", "a int, b Timestamp_LTZ(3)", rows);


dateFormatTblLTZ->timestampLtzTable

PHILO-HE · 2025-09-17T07:58:34Z

...link/ut/src/test/java/org/apache/gluten/table/runtime/stream/custom/ScalarFunctionsTest.java

+                2,
+                LocalDateTime.parse("2025-02-28 12:12:12", formatter).toInstant(ZoneOffset.UTC)));
+    createSimpleBoundedValuesTable("dateFormatTblLTZ", "a int, b Timestamp_LTZ(3)", rows);
+    String query1 =


Nit: maybe, just use "query = " to overwrite the previous value.

...link/ut/src/test/java/org/apache/gluten/table/runtime/stream/custom/ScalarFunctionsTest.java

PHILO-HE · 2025-09-17T08:22:42Z

...link/ut/src/test/java/org/apache/gluten/table/runtime/stream/custom/ScalarFunctionsTest.java

+            "+I[1, 2024-12-31, 2024-12-31 12:12:12]", "+I[2, 2025-02-28, 2025-02-28 12:12:12]"));
+
+    Map<String, String> configs = new HashMap<>();
+    configs.put(TableConfigOptions.LOCAL_TIME_ZONE.key(), "America/Los_Angeles");


I note in Flink, the following code can be called to set timezone. It may be better to directly call such API to do the setting before the test. Then, no need to do the setting inside runAndCheck.

tEnv.getConfig().setLocalTimeZone(ZoneId.of("Asia/Shanghai"));

KevinyhZou · 2025-09-19T01:29:49Z

I think the failure reason is the pr: bigo-sg/velox#9 that not merged. But from the ci log, it seems to be was blocked for 4 hours, and then was cancelled. @PHILO-HE . Please help to reiew:bigo-sg/velox#9 @shuai-xu , and merge it first if it's no problems.

PHILO-HE · 2025-09-19T06:02:25Z

@KevinyhZou, you can temporarily change the CI code to use your Velox branch for testing, then to see if your patch can fix this failure.

Another question.
With the code below that maps Flink Timestamp type to Velox Timestamp type, I am curious how to distinguish timestamp with timezone and timestamp without timezone at Velox side, given that Flink LocalZonedTimestampType is mapped to Velox Timestamp type also.

https://github.com/apache/incubator-gluten/blob/4d2a200c45d6a8849124d6eed4d54f1ae48568fd/gluten-flink/runtime/src/main/java/org/apache/gluten/util/LogicalTypeConverter.java#L67-L69

KevinyhZou · 2025-09-19T08:04:42Z

@KevinyhZou, you can temporarily change the CI code to use your Velox branch for testing, then to see if your patch can fix this failure.

Another question. With the code below that maps Flink Timestamp type to Velox Timestamp type, I am curious how to distinguish timestamp with timezone and timestamp without timezone at Velox side, given that Flink LocalZonedTimestampType is mapped to Velox Timestamp type also.

https://github.com/apache/incubator-gluten/blob/4d2a200c45d6a8849124d6eed4d54f1ae48568fd/gluten-flink/runtime/src/main/java/org/apache/gluten/util/LogicalTypeConverter.java#L67-L69

Timestamp with timezone would be specified with session_timezone config, while Timestamp without timezone will not be. @PHILO-HE

KevinyhZou · 2025-09-22T01:42:32Z

It seems something wrong with the ci, I use my own branch, and it still sutcked, it seems to have same problem as described in issue: #10258. @PHILO-HE

PHILO-HE · 2025-09-22T06:19:48Z

It seems something wrong with the ci, I use my own branch, and it still sutcked, it seems to have same problem as described in issue: #10258. @PHILO-HE

@KevinyhZou, please add the below command in flink.yml to install a new version of tzdata. We have merged some patch in upstream Velox to remove the dependency on new tzdata, and we've also removed the installation for new tzdata from docker image. I assume an old Velox version is used by Gluten Flink. So before the upgrade, we have to install new tzdata in runtime environment (not only the requirement for CI), as long as timezone is used in Velox.

yum install https://mirror.stream.centos.org/9-stream/BaseOS/x86_64/os/Packages/tzdata-2025a-1.el9.noarch.rpm -y

PHILO-HE · 2025-09-22T06:55:50Z

@KevinyhZou, you can temporarily change the CI code to use your Velox branch for testing, then to see if your patch can fix this failure.
Another question. With the code below that maps Flink Timestamp type to Velox Timestamp type, I am curious how to distinguish timestamp with timezone and timestamp without timezone at Velox side, given that Flink LocalZonedTimestampType is mapped to Velox Timestamp type also.
https://github.com/apache/incubator-gluten/blob/4d2a200c45d6a8849124d6eed4d54f1ae48568fd/gluten-flink/runtime/src/main/java/org/apache/gluten/util/LogicalTypeConverter.java#L67-L69

Timestamp with timezone would be specified with session_timezone config, while Timestamp without timezone will not be. @PHILO-HE

@KevinyhZou, would you mind clarifying how this works if both Flink's timestamp and timestampLTZ are used in one session, even in one query?

KevinyhZou · 2025-09-22T11:59:48Z

Both Timestamp and TimestampLTZ field in flink query will be converted to string and then passed to velox's FromElementSource to convert to timestamp of velox internal. Different from Timestamp field string, the TimestampLTZ field string will be end with a ‘utc’ timezone character Z, such as 2025-08-29T12:12:12Z, and it will be converted to timestamp with timezone by using TimestampConversion#fromTimestampWithTimezoneString in vlelox, on the contrast, the Timestamp field string will be converted to timestamp without timezone in the same way. And then TimestampConversion#fromParsedTimestampWithTimeZone will make different behavior to the different converted timestamps according to whether they have timezone and the timezone that in session_timezone config. @PHILO-HE

PHILO-HE · 2025-09-24T15:42:19Z

@KevinyhZou, thanks for the clarification. My understanding is that TimestampLTZ data is internally represented as a UTC timestamp, even though it is associated with a specific local timezone, which is different from Timestamp data.

PHILO-HE

Looks good. Thanks for your continuous efforts. Please revert the changes for Velox repo in flink.yml.

KevinyhZou · 2025-09-25T01:56:24Z

done

PHILO-HE · 2025-09-25T02:03:37Z

@shuai-xu, do you have any comments?

shuai-xu · 2025-09-25T07:45:25Z

...me/src/main/java/org/apache/gluten/table/runtime/operators/GlutenVectorTwoInputOperator.java

    memoryManager = MemoryManager.create(AllocationListener.NOOP);
    session = Velox4j.newSession(memoryManager);
-    query = new Query(glutenPlan, Config.empty(), ConnectorConfig.empty());
+    query =


It seems missing the GlutenVectorOneInputOperator

PHILO-HE · 2025-09-26T09:19:46Z

@KevinyhZou, could you check if the CI failure is related?

KevinyhZou · 2025-09-26T09:38:21Z

velox4j has not been updated yet，and we will re-run the ci after it update. @PHILO-HE

github-actions bot added the FLINK label Jul 23, 2025

lgbo-ustc approved these changes Jul 23, 2025

View reviewed changes

PHILO-HE changed the title ~~[GLUTEN-10247][FLINK]Support date_format function~~ [GLUTEN-10247][FLINK] Support date_format function Jul 23, 2025

PHILO-HE reviewed Jul 23, 2025

View reviewed changes

KevinyhZou added 2 commits July 24, 2025 09:42

Support date_format function

2dd79f2

consider about timezone

3ad14c9

KevinyhZou force-pushed the support_dateformat branch from ce89ecd to 3ad14c9 Compare July 24, 2025 09:43

fix ci

2573bbc

PHILO-HE reviewed Jul 28, 2025

View reviewed changes

KevinyhZou and others added 4 commits July 30, 2025 02:14

fix review

57471ad

Merge branch 'main' into support_dateformat

c05c6b1

run and check timezone set error

b7e055a

check error

86da02c

shuai-xu reviewed Jul 31, 2025

View reviewed changes

shuai-xu requested changes Aug 27, 2025

View reviewed changes

KevinyhZou and others added 4 commits August 27, 2025 08:49

Merge branch 'main' into support_dateformat

f99bf27

fix reviews

9700c1f

Merge branch 'main' into support_dateformat

ccc347c

test on remove arrow.memory.core from flink.yml

b07357a

github-actions bot added the INFRA label Sep 1, 2025

KevinyhZou added 3 commits September 1, 2025 06:12

fix ci

8b95500

Merge branch 'main' into support_dateformat

af21e3a

remove some useless changes

4bff55b

github-actions bot removed the INFRA label Sep 10, 2025

KevinyhZou added 2 commits September 10, 2025 06:45

fix ci

527eb42

fix ut add timestamp_ltz type for datetime format

6e20d64

PHILO-HE reviewed Sep 17, 2025

View reviewed changes

update flink.yml for test

16db006

github-actions bot added the INFRA label Sep 19, 2025

for ci testing

0bd0d77

KevinyhZou force-pushed the support_dateformat branch from f36f78a to 0bd0d77 Compare September 19, 2025 10:01

Merge branch 'main' into support_dateformat

d97c6f2

KevinyhZou and others added 2 commits September 22, 2025 01:44

check ci

f53101e

Merge branch 'main' into support_dateformat

f19774c

KevinyhZou added 3 commits September 22, 2025 07:51

update flink.yml

ca257e9

remove disabled label

948472f

fix reviews

5d0e7e5

PHILO-HE approved these changes Sep 24, 2025

View reviewed changes

revert flink.yml

3638ce2

shuai-xu requested changes Sep 25, 2025

View reviewed changes

fix reviews

40da044

KevinyhZou added 2 commits September 26, 2025 09:49

update flink.yml

17a8ebf

update flink.md

e40a221

shuai-xu approved these changes Oct 9, 2025

View reviewed changes

shuai-xu merged commit 73067f9 into apache:main Oct 9, 2025
4 checks passed

Conversation

KevinyhZou commented Jul 23, 2025

What changes were proposed in this pull request?

How was this patch tested?

Uh oh!

github-actions bot commented Jul 23, 2025

Uh oh!

PHILO-HE Jul 23, 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

KevinyhZou Jul 30, 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

PHILO-HE Jul 30, 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!

KevinyhZou Jul 30, 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!

Choose a reason for hiding this comment

Uh oh!

KevinyhZou Jul 30, 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

KevinyhZou commented Sep 12, 2025

Uh oh!

KevinyhZou commented Sep 12, 2025

Uh oh!

PHILO-HE left a comment • 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

KevinyhZou commented Sep 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

PHILO-HE commented Sep 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

KevinyhZou commented Sep 19, 2025

Uh oh!

KevinyhZou commented Sep 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

PHILO-HE commented Sep 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

PHILO-HE commented Sep 22, 2025

PHILO-HE Jul 23, 2025 •

edited

Loading

KevinyhZou Jul 30, 2025 •

edited

Loading

PHILO-HE Jul 30, 2025 •

edited

Loading

KevinyhZou Jul 30, 2025 •

edited

Loading

KevinyhZou Jul 30, 2025 •

edited

Loading

PHILO-HE left a comment •

edited

Loading

KevinyhZou commented Sep 19, 2025 •

edited

Loading

PHILO-HE commented Sep 19, 2025 •

edited

Loading

KevinyhZou commented Sep 22, 2025 •

edited

Loading

PHILO-HE commented Sep 22, 2025 •

edited

Loading

KevinyhZou commented Sep 22, 2025 •

edited

Loading