Skip to content

feat(tests): add enum argument support to FuncTestCase grammar#1010

Merged
nielspardon merged 1 commit intosubstrait-io:mainfrom
nielspardon:par-enum-arg
Mar 23, 2026
Merged

feat(tests): add enum argument support to FuncTestCase grammar#1010
nielspardon merged 1 commit intosubstrait-io:mainfrom
nielspardon:par-enum-arg

Conversation

@nielspardon
Copy link
Copy Markdown
Member

@nielspardon nielspardon commented Mar 17, 2026

In the FuncTestCase grammar enum arguments where not handled as enum arguments but as string literals (only example so far the datetime extract function).

  • This PR changes the grammar to explicitly support enum arguments with a new enum data type in the grammar.
  • The Python code is regenerated from the grammar.
  • The extract test cases are updated to use the new syntax.

Copy link
Copy Markdown
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.

Treating enums as their own type makes sense to me.

Copy link
Copy Markdown
Member

@benbellick benbellick left a comment

Choose a reason for hiding this comment

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

This is great, thanks!

It would be great in the future to validate that the enum values used in the tests are allowed enum values (e.g. HOUR is valid for extract, but BLAHBLAH is not) though I think leaving that for a future PR is wise.

Signed-off-by: Niels Pardon <par@zurich.ibm.com>
@nielspardon
Copy link
Copy Markdown
Member Author

I rebased and resolved the conflicts. Github offers me to merge the PR myself with the given approvals. Is it ok if I do this as a committer or should a PMC member push the button here?

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.

4 participants