[GLUTEN-4730][CH] feat: Support date_from_unix_date function: Add ClickHouse backend implementation#12410
Conversation
|
Run Gluten Clickhouse CI on x86 |
|
Failed to query issues imported from #12410: dial tcp 10.167.38.130:3306: connect: connection timed out |
|
Run Gluten Clickhouse CI on x86 |
…ith09/gluten into date-from-unix-date
|
Run Gluten Clickhouse CI on x86 |
|
|
Run Gluten Clickhouse CI on x86 |
|
@lgbo-ustc Ok, I think I have fixed the issue. Please let me know if anything else needs to be fixed. |
|
@navaneethsujith09 I'm curious about why we need to create a new function instead of reusing existed function like :) select toDate(1)
SELECT toDate(1)
Query id: b986ea69-9b6b-4184-b189-aae2b3ec2014
┌──toDate(1)─┐
1. │ 1970-01-02 │
└────────────┘
1 row in set. Elapsed: 0.002 sec.
:) select toDate32(1)
SELECT toDate32(1)
Query id: 3833fbb7-36b3-4ae5-8fb7-f3359fc22ab0
┌─toDate32(1)─┐
1. │ 1970-01-02 │
└─────────────┘
1 row in set. Elapsed: 0.002 sec. |
|
the ci still fails。 you can login the jenkins to view the log. |
|
Run Gluten Clickhouse CI on x86 |
| DB::DataTypePtr getReturnTypeImpl(const DB::ColumnsWithTypeAndName &) const override { return std::make_shared<DB::DataTypeDate32>(); } | ||
|
|
||
| DB::ColumnPtr executeImpl(const DB::ColumnsWithTypeAndName & arguments, const DB::DataTypePtr & result_type, size_t input_rows) const override | ||
| { | ||
| if (arguments.size() != 1) | ||
| throw DB::Exception(DB::ErrorCodes::NUMBER_OF_ARGUMENTS_DOESNT_MATCH, "Function {} requires exactly 1 argument", name); | ||
|
|
||
| const DB::ColumnWithTypeAndName & first_arg = arguments[0]; | ||
| DB::WhichDataType which(first_arg.type); | ||
| if (which.isInt32()) | ||
| return executeInternal<Int32>(first_arg.column, input_rows); | ||
| else | ||
| throw DB::Exception(DB::ErrorCodes::ILLEGAL_TYPE_OF_ARGUMENT, "Function {} requires Int32 argument", name); | ||
| } |
| testGluten("date_from_unix_date") { | ||
| // -100000 and 200000 are outside ClickHouse's native Date32 range | ||
| // [1900-01-01, 2299-12-31]. They guard against implementations that clamp to that | ||
| // range (e.g. mapping to CH toDate32), which would silently diverge from Spark. | ||
| val df = Seq(Some(0), Some(1000), Some(-100000), Some(200000), None).toDF("unix_date") |
|
@lgbo-ustc Ok, I have fixed the issue. The ci now passes. Thanks for giving me access to the jenkins. That helped a lot. |
|
@taiyang-li To answer this question, there were two main reasons for me to create a new function. While both toDate(1) and toDate32(1) return "1970-01-02" so that they both look equivalent for small values. ToDate and ToDate32 has different semantics at the edges like: So for out-of-range inputs, ClickHouse builtins can silently return the wrong date instead of Spark’s result. Our sparkDateFromUnixDate just passes the day count through unchanged, which preserves Spark parity. Also, I checked soupam05's PR on this issue and he also did it in a similar way. Hope that helps! |
Implement SparkFunctionDateFromUnixDate in C++
Register function in CommonScalarFunctionParser
Add unit test in GlutenDateFunctionsSuite (Spark 3.5) for correctness
This enables Spark SQL queries using date_from_unix_date to work with the ClickHouse backend.
Implemented the feedback recieved from @taiyang-li
What changes are proposed in this pull request?
This pull request adds support for the date_from_unix_date function in the ClickHouse backend of Apache Gluten.
Specifically, it includes:
Implementation of the SparkFunctionDateFromUnixDate C++ function, which converts a unix date (number of days since epoch) to a Spark-compatible date.
Registration of the function in the ClickHouse function factory and the scalar function parser, enabling Substrait-to-ClickHouse translation.
Addition of unit tests in GlutenDateFunctionsSuite (Spark 3.5) to verify correctness and expected behavior.
How was this patch tested?
Added unit tests in gluten-ut/spark35/src/test/scala/org/apache/spark/sql/GlutenDateFunctionsSuite.scala to cover:
Typical values (e.g., date_from_unix_date(0) returns 1970-01-01)
Larger values (e.g., date_from_unix_date(1000) returns 1972-09-27)
Null handling (date_from_unix_date(null) returns null)
Ran the full test suite to ensure no regressions.
Was this patch authored or co-authored using generative AI tooling?
Generated-by Claude Opus 4.8
Related issue: #4730