Generalizes back-tick Ion to a variant pair in the AST #1591
Conversation
CROSS-ENGINE Conformance Report ❌
Testing DetailsResult Details
Now Failing Tests ❌The following 1 test(s) were previously PASSING in BASE but are now FAILING in TARGET: Click here to see
Now Passing Tests535 test(s) were previously failing in BASE (LEGACY-A065D59) but now pass in TARGET (EVAL-A065D59). 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-LEGACY Conformance Report ✅
Testing DetailsResult Details
CROSS-COMMIT-EVAL Conformance Report ✅
Testing DetailsResult Details
|
| // simplified Ion value writing, as this intentionally omits formatting | ||
| val value = node.value.toString() | ||
| override fun visitExprVariant(node: Expr.Variant, head: SqlBlock): SqlBlock { | ||
| if (node.encoding != "ion") { |
There was a problem hiding this comment.
Maybe having the encoding as an enum is a better option.
There was a problem hiding this comment.
Agreed, however I'm not sure the best way to make it extensible/extendable
There was a problem hiding this comment.
I was thinking of it like the "content-type" header
There was a problem hiding this comment.
I see thinking of it same as "content-type" make sense if we are directly working on the serialized payload. But if the node is an in-memory (which can be deserialized from the payload), we may benefit from Enum.
Relevant Issues
Description
This PR removes eager-parsing of Ion in the AST (ONLY) and introduces the parameterized VARIANT type. I am working on a subsequent PR to remove PartiQLValue from the planner which will eliminate eager-parsing of Ion entirely.
This PR is limited to only the necessary AST changes and does not modify the grammar or other components.
Other Information
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.