Merge pull request #1592 from johnedquinn/v0_14_9_update_dml

Add dml update to partiql_logical pig domain and improve DML modeling
partiql · Sep 25, 2024 · 3dc2b5e · 3dc2b5e
2 parents 8634620 + 870d159
commit 3dc2b5e
Show file tree

Hide file tree

Showing 19 changed files with 830 additions and 1,358 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -25,22 +25,20 @@ Thank you to all who have contributed!
 
 ## [0.14.9]
 
-### Added
-
 ### Changed
 - With full, closed schema, the planner will now give a plan-time warning when it can prove an exclude path will never 
 exclude a value (relevant issue -- https://github.com/partiql/partiql-lang/issues/91).
 
-### Deprecated
-
-### Fixed
-
-### Removed
-
-### Security
+### Experimental Changes
+- **BREAKING**: For the _experimental_ `org.partiql.lang.domains` of `PartiqlLogical`, `PartiqlLogicalResolved`, and `PartiqlPhysical`,
+the modeling of DML has changed substantially. These changes, while considered breaking changes, are part of an
+experimental area of the PartiQL library and thus do not mandate a major-version bump of this library. Consumers
+of these experimental APIs should be wary of these changes.
 
 ### Contributors
 - @alancai98
+- @dlurton
+- @johnedquinn
 
 ## [0.14.8]
 

diff --git a/partiql-ast/src/main/pig/partiql.ion b/partiql-ast/src/main/pig/partiql.ion
@@ -802,55 +802,83 @@ may then be further optimized by selecting better implementations of each operat
             )
         )
 
+        (with statement
+              (exclude dml)
+              (include
+                  // An `INSERT` DML operation, which is fundamentally different from UPDATE and DELETE
+                  // because it lacks a FROM and WHERE clause while also including an ON CONFLICT clause.
+                  //
+                  // Models: INSERT INTO <dml_target> [AS <target_alias>] <rows_to_insert> [<on-conflict>]
+                  (dml_insert
+                      // The target is an expression indicates the table whose data is to be manipulated.
+                      // With current PartiQL Parser `SqlParser`, this can be an identifier or a simplified path expression
+                      // consisting of only literal path steps (and with no wildcard or unpivot operators).
+                      // Note: partiql_ast uses the `expr` sum type for this, which is too broad.  We're not
+                      // changing that at this time because `partiql_ast` is established public API, but we can
+                      // use dml_target instead which has the properly narrowed domain.
+                      (target dml_target)
+                      (target_alias var_decl)
+                      (rows_to_insert expr)
+                      (on_conflict (? on_conflict))
+                  )
+
+                  // Models: UPDATE <dml_target> [AS <target_alias>] SET <assignments> [WHERE <where>]
+                  (dml_update
+                      (target dml_target)
+                      (target_alias var_decl)
+                      (assignments (* set_assignment 0))
+                      (where (? expr))
+                  )
+
+                  // Models DELETE <from>
+                  (dml_delete (from bexpr)) // note: the bexpr includes filters, etc.
+              )
+        )
+
         (include
-            // Indicates kind of DML operation.
-            (sum dml_operation
-                (dml_insert target_alias::var_decl)
-                (dml_delete)
-
-                // Represents the REPLACE statement as well as INSERT ... ON CONFLICT DO REPLACE ...
-                // [target-alias]: represents the alias for the table name. See the following syntactical example:
-                //  `INSERT INTO Table1 AS <alias> << { 'id': 1, 'name': 'Arash' } >> ON CONFLICT DO REPLACE ...`
-                // [condition]: represents the condition by which a row should be replaced. See the following syntactical example:
-                //  `INSERT INTO x << {'id': 1, 'name': 'John'}} >> ON CONFLICT DO REPLACE EXCLUDED WHERE <condition>`
-                // [row_alias]: represents the alias given to the rows meant to be inserted/replaced. It is made optional
-                //  since dml_replace is currently shared by REPLACE (which does not allow the aliasing of rows) and
-                //  INSERT ... ON CONFLICT DO REPLACE ... (which aliases the rows as "EXCLUDED" for use within the [condition]).
-                (dml_replace target_alias::var_decl condition::(? expr) row_alias::(? var_decl))
-
-                // Represents the UPSERT statement as well as INSERT ... ON CONFLICT DO UPDATE ...
-                // [target-alias]: represents the alias for the table name. See the following syntactical example:
-                //  `INSERT INTO Table1 AS <alias> << { 'id': 1, 'name': 'Arash' } >> ON CONFLICT DO UPDATE ...`
-                // [condition]: represents the condition by which a row should be replaced. See the following syntactical example:
-                //  `INSERT INTO x << {'id': 1, 'name': 'John'}} >> ON CONFLICT DO UPDATE EXCLUDED WHERE <condition>`
-                // [row_alias]: represents the alias given to the rows meant to be inserted/updated. It is made optional
-                //  since dml_update is currently shared by UPSERT (which does not allow the aliasing of rows) and
-                //  INSERT ... ON CONFLICT DO UPDATE ... (which aliases the rows as "EXCLUDED" for use within the [condition]).
-                (dml_update target_alias::var_decl condition::(? expr) row_alias::(? var_decl))
+            // represents simple paths, i.e. suitable for the left side of an `=` operator within a `SET` clause.
+            // Example `a_field.nested_field[42]`
+            (record simple_path
+                // The first element, `a_field` in the example above.
+                (root identifier)
+                // The subsequent elements, `nested_field` and `[42]` in the example above.
+                (steps (* simple_path_step 0))
+            )
+            (sum simple_path_step
+                 // for bracket paths steps, i.e. `[42]` in the simple_path example above.
+                (sps_index (index int))
+                 // for symbols, i.e. `nested_field` in the simple_path example above.
+                (sps_identifier (identifier identifier))
             )
-        )
 
-        // Redefine statement.dml to be a simpler subset of the full DML functionality expressed with PartiQL's DML
-        // syntax.  Full functionality is out of scope for now.  This is factored minimally to support
-        // `INSERT INTO` and `DELETE FROM ... [WHERE <predicate>]` but this may need refactoring when
-        // `FROM ... UPDATE` and `UPDATE` is supported later.
-        (with statement
-            (exclude dml)
-            (include
-                // A DML operation, such as `INSERT`, `UPDATE` or `DELETE`
-                (dml
-                    // The target is an expression that is indicates the table whose data is to be manipulated.
-                    // With current PartiQL Parser `SqlParser`, this can be an identifier or a simplified path expression
-                    // consisting of only literal path steps (and with no wildcard or unpivot operators).
-                    // Note: partiql_ast uses the `expr` sum type for this, which is too broad.  We're not
-                    // changing that at this time because `partiql_ast` is established public API.
-                    target::identifier
-                    operation::dml_operation
-                    rows::expr
-                )
+            // The "target" of a DML operation, i.e. the table targeted for manipulation with INSERT, UPDATE, etc.
+            // This is a discrete type so it can be permuted in later domains to affect every use.
+            (record dml_target (identifier identifier))
+
+            // An assignment within a SET clause.
+            (record set_assignment
+                // The target, left of `=`
+                (set_target simple_path)
+                // The new value for the target, right of `=`
+                (value expr)
             )
+
+            // INSERT's ON CONFLICT Clause
+            (record on_conflict
+                (excluded_alias var_decl)
+                (condition (? expr))
+                (action on_conflict_action)
+            )
+
+            (sum on_conflict_action
+                 (do_update)
+                 (do_replace)
+            )
+
         )
 
+
+
         // Nodes excluded below this line will eventually have a representation in the logical algebra, but not
         // initially.
 
@@ -934,11 +962,9 @@ may then be further optimized by selecting better implementations of each operat
             )
         )
 
-        // Replace statement.dml.target with statement.dml.uniqueId (the "resolved" corollary).
-        (with statement
-            (exclude dml)
-            (include (dml uniqueId::symbol operation::dml_operation rows::expr))
-        )
+        // Replace statement.dml.uniqueId (the "resolved" corollary).
+        (exclude dml_target)
+        (include (record dml_target (uniqueId symbol)))
     )
 )
 

diff --git a/partiql-coverage/src/test/kotlin/org/partiql/coverage/api/impl/PartiQLTestExtensionTest.kt b/partiql-coverage/src/test/kotlin/org/partiql/coverage/api/impl/PartiQLTestExtensionTest.kt
@@ -31,7 +31,6 @@ import org.partiql.coverage.api.PartiQLTest
 import org.partiql.coverage.api.PartiQLTestCase
 import org.partiql.coverage.api.PartiQLTestProvider
 import org.partiql.lang.CompilerPipeline
-import org.partiql.lang.eval.EvaluationSession
 import org.partiql.lang.eval.PartiQLResult
 import java.lang.reflect.AnnotatedElement
 import java.lang.reflect.Method
@@ -76,19 +75,9 @@ class PartiQLTestExtensionTest {
         fun test2(tc: PartiQLTestCase, result: PartiQLResult.Value) {
         }
 
-        @Disabled
-        @PartiQLTest(provider = MockProvider::class)
-        @JvmName("test3")
-        @Suppress("UNUSED")
-        fun test3(tc: ValidTestCase, result: PartiQLResult.Delete) {
-        }
-
-        class ValidTestCase(override val session: EvaluationSession) : PartiQLTestCase
-
         override fun provideArguments(context: ExtensionContext?): Stream<out Arguments> = listOf(
             AbstractExtensionContext(ValidSignaturesProvider::class.java, "test1", PartiQLTestCase::class.java, PartiQLResult::class.java),
             AbstractExtensionContext(ValidSignaturesProvider::class.java, "test2", PartiQLTestCase::class.java, PartiQLResult.Value::class.java),
-            AbstractExtensionContext(ValidSignaturesProvider::class.java, "test3", ValidTestCase::class.java, PartiQLResult.Delete::class.java),
         ).map { Arguments.of(it) }.stream()
     }
 

diff --git a/partiql-lang/src/main/kotlin/org/partiql/lang/compiler/PartiQLCompilerAsyncDefault.kt b/partiql-lang/src/main/kotlin/org/partiql/lang/compiler/PartiQLCompilerAsyncDefault.kt
@@ -63,7 +63,9 @@ internal class PartiQLCompilerAsyncDefault(
 
     override suspend fun compile(statement: PartiqlPhysical.Plan): PartiQLStatementAsync {
         return when (val stmt = statement.stmt) {
-            is PartiqlPhysical.Statement.Dml -> compileDml(stmt, statement.locals.size)
+            is PartiqlPhysical.Statement.DmlDelete,
+            is PartiqlPhysical.Statement.DmlInsert,
+            is PartiqlPhysical.Statement.DmlUpdate -> TODO("DML compilation not supported.")
             is PartiqlPhysical.Statement.Exec,
             is PartiqlPhysical.Statement.Query -> {
                 val expression = exprConverter.compile(statement)
@@ -75,7 +77,9 @@ internal class PartiQLCompilerAsyncDefault(
 
     override suspend fun compile(statement: PartiqlPhysical.Plan, details: PartiQLPlanner.PlanningDetails): PartiQLStatementAsync {
         return when (val stmt = statement.stmt) {
-            is PartiqlPhysical.Statement.Dml -> compileDml(stmt, statement.locals.size)
+            is PartiqlPhysical.Statement.DmlDelete,
+            is PartiqlPhysical.Statement.DmlInsert,
+            is PartiqlPhysical.Statement.DmlUpdate -> TODO("DML compilation not supported.")
             is PartiqlPhysical.Statement.Exec,
             is PartiqlPhysical.Statement.Query -> compile(statement)
             is PartiqlPhysical.Statement.Explain -> PartiQLStatementAsync { compileExplain(stmt, details) }
@@ -93,18 +97,6 @@ internal class PartiQLCompilerAsyncDefault(
         PHYSICAL_TRANSFORMED
     }
 
-    private suspend fun compileDml(dml: PartiqlPhysical.Statement.Dml, localsSize: Int): PartiQLStatementAsync {
-        val rows = exprConverter.compile(dml.rows, localsSize)
-        return PartiQLStatementAsync { session ->
-            when (dml.operation) {
-                is PartiqlPhysical.DmlOperation.DmlReplace -> PartiQLResult.Replace(dml.uniqueId.text, (rows.eval(session) as PartiQLResult.Value).value)
-                is PartiqlPhysical.DmlOperation.DmlInsert -> PartiQLResult.Insert(dml.uniqueId.text, (rows.eval(session) as PartiQLResult.Value).value)
-                is PartiqlPhysical.DmlOperation.DmlDelete -> PartiQLResult.Delete(dml.uniqueId.text, (rows.eval(session) as PartiQLResult.Value).value)
-                is PartiqlPhysical.DmlOperation.DmlUpdate -> TODO("DML Update compilation not supported yet.")
-            }
-        }
-    }
-
     private fun compileExplain(statement: PartiqlPhysical.Statement.Explain, details: PartiQLPlanner.PlanningDetails): PartiQLResult.Explain.Domain {
         return when (val target = statement.target) {
             is PartiqlPhysical.ExplainTarget.Domain -> compileExplainDomain(target, details)

diff --git a/partiql-lang/src/main/kotlin/org/partiql/lang/compiler/PartiQLCompilerDefault.kt b/partiql-lang/src/main/kotlin/org/partiql/lang/compiler/PartiQLCompilerDefault.kt
@@ -65,7 +65,9 @@ internal class PartiQLCompilerDefault(
 
     override fun compile(statement: PartiqlPhysical.Plan): PartiQLStatement {
         return when (val stmt = statement.stmt) {
-            is PartiqlPhysical.Statement.Dml -> compileDml(stmt, statement.locals.size)
+            is PartiqlPhysical.Statement.DmlDelete,
+            is PartiqlPhysical.Statement.DmlInsert,
+            is PartiqlPhysical.Statement.DmlUpdate -> TODO("DML compilation not supported.")
             is PartiqlPhysical.Statement.Exec,
             is PartiqlPhysical.Statement.Query -> {
                 val expression = exprConverter.compile(statement)
@@ -77,7 +79,9 @@ internal class PartiQLCompilerDefault(
 
     override fun compile(statement: PartiqlPhysical.Plan, details: PartiQLPlanner.PlanningDetails): PartiQLStatement {
         return when (val stmt = statement.stmt) {
-            is PartiqlPhysical.Statement.Dml -> compileDml(stmt, statement.locals.size)
+            is PartiqlPhysical.Statement.DmlDelete,
+            is PartiqlPhysical.Statement.DmlInsert,
+            is PartiqlPhysical.Statement.DmlUpdate -> TODO("DML compilation not supported.")
             is PartiqlPhysical.Statement.Exec,
             is PartiqlPhysical.Statement.Query -> compile(statement)
             is PartiqlPhysical.Statement.Explain -> PartiQLStatement { compileExplain(stmt, details) }
@@ -95,18 +99,6 @@ internal class PartiQLCompilerDefault(
         PHYSICAL_TRANSFORMED
     }
 
-    private fun compileDml(dml: PartiqlPhysical.Statement.Dml, localsSize: Int): PartiQLStatement {
-        val rows = exprConverter.compile(dml.rows, localsSize)
-        return PartiQLStatement { session ->
-            when (dml.operation) {
-                is PartiqlPhysical.DmlOperation.DmlReplace -> PartiQLResult.Replace(dml.uniqueId.text, rows.eval(session))
-                is PartiqlPhysical.DmlOperation.DmlInsert -> PartiQLResult.Insert(dml.uniqueId.text, rows.eval(session))
-                is PartiqlPhysical.DmlOperation.DmlDelete -> PartiQLResult.Delete(dml.uniqueId.text, rows.eval(session))
-                is PartiqlPhysical.DmlOperation.DmlUpdate -> TODO("DML Update compilation not supported yet.")
-            }
-        }
-    }
-
     private fun compileExplain(statement: PartiqlPhysical.Statement.Explain, details: PartiQLPlanner.PlanningDetails): PartiQLResult.Explain.Domain {
         return when (val target = statement.target) {
             is PartiqlPhysical.ExplainTarget.Domain -> compileExplainDomain(target, details)

diff --git a/partiql-lang/src/main/kotlin/org/partiql/lang/eval/physical/PhysicalPlanCompilerAsyncImpl.kt b/partiql-lang/src/main/kotlin/org/partiql/lang/eval/physical/PhysicalPlanCompilerAsyncImpl.kt
@@ -226,7 +226,9 @@ internal class PhysicalPlanCompilerAsyncImpl(
         return when (ast) {
             is PartiqlPhysical.Statement.Query -> compileAstExpr(ast.expr)
             is PartiqlPhysical.Statement.Exec -> compileExec(ast)
-            is PartiqlPhysical.Statement.Dml,
+            is PartiqlPhysical.Statement.DmlDelete,
+            is PartiqlPhysical.Statement.DmlInsert,
+            is PartiqlPhysical.Statement.DmlUpdate -> TODO("DML compilation not supported.")
             is PartiqlPhysical.Statement.Explain -> {
                 val value = ExprValue.newBoolean(true)
                 thunkFactory.thunkEnvAsync(emptyMetaContainer()) { value }

diff --git a/partiql-lang/src/main/kotlin/org/partiql/lang/eval/physical/PhysicalPlanCompilerImpl.kt b/partiql-lang/src/main/kotlin/org/partiql/lang/eval/physical/PhysicalPlanCompilerImpl.kt
@@ -241,7 +241,9 @@ internal class PhysicalPlanCompilerImpl(
         return when (ast) {
             is PartiqlPhysical.Statement.Query -> compileAstExpr(ast.expr)
             is PartiqlPhysical.Statement.Exec -> compileExec(ast)
-            is PartiqlPhysical.Statement.Dml,
+            is PartiqlPhysical.Statement.DmlDelete,
+            is PartiqlPhysical.Statement.DmlInsert,
+            is PartiqlPhysical.Statement.DmlUpdate -> TODO("DML compilation not supported.")
             is PartiqlPhysical.Statement.Explain -> {
                 val value = ExprValue.newBoolean(true)
                 thunkFactory.thunkEnv(emptyMetaContainer()) { value }