Skip to content

feat: adding add_months datetime expression#4427

Open
athlcode wants to merge 3 commits into
apache:mainfrom
athlcode:add_months
Open

feat: adding add_months datetime expression#4427
athlcode wants to merge 3 commits into
apache:mainfrom
athlcode:add_months

Conversation

@athlcode
Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Part of #4150

Rationale for this change

add_months already has a native implementation in the datafusion-spark crate, but wasn't wired into Comet, so queries using it fell back to Spark.

What changes are included in this PR?

Wires Spark's AddMonths to the datafusion-spark SparkAddMonths UDF using the wire-datafusion-function skill. Also updates the supported-expression docs.

How are these changes tested?

Added a SQL file test (datetime/add_months.sql).

@athlcode
Copy link
Copy Markdown
Contributor Author

Hey @coderfender, @comphead. Created this for add_months spark expression, please review, thank you

@coderfender
Copy link
Copy Markdown
Contributor

Thank you @athlcode , I triggered CI but there seems to be some test failures

@andygrove
Copy link
Copy Markdown
Member

The root cause of the failures is that the upstream expression does not actually have a runnable invoke_with_args. It implements its logic via simplify, which rewrites add_months(d, m) into d + cast(m AS interval year-month) during DataFusion's optimizer pass. Comet's planner builds physical plans straight from the protobuf and does not run the simplifier, so the stub invoke_with_args runs and errors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants