[BEAM-9027] Unparse DOY/DOW/WEEK Enums properly for ZetaSQL#10539
[BEAM-9027] Unparse DOY/DOW/WEEK Enums properly for ZetaSQL#10539apilloud merged 1 commit intoapache:masterfrom
Conversation
|
R: @robinyqiu |
|
|
||
| /** Only sample scenarios are covered here. Excessive testing is done via Compliance tests. */ | ||
| @Test | ||
| @Ignore("ZetaSQL does not support EnumType to IdentifierLiteral") |
There was a problem hiding this comment.
How could this test pass without running BeamZetaSqlCalcRel? My understanding is that the changes in this PR is only used in BeamZetaSqlCalcRel. Is this not correct?
There was a problem hiding this comment.
BigQuery predicate push-down also makes use of the unparsing.
There was a problem hiding this comment.
I think it could pass because ZetaSQL somehow starts to support EnumType.
There was a problem hiding this comment.
Limited support for EnumType was added but the tests weren't turned on.
|
LGTM.
Is there anything we can do (short/long term) to workaround/fix it? |
|
For the offset by 0 or 1 issue we need to do some work to make it configurable in Calcite. There is probably a workaround but it isn't worth investing in at the moment. |
4 participants
Fixes enums for unparsing, passes compliance tests with ZetaSQL Calc. Unit tests aren't possible because of mismatches of these types being 0 or 1 indexed.
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
R: @username).[BEAM-XXX] Fixes bug in ApproximateQuantiles, where you replaceBEAM-XXXwith the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.See the Contributor Guide for more tips on how to make review process smoother.
Post-Commit Tests Status (on master branch)
Pre-Commit Tests Status (on master branch)
See .test-infra/jenkins/README for trigger phrase, status and link of all Jenkins jobs.