You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@gluten.apache.org by "rui-mo (via GitHub)" <gi...@apache.org> on 2026/06/04 01:32:58 UTC
[PR] [GLUTEN-11622][VL] Add basic TIMESTAMP_NTZ type support (#11939) [gluten]
rui-mo opened a new pull request, #12229:
URL: https://github.com/apache/gluten/pull/12229
<!--
Thank you for submitting a pull request! Here are some tips:
1. For first-time contributors, please read our contributing guide:
https://github.com/apache/gluten/blob/main/CONTRIBUTING.md
2. If necessary, create a GitHub issue for discussion beforehand to avoid duplicate work.
3. If the PR is specific to a single backend, include [VL] or [CH] in the PR title to indicate the
Velox or ClickHouse backend, respectively.
4. If the PR is not ready for review, please mark it as a draft.
-->
## What changes are proposed in this pull request?
<!--
Provide a clear and concise description of the changes introduced in this PR.
Ensure the PR description aligns with the code changes, especially after updates.
If applicable, include "Fixes #<GitHub_Issue_ID>" to automatically close the corresponding issue
when the PR is merged.
-->
## How was this patch tested?
<!--
Describe how the changes were tested, if applicable.
Include new tests to validate the functionality, if necessary.
For UI-related changes, attach screenshots to demonstrate the updates.
-->
## Was this patch authored or co-authored using generative AI tooling?
<!--
If generative AI tooling has been used in the process of authoring this patch, please include the
phrase: 'Generated-by: ' followed by the name of the tool and its version.
If no, write 'No'.
Please refer to the [ASF Generative Tooling Guidance](https://www.apache.org/legal/generative-tooling.html) for details.
-->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-11622][VL] Add basic TIMESTAMP_NTZ type support (#11939) [gluten]
Posted by "rui-mo (via GitHub)" <gi...@apache.org>.
rui-mo commented on PR #12229:
URL: https://github.com/apache/gluten/pull/12229#issuecomment-4688405035
Some unit tests were temporarily disabled due to unsupported functions and cast conversions involving TimestampNTZType. Let's merge the basic support first and continue expanding support for these cases afterward.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-11622][VL] Add basic TIMESTAMP_NTZ type support (#11939) [gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #12229:
URL: https://github.com/apache/gluten/pull/12229#issuecomment-4666092685
Run Gluten Clickhouse CI on x86
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-11622][VL] Add basic TIMESTAMP_NTZ type support (#11939) [gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #12229:
URL: https://github.com/apache/gluten/pull/12229#issuecomment-4666513192
Run Gluten Clickhouse CI on x86
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-11622][VL] Add basic TIMESTAMP_NTZ type support (#11939) [gluten]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #12229:
URL: https://github.com/apache/gluten/pull/12229#discussion_r3385244000
##########
gluten-substrait/src/main/scala/org/apache/gluten/extension/columnar/validator/Validators.scala:
##########
@@ -231,41 +218,30 @@ object Validators {
}
}
- private class FallbackByTimestampNTZ(veloxConf: Option[Any]) extends Validator {
- // Check if TimestampNTZ validation is enabled via VeloxConfig
- // Default to true (enabled) if VeloxConfig is not available or method call fails
- private val enableValidation: Boolean = veloxConf
- .flatMap {
- config =>
- try {
- val enableMethod = config.getClass.getMethod("enableTimestampNtzValidation")
- Some(enableMethod.invoke(config).asInstanceOf[Boolean])
- } catch {
- case _: Exception => None
- }
- }
- .getOrElse(true)
-
+ private class FallbackByTimestampNTZ(enableValidation: Boolean) extends Validator {
override def validate(plan: SparkPlan): Validator.OutCome = {
if (!enableValidation) {
- // Validation is disabled, allow TimestampNTZ
- return pass()
- }
-
- def containsNTZ(dataType: DataType): Boolean = dataType match {
- case dt if dt.catalogString == "timestamp_ntz" => true
- case st: StructType => st.exists(f => containsNTZ(f.dataType))
- case at: ArrayType => containsNTZ(at.elementType)
- case mt: MapType => containsNTZ(mt.keyType) || containsNTZ(mt.valueType)
- case _ => false
- }
- val hasNTZ = plan.output.exists(a => containsNTZ(a.dataType)) ||
- plan.children.exists(_.output.exists(a => containsNTZ(a.dataType)))
- if (hasNTZ) {
- fail(s"${plan.nodeName} has TimestampNTZType in input/output schema")
- } else {
- pass()
+ // Validation is disabled, allow supported operators.
+ def containsNTZ(dataType: DataType): Boolean = dataType match {
+ case dt if dt.typeName == "timestamp_ntz" => true
+ case st: StructType => st.exists(f => containsNTZ(f.dataType))
+ case at: ArrayType => containsNTZ(at.elementType)
+ case mt: MapType => containsNTZ(mt.keyType) || containsNTZ(mt.valueType)
+ case _ => false
+ }
+ val isScan = plan match {
+ case _: BatchScanExec => true
+ case _: FileSourceScanExec => true
+ case p if HiveTableScanExecTransformer.isHiveTableScan(p) => true
+ case _ => false
+ }
+ val hasNTZ = plan.output.exists(a => containsNTZ(a.dataType)) ||
+ plan.children.exists(_.output.exists(a => containsNTZ(a.dataType)))
+ if (isScan || !hasNTZ) {
+ return pass()
+ }
}
+ fail(s"${plan.nodeName} has TimestampNTZType in input/output schema")
}
Review Comment:
`FallbackByTimestampNTZ` currently fails validation unconditionally when `enableTimestampNtzValidation` is `true` (the default in `BackendSettingsApi`). This makes *all* Spark plans fail validation (and therefore fall back) even when the plan schema has no `timestamp_ntz`, which is a severe regression for non-Velox backends that don't override the setting.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-11622][VL] Add basic TIMESTAMP_NTZ type support (#11939) [gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #12229:
URL: https://github.com/apache/gluten/pull/12229#issuecomment-4676850202
Run Gluten Clickhouse CI on x86
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-11622][VL] Add basic TIMESTAMP_NTZ type support (#11939) [gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #12229:
URL: https://github.com/apache/gluten/pull/12229#issuecomment-4618335329
Run Gluten Clickhouse CI on x86
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-11622][VL] Add basic TIMESTAMP_NTZ type support (#11939) [gluten]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #12229:
URL: https://github.com/apache/gluten/pull/12229#discussion_r3393754588
##########
gluten-ut/spark41/src/test/scala/org/apache/gluten/utils/clickhouse/ClickHouseTestSettings.scala:
##########
@@ -703,6 +703,10 @@ class ClickHouseTestSettings extends BackendTestSettings {
.excludeCH("SPARK-31896: Handle am-pm timestamp parsing when hour is missing")
.excludeCH("UNIX_SECONDS")
.excludeCH("TIMESTAMP_SECONDS")
+ // TimestampNTZ evaluation is not supported.
+ .excludeCH("Seconds")
+ .excludeCH("Minute")
+ .excludeGlutenTest("Gluten - Hour")
Review Comment:
`excludeGlutenTest` automatically prepends the `Gluten - ` prefix to the provided test name. Passing an already-prefixed string here results in trying to exclude `Gluten - Gluten - Hour`, so the intended Gluten test (`Gluten - Hour`) will still run.
##########
gluten-ut/spark40/src/test/scala/org/apache/gluten/utils/clickhouse/ClickHouseTestSettings.scala:
##########
@@ -703,6 +703,10 @@ class ClickHouseTestSettings extends BackendTestSettings {
.excludeCH("SPARK-31896: Handle am-pm timestamp parsing when hour is missing")
.excludeCH("UNIX_SECONDS")
.excludeCH("TIMESTAMP_SECONDS")
+ // TimestampNTZ evaluation is not supported.
+ .excludeCH("Seconds")
+ .excludeCH("Minute")
+ .excludeGlutenTest("Gluten - Hour")
Review Comment:
`excludeGlutenTest` already prefixes test names with `Gluten - `. Using `excludeGlutenTest("Gluten - Hour")` won’t match any test and won’t exclude the intended Gluten test case.
##########
gluten-ut/spark35/src/test/scala/org/apache/gluten/utils/clickhouse/ClickHouseTestSettings.scala:
##########
@@ -716,6 +716,10 @@ class ClickHouseTestSettings extends BackendTestSettings {
.excludeCH("SPARK-31896: Handle am-pm timestamp parsing when hour is missing")
.excludeCH("UNIX_SECONDS")
.excludeCH("TIMESTAMP_SECONDS")
+ // TimestampNTZ evaluation is not supported.
+ .excludeCH("Seconds")
+ .excludeCH("Minute")
+ .excludeGlutenTest("Gluten - Hour")
Review Comment:
`excludeGlutenTest` prepends `Gluten - ` automatically. With the current code, the exclusion target becomes `Gluten - Gluten - Hour`, so `Gluten - Hour` will not be excluded as intended.
##########
gluten-ut/spark35/src/test/scala/org/apache/gluten/utils/velox/VeloxSQLQueryTestSettings.scala:
##########
@@ -58,7 +58,6 @@ object VeloxSQLQueryTestSettings extends SQLQueryTestSettings {
"double-quoted-identifiers.sql",
"except.sql",
"except-all.sql",
- "extract.sql",
"group-by.sql",
"group-by-all.sql",
Review Comment:
Spark 3.5 Velox SQL query test settings no longer include `extract.sql`, but other Spark versions’ Velox settings still do. This looks like an accidental omission and reduces coverage for date/time extraction behavior.
##########
gluten-ut/spark35/src/test/scala/org/apache/gluten/utils/velox/VeloxSQLQueryTestSettings.scala:
##########
@@ -180,8 +179,6 @@ object VeloxSQLQueryTestSettings extends SQLQueryTestSettings {
"postgreSQL/window_part2.sql",
"postgreSQL/with.sql",
"datetime-special.sql",
- "timestamp-ltz.sql",
- "timestamp-ntz.sql",
"timezone.sql",
"transform.sql",
"try-string-functions.sql",
Review Comment:
Spark 3.5 Velox SQL query test settings are missing `timestamp-ltz.sql` and `timestamp-ntz.sql` while Spark 3.3/3.4/4.0/4.1 Velox settings include them. If these tests are expected to pass for Velox, they should be re-enabled here for consistency and coverage.
##########
gluten-ut/spark33/src/test/scala/org/apache/gluten/utils/clickhouse/ClickHouseTestSettings.scala:
##########
@@ -758,6 +758,8 @@ class ClickHouseTestSettings extends BackendTestSettings {
.excludeGlutenTest("unix_timestamp")
.excludeGlutenTest("to_unix_timestamp")
.excludeGlutenTest("Hour")
+ // TimestampNTZ evaluation is not supported.
+ .excludeGlutenTest("Gluten - Hour")
Review Comment:
`excludeGlutenTest` prefixes with `Gluten - ` internally, so `excludeGlutenTest("Gluten - Hour")` won’t match any test. This suite already excludes the Gluten Hour test via `.excludeGlutenTest("Hour")`, so this line is redundant and should be removed to avoid confusion.
##########
gluten-substrait/src/main/scala/org/apache/gluten/expression/ConverterUtils.scala:
##########
@@ -160,6 +160,19 @@ object ConverterUtils extends Logging {
(StringType, isNullable(substraitType.getString.getNullability))
case Type.KindCase.BINARY =>
(BinaryType, isNullable(substraitType.getBinary.getNullability))
+ case Type.KindCase.TIMESTAMP =>
+ try {
+ (
+ Class
+ .forName("org.apache.spark.sql.types.TimestampNTZType$")
+ .getField("MODULE$")
+ .get(null)
+ .asInstanceOf[DataType],
+ isNullable(substraitType.getTimestamp.getNullability))
Review Comment:
With TIMESTAMP now mapped to Spark's TimestampNTZType, the type node flow can produce `TimestampNTZTypeNode` instances (via `TypeBuilder.makeTimestampNTZ`). However, `ConverterUtils.parseFromTypeNode` does not handle `TimestampNTZTypeNode`, which will throw `GlutenNotSupportException` when building list/map literals (see `ListLiteralNode`/`MapLiteralNode` calling `parseFromTypeNode`). Please add a `TimestampNTZTypeNode` case there (likely using the same reflection-based TimestampNTZType lookup used here).
##########
gluten-ut/spark34/src/test/scala/org/apache/gluten/utils/clickhouse/ClickHouseTestSettings.scala:
##########
@@ -657,6 +657,8 @@ class ClickHouseTestSettings extends BackendTestSettings {
.excludeGlutenTest("unix_timestamp")
.excludeGlutenTest("to_unix_timestamp")
.excludeGlutenTest("Hour")
+ // TimestampNTZ evaluation is not supported.
+ .excludeGlutenTest("Gluten - Hour")
Review Comment:
`excludeGlutenTest` automatically prefixes test names with `Gluten - `. Passing a prefixed name here means the exclusion won’t match (`Gluten - Gluten - Hour`). Since this suite already excludes the Gluten Hour test via `.excludeGlutenTest("Hour")` above, this extra line is redundant and can be removed to avoid confusion.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-11622][VL] Add basic TIMESTAMP_NTZ type support (#11939) [gluten]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #12229:
URL: https://github.com/apache/gluten/pull/12229#discussion_r3400452952
##########
gluten-ut/spark35/src/test/scala/org/apache/gluten/utils/velox/VeloxSQLQueryTestSettings.scala:
##########
@@ -58,7 +58,6 @@ object VeloxSQLQueryTestSettings extends SQLQueryTestSettings {
"double-quoted-identifiers.sql",
"except.sql",
"except-all.sql",
- "extract.sql",
"group-by.sql",
Review Comment:
`extract.sql` was removed from the supported SQL query list for Spark 3.5 Velox only, which looks unintentional (other Spark versions and CH still include it). This reduces SQL coverage and is unrelated to TIMESTAMP_NTZ.
##########
gluten-ut/spark41/src/test/scala/org/apache/spark/sql/GlutenCachedTableSuite.scala:
##########
@@ -164,4 +166,17 @@ class GlutenCachedTableSuite
uncacheTable("t2")
}
}
+
+ testGluten("SPARK-36120: Support cache/uncache table with TimestampNTZ type") {
+ val tableName = "ntzCache"
+ withTable(tableName) {
+ sql(s"CACHE TABLE $tableName AS SELECT TIMESTAMP_NTZ'2021-01-01 00:00:00'")
+ checkAnswer(spark.table(tableName), Row(LocalDateTime.parse("2021-01-01T00:00:00")))
+ spark.table(tableName).queryExecution.withCachedData.collect {
+ case cached: InMemoryRelation =>
+ assert(cached.stats.sizeInBytes === 60)
Review Comment:
The cached size assertion is a hard-coded constant (60 bytes) which is brittle across Spark/backend changes (and the test is already excluded for some backends because of size differences). Prefer asserting a property that indicates caching happened (e.g., sizeInBytes > 0) to keep the test stable and runnable.
##########
backends-velox/src/test/scala/org/apache/gluten/execution/FallbackSuite.scala:
##########
@@ -310,7 +310,7 @@ class FallbackSuite extends VeloxWholeStageTransformerSuite with AdaptiveSparkPl
}
}
- test("fallback with index based schema evolution") {
+ testWithMinSparkVersion("fallback with index based schema evolution", "3.4") {
Review Comment:
This test name says "fallback" but the assertion now requires a non-empty `GlutenPlan` (i.e., at least partial native offload). Renaming the test makes its intent consistent with the new behavior.
##########
gluten-ut/spark35/src/test/scala/org/apache/gluten/utils/velox/VeloxSQLQueryTestSettings.scala:
##########
@@ -180,8 +179,6 @@ object VeloxSQLQueryTestSettings extends SQLQueryTestSettings {
"postgreSQL/window_part2.sql",
"postgreSQL/with.sql",
"datetime-special.sql",
- "timestamp-ltz.sql",
- "timestamp-ntz.sql",
"timezone.sql",
Review Comment:
`timestamp-ltz.sql` and `timestamp-ntz.sql` were removed from the supported SQL query list for Spark 3.5 Velox only. Given this PR adds basic TIMESTAMP_NTZ support, dropping these tests will hide regressions; consider keeping them enabled (or explicitly excluding only failing queries with a reason).
##########
gluten-ut/spark35/src/test/scala/org/apache/spark/sql/GlutenCachedTableSuite.scala:
##########
@@ -155,4 +157,17 @@ class GlutenCachedTableSuite
uncacheTable("t2")
}
}
+
+ testGluten("SPARK-36120: Support cache/uncache table with TimestampNTZ type") {
+ val tableName = "ntzCache"
+ withTable(tableName) {
+ sql(s"CACHE TABLE $tableName AS SELECT TIMESTAMP_NTZ'2021-01-01 00:00:00'")
+ checkAnswer(spark.table(tableName), Row(LocalDateTime.parse("2021-01-01T00:00:00")))
+ spark.table(tableName).queryExecution.withCachedData.collect {
+ case cached: InMemoryRelation =>
+ assert(cached.stats.sizeInBytes === 60)
Review Comment:
The cached size assertion is a hard-coded constant (60 bytes) which is brittle across Spark/backend changes. Prefer asserting a property that indicates caching happened (e.g., sizeInBytes > 0) to keep the test stable.
##########
gluten-ut/spark40/src/test/scala/org/apache/spark/sql/GlutenCachedTableSuite.scala:
##########
@@ -164,4 +166,17 @@ class GlutenCachedTableSuite
uncacheTable("t2")
}
}
+
+ testGluten("SPARK-36120: Support cache/uncache table with TimestampNTZ type") {
+ val tableName = "ntzCache"
+ withTable(tableName) {
+ sql(s"CACHE TABLE $tableName AS SELECT TIMESTAMP_NTZ'2021-01-01 00:00:00'")
+ checkAnswer(spark.table(tableName), Row(LocalDateTime.parse("2021-01-01T00:00:00")))
+ spark.table(tableName).queryExecution.withCachedData.collect {
+ case cached: InMemoryRelation =>
+ assert(cached.stats.sizeInBytes === 60)
Review Comment:
The cached size assertion is a hard-coded constant (60 bytes) which is brittle across Spark/backend changes (and the test is already excluded for some backends because of size differences). Prefer asserting a property that indicates caching happened (e.g., sizeInBytes > 0) to keep the test stable and runnable.
##########
gluten-ut/spark34/src/test/scala/org/apache/spark/sql/GlutenCachedTableSuite.scala:
##########
@@ -40,4 +42,17 @@ class GlutenCachedTableSuite
assert(cached.stats.sizeInBytes === 1132)
}
}
+
+ testGluten("SPARK-36120: Support cache/uncache table with TimestampNTZ type") {
+ val tableName = "ntzCache"
+ withTable(tableName) {
+ sql(s"CACHE TABLE $tableName AS SELECT TIMESTAMP_NTZ'2021-01-01 00:00:00'")
+ checkAnswer(spark.table(tableName), Row(LocalDateTime.parse("2021-01-01T00:00:00")))
+ spark.table(tableName).queryExecution.withCachedData.collect {
+ case cached: InMemoryRelation =>
+ assert(cached.stats.sizeInBytes === 60)
Review Comment:
The cached size assertion is a hard-coded constant (60 bytes) which is brittle across Spark/backend changes. Prefer asserting a property that indicates caching happened (e.g., sizeInBytes > 0) to keep the test stable.
##########
gluten-ut/spark33/src/test/scala/org/apache/spark/sql/GlutenCachedTableSuite.scala:
##########
@@ -40,4 +42,17 @@ class GlutenCachedTableSuite
assert(cached.stats.sizeInBytes === 1132)
}
}
+
+ testGluten("SPARK-36120: Support cache/uncache table with TimestampNTZ type") {
+ val tableName = "ntzCache"
+ withTable(tableName) {
+ sql(s"CACHE TABLE $tableName AS SELECT TIMESTAMP_NTZ'2021-01-01 00:00:00'")
+ checkAnswer(spark.table(tableName), Row(LocalDateTime.parse("2021-01-01T00:00:00")))
+ spark.table(tableName).queryExecution.withCachedData.collect {
+ case cached: InMemoryRelation =>
+ assert(cached.stats.sizeInBytes === 60)
Review Comment:
The cached size assertion is a hard-coded constant (60 bytes) which is brittle across Spark/backend changes. Prefer asserting a property that indicates caching happened (e.g., sizeInBytes > 0) to keep the test stable.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-11622][VL] Add basic TIMESTAMP_NTZ type support (#11939) [gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #12229:
URL: https://github.com/apache/gluten/pull/12229#issuecomment-4685952784
Run Gluten Clickhouse CI on x86
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-11622][VL] Add basic TIMESTAMP_NTZ type support (#11939) [gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #12229:
URL: https://github.com/apache/gluten/pull/12229#issuecomment-4618179615
Run Gluten Clickhouse CI on x86
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-11622][VL] Add basic TIMESTAMP_NTZ type support (#11939) [gluten]
Posted by "zhouyuan (via GitHub)" <gi...@apache.org>.
zhouyuan commented on PR #12229:
URL: https://github.com/apache/gluten/pull/12229#issuecomment-4660496903
Run Gluten Clickhouse CI on x86
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-11622][VL] Add basic TIMESTAMP_NTZ type support (#11939) [gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #12229:
URL: https://github.com/apache/gluten/pull/12229#issuecomment-4656713211
Run Gluten Clickhouse CI on x86
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-11622][VL] Add basic TIMESTAMP_NTZ type support (#11939) [gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #12229:
URL: https://github.com/apache/gluten/pull/12229#issuecomment-4655144774
Run Gluten Clickhouse CI on x86
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-11622][VL] Add basic TIMESTAMP_NTZ type support (#11939) [gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #12229:
URL: https://github.com/apache/gluten/pull/12229#issuecomment-4668843448
Run Gluten Clickhouse CI on x86
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-11622][VL] Add basic TIMESTAMP_NTZ type support (#11939) [gluten]
Posted by "rui-mo (via GitHub)" <gi...@apache.org>.
rui-mo merged PR #12229:
URL: https://github.com/apache/gluten/pull/12229
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-11622][VL] Add basic TIMESTAMP_NTZ type support (#11939) [gluten]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #12229:
URL: https://github.com/apache/gluten/pull/12229#discussion_r3377322040
##########
cpp/velox/substrait/SubstraitParser.cc:
##########
@@ -78,6 +77,8 @@ TypePtr SubstraitParser::parseType(const ::substrait::Type& substraitType, bool
return DATE();
case ::substrait::Type::KindCase::kTimestampTz:
return TIMESTAMP();
+ case ::substrait::Type::KindCase::kTimestamp:
+ return TIMESTAMP_UTC();
case ::substrait::Type::KindCase::kDecimal: {
Review Comment:
`TIMESTAMP_UTC()` is referenced but not defined anywhere in the C++ codebase, which will fail compilation. If Substrait `timestamp` is intended to map to Velox's timestamp type, use `TIMESTAMP()` here (and handle any NTZ vs LTZ semantics elsewhere).
##########
backends-velox/src/main/scala/org/apache/gluten/config/VeloxConfig.scala:
##########
@@ -828,5 +828,5 @@ object VeloxConfig extends ConfigRegistry {
"containing TimestampNTZ will fall back to Spark execution. Set to false during " +
"development/testing of TimestampNTZ support to allow native execution.")
.booleanConf
Review Comment:
This docstring claims "When true (default)" but the config is now created with default `false`. Please update the docstring to match the actual default (or revert the default) to avoid misleading users/operators.
##########
backends-velox/src/main/scala/org/apache/gluten/backendsapi/velox/VeloxValidatorApi.scala:
##########
@@ -111,7 +111,10 @@ object VeloxValidatorApi {
StringType | BinaryType | _: DecimalType | DateType | TimestampType |
YearMonthIntervalType.DEFAULT | NullType =>
true
- case dt if !enableTimestampNtzValidation && dt.catalogString == "timestamp_ntz" => true
+ case dt if !enableTimestampNtzValidation && dt.catalogString == "timestamp_ntz" =>
+ // Allow TimestampNTZ when validation is disabled (for development/testing)
+ // Use reflection to avoid compile-time dependency on Spark 3.4+ TimestampNTZType
+ true
Review Comment:
The comment mentions using reflection to avoid a compile-time dependency on Spark's TimestampNTZType, but this branch only checks `dt.catalogString` (a String) and doesn't use reflection. Please fix/remove the comment to avoid confusion when maintaining this validation logic.
##########
gluten-substrait/src/main/scala/org/apache/gluten/expression/ConverterUtils.scala:
##########
@@ -160,6 +160,19 @@ object ConverterUtils extends Logging {
(StringType, isNullable(substraitType.getString.getNullability))
case Type.KindCase.BINARY =>
(BinaryType, isNullable(substraitType.getBinary.getNullability))
+ case Type.KindCase.TIMESTAMP =>
+ try {
+ (
+ Class
+ .forName("org.apache.spark.sql.types.TimestampNTZType$")
+ .getField("MODULE$")
+ .get(null)
+ .asInstanceOf[DataType],
+ isNullable(substraitType.getTimestamp.getNullability))
+ } catch {
+ case _: ClassNotFoundException =>
+ throw new GlutenNotSupportException(s"Type $substraitType not supported.")
+ }
Review Comment:
The reflection block can fail with exceptions other than ClassNotFoundException (e.g., NoSuchFieldException, IllegalAccessException). Catching only ClassNotFoundException can crash plan conversion instead of producing a clean GlutenNotSupportException fallback.
##########
cpp/velox/substrait/SubstraitToVeloxExpr.cc:
##########
@@ -133,6 +132,8 @@ TypePtr getScalarType(const ::substrait::Expression::Literal& literal) {
return DATE();
case ::substrait::Expression_Literal::LiteralTypeCase::kTimestampTz:
return TIMESTAMP();
+ case ::substrait::Expression_Literal::LiteralTypeCase::kTimestamp:
+ return TIMESTAMP_UTC();
case ::substrait::Expression_Literal::LiteralTypeCase::kString:
Review Comment:
`TIMESTAMP_UTC()` is referenced but not defined anywhere in the C++ codebase, which will fail compilation. Replace it with `TIMESTAMP()` (Velox timestamp) unless there is an actual custom UTC timestamp type available.
##########
gluten-substrait/src/main/java/org/apache/gluten/substrait/expression/TimestampNTZLiteralNode.java:
##########
@@ -0,0 +1,37 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.gluten.substrait.expression;
+
+import org.apache.gluten.substrait.type.TimestampTypeNode;
+import org.apache.gluten.substrait.type.TypeNode;
+
+import io.substrait.proto.Expression.Literal.Builder;
+
+public class TimestampNTZLiteralNode extends LiteralNodeWithValue<Long> {
+ public TimestampNTZLiteralNode(Long value) {
+ super(value, new TimestampTypeNode(true));
+ }
Review Comment:
TimestampNTZLiteralNode declares its type as TimestampTypeNode (Substrait timestamp_tz) but writes the value into the `timestamp` (no-tz) literal field. This type/value mismatch can produce invalid Substrait literals for TIMESTAMP_NTZ and break downstream parsing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-11622][VL] Add basic TIMESTAMP_NTZ type support (#11939) [gluten]
Posted by "zhouyuan (via GitHub)" <gi...@apache.org>.
zhouyuan commented on PR #12229:
URL: https://github.com/apache/gluten/pull/12229#issuecomment-4668107437
I saw some failures are due to plan change:
```
[2026-06-10T06:13:41.828Z] - don't add sort when the required ordering is empty *** FAILED ***
[2026-06-10T06:13:41.828Z] WriteFiles
[2026-06-10T06:13:41.828Z] +- *(1) ColumnarToRow
[2026-06-10T06:13:41.828Z] +- FileScan parquet spark_catalog.default.t0[i#19,j#20,k#21] Batched: true, DataFilters: [], Format: Parquet, Location: InMemoryFileIndex(1 paths)[file:/home/jenkins/agent/workspace/gluten/gluten-ci/ut-stage-6/backend..., PartitionFilters: [], PushedFilters: [], ReadSchema: struct<i:int,j:int,k:string>
[2026-06-10T06:13:41.828Z] was not instance of org.apache.gluten.execution.GlutenPlan (DeltaV1WritesSuite.scala:69)
[2026-06-10T06:13:41.932Z] 2026-06-10 06:13:41.798 [1][ScalaTest-main-running-DiscoverySuite] WARN org.apache.spark.sql.execution.GlutenFallbackReporter: Validation failed for plan: DropTable[QueryId=14], due to: DropTable has TimestampNTZType in input/output schema
[2026-06-10T06:13:41.935Z] 2026-06-10 06:13:41.827 [1][ScalaTest-main-running-DiscoverySuite] WARN org.apache.spark.sql.execution.GlutenFallbackReporter: Validation failed for plan: Execute CreateDataSourceTableCommand[QueryId=15], due to: Execute CreateDataSourceTableCommand has TimestampNTZType in input/output schema
[2026-06-10T06:13:41.935Z] 2026-06-10 06:13:41.848 [1][ScalaTest-main-running-DiscoverySuite] WARN org.apache.spark.sql.execution.GlutenFallbackReporter: Validation failed for plan: Execute RepairTableCommand[QueryId=16], due to: Execute RepairTableCommand has TimestampNTZType in input/output schema
[2026-06-10T06:13:42.081Z] 2026-06-10 06:13:41.856 [1][ScalaTest-main-running-DeltaV1WritesSuite] WARN org.apache.spark.sql.execution.GlutenFallbackReporter: Validation failed for plan: Scan parquet spark_catalog.default.t0, due to: Scan parquet spark_catalog.default.t0 has TimestampNTZType in input/output schema
[2026-06-10T06:13:42.085Z] 2026-06-10 06:13:41.856 [1][ScalaTest-main-running-DeltaV1WritesSuite] WARN org.apache.spark.sql.execution.GlutenFallbackReporter: Validation failed for plan: ColumnarToRow, due to: Scan parquet spark_catalog.default.t0 has TimestampNTZType in input/output schema
[2026-06-10T06:13:42.085Z] 2026-06-10 06:13:41.856 [1][ScalaTest-main-running-DeltaV1WritesSuite] WARN org.apache.spark.sql.execution.GlutenFallbackReporter: Validation failed for plan: Exchange, due to: Exchange has TimestampNTZType in input/output schema
[2026-06-10T06:13:42.085Z] 2026-06-10 06:13:41.856 [1][ScalaTest-main-running-DeltaV1WritesSuite] WARN org.apache.spark.sql.execution.GlutenFallbackReporter: Validation failed for plan: Sort, due to: Sort has TimestampNTZType in input/output schema
[2026-06-10T06:13:42.085Z] 2026-06-10 06:13:41.856 [1][ScalaTest-main-running-DeltaV1WritesSuite] WARN org.apache.spark.sql.execution.GlutenFallbackReporter: Validation failed for plan: WriteFiles, due to: WriteFiles has TimestampNTZType in input/output schema
[2026-06-10T06:13:42.085Z] - don't add sort when the required ordering is already satisfied *** FAILED ***
[2026-06-10T06:13:42.085Z] WriteFiles
[2026-06-10T06:13:42.085Z] +- *(2) Sort [k#21 ASC NULLS FIRST], true, 0
[2026-06-10T06:13:42.085Z] +- Exchange rangepartitioning(k#21 ASC NULLS FIRST, 5), ENSURE_REQUIREMENTS, [plan_id=174]
[2026-06-10T06:13:42.085Z] +- *(1) ColumnarToRow
[2026-06-10T06:13:42.085Z] +- FileScan parquet spark_catalog.default.t0[i#19,j#20,k#21] Batched: true, DataFilters: [], Format: Parquet, Location: InMemoryFileIndex(1 paths)[file:/home/jenkins/agent/workspace/gluten/gluten-ci/ut-stage-6/backend..., PartitionFilters: [], PushedFilters: [], ReadSchema: struct<i:int,j:int,k:string>
[2026-06-10T06:13:42.085Z] was not instance of org.apache.gluten.execution.GlutenPlan (DeltaV1WritesSuite.scala:83)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-11622][VL] Add basic TIMESTAMP_NTZ type support (#11939) [gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #12229:
URL: https://github.com/apache/gluten/pull/12229#issuecomment-4677750250
Run Gluten Clickhouse CI on x86
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-11622][VL] Add basic TIMESTAMP_NTZ type support (#11939) [gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #12229:
URL: https://github.com/apache/gluten/pull/12229#issuecomment-4687021427
Run Gluten Clickhouse CI on x86
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-11622][VL] Add basic TIMESTAMP_NTZ type support (#11939) [gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #12229:
URL: https://github.com/apache/gluten/pull/12229#issuecomment-4618194086
Run Gluten Clickhouse CI on x86
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org
Re: [PR] [GLUTEN-11622][VL] Add basic TIMESTAMP_NTZ type support (#11939) [gluten]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #12229:
URL: https://github.com/apache/gluten/pull/12229#issuecomment-4618753120
Run Gluten Clickhouse CI on x86
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org