Conversation
|
Thank you for the contribution @YutaLin! Could you add/migrate the expression tests to the new SQL test framework? https://datafusion.apache.org/comet/contributor-guide/sql-file-tests.html |
cb61473 to
379180e
Compare
|
Hi @mbutrovich, thanks for the review! |
|
Thank you for your contribution!
If I understand it correctly, you might need to create a table with Not sure if SQL statements can help you with table creation: https://datafusion.apache.org/comet/contributor-guide/sql-file-tests.html#statements |
|
Hi @mbutrovich @hsiang-c, i've add the test, let me know if i miss anything, appreciate! |
spark/src/test/resources/sql-tests/expressions/datetime/years.sql
Outdated
Show resolved
Hide resolved
| classOf[WeekOfYear] -> CometWeekOfYear, | ||
| classOf[Quarter] -> CometQuarter) | ||
| classOf[Quarter] -> CometQuarter, | ||
| classOf[Years] -> CometYears) |
There was a problem hiding this comment.
Since Years is a PartitionTransformExpression, maybe we can move it to a new Map(e.g. partitionTransformExpression instead of having it in temporalExpressions?
| event_date DATE, | ||
| value STRING | ||
| ) USING iceberg | ||
| PARTITIONED BY (years(event_date)) |
There was a problem hiding this comment.
years is supported by Iceberg only.
Switching to Parquet throws exception b/c CatalogV2Implicits::convertTransform of https://github.com/apache/spark/blob/branch-4.1/sql/catalyst/src/main/scala/org/apache/spark/sql/connector/catalog/CatalogV2Implicits.scala#L62-L101
I am not sure how to verify partition pruning is effective on partitioned Iceberg tables.
Which issue does this PR close?
Closes #3131
Rationale for this change
Comet does not currently support the Spark years function, causing queries using this function to fall back to Spark's JVM execution instead of running natively on DataFusion.
This patch adds serialization support for
Years, allowing Comet to recognize the expressionWhat changes are included in this PR?
Yearsexpression to DataFusion `datepart('year, child)' and cast to integergetSupportLevelto restrict support typeDateType,TimestampTypeandTimestampNTZTypeQueryPlanSerdeHow are these changes tested?
CometExpressionSerdeto verifyYearsexpression is identified correctly and serialized into Protobuf message.getSupportLevelreturnsCompatiblefor supported temporal types.