Add set op typing; fixes various behaviors in set op parsing and modeling #1506

alancai98 · 2024-07-10T18:41:28Z

Relevant Issues

Closes [main] Support UNION typing in the main branch #1501

Description

partiql-plan:
- adds set quantifier to Rel set ops. Plan is slightly simpler than the set operator modeling used in v1 branch. Primarily, no additional set class wrapping the set operators
- adds Rex set ops to plan
partiql-planner: adds typing support for set operator
- Currently just performs the schema size validation; comparability of column types will not be part of this release to unblock Support UNION in transpilation partiql-scribe#55
partiql-parser:
- distinguishes between PartiQL bag and SQL set ops
- updates and adds tests following this behavioral change
partiql-ast:
- SqlDialect -- fixes missing parens set op printing
Tested the updates in Scribe Upgrade to plk 0.14.6 + add UNION support partiql-scribe#56
Prepares v0.14.6 release

Other Information

Updated Unreleased Section in CHANGELOG: [YES]
Any backward-incompatible changes? [YES]
- Yes, from prior discussion, this is a backwards incompatible change due to adding an additional field (set quantifier) to set operators in the plan. But since the plan typer always gave an error when typing the plan, no customers should have been using the previous modeling.
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.

Adds compilation/evaluation support for UNION/INTERSECT/EXCEPT ALL/DISTINCT

github-actions · 2024-07-10T18:49:16Z

Conformance comparison report

	Base (`55a27e6`)	`c080b9f`	+/-
% Passing	92.54%	92.54%	0.00%
✅ Passing	5384	5384	0
❌ Failing	434	434	0
🔶 Ignored	0	0	0
Total Tests	5818	5818	0

Number passing in both: 5384

Number failing in both: 434

Number passing in Base (55a27e6) but now fail: 0

Number failing in Base (55a27e6) but now pass: 0

alancai98 · 2024-07-10T18:52:13Z

partiql-planner/src/main/kotlin/org/partiql/planner/internal/typer/PlanTyper.kt

+            for (i in 0..lhs.type.schema.lastIndex) {
+                val lhsBindingType = lhs.type.schema[i].type
+                val rhsBindingType = rhs.type.schema[i].type
+                if (lhsBindingType != rhsBindingType) {
+                    return false
+                }
+            }


Since the current schema type comparison is only partially implemented, I'll delete this validation block for the 0.14.6 release to unblock partiql/partiql-scribe#55. The different Scribe dialects may have their own notion of comparability and perform their own set operator checks.

alancai98 · 2024-07-10T23:28:17Z

Will mark this PR as a draft to account for some

parsing changes (current parser parses every set op to bag_op)
- Expr.SFW.set_op does not support left-associative set operations -- [main/v1] partiql-ast: modeling of s_f_w.set_op does not allow for left associative set operations #1507, so do not use this AST node
- Model SQL vs PartiQL set ops using DISTINCT node
  - Now, check if LHS or RHS is a SFW expr (or other SQL set op) to determine whether outer is true/false
plan changes
- current plan modeling maps bag_ops and SFW.set_ops as a Rel node w/ two Rel arguments
- we should have a separate node for bag_ops over Rexs and SFW.set_ops over Rels

… exprs; add/update tests

alancai98 · 2024-07-12T00:42:48Z

Above TODOs should be addressed. Still have a few more before PR is ready for review:

- Check SqlDialect behavior for printing BagOp -- seems to be omitting parens for right-associative case
- Test PR's changes in local maven release and Scribe -- Upgrade to plk 0.14.6 + add UNION support partiql-scribe#56
- Update added set op parse tests to use the AstBuilder to remove some unneeded code

partiql-ast/src/main/kotlin/org/partiql/ast/sql/internal/InternalSqlDialect.kt

alancai98 · 2024-07-12T22:38:11Z

partiql-ast/src/main/resources/partiql_ast.ion

@@ -500,7 +500,7 @@ expr::[
    where:    optional::expr,
    group_by: optional::group_by,
    having:   optional::expr,
-    set_op:   optional::{
+    set_op:   optional::{ // TODO modeling of `set_op` needs updated to support left-associative set ops https://github.com/partiql/partiql-lang-kotlin/issues/1507


The current modeling of SQL set ops is not correct in the AST (see #1507). I chose to just reuse the Expr.BagOp node rather than use this Expr.SFW.SetOp node for this PR to preserve backwards compatibility for this release.

We should fix the modeling before the v1 release.

alancai98 · 2024-07-12T22:39:50Z

partiql-lang/src/main/kotlin/org/partiql/lang/prettyprint/QueryPrettyPrinter.kt

@@ -320,7 +320,14 @@ class QueryPrettyPrinter {
            is PartiqlAst.Expr.Or -> writeNAryOperator("OR", node.operands, sb, level)
            is PartiqlAst.Expr.InCollection -> writeNAryOperator("IN", node.operands, sb, level)
            is PartiqlAst.Expr.BagOp -> {
-                var name = node.op.javaClass.simpleName.toUpperCase().replace("_", " ")
+                var name = when (node.op) {


Buggy previous behavior would print OUTER UNION node as OUTERUNION.

Could have been more clever w/ printing but since this is legacy code that will be removed ahead of v1, just opted for a simple solution.

alancai98 · 2024-07-12T22:42:41Z

partiql-parser/src/main/kotlin/org/partiql/parser/internal/PartiQLParserDefault.kt

+        /**
+         * Verifies if all of the [args] are
+         * 1. [Expr.SFW] or
+         * 2. [Expr.BagOp] and is a SQL Set op (i.e. [Expr.BagOp.outer] != true)
+         */
+        private fun argsAreSFW(args: List<Expr>): Boolean {
+            return args.all { arg ->
+                arg is Expr.SFW || (arg is Expr.BagOp && arg.outer != true)
+            }
+        }


New parsing logic to distinguish between SQL set ops and PartiQL bag ops

SQL set ops

require no OUTER before set operation and

require args are SQL set ops or SFW expression

PartiQL bag ops

use OUTER keyword or

do not use OUTER keyword but one or more arguments is a PartiQL bag op or non-SFW expression

alancai98 · 2024-07-12T22:44:58Z

partiql-planner/src/main/kotlin/org/partiql/planner/internal/typer/PlanTyper.kt

+            // TODO: [RFC-0007](https://github.com/partiql/partiql-lang/blob/main/RFCs/0007-rfc-bag-operators.md)
+            //  states that the types must be "comparable". For now, we will always return true. In the future, we need
+            //  to add support for checking comparable types.


For now, do not do comparability check of types. Only check same number of columns for both set ops.

RCHowell

Still reviewing, minor comments

RCHowell · 2024-07-13T01:55:58Z

gradle.properties

@@ -1,5 +1,5 @@
 group=org.partiql
-version=0.14.6-SNAPSHOT
+version=0.14.6


Did you drop SNAPSHOT for the release?

yeah -- was planning to cut the 0.14.6 release after this PR gets merged in

RCHowell · 2024-07-13T02:10:12Z

partiql-planner/src/main/kotlin/org/partiql/planner/internal/transforms/RelConverter.kt

+            assert(node.lhs is Expr.SFW || node.lhs is Expr.BagOp)
+            assert(node.rhs is Expr.SFW || node.rhs is Expr.BagOp)


I suggest adding a message here.

added a message for both arg asserts

partiql-planner/src/main/kotlin/org/partiql/planner/internal/transforms/RelConverter.kt

johnedquinn and others added 4 commits July 10, 2024 10:32

Merge pull request #1430 from johnedquinn/v1-union

9b1b677

Adds compilation/evaluation support for UNION/INTERSECT/EXCEPT ALL/DISTINCT

Delete partiql-eval specific changes

35760f7

Use simplified set op representation

f46dd22

Prepare 0.14.6 release

58df6f7

alancai98 self-assigned this Jul 10, 2024

alancai98 commented Jul 10, 2024

View reviewed changes

Ignore types of set op schemas

af7bc0b

alancai98 requested a review from RCHowell July 10, 2024 18:58

alancai98 marked this pull request as draft July 10, 2024 23:28

Adds separate plan node for PartiQL bag ops; fixes parsing of non-SFW…

0bd0572

… exprs; add/update tests

alancai98 added 2 commits July 12, 2024 11:13

Fix SqlDialect (and InternalSD) parens for set ops

2f7ccc7

Update parse tests to use AST builder

0dd43ea

alancai98 force-pushed the cherry-pick-setop-changes branch from c69dd6f to 0dd43ea Compare July 12, 2024 18:14

alancai98 mentioned this pull request Jul 12, 2024

Upgrade to plk 0.14.6 + add UNION support partiql/partiql-scribe#56

Merged

Update outdated plan comments

b7b6216

alancai98 commented Jul 12, 2024

View reviewed changes

alancai98 marked this pull request as ready for review July 12, 2024 22:49

alancai98 changed the title ~~partiql-plan: adds set quantifier to set ops; partiql-planner: add typing support for set ops~~ Add set op typing; fixes various behaviors in set op parsing and modeling Jul 12, 2024

RCHowell requested changes Jul 13, 2024

View reviewed changes

Add err messages; use nil

abdeaec

alancai98 requested a review from RCHowell July 15, 2024 20:07

RCHowell approved these changes Jul 15, 2024

View reviewed changes

alancai98 merged commit 54c6c47 into v0.14.6 Jul 15, 2024
10 checks passed

alancai98 deleted the cherry-pick-setop-changes branch July 15, 2024 22:55

This was referenced Jul 29, 2024

Fixes all DISTINCT conformance tests #1513

Merged

[v1] Refactor representation of set ops #1538

Merged

alancai98 mentioned this pull request Aug 12, 2024

[main] Support UNION typing in the main branch #1501

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add set op typing; fixes various behaviors in set op parsing and modeling #1506

Add set op typing; fixes various behaviors in set op parsing and modeling #1506

alancai98 commented Jul 10, 2024 •

edited

Loading

github-actions bot commented Jul 10, 2024 •

edited

Loading

alancai98 Jul 10, 2024

alancai98 commented Jul 10, 2024 •

edited

Loading

alancai98 commented Jul 12, 2024 •

edited

Loading

alancai98 Jul 12, 2024

alancai98 Jul 12, 2024

alancai98 Jul 12, 2024

alancai98 Jul 12, 2024

RCHowell left a comment

RCHowell Jul 13, 2024

alancai98 Jul 15, 2024

RCHowell Jul 13, 2024

alancai98 Jul 15, 2024

		assert(node.lhs is Expr.SFW \|\| node.lhs is Expr.BagOp)
		assert(node.rhs is Expr.SFW \|\| node.rhs is Expr.BagOp)

Add set op typing; fixes various behaviors in set op parsing and modeling #1506

Add set op typing; fixes various behaviors in set op parsing and modeling #1506

Conversation

alancai98 commented Jul 10, 2024 • edited Loading

Relevant Issues

Description

Other Information

License Information

github-actions bot commented Jul 10, 2024 • edited Loading

Conformance comparison report

Choose a reason for hiding this comment

alancai98 commented Jul 10, 2024 • edited Loading

alancai98 commented Jul 12, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RCHowell left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alancai98 commented Jul 10, 2024 •

edited

Loading

github-actions bot commented Jul 10, 2024 •

edited

Loading

alancai98 commented Jul 10, 2024 •

edited

Loading

alancai98 commented Jul 12, 2024 •

edited

Loading