parser: alter table actions more robust (#549)

Author: sbdchd

crates/squawk_parser/src/grammar.rs

@@ -6024,21 +6024,7 @@ fn alter_materialized_view(p: &mut Parser<'_>) -> CompletedMarker {
                 name_ref(p);
             }
             ALTER_KW | CLUSTER_KW | SET_KW | RESET_KW | OWNER_KW => {
-                // TODO: we should be robust to missing commas
-                while !p.at(EOF) {
-                    let action = p.start();
-                    match alter_table_action(p) {
-                        Some(action_kind) => {
-                            action.complete(p, action_kind);
-                        }
-                        None => {
-                            action.abandon(p);
-                        }
-                    };
-                    if !p.eat(COMMA) {
-                        break;
-                    }
-                }
+                opt_alter_table_action_list(p);
             }
             _ => {
                 p.error("Expected RENAME, SET SCHEMA, [NO] DEPENDS, or action (ALTER, CLUSTER, SET, RESET, OWNER)");
@@ -6048,6 +6034,24 @@ fn alter_materialized_view(p: &mut Parser<'_>) -> CompletedMarker {
     m.complete(p, ALTER_MATERIALIZED_VIEW)
 }
 
+fn opt_alter_table_action_list(p: &mut Parser<'_>) {
+    while !p.at(EOF) {
+        let m = p.start();
+        let Some(kind) = opt_alter_table_action(p) else {
+            m.abandon(p);
+            break;
+        };
+        m.complete(p, kind);
+        if !p.eat(COMMA) {
+            if p.at_ts(ALTER_TABLE_ACTION_FIRST) {
+                p.error("missing comma");
+            } else {
+                break;
+            }
+        }
+    }
+}
+
 // ALTER LARGE OBJECT large_object_oid OWNER TO { new_owner | CURRENT_ROLE | CURRENT_USER | SESSION_USER }
 fn alter_large_object(p: &mut Parser<'_>) -> CompletedMarker {
     assert!(p.at(ALTER_KW) && p.nth_at(1, LARGE_KW) && p.nth_at(2, OBJECT_KW));
@@ -6352,21 +6356,7 @@ fn alter_foreign_table(p: &mut Parser<'_>) -> CompletedMarker {
             name_ref(p);
         }
         _ => {
-            // TODO: we should be robust to missing commas
-            while !p.at(EOF) {
-                let action = p.start();
-                match alter_table_action(p) {
-                    Some(action_kind) => {
-                        action.complete(p, action_kind);
-                    }
-                    None => {
-                        action.abandon(p);
-                    }
-                };
-                if !p.eat(COMMA) {
-                    break;
-                }
-            }
+            opt_alter_table_action_list(p);
         }
     }
     m.complete(p, ALTER_FOREIGN_TABLE)
@@ -13278,25 +13268,37 @@ fn alter_table(p: &mut Parser<'_>) -> CompletedMarker {
             }
         }
     }
-    // TODO: we should be robust to missing commas
-    while !p.at(EOF) {
-        let action = p.start();
-        match alter_table_action(p) {
-            Some(action_kind) => {
-                action.complete(p, action_kind);
-            }
-            None => {
-                action.abandon(p);
-            }
-        };
-        if !p.eat(COMMA) {
-            break;
-        }
-    }
+    opt_alter_table_action_list(p);
     m.complete(p, ALTER_TABLE)
 }
 
-fn alter_table_action(p: &mut Parser<'_>) -> Option<SyntaxKind> {
+const ALTER_TABLE_ACTION_FIRST: TokenSet = TokenSet::new(&[
+    VALIDATE_KW,
+    REPLICA_KW,
+    OF_KW,
+    NOT_KW,
+    FORCE_KW,
+    NO_KW,
+    INHERIT_KW,
+    ENABLE_KW,
+    DISABLE_KW,
+    CLUSTER_KW,
+    OWNER_KW,
+    DETACH_KW,
+    DROP_KW,
+    ADD_KW,
+    ATTACH_KW,
+    SET_KW,
+    RESET_KW,
+    RENAME_KW,
+    ALTER_KW,
+    OPTIONS_KW,
+]);
+
+fn opt_alter_table_action(p: &mut Parser<'_>) -> Option<SyntaxKind> {
+    if !p.at_ts(ALTER_TABLE_ACTION_FIRST) {
+        return None;
+    }
     let kind = match p.current() {
         // VALIDATE CONSTRAINT constraint_name
         VALIDATE_KW => {

crates/squawk_parser/tests/data/err/alter_table.sql

@@ -4,5 +4,9 @@ add column foo boolean;
 -- mismatch options
 alter table t alter constraint c not deferrable initially deferred;
 
+alter table t 
+validate constraint foo validate constraint b ;
+--                     ^ missing comma
+
 -- pg 18 only, via: https://www.depesz.com/2025/05/01/waiting-for-postgresql-18-allow-not-null-constraints-to-be-added-as-not-valid/
 alter table public.copy_2 add constraint id_not_null not null id not valid;

crates/squawk_parser/tests/snapshots/tests__alter_table_err.snap

@@ -50,6 +50,37 @@ SOURCE_FILE
         DEFERRED_KW "deferred"
   SEMICOLON ";"
   WHITESPACE "\n\n"
+  ALTER_TABLE
+    ALTER_KW "alter"
+    WHITESPACE " "
+    TABLE_KW "table"
+    WHITESPACE " "
+    RELATION_NAME
+      PATH
+        PATH_SEGMENT
+          NAME_REF
+            IDENT "t"
+    WHITESPACE " \n"
+    VALIDATE_CONSTRAINT
+      VALIDATE_KW "validate"
+      WHITESPACE " "
+      CONSTRAINT_KW "constraint"
+      WHITESPACE " "
+      NAME_REF
+        IDENT "foo"
+    WHITESPACE " "
+    VALIDATE_CONSTRAINT
+      VALIDATE_KW "validate"
+      WHITESPACE " "
+      CONSTRAINT_KW "constraint"
+      WHITESPACE " "
+      NAME_REF
+        IDENT "b"
+  WHITESPACE " "
+  SEMICOLON ";"
+  WHITESPACE "\n"
+  COMMENT "--                     ^ missing comma"
+  WHITESPACE "\n\n"
   COMMENT "-- pg 18 only, via: https://www.depesz.com/2025/05/01/waiting-for-postgresql-18-allow-not-null-constraints-to-be-added-as-not-valid/"
   WHITESPACE "\n"
   ALTER_TABLE
@@ -95,3 +126,4 @@ ERROR@23: expected command, found ADD_KW
 ERROR@27: expected command, found COLUMN_KW
 ERROR@34: expected command, found IDENT
 ERROR@38: expected command, found BOOLEAN_KW
+ERROR@175: missing comma