Merge pull request #63 from dolthub/daylon/fixing-issues

Fixes for issues: 40, 42, 43

server/ast/aliased_table_expr.go

@@ -16,18 +16,13 @@ package ast
 
 import (
 	"fmt"
-	"sync/atomic"
 
 	vitess "github.com/dolthub/vitess/go/vt/sqlparser"
 
 	"github.com/dolthub/doltgresql/postgres/parser/sem/tree"
+	"github.com/dolthub/doltgresql/utils"
 )
 
-// uniqueAliasCounter is used to create unique aliases. Aliases are required by GMS when some expressions are contained
-// within a vitess.AliasedTableExpr. The Postgres AST does not have this restriction, so we must do this for
-// compatibility. Callers are free to change the alias if need be, but we'll always set an alias to be safe.
-var uniqueAliasCounter atomic.Uint64
-
 // nodeAliasedTableExpr handles *tree.AliasedTableExpr nodes.
 func nodeAliasedTableExpr(node *tree.AliasedTableExpr) (*vitess.AliasedTableExpr, error) {
 	if node.Ordinality {
@@ -66,7 +61,7 @@ func nodeAliasedTableExpr(node *tree.AliasedTableExpr) (*vitess.AliasedTableExpr
 	}
 	alias := string(node.As.Alias)
 	if len(alias) == 0 {
-		alias = generateUniqueAlias()
+		alias = utils.GenerateUniqueAlias()
 	}
 	return &vitess.AliasedTableExpr{
 		Expr:    aliasExpr,
@@ -75,8 +70,3 @@ func nodeAliasedTableExpr(node *tree.AliasedTableExpr) (*vitess.AliasedTableExpr
 		Lateral: node.Lateral,
 	}, nil
 }
-
-// generateUniqueAlias generates a unique alias. This is thread-safe.
-func generateUniqueAlias() string {
-	return fmt.Sprintf("doltgres!|alias|%d", uniqueAliasCounter.Add(1))
-}

server/ast/insert.go

@@ -59,7 +59,7 @@ func nodeInsert(node *tree.Insert) (*vitess.Insert, error) {
 	}
 	var columns []vitess.ColIdent
 	if len(node.Columns) > 0 {
-		columns := make([]vitess.ColIdent, len(node.Columns))
+		columns = make([]vitess.ColIdent, len(node.Columns))
 		for i := range node.Columns {
 			columns[i] = vitess.NewColIdent(string(node.Columns[i]))
 		}

server/ast/select_clause.go

@@ -35,6 +35,40 @@ func nodeSelectClause(node *tree.SelectClause) (*vitess.Select, error) {
 	if err != nil {
 		return nil, err
 	}
+	// We use TableFuncExprs to represent queries on functions that behave as though they were tables. This is something
+	// that we have to situationally support, as inner nodes do not have the proper context to output a TableFuncExpr,
+	// since TableFuncExprs pertain only to SELECT statements.
+	for i, fromExpr := range from {
+		// Nodes are very liberal in wrapping themselves within other nodes, which gives them a technically correct
+		// tree, however GMS makes assumptions about the makeup of the trees that it receives. We'll eventually
+		// generalize this on the GMS side, but for now we need to transform our tree in case we need to use a TableFuncExpr.
+		if aliasedTableExpr, ok := fromExpr.(*vitess.AliasedTableExpr); ok {
+			subquery, ok := aliasedTableExpr.Expr.(*vitess.Subquery)
+			// If all of these are true, then the AliasedTableExpr is probably a wrapper around a subquery, but we have
+			// to confirm that the subquery contains a *Select with a single child in its From expressions.
+			if !aliasedTableExpr.Lateral &&
+				aliasedTableExpr.Hints == nil &&
+				len(aliasedTableExpr.Partitions) == 0 &&
+				ok && len(subquery.Columns) == 0 {
+				// If this is true, then we can confirm that it's just a wrapper (and not an explicit AliasedTableExpr).
+				// This may seem like a lot of fragile checks, but AliasedTableExpr explicitly sets its state to this in
+				// this circumstance. We do not want to create a TableFuncExpr except under very specific circumstances.
+				if subquerySelect, ok := subquery.Select.(*vitess.Select); ok && len(subquerySelect.From) == 1 {
+					if valuesStatement, ok := subquerySelect.From[0].(*vitess.ValuesStatement); ok {
+						if len(valuesStatement.Columns) == 0 && len(valuesStatement.Rows) == 1 && len(valuesStatement.Rows[0]) == 1 {
+							if funcExpr, ok := valuesStatement.Rows[0][0].(*vitess.FuncExpr); ok {
+								from[i] = &vitess.TableFuncExpr{
+									Name:  funcExpr.Name.String(),
+									Exprs: funcExpr.Exprs,
+									Alias: aliasedTableExpr.As,
+								}
+							}
+						}
+					}
+				}
+			}
+		}
+	}
 	if len(node.DistinctOn) > 0 {
 		return nil, fmt.Errorf("DISTINCT ON is not yet supported")
 	}

server/ast/subquery.go

@@ -20,6 +20,7 @@ import (
 	vitess "github.com/dolthub/vitess/go/vt/sqlparser"
 
 	"github.com/dolthub/doltgresql/postgres/parser/sem/tree"
+	"github.com/dolthub/doltgresql/utils"
 )
 
 // nodeSubquery handles *tree.Subquery nodes.
@@ -47,6 +48,6 @@ func nodeSubqueryToTableExpr(node *tree.Subquery) (vitess.TableExpr, error) {
 	}
 	return &vitess.AliasedTableExpr{
 		Expr: subquery,
-		As:   vitess.NewTableIdent(generateUniqueAlias()),
+		As:   vitess.NewTableIdent(utils.GenerateUniqueAlias()),
 	}, nil
 }

server/ast/table_def.go

@@ -16,13 +16,18 @@ package ast
 
 import (
 	"fmt"
+	"sort"
 
 	vitess "github.com/dolthub/vitess/go/vt/sqlparser"
 
 	"github.com/dolthub/doltgresql/postgres/parser/sem/tree"
+	"github.com/dolthub/doltgresql/utils"
 )
 
-// assignTableDef handles tree.TableDef nodes for *vitess.DDL targets.
+// assignTableDef handles tree.TableDef nodes for *vitess.DDL targets. Some table defs, such as indexes, affect other
+// defs, such as columns, and they're therefore dependent on columns being handled first. It is up to the caller to
+// ensure that all defs have been ordered properly before calling. assignTableDefs handles the sort for you, so this
+// notice is only relevant when individually calling assignTableDef.
 func assignTableDef(node tree.TableDef, target *vitess.DDL) error {
 	switch node := node.(type) {
 	case *tree.CheckConstraintTableDef:
@@ -98,6 +103,18 @@ func assignTableDef(node tree.TableDef, target *vitess.DDL) error {
 		}
 		indexDef.Info.Unique = true
 		indexDef.Info.Primary = node.PrimaryKey
+		// If we're setting a primary key, then we need to make sure that all of the columns are also set to NOT NULL
+		if indexDef.Info.Primary {
+			tableColumns := utils.SliceToMapValues(target.TableSpec.Columns, func(col *vitess.ColumnDefinition) string {
+				return col.Name.String()
+			})
+			for _, indexedColumn := range indexDef.Columns {
+				if column, ok := tableColumns[indexedColumn.Column.String()]; ok {
+					column.Type.Null = false
+					column.Type.NotNull = true
+				}
+			}
+		}
 		target.TableSpec.Indexes = append(target.TableSpec.Indexes, indexDef)
 		return nil
 	case nil:
@@ -107,10 +124,28 @@ func assignTableDef(node tree.TableDef, target *vitess.DDL) error {
 	}
 }
 
-// assignTableDefs handles tree.TableDefs nodes for *vitess.DDL targets.
+// assignTableDefs handles tree.TableDefs nodes for *vitess.DDL targets. This also sorts table defs by whether they're
+// dependent on other table defs evaluating first. Some table defs, such as indexes, affect other defs, such as columns,
+// and they're therefore dependent on columns being handled first.
 func assignTableDefs(node tree.TableDefs, target *vitess.DDL) error {
-	for i := range node {
-		if err := assignTableDef(node[i], target); err != nil {
+	sortedNode := make(tree.TableDefs, len(node))
+	copy(sortedNode, node)
+	sort.Slice(sortedNode, func(i, j int) bool {
+		var cmps [2]int
+		for cmpsIdx := range []tree.TableDef{sortedNode[i], sortedNode[j]} {
+			switch sortedNode[i].(type) {
+			case *tree.IndexTableDef:
+				cmps[cmpsIdx] = 1
+			case *tree.UniqueConstraintTableDef:
+				cmps[cmpsIdx] = 2
+			default:
+				cmps[cmpsIdx] = 0
+			}
+		}
+		return cmps[0] < cmps[1]
+	})
+	for i := range sortedNode {
+		if err := assignTableDef(sortedNode[i], target); err != nil {
 			return err
 		}
 	}

server/listener.go

@@ -572,6 +572,9 @@ func (l *Listener) convertQuery(query string) (ConvertedQuery, error) {
 	if len(s) > 1 {
 		return ConvertedQuery{}, fmt.Errorf("only a single statement at a time is currently supported")
 	}
+	if len(s) == 0 {
+		return ConvertedQuery{String: query}, nil
+	}
 	vitessAST, err := ast.Convert(s[0])
 	if err != nil {
 		return ConvertedQuery{}, err

server/server.go

@@ -324,8 +324,8 @@ func runServer(args []string, fs filesys.Filesys) (*int, *sync.WaitGroup) {
 		}
 		// Server mode automatically initializes a doltgres database
 		if !dEnv.HasDoltDir() {
-			// Need to make sure that there isn't a doltgres subdirectory. If there is, we'll assume it's a db.
-			if exists, _ := dEnv.FS.Exists("doltgres"); !exists {
+			// Need to make sure that there isn't a doltgres item in the path.
+			if exists, isDirectory := dEnv.FS.Exists("doltgres"); !exists {
 				dEnv.FS.MkDirs("doltgres")
 				subdirectoryFS, err := dEnv.FS.WithWorkingDir("doltgres")
 				if err != nil {
@@ -336,6 +336,14 @@ func runServer(args []string, fs filesys.Filesys) (*int, *sync.WaitGroup) {
 				// We'll use a temporary environment to instantiate the subdirectory
 				tempDEnv := env.Load(ctx, env.GetCurrentUserHomeDir, subdirectoryFS, doltdb.LocalDirDoltDB, Version)
 				_ = doltCommand.Exec(ctx, "dolt", []string{"init"}, tempDEnv, cliCtx)
+			} else if !isDirectory {
+				// The else branch means that there's a Doltgres item, so we need to error if it's a file since we
+				// enforce the creation of a Doltgres database/directory, which would create a name conflict with the file
+				stop()
+				cli.PrintErrln(fmt.Errorf("Attempted to create the default `doltgres` database, but a file with" +
+					" the same name was found. Please run the doltgres command in a directory that does not contain a" +
+					" file with the name doltgres"))
+				return intPointer(1), wg
 			}
 		}
 	}

testing/go/smoke_test.go

@@ -56,6 +56,37 @@ func TestSmokeTests(t *testing.T) {
 				},
 			},
 		},
+		{
+			Name: "Dolt Getting Started example", /* https://docs.dolthub.com/introduction/getting-started/database */
+			SetUpScript: []string{
+				"create table employees (id int, last_name varchar(255), first_name varchar(255), primary key(id));",
+				"create table teams (id int, team_name varchar(255), primary key(id));",
+				"create table employees_teams(team_id int, employee_id int, primary key(team_id, employee_id), foreign key (team_id) references teams(id), foreign key (employee_id) references employees(id));",
+				"call dolt_add('teams', 'employees', 'employees_teams');",
+				"call dolt_commit('-m', 'Created initial schema');",
+				"insert into employees values (0, 'Sehn', 'Tim'), (1, 'Hendriks', 'Brian'), (2, 'Son','Aaron'), (3, 'Fitzgerald', 'Brian');",
+				"insert into teams values (0, 'Engineering'), (1, 'Sales');",
+				"insert into employees_teams(employee_id, team_id) values (0,0), (1,0), (2,0), (0,1), (3,1);",
+				"call dolt_commit('-am', 'Populated tables with data');",
+				"call dolt_checkout('-b','modifications');",
+				"update employees SET first_name='Timothy' where first_name='Tim';",
+				"insert INTO employees (id, first_name, last_name) values (4,'Daylon', 'Wilkins');",
+				"insert into employees_teams(team_id, employee_id) values (0,4);",
+				"delete from employees_teams where employee_id=0 and team_id=1;",
+				"call dolt_commit('-am', 'Modifications on a branch')",
+				"call dolt_checkout('modifications');",
+			},
+			Assertions: []ScriptTestAssertion{
+				{
+					Query: "select to_last_name, to_first_name, to_id, to_commit, from_last_name, from_first_name," +
+						"from_id, from_commit, diff_type from dolt_diff('main', 'modifications', 'employees');",
+					Expected: []sql.Row{
+						{"Sehn", "Timothy", 0, "modifications", "Sehn", "Tim", 0, "main", "modified"},
+						{"Wilkins", "Daylon", 4, "modifications", nil, nil, nil, "main", "added"},
+					},
+				},
+			},
+		},
 		{
 			Name: "Boolean results",
 			Assertions: []ScriptTestAssertion{
@@ -122,6 +153,15 @@ func TestSmokeTests(t *testing.T) {
 				},
 			},
 		},
+		{
+			Name: "Empty statement",
+			Assertions: []ScriptTestAssertion{
+				{
+					Query:    ";",
+					Expected: []sql.Row{},
+				},
+			},
+		},
 		{
 			Name: "Unsupported MySQL statements",
 			Assertions: []ScriptTestAssertion{

utils/alias.go

@@ -0,0 +1,45 @@
+// Copyright 2023 Dolthub, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package utils
+
+import (
+	"fmt"
+	"strconv"
+	"strings"
+	"sync/atomic"
+)
+
+// uniqueAliasCounter is used to create unique aliases. Aliases are required by GMS when some expressions are contained
+// within an AliasedTableExpr. Postgres does not have this restriction, so we must do this for compatibility.
+var uniqueAliasCounter atomic.Uint64
+
+// uniqueAliasPrefix is the prefix given to aliases that have been generated by GenerateUniqueAlias.
+var uniqueAliasPrefix = "doltgres!|!alias!|_"
+
+// GenerateUniqueAlias generates a unique alias. This is thread-safe.
+func GenerateUniqueAlias() string {
+	return fmt.Sprintf("%s%d", uniqueAliasPrefix, uniqueAliasCounter.Add(1))
+}
+
+// IsGeneratedAlias returns whether the given alias was generated using GenerateUniqueAlias.
+func IsGeneratedAlias(alias string) bool {
+	if strings.HasPrefix(alias, uniqueAliasPrefix) && len(alias) > len(uniqueAliasPrefix) {
+		aliasDigits := alias[len(uniqueAliasPrefix):]
+		if _, err := strconv.ParseUint(aliasDigits, 10, 64); err == nil {
+			return true
+		}
+	}
+	return false
+}

utils/map_keys.go

@@ -30,7 +30,8 @@ func GetMapKeys[K comparable, V any](m map[K]V) []K {
 	return allKeys
 }
 
-// GetMapKeysSorted returns the map's keys as a sorted slice.
+// GetMapKeysSorted returns the map's keys as a sorted slice. The keys are sorted in ascending order. For descending
+// order, use GetMapKeysSortedDescending.
 func GetMapKeysSorted[K cmp.Ordered, V any](m map[K]V) []K {
 	allKeys := make([]K, len(m))
 	i := 0
@@ -44,6 +45,21 @@ func GetMapKeysSorted[K cmp.Ordered, V any](m map[K]V) []K {
 	return allKeys
 }
 
+// GetMapKeysSortedDescending returns the map's keys as a sorted slice. The keys are sorted in descending order. For
+// ascending order, use GetMapKeysSorted.
+func GetMapKeysSortedDescending[K cmp.Ordered, V any](m map[K]V) []K {
+	allKeys := make([]K, len(m))
+	i := 0
+	for k := range m {
+		allKeys[i] = k
+		i++
+	}
+	sort.Slice(allKeys, func(i, j int) bool {
+		return allKeys[i] > allKeys[j]
+	})
+	return allKeys
+}
+
 // GetMapValues returns the map's values as a slice. Due to Go's map iteration, the values will be in a
 // non-deterministic order.
 func GetMapValues[K comparable, V any](m map[K]V) []V {

utils/slice_to_map.go

@@ -0,0 +1,34 @@
+// Copyright 2023 Dolthub, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package utils
+
+// SliceToMapKeys converts the given slice into a map where all of the keys are represented by all of the slice's values.
+func SliceToMapKeys[T comparable](slice []T) map[T]struct{} {
+	m := make(map[T]struct{})
+	for _, val := range slice {
+		m[val] = struct{}{}
+	}
+	return m
+}
+
+// SliceToMapValues converts the given slice into a map where all of the slice's values are keyed by the output of the
+// `getKey` function, which takes a value and returns a key. It is up to the caller to return a unique key.
+func SliceToMapValues[K comparable, V any](slice []V, getKey func(V) K) map[K]V {
+	m := make(map[K]V)
+	for _, val := range slice {
+		m[getKey(val)] = val
+	}
+	return m
+}