Repository: cloudspannerecosystem/memefish
Pull request #149 (2024-10-19 14:00:54)

Fix some `Pos` and `End` issues on `TableConstraint`
- In `TableConstraint` definition, comments about `pos` and `end` were missing. This PR adds them.
- When `ConstraintPos` is missing, it sets `InvalidPos` explicitly.
- Also, that `ForeignKey` and `Check` set `expect(")").End` to `Rparen` on parsing seems incorrect, and `expect(")").Pos` is correct.

Note that `expect(")").End` also appears in `parseColumnDefOptions`, but it will be removed in #142.

makenowjust: Thank you @apstndb.

Patch:
diff --git a/ast/ast.go b/ast/ast.go
index ba0da57..e11ace4 100644
--- a/ast/ast.go
+++ b/ast/ast.go
@@ -1606,6 +1606,9 @@ type ColumnDefOptions struct {
 //
 //	{{if .Name}}CONSTRAINT {{.Name}}{{end}}{{.Constraint | sql}}
 type TableConstraint struct {
+	// pos = ConstraintPos || Constraint.pos
+	// end = Constraint.end
+
 	ConstraintPos token.Pos // position of "CONSTRAINT" keyword when Name presents
 
 	Name       *Ident // optional
diff --git a/parser.go b/parser.go
index 24ad76a..8242f81 100644
--- a/parser.go
+++ b/parser.go
@@ -2151,10 +2151,16 @@ func (p *Parser) parseCreateTable(pos token.Pos) *ast.CreateTable {
 			constraints = append(constraints, p.parseConstraint())
 		case p.Token.IsKeywordLike("FOREIGN"):
 			fk := p.parseForeignKey()
-			constraints = append(constraints, &ast.TableConstraint{Constraint: fk})
+			constraints = append(constraints, &ast.TableConstraint{
+				ConstraintPos: token.InvalidPos,
+				Constraint:    fk,
+			})
 		case p.Token.IsKeywordLike("CHECK"):
 			c := p.parseCheck()
-			constraints = append(constraints, &ast.TableConstraint{Constraint: c})
+			constraints = append(constraints, &ast.TableConstraint{
+				ConstraintPos: token.InvalidPos,
+				Constraint:    c,
+			})
 		default:
 			columns = append(columns, p.parseColumnDef())
 		}
@@ -2339,7 +2345,7 @@ func (p *Parser) parseForeignKey() *ast.ForeignKey {
 
 	p.expect("(")
 	refColumns := parseCommaSeparatedList(p, p.parseIdent)
-	rparen := p.expect(")").End
+	rparen := p.expect(")").Pos
 
 	onDelete, onDeleteEnd := p.tryParseOnDeleteAction()
 
@@ -2358,7 +2364,7 @@ func (p *Parser) parseCheck() *ast.Check {
 	pos := p.expectKeywordLike("CHECK").Pos
 	p.expect("(")
 	expr := p.parseExpr()
-	rparen := p.expect(")").End
+	rparen := p.expect(")").Pos
 	return &ast.Check{
 		Check:  pos,
 		Rparen: rparen,


Test patch:
diff --git a/testdata/result/ddl/alter_table_add_check.sql.txt b/testdata/result/ddl/alter_table_add_check.sql.txt
index 24cf9b7..5ecdaf7 100644
--- a/testdata/result/ddl/alter_table_add_check.sql.txt
+++ b/testdata/result/ddl/alter_table_add_check.sql.txt
@@ -16,7 +16,7 @@ alter table foo add check (c1 > 0)
       Name:          (*ast.Ident)(nil),
       Constraint:    &ast.Check{
         Check:  20,
-        Rparen: 34,
+        Rparen: 33,
         Expr:   &ast.BinaryExpr{
           Op:   ">",
           Left: &ast.Ident{
diff --git a/testdata/result/ddl/alter_table_add_constraint_check.sql.txt b/testdata/result/ddl/alter_table_add_constraint_check.sql.txt
index 8da9846..187b07e 100644
--- a/testdata/result/ddl/alter_table_add_constraint_check.sql.txt
+++ b/testdata/result/ddl/alter_table_add_constraint_check.sql.txt
@@ -20,7 +20,7 @@ alter table foo add constraint cname check (c1 > 0)
       },
       Constraint: &ast.Check{
         Check:  37,
-        Rparen: 51,
+        Rparen: 50,
         Expr:   &ast.BinaryExpr{
           Op:   ">",
           Left: &ast.Ident{
diff --git a/testdata/result/ddl/alter_table_add_constraint_foreign_key.sql.txt b/testdata/result/ddl/alter_table_add_constraint_foreign_key.sql.txt
index 7375568..6c0720c 100644
--- a/testdata/result/ddl/alter_table_add_constraint_foreign_key.sql.txt
+++ b/testdata/result/ddl/alter_table_add_constraint_foreign_key.sql.txt
@@ -20,7 +20,7 @@ alter table foo add constraint fkname foreign key (foo, bar) references t2 (t2ke
       },
       Constraint: &ast.ForeignKey{
         Foreign:     38,
-        Rparen:      91,
+        Rparen:      90,
         OnDeleteEnd: -1,
         Columns:     []*ast.Ident{
           &ast.Ident{
diff --git a/testdata/result/ddl/alter_table_add_constraint_foreign_key_on_delete_cascade.sql.txt b/testdata/result/ddl/alter_table_add_constraint_foreign_key_on_delete_cascade.sql.txt
index 77fac40..ebb2d03 100644
--- a/testdata/result/ddl/alter_table_add_constraint_foreign_key_on_delete_cascade.sql.txt
+++ b/testdata/result/ddl/alter_table_add_constraint_foreign_key_on_delete_cascade.sql.txt
@@ -20,7 +20,7 @@ alter table foo add constraint fkname foreign key (foo, bar) references t2 (t2ke
       },
       Constraint: &ast.ForeignKey{
         Foreign:     38,
-        Rparen:      91,
+        Rparen:      90,
         OnDeleteEnd: 109,
         Columns:     []*ast.Ident{
           &ast.Ident{
diff --git a/testdata/result/ddl/alter_table_add_constraint_foreign_key_on_delete_no_action.sql.txt b/testdata/result/ddl/alter_table_add_constraint_foreign_key_on_delete_no_action.sql.txt
index 6a91159..e7e7251 100644
--- a/testdata/result/ddl/alter_table_add_constraint_foreign_key_on_delete_no_action.sql.txt
+++ b/testdata/result/ddl/alter_table_add_constraint_foreign_key_on_delete_no_action.sql.txt
@@ -20,7 +20,7 @@ alter table foo add constraint fkname foreign key (foo, bar) references t2 (t2ke
       },
       Constraint: &ast.ForeignKey{
         Foreign:     38,
-        Rparen:      91,
+        Rparen:      90,
         OnDeleteEnd: 111,
         Columns:     []*ast.Ident{
           &ast.Ident{
diff --git a/testdata/result/ddl/alter_table_add_foreign_key.sql.txt b/testdata/result/ddl/alter_table_add_foreign_key.sql.txt
index 6f40ab6..1c01e2f 100644
--- a/testdata/result/ddl/alter_table_add_foreign_key.sql.txt
+++ b/testdata/result/ddl/alter_table_add_foreign_key.sql.txt
@@ -16,7 +16,7 @@ alter table foo add foreign key (bar) references t2 (t2key1)
       Name:          (*ast.Ident)(nil),
       Constraint:    &ast.ForeignKey{
         Foreign:     20,
-        Rparen:      60,
+        Rparen:      59,
         OnDeleteEnd: -1,
         Columns:     []*ast.Ident{
           &ast.Ident{
diff --git a/testdata/result/ddl/create_table.sql.txt b/testdata/result/ddl/create_table.sql.txt
index 09e7fed..e263d4b 100644
--- a/testdata/result/ddl/create_table.sql.txt
+++ b/testdata/result/ddl/create_table.sql.txt
@@ -187,11 +187,11 @@ create table foo (
   },
   TableConstraints: []*ast.TableConstraint{
     &ast.TableConstraint{
-      ConstraintPos: 0,
+      ConstraintPos: -1,
       Name:          (*ast.Ident)(nil),
       Constraint:    &ast.ForeignKey{
         Foreign:     182,
-        Rparen:      222,
+        Rparen:      221,
         OnDeleteEnd: -1,
         Columns:     []*ast.Ident{
           &ast.Ident{
@@ -216,11 +216,11 @@ create table foo (
       },
     },
     &ast.TableConstraint{
-      ConstraintPos: 0,
+      ConstraintPos: -1,
       Name:          (*ast.Ident)(nil),
       Constraint:    &ast.ForeignKey{
         Foreign:     226,
-        Rparen:      266,
+        Rparen:      265,
         OnDeleteEnd: 284,
         Columns:     []*ast.Ident{
           &ast.Ident{
@@ -245,11 +245,11 @@ create table foo (
       },
     },
     &ast.TableConstraint{
-      ConstraintPos: 0,
+      ConstraintPos: -1,
       Name:          (*ast.Ident)(nil),
       Constraint:    &ast.ForeignKey{
         Foreign:     288,
-        Rparen:      328,
+        Rparen:      327,
         OnDeleteEnd: 348,
         Columns:     []*ast.Ident{
           &ast.Ident{
@@ -282,7 +282,7 @@ create table foo (
       },
       Constraint: &ast.ForeignKey{
         Foreign:     370,
-        Rparen:      423,
+        Rparen:      422,
         OnDeleteEnd: -1,
         Columns:     []*ast.Ident{
           &ast.Ident{
@@ -317,11 +317,11 @@ create table foo (
       },
     },
     &ast.TableConstraint{
-      ConstraintPos: 0,
+      ConstraintPos: -1,
       Name:          (*ast.Ident)(nil),
       Constraint:    &ast.Check{
         Check:  427,
-        Rparen: 442,
+        Rparen: 441,
         Expr:   &ast.BinaryExpr{
           Op:   ">",
           Left: &ast.Ident{
@@ -347,7 +347,7 @@ create table foo (
       },
       Constraint: &ast.Check{
         Check:  463,
-        Rparen: 478,
+        Rparen: 477,
         Expr:   &ast.BinaryExpr{
           Op:   ">",
           Left: &ast.Ident{
diff --git a/testdata/result/statement/alter_table_add_check.sql.txt b/testdata/result/statement/alter_table_add_check.sql.txt
index 24cf9b7..5ecdaf7 100644
--- a/testdata/result/statement/alter_table_add_check.sql.txt
+++ b/testdata/result/statement/alter_table_add_check.sql.txt
@@ -16,7 +16,7 @@ alter table foo add check (c1 > 0)
       Name:          (*ast.Ident)(nil),
       Constraint:    &ast.Check{
         Check:  20,
-        Rparen: 34,
+        Rparen: 33,
         Expr:   &ast.BinaryExpr{
           Op:   ">",
           Left: &ast.Ident{
diff --git a/testdata/result/statement/alter_table_add_constraint_check.sql.txt b/testdata/result/statement/alter_table_add_constraint_check.sql.txt
index 8da9846..187b07e 100644
--- a/testdata/result/statement/alter_table_add_constraint_check.sql.txt
+++ b/testdata/result/statement/alter_table_add_constraint_check.sql.txt
@@ -20,7 +20,7 @@ alter table foo add constraint cname check (c1 > 0)
       },
       Constraint: &ast.Check{
         Check:  37,
-        Rparen: 51,
+        Rparen: 50,
         Expr:   &ast.BinaryExpr{
           Op:   ">",
           Left: &ast.Ident{
diff --git a/testdata/result/statement/alter_table_add_constraint_foreign_key.sql.txt b/testdata/result/statement/alter_table_add_constraint_foreign_key.sql.txt
index 7375568..6c0720c 100644
--- a/testdata/result/statement/alter_table_add_constraint_foreign_key.sql.txt
+++ b/testdata/result/statement/alter_table_add_constraint_foreign_key.sql.txt
@@ -20,7 +20,7 @@ alter table foo add constraint fkname foreign key (foo, bar) references t2 (t2ke
       },
       Constraint: &ast.ForeignKey{
         Foreign:     38,
-        Rparen:      91,
+        Rparen:      90,
         OnDeleteEnd: -1,
         Columns:     []*ast.Ident{
           &ast.Ident{
diff --git a/testdata/result/statement/alter_table_add_constraint_foreign_key_on_delete_cascade.sql.txt b/testdata/result/statement/alter_table_add_constraint_foreign_key_on_delete_cascade.sql.txt
index 77fac40..ebb2d03 100644
--- a/testdata/result/statement/alter_table_add_constraint_foreign_key_on_delete_cascade.sql.txt
+++ b/testdata/result/statement/alter_table_add_constraint_foreign_key_on_delete_cascade.sql.txt
@@ -20,7 +20,7 @@ alter table foo add constraint fkname foreign key (foo, bar) references t2 (t2ke
       },
       Constraint: &ast.ForeignKey{
         Foreign:     38,
-        Rparen:      91,
+        Rparen:      90,
         OnDeleteEnd: 109,
         Columns:     []*ast.Ident{
           &ast.Ident{
diff --git a/testdata/result/statement/alter_table_add_constraint_foreign_key_on_delete_no_action.sql.txt b/testdata/result/statement/alter_table_add_constraint_foreign_key_on_delete_no_action.sql.txt
index 6a91159..e7e7251 100644
--- a/testdata/result/statement/alter_table_add_constraint_foreign_key_on_delete_no_action.sql.txt
+++ b/testdata/result/statement/alter_table_add_constraint_foreign_key_on_delete_no_action.sql.txt
@@ -20,7 +20,7 @@ alter table foo add constraint fkname foreign key (foo, bar) references t2 (t2ke
       },
       Constraint: &ast.ForeignKey{
         Foreign:     38,
-        Rparen:      91,
+        Rparen:      90,
         OnDeleteEnd: 111,
         Columns:     []*ast.Ident{
           &ast.Ident{
diff --git a/testdata/result/statement/alter_table_add_foreign_key.sql.txt b/testdata/result/statement/alter_table_add_foreign_key.sql.txt
index 6f40ab6..1c01e2f 100644
--- a/testdata/result/statement/alter_table_add_foreign_key.sql.txt
+++ b/testdata/result/statement/alter_table_add_foreign_key.sql.txt
@@ -16,7 +16,7 @@ alter table foo add foreign key (bar) references t2 (t2key1)
       Name:          (*ast.Ident)(nil),
       Constraint:    &ast.ForeignKey{
         Foreign:     20,
-        Rparen:      60,
+        Rparen:      59,
         OnDeleteEnd: -1,
         Columns:     []*ast.Ident{
           &ast.Ident{
diff --git a/testdata/result/statement/create_table.sql.txt b/testdata/result/statement/create_table.sql.txt
index 09e7fed..e263d4b 100644
--- a/testdata/result/statement/create_table.sql.txt
+++ b/testdata/result/statement/create_table.sql.txt
@@ -187,11 +187,11 @@ create table foo (
   },
   TableConstraints: []*ast.TableConstraint{
     &ast.TableConstraint{
-      ConstraintPos: 0,
+      ConstraintPos: -1,
       Name:          (*ast.Ident)(nil),
       Constraint:    &ast.ForeignKey{
         Foreign:     182,
-        Rparen:      222,
+        Rparen:      221,
         OnDeleteEnd: -1,
         Columns:     []*ast.Ident{
           &ast.Ident{
@@ -216,11 +216,11 @@ create table foo (
       },
     },
     &ast.TableConstraint{
-      ConstraintPos: 0,
+      ConstraintPos: -1,
       Name:          (*ast.Ident)(nil),
       Constraint:    &ast.ForeignKey{
         Foreign:     226,
-        Rparen:      266,
+        Rparen:      265,
         OnDeleteEnd: 284,
         Columns:     []*ast.Ident{
           &ast.Ident{
@@ -245,11 +245,11 @@ create table foo (
       },
     },
     &ast.TableConstraint{
-      ConstraintPos: 0,
+      ConstraintPos: -1,
       Name:          (*ast.Ident)(nil),
       Constraint:    &ast.ForeignKey{
         Foreign:     288,
-        Rparen:      328,
+        Rparen:      327,
         OnDeleteEnd: 348,
         Columns:     []*ast.Ident{
           &ast.Ident{
@@ -282,7 +282,7 @@ create table foo (
       },
       Constraint: &ast.ForeignKey{
         Foreign:     370,
-        Rparen:      423,
+        Rparen:      422,
         OnDeleteEnd: -1,
         Columns:     []*ast.Ident{
           &ast.Ident{
@@ -317,11 +317,11 @@ create table foo (
       },
     },
     &ast.TableConstraint{
-      ConstraintPos: 0,
+      ConstraintPos: -1,
       Name:          (*ast.Ident)(nil),
       Constraint:    &ast.Check{
         Check:  427,
-        Rparen: 442,
+        Rparen: 441,
         Expr:   &ast.BinaryExpr{
           Op:   ">",
           Left: &ast.Ident{
@@ -347,7 +347,7 @@ create table foo (
       },
       Constraint: &ast.Check{
         Check:  463,
-        Rparen: 478,
+        Rparen: 477,
         Expr:   &ast.BinaryExpr{
           Op:   ">",
           Left: &ast.Ident{