Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 10 commits into from
Jul 15, 2024
9 changes: 8 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,11 @@ unconstrained which is not SQL-conformant and is causing issues in integrating w
INTEGER an alias for INT4 which is the internal type name. In a later release, we will make INTEGER the default 32-bit
integer with INT/INT4/INTEGER4 being aliases per other systems. This change only applies to
org.partiql.parser.PartiQLParser, not the org.partiql.lang.syntax.PartiQLParser.
- **Breaking change**: partiql-plan: adds a set quantifier field to SQL set operators `UNION`, `INTERSECT`, and `EXCEPT`
- partiql-plan: adds a dedicated Rex node for PartiQL bag operators `UNION`, `INTERSECT`, and `EXCEPT`
- partiql-planner: Adds typing support for set operators
- partiql-parser: parses non-SFW expressions to be PartiQL `OUTER` bag operators
- partiql-ast: fixes missing parens from `bag_op` when printing using `SqlDialect`

### Deprecated

Expand Down Expand Up @@ -1082,7 +1087,9 @@ breaking changes if migrating from v0.9.2. The breaking changes accidentally int
### Added
Initial alpha release of PartiQL.

[Unreleased]: https://github.com/partiql/partiql-lang-kotlin/compare/v0.14.4...HEAD
[Unreleased]: https://github.com/partiql/partiql-lang-kotlin/compare/v0.14.6...HEAD
[0.14.6]: https://github.com/partiql/partiql-lang-kotlin/compare/v0.14.5...v0.14.6
[0.14.5]: https://github.com/partiql/partiql-lang-kotlin/compare/v0.14.4...v0.14.5
[0.14.4]: https://github.com/partiql/partiql-lang-kotlin/compare/v0.14.3...v0.14.4
[0.14.3]: https://github.com/partiql/partiql-lang-kotlin/compare/v0.14.2...v0.14.3
[0.14.2]: https://github.com/partiql/partiql-lang-kotlin/compare/v0.14.1...v0.14.2
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ This project is published to [Maven Central](https://search.maven.org/artifact/o

| Group ID | Artifact ID | Recommended Version |
|---------------|-----------------------|---------------------|
| `org.partiql` | `partiql-lang-kotlin` | `0.14.5` |
| `org.partiql` | `partiql-lang-kotlin` | `0.14.6` |


For Maven builds, add the following to your `pom.xml`:
Expand Down
2 changes: 1 addition & 1 deletion gradle.properties
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
group=org.partiql
version=0.14.6-SNAPSHOT
version=0.14.6
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you drop SNAPSHOT for the release?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


ossrhUsername=EMPTY
ossrhPassword=EMPTY
Expand Down
7 changes: 7 additions & 0 deletions partiql-ast/src/main/kotlin/org/partiql/ast/sql/SqlDialect.kt
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,13 @@ public abstract class SqlDialect : AstBaseVisitor<SqlBlock, SqlBlock>() {
h = h concat ")"
h
}
is Expr.BagOp -> {
var h = head
h = h concat "("
h = visitExprBagOp(node, h)
h = h concat ")"
h
}
else -> visitExpr(node, head)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,13 @@ internal abstract class InternalSqlDialect : AstBaseVisitor<InternalSqlBlock, In
t = t concat ")"
t
}
is Expr.BagOp -> {
alancai98 marked this conversation as resolved.
Show resolved Hide resolved
var t = tail
t = t concat "("
t = visitExprBagOp(node, t)
t = t concat ")"
t
}
else -> visitExpr(node, tail)
}

Expand Down
2 changes: 1 addition & 1 deletion partiql-ast/src/main/resources/partiql_ast.ion
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

type: '.set_op',
operand: '.expr.s_f_w',
},
Expand Down
114 changes: 114 additions & 0 deletions partiql-ast/src/test/kotlin/org/partiql/ast/sql/SqlDialectTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -986,6 +986,120 @@ class SqlDialectTest {
rhs = v("y")
}
},
expect("(x UNION y) UNION z") {
exprBagOp {
type = setOp {
type = SetOp.Type.UNION
setq = null
}
outer = false
lhs = exprBagOp {
type = setOp {
type = SetOp.Type.UNION
setq = null
}
outer = false
lhs = v("x")
rhs = v("y")
}
rhs = v("z")
}
},
expect("x UNION (y UNION z)") {
exprBagOp {
type = setOp {
type = SetOp.Type.UNION
setq = null
}
outer = false
lhs = v("x")
rhs = exprBagOp {
type = setOp {
type = SetOp.Type.UNION
setq = null
}
outer = false
lhs = v("y")
rhs = v("z")
}
}
},
expect("(x EXCEPT y) EXCEPT z") {
exprBagOp {
type = setOp {
type = SetOp.Type.EXCEPT
setq = null
}
outer = false
lhs = exprBagOp {
type = setOp {
type = SetOp.Type.EXCEPT
setq = null
}
outer = false
lhs = v("x")
rhs = v("y")
}
rhs = v("z")
}
},
expect("x EXCEPT (y EXCEPT z)") {
exprBagOp {
type = setOp {
type = SetOp.Type.EXCEPT
setq = null
}
outer = false
lhs = v("x")
rhs = exprBagOp {
type = setOp {
type = SetOp.Type.EXCEPT
setq = null
}
outer = false
lhs = v("y")
rhs = v("z")
}
}
},
expect("(x INTERSECT y) INTERSECT z") {
exprBagOp {
type = setOp {
type = SetOp.Type.INTERSECT
setq = null
}
outer = false
lhs = exprBagOp {
type = setOp {
type = SetOp.Type.INTERSECT
setq = null
}
outer = false
lhs = v("x")
rhs = v("y")
}
rhs = v("z")
}
},
expect("x INTERSECT (y INTERSECT z)") {
exprBagOp {
type = setOp {
type = SetOp.Type.INTERSECT
setq = null
}
outer = false
lhs = v("x")
rhs = exprBagOp {
type = setOp {
type = SetOp.Type.INTERSECT
setq = null
}
outer = false
lhs = v("y")
rhs = v("z")
}
}
},
)

@JvmStatic
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

is PartiqlAst.BagOpType.Except -> "EXCEPT"
is PartiqlAst.BagOpType.Intersect -> "INTERSECT"
is PartiqlAst.BagOpType.Union -> "UNION"
is PartiqlAst.BagOpType.OuterExcept -> "OUTER EXCEPT"
is PartiqlAst.BagOpType.OuterIntersect -> "OUTER INTERSECT"
is PartiqlAst.BagOpType.OuterUnion -> "OUTER UNION"
}
if (node.quantifier is PartiqlAst.SetQuantifier.All) {
name += " ALL"
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -752,12 +752,33 @@ internal class PartiQLPigVisitor(
*
*/

/**
* Verifies if all of the [args] are
* 1. [PartiqlAst.Expr.Select] or
* 2. [PartiqlAst.Expr.BagOp] and is a SQL Set op (i.e. not an `OUTER` bag op)
*/
private fun argsAreSFW(args: List<PartiqlAst.Expr>): Boolean {
return args.all { arg ->
arg is PartiqlAst.Expr.Select || (arg is PartiqlAst.Expr.BagOp && isOuter(arg.op))
}
}

private fun isOuter(op: PartiqlAst.BagOpType): Boolean {
return op is PartiqlAst.BagOpType.OuterUnion || op is PartiqlAst.BagOpType.OuterExcept || op is PartiqlAst.BagOpType.OuterIntersect
}

override fun visitIntersect(ctx: PartiQLParser.IntersectContext) = PartiqlAst.build {
val lhs = visit(ctx.lhs) as PartiqlAst.Expr
val rhs = visit(ctx.rhs) as PartiqlAst.Expr
val quantifier = if (ctx.ALL() != null) all() else distinct()
val (intersect, metas) = when (ctx.OUTER()) {
null -> intersect() to ctx.INTERSECT().getSourceMetaContainer()
null -> {
if (argsAreSFW(listOf(lhs, rhs))) {
intersect() to ctx.INTERSECT().getSourceMetaContainer()
} else {
outerIntersect() to ctx.OUTER().getSourceMetaContainer()
}
}
else -> outerIntersect() to ctx.OUTER().getSourceMetaContainer()
}
bagOp(intersect, quantifier, listOf(lhs, rhs), metas)
Expand All @@ -768,7 +789,13 @@ internal class PartiQLPigVisitor(
val rhs = visit(ctx.rhs) as PartiqlAst.Expr
val quantifier = if (ctx.ALL() != null) all() else distinct()
val (except, metas) = when (ctx.OUTER()) {
null -> except() to ctx.EXCEPT().getSourceMetaContainer()
null -> {
if (argsAreSFW(listOf(lhs, rhs))) {
except() to ctx.EXCEPT().getSourceMetaContainer()
} else {
outerExcept() to ctx.OUTER().getSourceMetaContainer()
}
}
else -> outerExcept() to ctx.OUTER().getSourceMetaContainer()
}
bagOp(except, quantifier, listOf(lhs, rhs), metas)
Expand All @@ -779,7 +806,13 @@ internal class PartiQLPigVisitor(
val rhs = visit(ctx.rhs) as PartiqlAst.Expr
val quantifier = if (ctx.ALL() != null) all() else distinct()
val (union, metas) = when (ctx.OUTER()) {
null -> union() to ctx.UNION().getSourceMetaContainer()
null -> {
if (argsAreSFW(listOf(lhs, rhs))) {
union() to ctx.UNION().getSourceMetaContainer()
} else {
outerUnion() to ctx.OUTER().getSourceMetaContainer()
}
}
else -> outerUnion() to ctx.OUTER().getSourceMetaContainer()
}
bagOp(union, quantifier, listOf(lhs, rhs), metas)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -539,7 +539,7 @@ class ASTPrettyPrinterTest {
checkPrettyPrintAst(
"a UNION b",
"""
Union
OuterUnion
Id a (case_insensitive) (unqualified)
Id b (case_insensitive) (unqualified)
""".trimIndent()
Expand All @@ -551,7 +551,7 @@ class ASTPrettyPrinterTest {
checkPrettyPrintAst(
"a EXCEPT b",
"""
Except
OuterExcept
Id a (case_insensitive) (unqualified)
Id b (case_insensitive) (unqualified)
""".trimIndent()
Expand All @@ -563,7 +563,7 @@ class ASTPrettyPrinterTest {
checkPrettyPrintAst(
"a INTERSECT b",
"""
Intersect
OuterIntersect
Id a (case_insensitive) (unqualified)
Id b (case_insensitive) (unqualified)
""".trimIndent()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -465,22 +465,22 @@ class QueryPrettyPrinterTest {

@Test
fun union1() {
checkPrettyPrintQuery("a UNION b", "a UNION b")
checkPrettyPrintQuery("a UNION b", "a OUTER UNION b")
}

@Test
fun union2() {
checkPrettyPrintQuery("a UNION ALL b", "a UNION ALL b")
checkPrettyPrintQuery("a UNION ALL b", "a OUTER UNION ALL b")
}

@Test
fun except() {
checkPrettyPrintQuery("a EXCEPT b", "a EXCEPT b")
checkPrettyPrintQuery("a EXCEPT b", "a OUTER EXCEPT b")
}

@Test
fun intersect() {
checkPrettyPrintQuery("a INTERSECT b", "a INTERSECT b")
checkPrettyPrintQuery("a INTERSECT b", "a OUTER INTERSECT b")
}

@Test
Expand Down Expand Up @@ -822,7 +822,7 @@ class QueryPrettyPrinterTest {
checkPrettyPrintQuery(
"(SELECT a FROM b) UNION (SELECT c FROM d) UNION (SELECT e FROM f)",
"""
((SELECT a FROM b) UNION (SELECT c FROM d)) UNION (SELECT e FROM f)
((SELECT a FROM b) UNION (SELECT c FROM d)) OUTER UNION (SELECT e FROM f)
""".trimIndent()
)
}
Expand All @@ -832,7 +832,7 @@ class QueryPrettyPrinterTest {
checkPrettyPrintQuery(
"(SELECT a FROM b) UNION c",
"""
(SELECT a FROM b) UNION c
(SELECT a FROM b) OUTER UNION c
""".trimIndent()
)
}
Expand Down Expand Up @@ -873,7 +873,7 @@ class QueryPrettyPrinterTest {
"CASE (SELECT name FROM t) WHEN (SELECT a FROM b) UNION c THEN 1 WHEN (SELECT c FROM d) THEN 2 ELSE (SELECT e FROM f) END",
"""
CASE (SELECT name FROM t)
WHEN (SELECT a FROM b) UNION c THEN 1
WHEN (SELECT a FROM b) OUTER UNION c THEN 1
WHEN (SELECT c FROM d) THEN 2
ELSE (SELECT e FROM f)
END
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ class PartiQLParserMatchTest : PartiQLParserTestBase() {
"(MyGraph MATCH (x)) UNION SELECT * FROM tbl1"
) {
bagOp(
op = union(),
op = outerUnion(), // TODO decide if graph match set op maps to PartiQL or SQL
quantifier = distinct(),
operands = listOf(
astMygraphMatchAllNodes,
Expand Down Expand Up @@ -142,7 +142,7 @@ class PartiQLParserMatchTest : PartiQLParserTestBase() {
"SELECT * FROM tbl1 UNION (MyGraph MATCH (x))"
) {
bagOp(
op = union(),
op = outerUnion(), // TODO decide if graph match set op maps to PartiQL or SQL
quantifier = distinct(),
operands = listOf(
astSelectStarFromTbl1,
Expand Down
Loading
Loading