Addresses PR feedback

Updates products to records in PIG

Updates naming of root/parts/qualifier

Simplifies BindingId and moves to separate file

Simplifies modeling of identifier chain

examples/src/main/java/org/partiql/examples/PartiQLCompilerPipelineAsyncJavaExample.java

@@ -3,6 +3,7 @@
 import com.amazon.ion.IonSystem;
 import com.amazon.ion.system.IonSystemBuilder;
 
+import java.util.List;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.ExecutionException;
 import kotlin.OptIn;
@@ -70,8 +71,9 @@ public void run() {
             @Override
             public GlobalResolutionResult resolveGlobal(@NotNull BindingId bindingId) {
                 // In this example, we don't allow for qualified identifiers.
-                if (!bindingId.hasQualifier()) {
-                    return resolveGlobal(bindingId.getIdentifier());
+                List<BindingName> parts = bindingId.getParts();
+                if (parts.size() == 1) {
+                    return resolveGlobal(parts.get(0));
                 }
                 return GlobalResolutionResult.Undefined.INSTANCE;
             }

partiql-ast/src/main/kotlin/org/partiql/ast/helpers/ToLegacyAst.kt

@@ -1002,15 +1002,12 @@ private class AstTranslator(val metas: Map<String, MetaContainer>) : AstBaseVisi
 
     private fun tableName(id: Identifier): PartiqlAst.TableName = translate(id) { metas ->
         val identifierChain = when (id) {
-            is Identifier.Symbol -> identifierChain(identifier(id), emptyList())
+            is Identifier.Symbol -> identifierChain(listOf(identifier(id)))
             is Identifier.Qualified -> {
                 val root = identifier(id.root)
                 val steps = id.steps.map { identifier(it) }
-                val (head, qualifier) = when (id.steps.isEmpty()) {
-                    true -> root to emptyList()
-                    false -> steps.last() to listOf(root) + steps.dropLast(1)
-                }
-                identifierChain(head, qualifier)
+                val parts = listOf(root) + steps
+                identifierChain(parts)
             }
         }
         tableName(identifierChain)

partiql-ast/src/main/pig/partiql.ion

@@ -525,7 +525,7 @@ may then be further optimized by selecting better implementations of each operat
         )
 
         // This refers to the <table name> EBNF rule in SQL:1999.
-        (product table_name id::identifier_chain)
+        (record table_name (id identifier_chain))
 
         // `identifier` can be used for names that need to be looked up with a notion of case-sensitivity.
 
@@ -541,7 +541,7 @@ may then be further optimized by selecting better implementations of each operat
         // `expr`.
         // For how this maps to syntax, consider `INSERT INTO cat1.schema1.table1`. In this scenario, `table1` is the
         // head. The qualifier is `cat1.schema1`.
-        (product identifier_chain head::identifier qualifier::(* identifier 0))
+        (record identifier_chain (parts (* identifier 1)))
 
         // This refers to a regular/delimited identifier in the SQL EBNF.
         (product identifier name::symbol case::case_sensitivity)

partiql-lang/src/main/kotlin/org/partiql/lang/eval/BindingId.kt

@@ -0,0 +1,27 @@
+package org.partiql.lang.eval
+
+/**
+ * Represents a namespaced global binding.
+ * @property parts a collection of the parts of the identifier.
+ */
+data class BindingId(
+    val parts: List<BindingName>
+) : Iterable<BindingName> {
+
+    init {
+        assert(parts.any())
+    }
+
+    constructor(vararg names: BindingName) : this(names.toList())
+
+    override fun iterator(): Iterator<BindingName> {
+        return parts.iterator()
+    }
+
+    override fun toString(): String {
+        return when (this.parts.size > 1) {
+            true -> parts.joinToString(".")
+            false -> parts[0].toString()
+        }
+    }
+}

partiql-lang/src/main/kotlin/org/partiql/lang/eval/Bindings.kt

@@ -56,63 +56,6 @@ fun PartiqlAst.CaseSensitivity.toBindingCase(): BindingCase = when (this) {
     is PartiqlAst.CaseSensitivity.CaseSensitive -> BindingCase.SENSITIVE
 }
 
-/**
- * Represents a namespaced global binding.
- */
-class BindingId(
-    private val qualifier: List<BindingName>,
-    private val id: BindingName
-) : Iterable<BindingName> {
-    /**
-     * For input such as `SELECT a FROM catalog1."schema1".table1`, the identifier is `table1`.
-     */
-    fun getIdentifier(): BindingName = id
-
-    /**
-     * For input such as `SELECT a FROM catalog1."schema1".table1`, the qualifier is `catalog1."schema1"`
-     */
-    fun getQualifier(): List<BindingName> = qualifier
-
-    /**
-     * Returns whether the id is qualified.
-     */
-    fun hasQualifier(): Boolean = qualifier.isNotEmpty()
-
-    /**
-     * Returns a collection of the parts of the identifier.
-     */
-    fun getParts(): List<BindingName> {
-        return qualifier + id
-    }
-
-    override fun iterator(): Iterator<BindingName> {
-        return getParts().iterator()
-    }
-
-    override fun toString(): String {
-        return when (hasQualifier()) {
-            true -> "${qualifier.joinToString(".")}.$id"
-            false -> id.toString()
-        }
-    }
-
-    override fun equals(other: Any?): Boolean {
-        if (this === other) return true
-        if (other !is BindingId) return false
-
-        if (qualifier != other.qualifier) return false
-        if (id != other.id) return false
-
-        return true
-    }
-
-    override fun hashCode(): Int {
-        var result = qualifier.hashCode()
-        result = 31 * result + id.hashCode()
-        return result
-    }
-}
-
 /**
  * Encapsulates the data necessary to perform a binding lookup.
  */

partiql-lang/src/main/kotlin/org/partiql/lang/planner/GlobalVariableResolver.kt

@@ -78,8 +78,9 @@ fun interface GlobalVariableResolver {
      * the unqualified identifier. If it is qualified, it will return a [GlobalResolutionResult.Undefined].
      */
     fun resolveGlobal(bindingId: BindingId): GlobalResolutionResult {
-        if (!bindingId.hasQualifier()) {
-            return resolveGlobal(bindingId.getIdentifier())
+        val parts = bindingId.parts
+        if (parts.size == 1) {
+            return resolveGlobal(parts.last())
         }
         return GlobalResolutionResult.Undefined
     }

partiql-lang/src/main/kotlin/org/partiql/lang/planner/PlanningProblemDetails.kt

@@ -38,16 +38,16 @@ sealed class PlanningProblemDetails(
     data class UndefinedDmlTarget(val id: BindingId) : PlanningProblemDetails(
         ProblemSeverity.ERROR,
         {
-            "Data manipulation target table '$id' is undefined. Hint: this must be a name in the global scope. " + quotationHint(id.getIdentifier().bindingCase == BindingCase.SENSITIVE)
+            "Data manipulation target table '$id' is undefined. Hint: this must be a name in the global scope. " + quotationHint(id.parts.last().bindingCase == BindingCase.SENSITIVE)
         }
     ) {
 
-        val variableName = id.getIdentifier().name
-        val caseSensitive = id.getIdentifier().bindingCase == BindingCase.SENSITIVE
+        private val lastPart = id.parts.last()
+        val variableName = lastPart.name
+        val caseSensitive = lastPart.bindingCase == BindingCase.SENSITIVE
 
         constructor(variableName: String, caseSensitive: Boolean) : this(
             BindingId(
-                emptyList(),
                 BindingName(
                     variableName,
                     when (caseSensitive) {

partiql-lang/src/main/kotlin/org/partiql/lang/planner/transforms/AstToLogicalVisitorTransform.kt

@@ -486,14 +486,13 @@ internal class AstToLogicalVisitorTransform(
 
     override fun transformIdentifierChain(node: PartiqlAst.IdentifierChain): PartiqlLogical.IdentifierChain {
         return PartiqlLogical.IdentifierChain(
-            head = transformIdentifier(node.head),
-            qualifier = node.qualifier.map { transformIdentifier(it) }
+            parts = node.parts.map { transformIdentifier(it) },
         )
     }
 
     private fun name(tableName: PartiqlLogical.TableName): SymbolPrimitive {
         val id = tableName.id
-        return id.head.name
+        return id.parts.last().name
     }
 
     private fun transformConflictAction(conflictAction: PartiqlAst.ConflictAction?) =
@@ -529,8 +528,7 @@ internal class AstToLogicalVisitorTransform(
                 dmlTarget(
                     tableName(
                         identifierChain(
-                            head = identifier_(name, transformCaseSensitivity(case), metas),
-                            qualifier = emptyList()
+                            listOf(identifier_(name, transformCaseSensitivity(case), metas)),
                         )
                     )
                 )
@@ -866,8 +864,7 @@ private val INVALID_EXPR = PartiqlLogical.build {
 private val INVALID_DML_TARGET_ID = PartiqlLogical.build {
     tableName(
         identifierChain(
-            head = identifier("this is a placeholder for an invalid DML target - do not run", caseInsensitive()),
-            qualifier = emptyList()
+            listOf(identifier("this is a placeholder for an invalid DML target - do not run", caseInsensitive())),
         )
     )
 }

partiql-lang/src/main/kotlin/org/partiql/lang/planner/transforms/LogicalToLogicalResolvedVisitorTransform.kt

@@ -358,16 +358,14 @@ internal data class LogicalToLogicalResolvedVisitorTransform(
         val tableUniqueId = when (val resolvedVariable = globals.resolveGlobal(bindingId)) {
             is GlobalResolutionResult.GlobalVariable -> resolvedVariable.uniqueId
             is GlobalResolutionResult.NamespacedVariable -> {
-                if (resolvedVariable.getRemainingSteps().isNotEmpty()) {
+                if (resolvedVariable.getRemainingSteps().any()) {
                     problemHandler.handleProblem(
                         Problem(
                             node.metas.sourceLocationMetaOrUnknown.toProblemLocation(),
-                            PlanningProblemDetails.UndefinedDmlTarget(
-                                bindingId,
-                            )
+                            PlanningProblemDetails.UnimplementedFeature("Qualified identifiers as a DML target")
                         )
                     )
-                    "DML Target could not be completely resolved - do not run"
+                    "Undefined DML target: $bindingId - do not run"
                 } else {
                     resolvedVariable.uniqueId
                 }
@@ -389,11 +387,8 @@ internal data class LogicalToLogicalResolvedVisitorTransform(
 
     private fun bindingId(tableName: PartiqlLogical.TableName): BindingId {
         val id = tableName.id
-        val headName = BindingName(id.head.name.text, id.head.case.toBindingCase())
-        return BindingId(
-            id.qualifier.map { BindingName(it.name.text, it.case.toBindingCase()) },
-            headName
-        )
+        val parts = id.parts.map { BindingName(it.name.text, it.case.toBindingCase()) }
+        return BindingId(parts)
     }
 
     override fun transformStatementDmlInsert_onConflict(node: PartiqlLogical.Statement.DmlInsert): PartiqlLogicalResolved.OnConflict? {

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

@@ -137,7 +137,7 @@ class ASTPrettyPrinter {
     private fun toRecursionTree(node: PartiqlAst.IdentifierChain, attrOfParent: String? = null): RecursionTree =
         RecursionTree(
             astType = "IdentifierChain",
-            children = node.qualifier.map { step -> toRecursionTree(step) } + listOf(toRecursionTree(node.head)),
+            children = node.parts.map { step -> toRecursionTree(step) },
             attrOfParent = attrOfParent
         )
 

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

@@ -113,11 +113,13 @@ class QueryPrettyPrinter {
     }
 
     private fun writeAstNode(node: PartiqlAst.IdentifierChain, sb: StringBuilder) {
-        node.qualifier.forEach { step ->
+        val last = node.parts.last()
+        val preLast = node.parts.dropLast(1)
+        preLast.forEach { step ->
             writeAstNode(step, sb)
             sb.append('.')
         }
-        writeAstNode(node.head, sb)
+        writeAstNode(last, sb)
     }
 
     private fun writeAstNode(node: PartiqlAst.Identifier, sb: StringBuilder) {

partiql-lang/src/main/kotlin/org/partiql/lang/syntax/impl/PartiQLPigVisitor.kt

@@ -373,11 +373,10 @@ internal class PartiQLPigVisitor(
     }
 
     override fun visitIdentifierChain(ctx: PartiQLParser.IdentifierChainContext) = PartiqlAst.build {
-        val steps = ctx.idSteps.map { step ->
+        val parts = ctx.idSteps.map { step ->
             visitSymbolPrimitive(step).toIdentifier()
         }
-        val head = steps.last()
-        identifierChain(head, steps.dropLast(1), steps.first().metas)
+        identifierChain(parts, parts.first().metas)
     }
 
     override fun visitReplaceCommand(ctx: PartiQLParser.ReplaceCommandContext) = PartiqlAst.build {

partiql-lang/src/test/kotlin/org/partiql/lang/planner/transforms/AstToLogicalVisitorTransformTests.kt

@@ -1353,8 +1353,7 @@ fun dmlTarget(node: PartiqlLogical.Identifier): PartiqlLogical.DmlTarget {
     return PartiqlLogical.DmlTarget(
         PartiqlLogical.TableName(
             PartiqlLogical.IdentifierChain(
-                head = node,
-                qualifier = emptyList()
+                listOf(node)
             )
         )
     )