Skip to content

Conversation

@nielspardon
Copy link
Member

Currently, the Substrait to Calcite mappings only support mapping scalar functions with enum arguments for the extract:req_ts function which means it only supports the extract function with an enum argument and a timestamp but none of the other datetime extract functions defined in the default datetime extension.

This PR adds basic support for all datetime extract functions which are not using the indexing argument which indicates whether e.g. months should start to be counted from 0 instead of 1.

It is still a naive mapping from the enum values defined in the datetime extension YAML to Apache Calcite's TimeUnitRange enum which misses some of the Substrait enum values (e.g. US_YEAR, PICOSECONDS) or has spelling differences for some of the enum values (ISOYEAR vs. ISO_YEAR).

This PR at least allows some of the TPC-H queries to be mapped back from Substrait to Calcite which currently throws an UnsupportedOperationException due to missing support for date extract.

I would suggest to revisit the function mapping code in a follow-up PR to create a framework which supports more sophisticated mapping logic.

@nielspardon nielspardon force-pushed the par-extract-datetime branch 3 times, most recently from 20199a4 to 77cef26 Compare April 9, 2025 09:44
Copy link
Member

@vbarua vbarua left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The functional changes look good overall. I did leave some simplification suggestions, and one request to move tests.

@nielspardon nielspardon force-pushed the par-extract-datetime branch from 77cef26 to b89e245 Compare April 11, 2025 08:31
@nielspardon nielspardon requested a review from vbarua April 11, 2025 13:47
@vbarua vbarua merged commit 134c224 into substrait-io:main Apr 11, 2025
12 checks passed
@nielspardon nielspardon deleted the par-extract-datetime branch August 5, 2025 06:03
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