Fix ALTER TABLE SETTINGS being parsed per clause

parser/ast.go

@@ -202,6 +202,9 @@ type AlterTable struct {
 	TableIdentifier *TableIdentifier
 	OnCluster       *ClusterClause
 	AlterExprs      []AlterTableClause
+	// Settings holds the query-level SETTINGS clause. ClickHouse only
+	// accepts it once, after the last alter clause.
+	Settings *SettingsClause
 }
 
 func (a *AlterTable) Pos() Pos {
@@ -233,6 +236,11 @@ func (a *AlterTable) Accept(visitor ASTVisitor) error {
 			return err
 		}
 	}
+	if a.Settings != nil {
+		if err := a.Settings.Accept(visitor); err != nil {
+			return err
+		}
+	}
 	return visitor.VisitAlterTable(a)
 }
 
@@ -275,7 +283,6 @@ func (a *AlterTableAttachPartition) Accept(visitor ASTVisitor) error {
 type AlterTableDetachPartition struct {
 	DetachPos Pos
 	Partition *PartitionClause
-	Settings  *SettingsClause
 }
 
 func (a *AlterTableDetachPartition) Pos() Pos {
@@ -296,29 +303,20 @@ func (a *AlterTableDetachPartition) Accept(visitor ASTVisitor) error {
 	if err := a.Partition.Accept(visitor); err != nil {
 		return err
 	}
-	if a.Settings != nil {
-		if err := a.Settings.Accept(visitor); err != nil {
-			return err
-		}
-	}
 	return visitor.VisitAlterTableDetachPartition(a)
 }
 
 type AlterTableDropPartition struct {
 	DropPos     Pos
 	HasDetached bool
 	Partition   *PartitionClause
-	Settings    *SettingsClause
 }
 
 func (a *AlterTableDropPartition) Pos() Pos {
 	return a.DropPos
 }
 
 func (a *AlterTableDropPartition) End() Pos {
-	if a.Settings != nil {
-		return a.Settings.End()
-	}
 	return a.Partition.End()
 }
 
@@ -332,11 +330,6 @@ func (a *AlterTableDropPartition) Accept(visitor ASTVisitor) error {
 	if err := a.Partition.Accept(visitor); err != nil {
 		return err
 	}
-	if a.Settings != nil {
-		if err := a.Settings.Accept(visitor); err != nil {
-			return err
-		}
-	}
 	return visitor.VisitAlterTableDropPartition(a)
 }
 
@@ -444,17 +437,13 @@ type AlterTableAddColumn struct {
 	Column      *ColumnDef
 	IfNotExists bool
 	After       *NestedIdentifier
-	Settings    *SettingsClause
 }
 
 func (a *AlterTableAddColumn) Pos() Pos {
 	return a.AddPos
 }
 
 func (a *AlterTableAddColumn) End() Pos {
-	if a.Settings != nil {
-		return a.Settings.End()
-	}
 	return a.StatementEnd
 }
 
@@ -473,11 +462,6 @@ func (a *AlterTableAddColumn) Accept(visitor ASTVisitor) error {
 			return err
 		}
 	}
-	if a.Settings != nil {
-		if err := a.Settings.Accept(visitor); err != nil {
-			return err
-		}
-	}
 	return visitor.VisitAlterTableAddColumn(a)
 }
 
@@ -488,17 +472,13 @@ type AlterTableAddIndex struct {
 	Index       *TableIndex
 	IfNotExists bool
 	After       *NestedIdentifier
-	Settings    *SettingsClause
 }
 
 func (a *AlterTableAddIndex) Pos() Pos {
 	return a.AddPos
 }
 
 func (a *AlterTableAddIndex) End() Pos {
-	if a.Settings != nil {
-		return a.Settings.End()
-	}
 	return a.StatementEnd
 }
 
@@ -517,11 +497,6 @@ func (a *AlterTableAddIndex) Accept(visitor ASTVisitor) error {
 			return err
 		}
 	}
-	if a.Settings != nil {
-		if err := a.Settings.Accept(visitor); err != nil {
-			return err
-		}
-	}
 	return visitor.VisitAlterTableAddIndex(a)
 }
 

parser/format.go

@@ -230,6 +230,10 @@ func (a *AlterTable) FormatSQL(formatter *Formatter) {
 			formatter.WriteString(",")
 		}
 	}
+	if a.Settings != nil {
+		formatter.Break()
+		formatter.WriteExpr(a.Settings)
+	}
 }
 
 func (a *AlterTableAddColumn) FormatSQL(formatter *Formatter) {
@@ -242,10 +246,6 @@ func (a *AlterTableAddColumn) FormatSQL(formatter *Formatter) {
 		formatter.WriteString(" AFTER ")
 		formatter.WriteExpr(a.After)
 	}
-	if a.Settings != nil {
-		formatter.Break()
-		formatter.WriteExpr(a.Settings)
-	}
 }
 
 func (a *AlterTableAddIndex) FormatSQL(formatter *Formatter) {
@@ -258,10 +258,6 @@ func (a *AlterTableAddIndex) FormatSQL(formatter *Formatter) {
 		formatter.WriteString(" AFTER ")
 		formatter.WriteExpr(a.After)
 	}
-	if a.Settings != nil {
-		formatter.Break()
-		formatter.WriteExpr(a.Settings)
-	}
 }
 
 func (a *AlterTableAddProjection) FormatSQL(formatter *Formatter) {
@@ -337,10 +333,6 @@ func (a *AlterTableDelete) FormatSQL(formatter *Formatter) {
 func (a *AlterTableDetachPartition) FormatSQL(formatter *Formatter) {
 	formatter.WriteString("DETACH ")
 	formatter.WriteExpr(a.Partition)
-	if a.Settings != nil {
-		formatter.Break()
-		formatter.WriteExpr(a.Settings)
-	}
 }
 
 func (a *AlterTableDropColumn) FormatSQL(formatter *Formatter) {
@@ -365,10 +357,6 @@ func (a *AlterTableDropPartition) FormatSQL(formatter *Formatter) {
 		formatter.WriteString("DETACHED ")
 	}
 	formatter.WriteExpr(a.Partition)
-	if a.Settings != nil {
-		formatter.Break()
-		formatter.WriteExpr(a.Settings)
-	}
 }
 
 func (a *AlterTableDropProjection) FormatSQL(formatter *Formatter) {

parser/parser_alter.go

@@ -78,6 +78,15 @@ func (p *Parser) parseAlterTable(pos Pos) (*AlterTable, error) {
 	}
 	alterTable.StatementEnd = alterTable.AlterExprs[len(alterTable.AlterExprs)-1].End()
 
+	settings, err := p.tryParseSettingsClause(p.Pos())
+	if err != nil {
+		return nil, err
+	}
+	if settings != nil {
+		alterTable.Settings = settings
+		alterTable.StatementEnd = settings.End()
+	}
+
 	return alterTable, nil
 }
 
@@ -122,21 +131,12 @@ func (p *Parser) parseAlterTableAddColumn(pos Pos) (*AlterTableAddColumn, error)
 		statementEnd = after.End()
 	}
 
-	settings, err := p.tryParseSettingsClause(p.Pos())
-	if err != nil {
-		return nil, err
-	}
-	if settings != nil {
-		statementEnd = settings.End()
-	}
-
 	return &AlterTableAddColumn{
 		AddPos:       pos,
 		StatementEnd: statementEnd,
 		Column:       column,
 		IfNotExists:  ifNotExists,
 		After:        after,
-		Settings:     settings,
 	}, nil
 }
 
@@ -162,20 +162,12 @@ func (p *Parser) parseAlterTableAddIndex(pos Pos) (*AlterTableAddIndex, error) {
 	if after != nil {
 		statementEnd = after.End()
 	}
-	settings, err := p.tryParseSettingsClause(p.Pos())
-	if err != nil {
-		return nil, err
-	}
-	if settings != nil {
-		statementEnd = settings.End()
-	}
 	return &AlterTableAddIndex{
 		AddPos:       pos,
 		StatementEnd: statementEnd,
 		IfNotExists:  ifNotExists,
 		Index:        index,
 		After:        after,
-		Settings:     settings,
 	}, nil
 }
 
@@ -352,15 +344,9 @@ func (p *Parser) parseAlterTableDetachPartition(pos Pos) (AlterTableClause, erro
 	}
 	partition.Expr = expr
 
-	settings, err := p.tryParseSettingsClause(p.Pos())
-	if err != nil {
-		return nil, err
-	}
-
 	return &AlterTableDetachPartition{
 		DetachPos: pos,
 		Partition: partition,
-		Settings:  settings,
 	}, nil
 }
 
@@ -493,16 +479,10 @@ func (p *Parser) parseAlterTableDropPartition(pos Pos) (AlterTableClause, error)
 	}
 	partition.Expr = expr
 
-	settings, err := p.tryParseSettingsClause(p.Pos())
-	if err != nil {
-		return nil, err
-	}
-
 	return &AlterTableDropPartition{
 		DropPos:     pos,
 		Partition:   partition,
 		HasDetached: hasDetached,
-		Settings:    settings,
 	}, nil
 }
 

parser/position_test.go

@@ -70,14 +70,13 @@ func TestAlterDetachPartitionPos(t *testing.T) {
 	require.Equal(t, Pos(14), detach.Pos())
 }
 
-func TestAlterDropPartitionEndIncludesSettings(t *testing.T) {
+func TestAlterTableEndIncludesSettings(t *testing.T) {
 	sql := "ALTER TABLE t DROP PARTITION p SETTINGS mutations_sync=1"
 	stmt := parseOneStmt(t, sql).(*AlterTable)
-	drop := stmt.AlterExprs[0].(*AlterTableDropPartition)
-	require.NotNil(t, drop.Settings)
-	// End() used to discard the Settings end and stop at the partition
-	require.Equal(t, drop.Settings.End(), drop.End())
-	require.Greater(t, drop.End(), drop.Partition.End())
+	require.NotNil(t, stmt.Settings)
+	// End() used to discard the Settings end and stop at the last alter clause
+	require.Equal(t, stmt.Settings.End(), stmt.End())
+	require.Greater(t, stmt.End(), stmt.AlterExprs[0].End())
 }
 
 func TestDictionaryAttributeEnd(t *testing.T) {

parser/testdata/ddl/alter_table_settings.sql

@@ -0,0 +1,3 @@
+ALTER TABLE test.events_local MODIFY COLUMN f0 Int64 SETTINGS alter_sync = 2;
+ALTER TABLE test.events_local ADD COLUMN f1 String, DROP COLUMN f2 SETTINGS alter_sync = 2;
+ALTER TABLE test.events_local ON CLUSTER 'default_cluster' MATERIALIZE INDEX IF EXISTS created_at_idx SETTINGS mutations_sync = 2;

parser/testdata/ddl/format/alter_table_settings.sql

@@ -0,0 +1,10 @@
+-- Origin SQL:
+ALTER TABLE test.events_local MODIFY COLUMN f0 Int64 SETTINGS alter_sync = 2;
+ALTER TABLE test.events_local ADD COLUMN f1 String, DROP COLUMN f2 SETTINGS alter_sync = 2;
+ALTER TABLE test.events_local ON CLUSTER 'default_cluster' MATERIALIZE INDEX IF EXISTS created_at_idx SETTINGS mutations_sync = 2;
+
+
+-- Format SQL:
+ALTER TABLE test.events_local MODIFY COLUMN f0 Int64 SETTINGS alter_sync=2;
+ALTER TABLE test.events_local ADD COLUMN f1 String, DROP COLUMN f2 SETTINGS alter_sync=2;
+ALTER TABLE test.events_local ON CLUSTER 'default_cluster' MATERIALIZE INDEX IF EXISTS created_at_idx SETTINGS mutations_sync=2;

parser/testdata/ddl/format/beautify/alter_table_settings.sql

@@ -0,0 +1,21 @@
+-- Origin SQL:
+ALTER TABLE test.events_local MODIFY COLUMN f0 Int64 SETTINGS alter_sync = 2;
+ALTER TABLE test.events_local ADD COLUMN f1 String, DROP COLUMN f2 SETTINGS alter_sync = 2;
+ALTER TABLE test.events_local ON CLUSTER 'default_cluster' MATERIALIZE INDEX IF EXISTS created_at_idx SETTINGS mutations_sync = 2;
+
+
+-- Beautify SQL:
+ALTER TABLE test.events_local
+MODIFY COLUMN f0 Int64
+SETTINGS
+  alter_sync=2;
+ALTER TABLE test.events_local
+ADD COLUMN f1 String,
+DROP COLUMN f2
+SETTINGS
+  alter_sync=2;
+ALTER TABLE test.events_local
+ON CLUSTER 'default_cluster'
+MATERIALIZE INDEX IF EXISTS created_at_idx
+SETTINGS
+  mutations_sync=2;

parser/testdata/ddl/output/alter_table_add_column.sql.golden.json

@@ -27,7 +27,7 @@
     "AlterExprs": [
       {
         "AddPos": 59,
-        "StatementEnd": 112,
+        "StatementEnd": 88,
         "Column": {
           "NamePos": 70,
           "ColumnEnd": 79,
@@ -67,29 +67,29 @@
             "NameEnd": 88
           },
           "DotIdent": null
-        },
-        "Settings": {
-          "SettingsPos": 89,
-          "ListEnd": 112,
-          "Items": [
-            {
-              "SettingsPos": 98,
-              "Name": {
-                "Name": "alter_sync",
-                "QuoteType": 1,
-                "NamePos": 98,
-                "NameEnd": 108
-              },
-              "Expr": {
-                "NumPos": 111,
-                "NumEnd": 112,
-                "Literal": "2",
-                "Base": 10
-              }
-            }
-          ]
         }
       }
-    ]
+    ],
+    "Settings": {
+      "SettingsPos": 89,
+      "ListEnd": 112,
+      "Items": [
+        {
+          "SettingsPos": 98,
+          "Name": {
+            "Name": "alter_sync",
+            "QuoteType": 1,
+            "NamePos": 98,
+            "NameEnd": 108
+          },
+          "Expr": {
+            "NumPos": 111,
+            "NumEnd": 112,
+            "Literal": "2",
+            "Base": 10
+          }
+        }
+      ]
+    }
   }
 ]