Repository: genjidb/genji (go)

Issue:
Support primary key specified after the fields
The following new test should probably pass (PRIMARY KEY after the fields):

```
ldiff --git a/sql/query/create_test.go b/sql/query/create_test.go
index 54c09e2..55520a2 100644
--- a/sql/query/create_test.go
+++ b/sql/query/create_test.go
@@ -28,6 +28,7 @@ func TestCreateTable(t *testing.T) {
 		{"If not exists, twice", "CREATE TABLE IF NOT EXISTS test;CREATE TABLE IF NOT EXISTS test", false},
 		{"With primary key", "CREATE TABLE test(foo TEXT PRIMARY KEY)", false},
 		{"With field constraints", "CREATE TABLE test(foo.a[1][2] TEXT primary key, bar[4][0].bat INTEGER not null, baz not null)", false},
+		{"With primary key after", "CREATE TABLE test(a TEXT, PRIMARY KEY(`id`));", false},
 		{"With no constraints", "CREATE TABLE test(a, b)", false},
 		{"With coherent constraint(common)", "CREATE TABLE test(a DOCUMENT, a.b ARRAY, a.b[0] TEXT);", false},
 		{"With coherent constraint(any)", "CREATE TABLE test(a, a.b[0] TEXT);", false},

```


Pull request:
Add PRIMARY KEY table constraint
This adds support for the `PRIMARY KEY` table constraint:

```sql
CREATE TABLE foo(a INT, PRIMARY KEY(b))
```

Fixes #333 

Patch:
diff --git a/database/constraint.go b/database/constraint.go
index a45aea10..2ef05884 100644
--- a/database/constraint.go
+++ b/database/constraint.go
@@ -188,6 +188,17 @@ func NewFieldConstraints(userConstraints []*FieldConstraint) (FieldConstraints,
 	return FieldConstraints(userConstraints).Infer()
 }
 
+// Get a field constraint by path. Returns nil if not found.
+func (f FieldConstraints) Get(path document.Path) *FieldConstraint {
+	for _, fc := range f {
+		if fc.Path.IsEqual(path) {
+			return fc
+		}
+	}
+
+	return nil
+}
+
 // Infer additional constraints based on user defined ones.
 // For example, given the following table:
 //   CREATE TABLE foo (a.b[0] TEXT)
diff --git a/sql/parser/create.go b/sql/parser/create.go
index 0cdf4b5e..cdfd0779 100644
--- a/sql/parser/create.go
+++ b/sql/parser/create.go
@@ -2,6 +2,7 @@ package parser
 
 import (
 	"github.com/genjidb/genji/database"
+	"github.com/genjidb/genji/document"
 	"github.com/genjidb/genji/expr"
 	"github.com/genjidb/genji/query"
 	"github.com/genjidb/genji/sql/scanner"
@@ -47,7 +48,7 @@ func (p *Parser) parseCreateTableStatement() (query.CreateTableStmt, error) {
 	}
 
 	// parse field constraints
-	err = p.parseFieldConstraints(&stmt.Info)
+	err = p.parseConstraints(&stmt)
 	if err != nil {
 		return stmt, err
 	}
@@ -99,25 +100,64 @@ func (p *Parser) parseFieldDefinition(fc *database.FieldConstraint) (err error)
 	return nil
 }
 
-func (p *Parser) parseFieldConstraints(info *database.TableInfo) error {
+func (p *Parser) parseConstraints(stmt *query.CreateTableStmt) error {
 	// Parse ( token.
 	if tok, _, _ := p.ScanIgnoreWhitespace(); tok != scanner.LPAREN {
 		p.Unscan()
 		return nil
 	}
 
-	var err error
+	// if set to true, the parser must no longer
+	// expect field definitions, but only table constraints.
+	var parsingTableConstraints bool
 
 	// Parse constraints.
 	for {
-		var fc database.FieldConstraint
-
-		err = p.parseFieldDefinition(&fc)
+		// we start by checking if it is a table constraint,
+		// as it's easier to determine
+		tc, err := p.parseTableConstraint()
 		if err != nil {
 			return err
 		}
 
-		info.FieldConstraints = append(info.FieldConstraints, &fc)
+		// no table constraint found
+		if tc == nil && parsingTableConstraints {
+			tok, pos, lit := p.ScanIgnoreWhitespace()
+			return newParseError(scanner.Tokstr(tok, lit), []string{"CONSTRAINT", ")"}, pos)
+		}
+
+		// only PRIMARY KEY(path) is currently supported.
+		if tc != nil {
+			parsingTableConstraints = true
+
+			if pk := stmt.Info.GetPrimaryKey(); pk != nil {
+				return stringutil.Errorf("table %q has more than one primary key", stmt.TableName)
+			}
+			fc := stmt.Info.FieldConstraints.Get(tc.primaryKey)
+			if fc == nil {
+				err = stmt.Info.FieldConstraints.Add(&database.FieldConstraint{
+					Path:         tc.primaryKey,
+					IsPrimaryKey: true,
+				})
+				if err != nil {
+					return err
+				}
+			} else {
+				fc.IsPrimaryKey = true
+			}
+		}
+
+		// if set to false, we are still parsing field definitions
+		if !parsingTableConstraints {
+			var fc database.FieldConstraint
+
+			err = p.parseFieldDefinition(&fc)
+			if err != nil {
+				return err
+			}
+
+			stmt.Info.FieldConstraints = append(stmt.Info.FieldConstraints, &fc)
+		}
 
 		if tok, _, _ := p.ScanIgnoreWhitespace(); tok != scanner.COMMA {
 			p.Unscan()
@@ -131,15 +171,16 @@ func (p *Parser) parseFieldConstraints(info *database.TableInfo) error {
 	}
 
 	// ensure only one primary key
-	var pkCount int
-	for _, fc := range info.FieldConstraints {
+	var pkFound bool
+	for _, fc := range stmt.Info.FieldConstraints {
 		if fc.IsPrimaryKey {
-			pkCount++
+			if pkFound {
+				return stringutil.Errorf("table %q has more than one primary key", stmt.TableName)
+			}
+
+			pkFound = true
 		}
 	}
-	if pkCount > 1 {
-		return &ParseError{Message: stringutil.Sprintf("only one primary key is allowed, got %d", pkCount)}
-	}
 
 	return nil
 }
@@ -197,6 +238,40 @@ func (p *Parser) parseFieldConstraint(fc *database.FieldConstraint) error {
 	}
 }
 
+func (p *Parser) parseTableConstraint() (*tableConstraint, error) {
+	var tc tableConstraint
+	var err error
+
+	tok, _, _ := p.ScanIgnoreWhitespace()
+	switch tok {
+	case scanner.PRIMARY:
+		// Parse "KEY"
+		if tok, pos, lit := p.ScanIgnoreWhitespace(); tok != scanner.KEY {
+			return nil, newParseError(scanner.Tokstr(tok, lit), []string{"KEY"}, pos)
+		}
+
+		// Parse "("
+		if tok, pos, lit := p.ScanIgnoreWhitespace(); tok != scanner.LPAREN {
+			return nil, newParseError(scanner.Tokstr(tok, lit), []string{"("}, pos)
+		}
+
+		tc.primaryKey, err = p.parsePath()
+		if err != nil {
+			return nil, err
+		}
+
+		// Parse ")"
+		if tok, pos, lit := p.ScanIgnoreWhitespace(); tok != scanner.RPAREN {
+			return nil, newParseError(scanner.Tokstr(tok, lit), []string{")"}, pos)
+		}
+
+		return &tc, nil
+	default:
+		p.Unscan()
+		return nil, nil
+	}
+}
+
 // parseCreateIndexStatement parses a create index string and returns a Statement AST object.
 // This function assumes the CREATE INDEX or CREATE UNIQUE INDEX tokens have already been consumed.
 func (p *Parser) parseCreateIndexStatement(unique bool) (query.CreateIndexStmt, error) {
@@ -256,3 +331,7 @@ func (p *Parser) parseCreateIndexStatement(unique bool) (query.CreateIndexStmt,
 
 	return stmt, nil
 }
+
+type tableConstraint struct {
+	primaryKey document.Path
+}


Test patch:
diff --git a/sql/parser/create_test.go b/sql/parser/create_test.go
index e8f7e69e..d2ed0cc5 100644
--- a/sql/parser/create_test.go
+++ b/sql/parser/create_test.go
@@ -99,6 +99,29 @@ func TestParserCreateTable(t *testing.T) {
 					},
 				},
 			}, false},
+		{"With table constraints / PK on defined field", "CREATE TABLE test(foo INTEGER, bar NOT NULL, PRIMARY KEY (foo))",
+			query.CreateTableStmt{
+				TableName: "test",
+				Info: database.TableInfo{
+					FieldConstraints: []*database.FieldConstraint{
+						{Path: document.Path(parsePath(t, "foo")), Type: document.IntegerValue, IsPrimaryKey: true},
+						{Path: document.Path(parsePath(t, "bar")), IsNotNull: true},
+					},
+				},
+			}, false},
+		{"With table constraints / PK on undefined field", "CREATE TABLE test(foo INTEGER, PRIMARY KEY (bar))",
+			query.CreateTableStmt{
+				TableName: "test",
+				Info: database.TableInfo{
+					FieldConstraints: []*database.FieldConstraint{
+						{Path: document.Path(parsePath(t, "foo")), Type: document.IntegerValue},
+						{Path: document.Path(parsePath(t, "bar")), IsPrimaryKey: true},
+					},
+				},
+			}, false},
+		{"With table constraints / field constraint after table constraint", "CREATE TABLE test(PRIMARY KEY (bar), foo INTEGER)", nil, true},
+		{"With table constraints / duplicate pk", "CREATE TABLE test(foo INTEGER PRIMARY KEY, PRIMARY KEY (bar))", nil, true},
+		{"With table constraints / duplicate pk on same path", "CREATE TABLE test(foo INTEGER PRIMARY KEY, PRIMARY KEY (foo))", nil, true},
 		{"With multiple primary keys", "CREATE TABLE test(foo PRIMARY KEY, bar PRIMARY KEY)",
 			query.CreateTableStmt{}, true},
 		{"With all supported fixed size data types",
@@ -155,7 +178,6 @@ func TestParserCreateTable(t *testing.T) {
 					},
 				},
 			}, false},
-
 		{"With text aliases types",
 			"CREATE TABLE test(v VARCHAR(255), c CHARACTER(64), t TEXT)",
 			query.CreateTableStmt{
@@ -168,7 +190,6 @@ func TestParserCreateTable(t *testing.T) {
 					},
 				},
 			}, false},
-
 		{"With errored text aliases types",
 			"CREATE TABLE test(v VARCHAR(1 IN [1, 2, 3] AND foo > 4) )",
 			query.CreateTableStmt{