Revert "V1 ddl extended keyword (#1455)"

This reverts commit 2879f3a

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

@@ -138,12 +138,6 @@ private class AstTranslator(val metas: Map<String, MetaContainer>) : AstBaseVisi
         }
         val tableName = node.name.symbol
         val def = node.definition?.let { visitTableDefinition(it, ctx) }
-        if (node.partitionBy != null) {
-            error("The legacy AST does not support Partition BY in create Table")
-        }
-        if (node.tableProperties.isNotEmpty()) {
-            error("The legacy AST does not support TBLProperties in create Table")
-        }
         ddl(createTable(tableName, def), metas)
     }
 
@@ -195,12 +189,6 @@ private class AstTranslator(val metas: Map<String, MetaContainer>) : AstBaseVisi
         val name = node.name.symbol
         val type = visitType(node.type, ctx)
         val constraints = node.constraints.translate<PartiqlAst.ColumnConstraint>(ctx)
-        if (node.isOptional) {
-            error("The legacy AST does not support optional field declaration")
-        }
-        if (node.comment != null) {
-            error("The legacy AST does not support comment clause")
-        }
         columnDeclaration(name, type, constraints, metas)
     }
 

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

@@ -133,12 +133,6 @@ ddl_op::[
   create_table::{
     name:       identifier,
     definition: optional::table_definition,
-    // This is an expression for backward compatibility
-    // For now, we support `PARTITION BY (<id> (, <id>)* ))`
-    // But in the future, we might support additional partition expressions:
-    // such as `PARTITION BY RANGE(..)`
-    partition_by: optional::partition_by,
-    table_properties: list::[table_property],
   },
 
   // CREATE INDEX [<identifier>] ON <identifier> (<path> [, <path>]...)
@@ -239,8 +233,6 @@ type::[
         // for struct subfield. But modeling this to be a list of constraints
         // to prevent future breaking changes.
         constraints: list::[constraint],
-        is_optional: bool,
-        comment: optional::string,
       }
     ],
   },                                                           // STRUCT <fields>
@@ -838,9 +830,6 @@ table_definition::{
       name:        '.identifier.symbol',
       type:        '.type',
       constraints: list::[constraint],
-      // TODO: Consider to model this as a constraint?
-      is_optional: bool,
-      comment: optional::string,
     }
   ],
 }
@@ -859,18 +848,6 @@ constraint::{
   ],
 }
 
-partition_by::[
-  attr_list :: {
-    list: list::['.identifier.symbol']
-  },
-  // We can add other commonly support Partition Expression like `range` later
-]
-
-table_property::{
-  name: string,
-  value: '.value',
-}
-
 // SQL-99 Table 11
 datetime_field::[
   YEAR,             // 0001-9999

partiql-ast/src/test/kotlin/org/partiql/ast/helpers/ToLegacyAstTest.kt

@@ -423,9 +423,7 @@ class ToLegacyAstTest {
                     fields += org.partiql.ast.typeStructField(
                         org.partiql.ast.identifierSymbol("a", Identifier.CaseSensitivity.INSENSITIVE),
                         typeInt2(),
-                        emptyList(),
-                        false,
-                        null
+                        emptyList()
                     )
                 }
             },

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

@@ -259,7 +259,6 @@ internal class PartiQLPigVisitor(
             throw ParserException("PIG Parser does not support qualified name as table name", ErrorCode.PARSE_UNEXPECTED_TOKEN)
         }
         val def = ctx.tableDef()?.let { visitTableDef(it) }
-        ctx.tableExtension().map { visit(it) }
         createTable_(name, def, ctx.CREATE().getSourceMetaContainer())
     }
 
@@ -278,12 +277,6 @@ internal class PartiQLPigVisitor(
         val name = visitSymbolPrimitive(ctx.columnName().symbolPrimitive()).name.text
         val type = visit(ctx.type()) as PartiqlAst.Type
         val constrs = ctx.columnConstraint().map { visitColumnConstraint(it) }
-        if (ctx.OPTIONAL() != null) {
-            throw ParserException("PIG Parser does not support OPTIONAL Field", ErrorCode.PARSE_UNEXPECTED_TOKEN)
-        }
-        if (ctx.comment() != null) {
-            throw ParserException("PIG Parser does not support COMMENT Clause", ErrorCode.PARSE_UNEXPECTED_TOKEN)
-        }
         columnDeclaration(name, type, constrs)
     }
 
@@ -301,12 +294,6 @@ internal class PartiQLPigVisitor(
         columnNull()
     }
 
-    override fun visitTableConstrDeclaration(ctx: PartiQLParser.TableConstrDeclarationContext) = throw ParserException("PIG Parser does not support tuple level constraint", ErrorCode.PARSE_UNEXPECTED_TOKEN)
-
-    override fun visitTblExtensionPartition(ctx: PartiQLParser.TblExtensionPartitionContext) = throw ParserException("PIG Parser does not support PARTITION BY Clause", ErrorCode.PARSE_UNEXPECTED_TOKEN)
-
-    override fun visitTblExtensionTblProperties(ctx: PartiQLParser.TblExtensionTblPropertiesContext) = throw ParserException("PIG Parser does not support TBLPROPERTIES Clause", ErrorCode.PARSE_UNEXPECTED_TOKEN)
-
     /**
      *
      * EXECUTE

partiql-lang/src/test/kotlin/org/partiql/lang/syntax/PartiQLParserDDLTest.kt

@@ -96,44 +96,6 @@ internal class PartiQLParserDDLTest : PartiQLParserTestBase() {
                 code = ErrorCode.PARSE_UNEXPECTED_TOKEN,
                 context = mapOf(),
             ),
-            ParserErrorTestCase(
-                description = "PIG Parser does not support OPTIONAL Attribute",
-                query = """
-                    CREATE TABLE tbl (
-                        a OPTIONAL INT2
-                    )
-                """.trimIndent(),
-                code = ErrorCode.PARSE_UNEXPECTED_TOKEN,
-                context = mapOf(),
-            ),
-            ParserErrorTestCase(
-                description = "PIG Parser does not support COMMENT keyword",
-                query = """
-                    CREATE TABLE tbl (
-                        a INT2 COMMENT 'this is a comment'
-                    )
-                """.trimIndent(),
-                code = ErrorCode.PARSE_UNEXPECTED_TOKEN,
-                context = mapOf(),
-            ),
-            ParserErrorTestCase(
-                description = "PIG Parser does not support PARTITION BY keyword",
-                query = """
-                    CREATE TABLE tbl 
-                        PARTITION BY (a, b)
-                """.trimIndent(),
-                code = ErrorCode.PARSE_UNEXPECTED_TOKEN,
-                context = mapOf(),
-            ),
-            ParserErrorTestCase(
-                description = "PIG Parser does not support TBLPROPERTIES keyword",
-                query = """
-                    CREATE TABLE tbl 
-                        TBLPROPERTIES ('k1' = 'v1')
-                """.trimIndent(),
-                code = ErrorCode.PARSE_UNEXPECTED_TOKEN,
-                context = mapOf(),
-            ),
 
             // Putting those tests here are they are impacted by DDL implementation
             ParserErrorTestCase(

partiql-lang/src/test/kotlin/org/partiql/lang/syntax/PartiQLParserMatchTest.kt

@@ -1067,7 +1067,7 @@ class PartiQLParserMatchTest : PartiQLParserTestBase() {
 
     @Test
     fun prefilters() = assertExpression(
-        "SELECT u as banCandidate FROM g MATCH (p:Post Where p.isFlagged = true) <-[:createdPost]- (u:Usr WHERE u.isBanned = false AND u.karma < 20) -[:createdComment]->(c:\"Comment\" WHERE c.isFlagged = true) WHERE p.title LIKE '%considered harmful%'",
+        "SELECT u as banCandidate FROM g MATCH (p:Post Where p.isFlagged = true) <-[:createdPost]- (u:Usr WHERE u.isBanned = false AND u.karma < 20) -[:createdComment]->(c:Comment WHERE c.isFlagged = true) WHERE p.title LIKE '%considered harmful%'",
     ) {
         PartiqlAst.build {
             select(

partiql-parser/src/main/antlr/PartiQLParser.g4

@@ -79,15 +79,14 @@ qualifiedName : (qualifier+=symbolPrimitive PERIOD)* name=symbolPrimitive;
 tableName : symbolPrimitive;
 columnName : symbolPrimitive;
 constraintName : symbolPrimitive;
-comment : COMMENT LITERAL_STRING;
 
 ddl
     : createCommand
     | dropCommand
     ;
 
 createCommand
-    : CREATE TABLE qualifiedName ( PAREN_LEFT tableDef PAREN_RIGHT )? tableExtension*               # CreateTable
+    : CREATE TABLE qualifiedName ( PAREN_LEFT tableDef PAREN_RIGHT )?                           # CreateTable
     | CREATE INDEX ON symbolPrimitive PAREN_LEFT pathSimple ( COMMA pathSimple )* PAREN_RIGHT   # CreateIndex
     ;
 
@@ -101,7 +100,7 @@ tableDef
     ;
 
 tableDefPart
-    : columnName OPTIONAL? type columnConstraint* comment?          # ColumnDeclaration
+    : columnName type columnConstraint*                             # ColumnDeclaration
     | ( CONSTRAINT constraintName )?  tableConstraintDef            # TableConstrDeclaration
     ;
 
@@ -139,20 +138,6 @@ uniqueConstraintDef
 // but we at least can eliminate SFW query here.
 searchCondition : exprOr;
 
-// SQL/HIVE DDL Extension, Support additional table metadatas such as partition by, tblProperties, etc.
-tableExtension
-    : PARTITION BY partitionBy                                                             # TblExtensionPartition
-    | TBLPROPERTIES PAREN_LEFT keyValuePair (COMMA keyValuePair)* PAREN_RIGHT              # TblExtensionTblProperties
-    ;
-
-// Limiting the scope to only allow String as valid value for now
-keyValuePair : key=LITERAL_STRING EQ value=LITERAL_STRING;
-
-// For now: just support a list of column name
-// In the future, we might support common partition expression such as Hash(), Range(), etc.
-partitionBy
-    : PAREN_LEFT columnName (COMMA columnName)* PAREN_RIGHT                   #PartitionColList
-    ;
 /**
  *
  * DATA MANIPULATION LANGUAGE (DML)
@@ -897,5 +882,5 @@ type
     ;
 
 structField
-    : columnName OPTIONAL? COLON type columnConstraint* comment?
+    : columnName COLON type columnConstraint*
     ;

partiql-parser/src/main/antlr/PartiQLTokens.g4

@@ -59,7 +59,6 @@ COALESCE: 'COALESCE';
 COLLATE: 'COLLATE';
 COLLATION: 'COLLATION';
 COLUMN: 'COLUMN';
-COMMENT: 'COMMENT';
 COMMIT: 'COMMIT';
 CONNECT: 'CONNECT';
 CONNECTION: 'CONNECTION';
@@ -176,7 +175,6 @@ ON: 'ON';
 ONLY: 'ONLY';
 OPEN: 'OPEN';
 OPTION: 'OPTION';
-OPTIONAL: 'OPTIONAL';
 OR: 'OR';
 ORDER: 'ORDER';
 OUTER: 'OUTER';
@@ -289,7 +287,6 @@ MODIFIED: 'MODIFIED';
 NEW: 'NEW';
 OLD: 'OLD';
 NOTHING: 'NOTHING';
-TBLPROPERTIES: 'TBLPROPERTIES';
 
 /**
  *

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

@@ -44,7 +44,6 @@ import org.partiql.ast.GroupBy
 import org.partiql.ast.Identifier
 import org.partiql.ast.Let
 import org.partiql.ast.OnConflict
-import org.partiql.ast.PartitionBy
 import org.partiql.ast.Path
 import org.partiql.ast.Returning
 import org.partiql.ast.Select
@@ -141,7 +140,6 @@ import org.partiql.ast.onConflictActionDoUpdate
 import org.partiql.ast.onConflictTargetConstraint
 import org.partiql.ast.onConflictTargetSymbols
 import org.partiql.ast.orderBy
-import org.partiql.ast.partitionByAttrList
 import org.partiql.ast.path
 import org.partiql.ast.pathStepIndex
 import org.partiql.ast.pathStepSymbol
@@ -179,7 +177,6 @@ import org.partiql.ast.statementExplainTargetDomain
 import org.partiql.ast.statementQuery
 import org.partiql.ast.tableDefinition
 import org.partiql.ast.tableDefinitionAttribute
-import org.partiql.ast.tableProperty
 import org.partiql.ast.typeAny
 import org.partiql.ast.typeArray
 import org.partiql.ast.typeBag
@@ -621,26 +618,7 @@ internal class PartiQLParserDefault : PartiQLParser {
         override fun visitCreateTable(ctx: GeneratedParser.CreateTableContext) = translate(ctx) {
             val table = visitQualifiedName(ctx.qualifiedName())
             val definition = ctx.tableDef()?.let { visitTableDef(it) }
-            val partitionBy = ctx
-                .tableExtension()
-                .filterIsInstance<GeneratedParser.TblExtensionPartitionContext>()
-                .let {
-                    if (it.size > 1) throw error(ctx, "Expect one PARTITION BY clause.")
-                    it.firstOrNull()?.let { visitTblExtensionPartition(it) }
-                }
-            val tblProperties = ctx
-                .tableExtension()
-                .filterIsInstance<GeneratedParser.TblExtensionTblPropertiesContext>()
-                .let {
-                    if (it.size > 1) throw error(ctx, "Expect one TBLPROPERTIES clause.")
-                    val tblPropertiesCtx = it.firstOrNull()
-                    tblPropertiesCtx?.keyValuePair()?.map {
-                        val key = it.key.getStringValue()
-                        val value = it.value.getStringValue()
-                        tableProperty(key, stringValue(value))
-                    } ?: emptyList()
-                }
-            ddlOpCreateTable(table, definition, partitionBy, tblProperties)
+            ddlOpCreateTable(table, definition)
         }
 
         override fun visitCreateIndex(ctx: GeneratedParser.CreateIndexContext) = translate(ctx) {
@@ -670,12 +648,7 @@ internal class PartiQLParserDefault : PartiQLParser {
                 isValidTypeDeclarationOrThrow(it, ctx.type())
             }
             val constraints = ctx.columnConstraint().map { visitColumnConstraint(it) }
-            val optional = when (ctx.OPTIONAL()) {
-                null -> false
-                else -> true
-            }
-            val comment = ctx.comment()?.LITERAL_STRING()?.getStringValue()
-            tableDefinitionAttribute(name, type, constraints, optional, comment)
+            tableDefinitionAttribute(name, type, constraints)
         }
 
         /**
@@ -745,13 +718,6 @@ internal class PartiQLParserDefault : PartiQLParser {
             constraint(identifier, body)
         }
 
-        override fun visitTblExtensionPartition(ctx: GeneratedParser.TblExtensionPartitionContext) =
-            ctx.partitionBy().accept(this) as PartitionBy
-
-        override fun visitPartitionColList(ctx: GeneratedParser.PartitionColListContext) = translate(ctx) {
-            partitionByAttrList(ctx.columnName().map { visitSymbolPrimitive(it.symbolPrimitive()) })
-        }
-
         /**
          *
          * EXECUTE
@@ -2262,13 +2228,8 @@ internal class PartiQLParserDefault : PartiQLParser {
                         else -> throw error(it, "Only NULL Constraint and NOT NULL Constraint are allowed in Struct field")
                     }
                 }
-                val optional = when (structFieldCtx.OPTIONAL()) {
-                    null -> false
-                    else -> true
-                }
-                val comment = structFieldCtx.comment()?.LITERAL_STRING()?.getStringValue()
 
-                typeStructField(name, type, constraints, optional, comment)
+                typeStructField(name, type, constraints)
             }
             typeStruct(fields)
         }