[v1] Port SqlDialect to v1 AST #1638
Conversation
CROSS-ENGINE-REPORT ❌
Testing Details
Result Details
Now FAILING Tests ❌The following 3 test(s) were previously PASSING in BASE but are now FAILING in TARGET: Click here to see
Now IGNORED Tests ❌The complete list can be found in GitHub CI summary, either from Step Summary or in the Artifact. Now Passing Tests180 test(s) were previously failing in BASE (LEGACY-V0.14.8) but now pass in TARGET (EVAL-507F4D3). Before merging, confirm they are intended to pass. The complete list can be found in GitHub CI summary, either from Step Summary or in the Artifact. CROSS-COMMIT-REPORT ✅
Testing DetailsResult Details
|
alancai98
force-pushed
the
v1-migrate-ast-sqldialect
branch
from
91af96d
to
fc93151
Compare
| */ | ||
| @lombok.Builder(builderClassName = "Builder") | ||
| @EqualsAndHashCode(callSuper = false) | ||
| public class ExprRowValue extends Expr { |
There was a problem hiding this comment.
(self-review): slight adjustment to RowValue modeling to be more consistent with what SQL99 defines -- https://ronsavage.github.io/SQL/sql-99.bnf.html#row%20value%20constructor. In the future, can consider also adding the optional ROW keyword.
| @@ -12,14 +12,15 @@ | |||
|
|
|||
| /** | |||
| * TODO docs, equals, hashcode | |||
| * Also may not be an [Expr]? | |||
There was a problem hiding this comment.
(self-review): at some point before v1, need to take a closer look at the VALUES modeling. I'm not sure yet if it should be an Expr or something different.
There was a problem hiding this comment.
Subqueries generally need to be properly supported. #1641
| // Special case -- DATE_ADD('<datetime_field>', <lhs>, <rhs>) -> DATE_ADD(<datetime_field>, <lhs>, <rhs>) | ||
| // Special case -- DATE_DIFF('<datetime_field>', <lhs>, <rhs>) -> DATE_DIFF(<datetime_field>, <lhs>, <rhs>) | ||
| // TODO this will need to be rewritten once PartiQLValue is removed from AST |
There was a problem hiding this comment.
(self-review) special conversion for DATE_ADD and DATE_DIFF since in the v1 AST, we model them as ExprCalls rather than dedicated nodes.
| ) | ||
| ) | ||
| ), | ||
| // SEXP not in v1 |
There was a problem hiding this comment.
(self-review) the SEXP constructor was removed in the v1 AST. IIRC, these were the only tests I commented out from SqlDialect.
alancai98
changed the title
| @@ -12,14 +12,15 @@ | |||
|
|
|||
| /** | |||
| * TODO docs, equals, hashcode | |||
| * Also may not be an [Expr]? | |||
There was a problem hiding this comment.
Subqueries generally need to be properly supported. #1641
Relevant Issues
Description
SqlDialectto the v1 ASTDataTypeto visitor methodsOther Information
Updated Unreleased Section in CHANGELOG: [NO]
Any backward-incompatible changes? [YES]
Any new external dependencies? [NO]
Do your changes comply with the Contributing Guidelines
and Code Style Guidelines? [YES]
License Information
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.