Adds support for DML INSERT's WHERE/ALIAS in Parser, AST, and Plans

[johnedquinn]

Relevant Issues

• Closes Parser support for the WHERE clause with INSERT INTO ... ON CONFLICT REPLACE EXCLUDED #1038
• Closes INSERT with alias loses original table name which is replaced with the alias #1043
• Closes Allow PartiQL applications to rewrite DML nodes into custom project operators #1052
  • By adding support for the alias and condition in the physical plan, there is no longer a need to rewrite the DML statement into a projection operator -- as requested in 1052.

Description

• Adds support for DML INSERT's WHERE/ALIAS in Parser, AST, and Plans

Changes

• Adds support for ALIAS syntax
  • Adds a new property as_alias to the insert AST node.
  • Adds target_alias property to the dml_insert, dml_replace, and dml_update nodes within the
    Logical and Logical Resolved plans
  • Adds new physical plan field representing the register allocated to the index.
• Adds support for CONDITION in ON CONFLICT DO [REPLACE|UPDATE] EXCLUDED WHERE <condition>
  • Adds new property condition to the AST nodes of do_replace and do_update
  • Adds condition property to the dml_replace and dml_update nodes within the
    Logical and Logical Resolved plans
  • Adds new physical plan field representing the conflict condition.
• Removes dml_query from the physical plan and adds the statement.dml and dml_operation to the physical plan.
  • This allows users to rewrite the physical plan without using the "loose" contract that used to be dml_query -- which was effectively just a struct with key-value pairs.

Note: The pipeline currently doesn't support the physical plan for ON CONFLICT DO UPDATE EXCLUDED. This PR still provides the changes for this clause, but it does not complete the required physical plan changes to support full planning of this statement.

Testing

• Added tests for the PartiQLParser, the AstToLogicalVisitorTransform, the LogicalToLogicalResolvedVisitorTransform, and the LogicalResolvedToDefaultPhysicalVisitorTransform
• Compilation of DML has changed from extracting the key-value pairs from the evaluated physical plan to immediately evaluating the table_unique_id and rows expressions. These cases are covered by the integration tests under IntegrationTests.kt.

To Check

For the PR reviewer, you can use these features by starting up the experimental planner in the CLI and adding an empty table to the global environment:

Then, explain the ast, logical, logical_resolved, and physical plans for example queries. For example, here is the logical_resolved:

Other Information

• Updated Unreleased Section in CHANGELOG: YES
• Any backward-incompatible changes? YES
  • partiql.ion has modifications. See the CHANGELOG.
• Any new external dependencies? NO

License Information

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[github-actions]

Conformance comparison report

	Base (ef2847e)	bbb26dd	+/-
% Passing	97.42%	97.42%	0.00%
✅ Passing	4271	4271	0
❌ Failing	113	113	0
🔶 Ignored	0	0	0
Total Tests	4384	4384	0

Number passing in both: 4271

Number failing in both: 113

Number passing in Base (ef2847e) but now fail: 0

Number failing in Base (ef2847e) but now pass: 0

[codecov-commenter]

Codecov Report

Patch coverage: 78.12% and project coverage change: +0.02 🎉

Comparison is base (ef2847e) 74.93% compared to head (6fe4d96) 74.96%.

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #1061      +/-   ##
============================================
+ Coverage     74.93%   74.96%   +0.02%     
  Complexity     2334     2334              
============================================
  Files           249      249              
  Lines         18559    18537      -22     
  Branches       3360     3354       -6     
============================================
- Hits          13907    13896      -11     
+ Misses         3627     3617      -10     
+ Partials       1025     1024       -1     

Flag	Coverage Δ
CLI	14.94% <ø> (ø)
EXAMPLES	80.44% <ø> (ø)
LANG	79.76% <78.12%> (+0.03%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
.../main/kotlin/org/partiql/lang/planner/QueryPlan.kt	0.00% <ø> (-12.50%)	⬇️
...ogicalResolvedToDefaultPhysicalVisitorTransform.kt	90.21% <0.00%> (-5.87%)	⬇️
...rg/partiql/lang/compiler/PartiQLCompilerDefault.kt	90.62% <66.66%> (+4.91%)	⬆️
...n/kotlin/org/partiql/lang/syntax/PartiQLVisitor.kt	91.56% <78.57%> (-0.25%)	⬇️
...iql/lang/eval/physical/PhysicalPlanCompilerImpl.kt	85.13% <85.71%> (-0.02%)	⬇️
...planner/transforms/AstToLogicalVisitorTransform.kt	86.66% <95.00%> (+1.15%)	⬆️
...sforms/LogicalToLogicalResolvedVisitorTransform.kt	94.60% <100.00%> (+0.98%)	⬆️

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[dlurton]

While this does satisfy #1038 and #1043 issues, we don't have a solution for #1052 yet in this PR.

I was hoping we could delete this line, in order to preserve DML in the original plan. We'd also have to delete this function so as to avoid rewriting DML into an SFW query at all.

Doing this would allow your customers to do whatever interesting things they want with DML, with the benefit that you don't have to expose the logical-resolved plan as API, and your customers for whom DML-as-SFW won't work don't have to jump thru hoops with trying to rewrite the DML-as-SFW query into something usable.

[dlurton]

I also think that PartiQLResult.Insert, and PartiQLResult.Delete (and related code) may need to be removed as well. I see that PartiQLResult.Explain was added not to long ago, so probably it and PartiQLResult.Value can be retained.

[johnedquinn]

I completely agree with retaining the structure of the DML within the physical plan and have consulted @am357 about it as well. I've just added recent changes to reflect our position. If you'll notice, I didn't remove the DML PartiQLResults -- the reason for this is that we can still retain its intended use by modifying how we compile the plan. See the changes in PartiQLCompilerDefault and PhysicalPlanCompilerImpl. Re-marking as Ready For Review.

[dlurton]

I did not review every detail--but I did bring in my DML rewrite WIP into this branch on my local machine to verify that this would be sufficient to meet my use case, and it looks like it is. So, overall, this LGTM.

However, I did accidentally discover that inclusion of the VALUE <expr> clause (which I'm pretty sure was a mistake from the past since it's redundant with VALUES << <expr> >>), causes an internal error caused by a ClassCastException within the ANTLR AST -> PartiQL AST visitor. (We have to keep around the VALUE clause for backward compatibility...)

i.e. this query will reproduce the issue: INSERT INTO foo VALUE { 'id': 42 } ON CONFLICT DO REPLACE EXCLUDED WHERE TRUE

[johnedquinn]

@dlurton -- regarding the query for reproducing the issue. It seems that this actually has to do with the mixing-and-matching of the "legacy" INSERT command with the "current" ON CONFLICT clause. The grammar says:

insertCommand
    : INSERT INTO pathSimple VALUE value=expr ( AT pos=expr )? onConflictClause?  # InsertLegacy
    | INSERT INTO symbolPrimitive asIdent? value=expr onConflictClause?           # Insert
    ;

onConflictClause
    : ON CONFLICT WHERE expr DO NOTHING                                           # OnConflictLegacy
    | ON CONFLICT conflictTarget? conflictAction                                  # OnConflict
    ;

While the grammar says we can mix-and-match the "legacy" and "current" commands/clauses, we actually don't allow it in the visitor. Ideally, this should be expressed more explicitly within our grammar. I spoke to @am357 , and it seems that we are maintaining the "legacy" insert and "legacy" on-conflict clauses for backward-compatibility.

Anyways, this issue is present in main, and I'll leave it out of the scope of this PR.