Merge pull request #939 from dolthub/zachmu/enginetests2

zachmu · web-flow · commit 28bca52450e1 · 2024-11-12T12:31:39.000-08:00
Unskip many InsertInto engine tests
diff --git a/server/analyzer/assign_insert_casts.go b/server/analyzer/assign_insert_casts.go
@@ -16,6 +16,7 @@ package analyzer
 
 import (
 	"fmt"
+	"strings"
 
 	"github.com/dolthub/go-mysql-server/sql"
 	"github.com/dolthub/go-mysql-server/sql/analyzer"
@@ -42,12 +43,12 @@ func AssignInsertCasts(ctx *sql.Context, a *analyzer.Analyzer, node sql.Node, sc
 		if !ok {
 			return nil, transform.NewTree, fmt.Errorf("INSERT: non-Doltgres type found in destination: %s", col.Type.String())
 		}
-		destinationNameToType[col.Name] = colType
+		destinationNameToType[strings.ToLower(col.Name)] = colType
 	}
 	// Create the destination type slice that will match each inserted column
 	destinationTypes := make([]pgtypes.DoltgresType, len(insertInto.ColumnNames))
 	for i, colName := range insertInto.ColumnNames {
-		destinationTypes[i], ok = destinationNameToType[colName]
+		destinationTypes[i], ok = destinationNameToType[strings.ToLower(colName)]
 		if !ok {
 			return nil, transform.NewTree, fmt.Errorf("INSERT: cannot find destination column with name `%s`", colName)
 		}
@@ -72,7 +73,7 @@ func AssignInsertCasts(ctx *sql.Context, a *analyzer.Analyzer, node sql.Node, sc
 				}
 			}
 		}
-		return insertInto.WithSource(plan.NewValues(newValues)), transform.NewTree, nil
+		insertInto = insertInto.WithSource(plan.NewValues(newValues))
 	} else {
 		sourceSchema := insertInto.Source.Schema()
 		projections := make([]sql.Expression, len(sourceSchema))
@@ -90,6 +91,23 @@ func AssignInsertCasts(ctx *sql.Context, a *analyzer.Analyzer, node sql.Node, sc
 				projections[i] = pgexprs.NewAssignmentCast(getField, fromColType, toColType)
 			}
 		}
-		return insertInto.WithSource(plan.NewProject(projections, insertInto.Source)), transform.NewTree, nil
+		insertInto = insertInto.WithSource(plan.NewProject(projections, insertInto.Source))
 	}
+
+	// handle on conflict clause if present
+	if len(insertInto.OnDupExprs) > 0 {
+		newDupExprs, err := assignUpdateFieldCasts(insertInto.OnDupExprs)
+		if err != nil {
+			return nil, false, err
+		}
+		// TODO: this relies on a particular implementation detail InsertInto.WithExpressions
+		newInsertInto, err := insertInto.WithExpressions(append(newDupExprs, insertInto.Checks().ToExpressions()...)...)
+		if err != nil {
+			return nil, false, err
+		}
+
+		insertInto = newInsertInto.(*plan.InsertInto)
+	}
+
+	return insertInto, transform.NewTree, nil
 }
diff --git a/server/analyzer/assign_update_casts.go b/server/analyzer/assign_update_casts.go
@@ -69,8 +69,21 @@ func AssignUpdateCasts(ctx *sql.Context, a *analyzer.Analyzer, node sql.Node, sc
 
 // assignUpdateCastsHandleSource handles the *plan.UpdateSource portion of AssignUpdateCasts.
 func assignUpdateCastsHandleSource(updateSource *plan.UpdateSource) (*plan.UpdateSource, error) {
-	newUpdateExprs := make([]sql.Expression, len(updateSource.UpdateExprs))
-	for i, updateExpr := range updateSource.UpdateExprs {
+	updateExprs := updateSource.UpdateExprs
+	newUpdateExprs, err := assignUpdateFieldCasts(updateExprs)
+	if err != nil {
+		return nil, err
+	}
+	newUpdateSource, err := updateSource.WithExpressions(newUpdateExprs...)
+	if err != nil {
+		return nil, err
+	}
+	return newUpdateSource.(*plan.UpdateSource), nil
+}
+
+func assignUpdateFieldCasts(updateExprs []sql.Expression) ([]sql.Expression, error) {
+	newUpdateExprs := make([]sql.Expression, len(updateExprs))
+	for i, updateExpr := range updateExprs {
 		setField, ok := updateExpr.(*expression.SetField)
 		if !ok {
 			return nil, fmt.Errorf("UPDATE: assumption that expression is always SetField is incorrect: %T", updateExpr)
@@ -94,9 +107,5 @@ func assignUpdateCastsHandleSource(updateSource *plan.UpdateSource) (*plan.Updat
 			newUpdateExprs[i] = newSetField
 		}
 	}
-	newUpdateSource, err := updateSource.WithExpressions(newUpdateExprs...)
-	if err != nil {
-		return nil, err
-	}
-	return newUpdateSource.(*plan.UpdateSource), nil
+	return newUpdateExprs, nil
 }
diff --git a/server/ast/insert.go b/server/ast/insert.go
@@ -35,16 +35,23 @@ func nodeInsert(ctx *Context, node *tree.Insert) (*vitess.Insert, error) {
 		return nil, fmt.Errorf("RETURNING is not yet supported")
 	}
 	var ignore string
+	var onDuplicate vitess.OnDup
+
 	if node.OnConflict != nil {
-		// Currently, only ON CONFLICT DO NOTHING is supported, which is equivalent to INSERT IGNORE in GMS
-		if node.OnConflict.Columns != nil ||
-			node.OnConflict.ArbiterPredicate != nil ||
-			node.OnConflict.Exprs != nil ||
-			node.OnConflict.Where != nil ||
-			!node.OnConflict.DoNothing {
+		if isIgnore(node.OnConflict) {
+			ignore = vitess.IgnoreStr
+		} else if supportedOnConflictClause(node.OnConflict) {
+			// TODO: we are ignoring the column names, which are used to infer which index under conflict is to be checked
+			updateExprs, err := nodeUpdateExprs(ctx, node.OnConflict.Exprs)
+			if err != nil {
+				return nil, err
+			}
+			for _, updateExpr := range updateExprs {
+				onDuplicate = append(onDuplicate, updateExpr)
+			}
+		} else {
 			return nil, fmt.Errorf("the ON CONFLICT clause provided is not yet supported")
 		}
-		ignore = vitess.IgnoreStr
 	}
 	var tableName vitess.TableName
 	switch node := node.Table.(type) {
@@ -78,7 +85,7 @@ func nodeInsert(ctx *Context, node *tree.Insert) (*vitess.Insert, error) {
 		return nil, err
 	}
 
-	// GMS For a ValuesStatement with simple rows, GMS expects AliasedValues
+	// For a ValuesStatement with simple rows, GMS expects AliasedValues
 	if vSelect, ok := rows.(*vitess.Select); ok && len(vSelect.From) == 1 {
 		if valsStmt, ok := vSelect.From[0].(*vitess.ValuesStatement); ok {
 			rows = &vitess.AliasedValues{
@@ -93,10 +100,31 @@ func nodeInsert(ctx *Context, node *tree.Insert) (*vitess.Insert, error) {
 		With:    with,
 		Columns: columns,
 		Rows:    rows,
+		OnDup:   onDuplicate,
 		Auth: vitess.AuthInformation{
 			AuthType:    auth.AuthType_INSERT,
 			TargetType:  auth.AuthTargetType_SingleTableIdentifier,
 			TargetNames: []string{tableName.SchemaQualifier.String(), tableName.Name.String()},
 		},
 	}, nil
 }
+
+// isIgnore returns true if the ON CONFLICT clause provided is equivalent to INSERT IGNORE in GMS
+func isIgnore(conflict *tree.OnConflict) bool {
+	return conflict.ArbiterPredicate == nil &&
+		conflict.Exprs == nil &&
+		conflict.Where == nil &&
+		conflict.DoNothing
+}
+
+// supportedOnConflictClause returns true if the ON CONFLICT clause given can be represented as
+// an ON DUPLICATE KEY UPDATE clause in GMS
+func supportedOnConflictClause(conflict *tree.OnConflict) bool {
+	if conflict.ArbiterPredicate != nil {
+		return false
+	}
+	if conflict.Where != nil {
+		return false
+	}
+	return true
+}
diff --git a/server/ast/select.go b/server/ast/select.go
@@ -55,7 +55,7 @@ func nodeSelect(ctx *Context, node *tree.Select) (vitess.SelectStatement, error)
 
 	switch selectStmt := selectStmt.(type) {
 	case *vitess.ParenSelect:
-		//TODO: figure out if this is even correct, not sure what statement would produce this AST
+		// TODO: figure out if this is even correct, not sure what statement would produce this AST
 		// perhaps we should use the inner select statement, but maybe it has its own order by, limit, etc.
 		return &vitess.Select{
 			SelectExprs: vitess.SelectExprs{
@@ -144,13 +144,15 @@ func nodeSelectExpr(ctx *Context, node tree.SelectExpr) (vitess.SelectExpr, erro
 			if expr.NumParts == 2 {
 				tableName.Name = vitess.NewTableIdent(expr.Parts[1])
 			}
+			// We don't set the InputExpression for ColName expressions. This matches the behavior in vitess's
+			// post-processing found in ast.go. Input expressions are load bearing for some parts of plan building
+			// so we need to match the behavior exactly.
 			return &vitess.AliasedExpr{
 				Expr: &vitess.ColName{
 					Name:      vitess.NewColIdent(expr.Parts[0]),
 					Qualifier: tableName,
 				},
-				As:              vitess.NewColIdent(string(node.As)),
-				InputExpression: tree.AsString(&node),
+				As: vitess.NewColIdent(string(node.As)),
 			}, nil
 		}
 	default:
diff --git a/testing/go/enginetest/doltgres_engine_test.go b/testing/go/enginetest/doltgres_engine_test.go
@@ -25,6 +25,7 @@ import (
 	"github.com/dolthub/go-mysql-server/enginetest/queries"
 	"github.com/dolthub/go-mysql-server/enginetest/scriptgen/setup"
 	"github.com/dolthub/go-mysql-server/sql"
+	"github.com/dolthub/go-mysql-server/sql/types"
 	"github.com/stretchr/testify/require"
 
 	"github.com/dolthub/dolt/go/libraries/doltcore/dtestutils"
@@ -59,6 +60,28 @@ func TestQueries(t *testing.T) {
 	enginetest.TestQueries(t, h)
 }
 
+func TestSingleWriteQuery(t *testing.T) {
+	t.Skip()
+	h := newDoltgresServerHarness(t)
+	defer h.Close()
+
+	h.Setup(setup.MydbData, setup.MytableData, setup.Mytable_del_idxData, setup.KeylessData, setup.Keyless_idxData, setup.NiltableData, setup.TypestableData, setup.EmptytableData, setup.AutoincrementData, setup.OthertableData, setup.Othertable_del_idxData)
+
+	test := queries.WriteQueryTest{
+		WriteQuery: `INSERT INTO emptytable (s,i) SELECT s,i from mytable where i = 1
+			union select s,i from mytable where i = 3
+			union select s,i from mytable where i > 2`,
+		ExpectedWriteResult: []sql.Row{{types.NewOkResult(2)}},
+		SelectQuery:         "SELECT * FROM emptytable ORDER BY i,s",
+		ExpectedSelect: []sql.Row{
+			{int64(1), "first row"},
+			{int64(3), "third row"},
+		},
+	}
+
+	enginetest.RunWriteQueryTest(t, h, test)
+}
+
 func TestSingleQuery(t *testing.T) {
 	t.Skip()
 
@@ -148,34 +171,35 @@ func TestSingleScript(t *testing.T) {
 
 	var scripts = []queries.ScriptTest{
 		{
-			Name: "Group by BINARY: https://github.com/dolthub/dolt/issues/6179",
+			Name: "Insert throws primary key violations",
 			SetUpScript: []string{
-				"create table t (s varchar(100));",
-				"insert into t values ('abc'), ('def');",
-				"create table t1 (b binary(3));",
-				"insert into t1 values ('abc'), ('abc'), ('def'), ('abc'), ('def');",
+				"CREATE TABLE t (pk int PRIMARY key);",
+				"CREATE TABLE t2 (pk1 int, pk2 int, PRIMARY KEY (pk1, pk2));",
 			},
 			Assertions: []queries.ScriptTestAssertion{
 				{
-					Query: "select binary s from t group by binary s order by binary s",
-					Expected: []sql.Row{
-						{[]uint8("abc")},
-						{[]uint8("def")},
-					},
+					Query:    "INSERT INTO t VALUES (1), (2);",
+					Expected: []sql.Row{{types.NewOkResult(2)}},
 				},
 				{
-					Query: "select count(b), b from t1 group by b order by b",
-					Expected: []sql.Row{
-						{3, []uint8("abc")},
-						{2, []uint8("def")},
-					},
+					Query:       "INSERT into t VALUES (1);",
+					ExpectedErr: sql.ErrPrimaryKeyViolation,
 				},
 				{
-					Query: "select binary s from t group by binary s order by s",
-					Expected: []sql.Row{
-						{[]uint8("abc")},
-						{[]uint8("def")},
-					},
+					Query:    "SELECT * from t;",
+					Expected: []sql.Row{{1}, {2}},
+				},
+				{
+					Query:    "INSERT into t2 VALUES (1, 1), (2, 2);",
+					Expected: []sql.Row{{types.NewOkResult(2)}},
+				},
+				{
+					Query:       "INSERT into t2 VALUES (1, 1);",
+					ExpectedErr: sql.ErrPrimaryKeyViolation,
+				},
+				{
+					Query:    "SELECT * from t2;",
+					Expected: []sql.Row{{1, 1}, {2, 2}},
 				},
 			},
 		},
@@ -286,15 +310,46 @@ func TestOrderByGroupBy(t *testing.T) {
 }
 
 func TestAmbiguousColumnResolution(t *testing.T) {
-	t.Skip()
 	h := newDoltgresServerHarness(t)
 	defer h.Close()
 	enginetest.TestAmbiguousColumnResolution(t, h)
 }
 
 func TestInsertInto(t *testing.T) {
-	t.Skip()
-	h := newDoltgresServerHarness(t)
+	h := newDoltgresServerHarness(t).WithSkippedQueries([]string{
+		"INSERT INTO keyless VALUES ();",                                                                                                          // unsupported syntax
+		"INSERT INTO keyless () VALUES ();",                                                                                                       // unsupported syntax
+		"INSERT INTO mytable (s, i) VALUES ('x', '10.0');",                                                                                        // type mismatch
+		"INSERT INTO mytable (s, i) VALUES ('x', '64.6');",                                                                                        // type mismatch
+		"INSERT INTO mytable SET s = 'x', i = 999;",                                                                                               // unsupported syntax
+		"INSERT INTO mytable SET i = 999, s = 'x';",                                                                                               // unsupported syntax
+		`INSERT INTO mytable (i,s) SELECT i * 2, concat(s,s) from mytable order by 1 desc limit 1`,                                                // type error
+		"INSERT INTO mytable VALUES (999, _binary 'x');",                                                                                          // unsupported syntax
+		"INSERT INTO mytable SET i = 999, s = _binary 'x';",                                                                                       // unsupported syntax
+		"INSERT INTO mytable (i,s) values (1,'hi') ON DUPLICATE KEY UPDATE s=VALUES(s)",                                                           // unsupported syntax
+		"INSERT INTO mytable (s,i) values ('dup',1) ON DUPLICATE KEY UPDATE s=CONCAT(VALUES(s), 'licate')",                                        // unsupported syntax
+		"INSERT INTO mytable (i,s) values (1,'mar'), (2,'par') ON DUPLICATE KEY UPDATE s=CONCAT(VALUES(s), 'tial')",                               // bad translation
+		"INSERT INTO mytable (i,s) values (1,'maybe') ON DUPLICATE KEY UPDATE i=VALUES(i)+8000, s=VALUES(s)",                                      // unsupported syntax
+		`insert into keyless (c0, c1) select a.c0, a.c1 from (select 1, 1) as a(c0, c1) join keyless on a.c0 = keyless.c0`,                        // missing result element, needs investigation
+		"with t (i,f) as (select 4,'fourth row' from dual) insert into mytable select i,f from t",                                                 // WITH unsupported syntax
+		"with recursive t (i,f) as (select 4,4 from dual union all select i + 1, i + 1 from t where i < 5) insert into mytable select i,f from t", // WITH unsupported syntax
+		"issue 6675: on duplicate rearranged getfield indexes from select source",                                                                 // panic
+		"issue 4857: insert cte column alias with table alias qualify panic",                                                                      // WITH unsupported syntax
+		"sql_mode=NO_auto_value_ON_ZERO",                                                                                                          // unsupported
+		"explicit DEFAULT",                                                                                                                        // enum type unsupported
+		// "Try INSERT IGNORE with primary key, non null, and single row violations", // insert ignore not supported
+		"Insert on duplicate key references table in subquery",           // bad translation?
+		"Insert on duplicate key references table in aliased subquery",   // bad translation?
+		"Insert on duplicate key references table in cte",                // CTE not supported
+		"insert on duplicate key with incorrect row alias",               // column "c" could not be found in any table in scope
+		"insert on duplicate key update errors",                          // failing
+		"Insert on duplicate key references table in subquery with join", // untranslated
+		"INSERT INTO ... SELECT works properly with ENUM",                // enum unsupported
+		"INSERT INTO ... SELECT works properly with SET",                 // set unsupported
+		"INSERT INTO ... SELECT with TEXT types",                         // typecasts needed
+		"check IN TUPLE constraint with duplicate key update",            // error not being thrown
+		"INSERT IGNORE works with FK Violations",                         // ignore not supported
+	})
 	defer h.Close()
 	enginetest.TestInsertInto(t, h)
 }
diff --git a/testing/go/enginetest/doltgres_harness_test.go b/testing/go/enginetest/doltgres_harness_test.go
@@ -217,8 +217,6 @@ var skippedSetupWords = []string{
 }
 
 var commentClause = regexp.MustCompile(`(?i)comment '.*?'`)
-var createIndexStatement = regexp.MustCompile(`(?i)create( unique)? index`)
-var alterTableStatement = regexp.MustCompile(`(?i)alter table`)
 
 // sanitizeQuery strips the query string given of any unsupported constructs without attempting to actually convert
 // to Postgres syntax.
@@ -229,13 +227,6 @@ func sanitizeQuery(s string) (bool, string) {
 		}
 	}
 
-	if createIndexStatement.MatchString(s) {
-		return false, ""
-	}
-	if alterTableStatement.MatchString(s) {
-		return false, ""
-	}
-
 	s = commentClause.ReplaceAllString(s, "")
 	return true, s
 }
diff --git a/testing/go/enginetest/query_converter_test.go b/testing/go/enginetest/query_converter_test.go
diff --git a/testing/go/insert_test.go b/testing/go/insert_test.go

Original file line number	Diff line number	Diff line change
`@@ -16,6 +16,7 @@ package analyzer`
`16`	`16`
`17`	`17`	`import (`
`18`	`18`	`"fmt"`
	`19`	`+ "strings"`
`19`	`20`
`20`	`21`	`"github.com/dolthub/go-mysql-server/sql"`
`21`	`22`	`"github.com/dolthub/go-mysql-server/sql/analyzer"`
`@@ -42,12 +43,12 @@ func AssignInsertCasts(ctx sql.Context, a analyzer.Analyzer, node sql.Node, sc`
`42`	`43`	`if !ok {`
`43`	`44`	`return nil, transform.NewTree, fmt.Errorf("INSERT: non-Doltgres type found in destination: %s", col.Type.String())`
`44`	`45`	`}`
`45`		`- destinationNameToType[col.Name] = colType`
	`46`	`+ destinationNameToType[strings.ToLower(col.Name)] = colType`
`46`	`47`	`}`
`47`	`48`	`// Create the destination type slice that will match each inserted column`
`48`	`49`	`destinationTypes := make([]pgtypes.DoltgresType, len(insertInto.ColumnNames))`
`49`	`50`	`for i, colName := range insertInto.ColumnNames {`
`50`		`- destinationTypes[i], ok = destinationNameToType[colName]`
	`51`	`+ destinationTypes[i], ok = destinationNameToType[strings.ToLower(colName)]`
`51`	`52`	`if !ok {`
`52`	`53`	return nil, transform.NewTree, fmt.Errorf("INSERT: cannot find destination column with name `%s`", colName)
`53`	`54`	`}`
`@@ -72,7 +73,7 @@ func AssignInsertCasts(ctx sql.Context, a analyzer.Analyzer, node sql.Node, sc`
`72`	`73`	`}`
`73`	`74`	`}`
`74`	`75`	`}`
`75`		`- return insertInto.WithSource(plan.NewValues(newValues)), transform.NewTree, nil`
	`76`	`+ insertInto = insertInto.WithSource(plan.NewValues(newValues))`
`76`	`77`	`} else {`
`77`	`78`	`sourceSchema := insertInto.Source.Schema()`
`78`	`79`	`projections := make([]sql.Expression, len(sourceSchema))`
`@@ -90,6 +91,23 @@ func AssignInsertCasts(ctx sql.Context, a analyzer.Analyzer, node sql.Node, sc`
`90`	`91`	`projections[i] = pgexprs.NewAssignmentCast(getField, fromColType, toColType)`
`91`	`92`	`}`
`92`	`93`	`}`
`93`		`- return insertInto.WithSource(plan.NewProject(projections, insertInto.Source)), transform.NewTree, nil`
	`94`	`+ insertInto = insertInto.WithSource(plan.NewProject(projections, insertInto.Source))`
`94`	`95`	`}`
	`96`	`+`
	`97`	`+ // handle on conflict clause if present`
	`98`	`+ if len(insertInto.OnDupExprs) > 0 {`
	`99`	`+ newDupExprs, err := assignUpdateFieldCasts(insertInto.OnDupExprs)`
	`100`	`+ if err != nil {`
	`101`	`+ return nil, false, err`
	`102`	`+ }`
	`103`	`+ // TODO: this relies on a particular implementation detail InsertInto.WithExpressions`
	`104`	`+ newInsertInto, err := insertInto.WithExpressions(append(newDupExprs, insertInto.Checks().ToExpressions()...)...)`
	`105`	`+ if err != nil {`
	`106`	`+ return nil, false, err`
	`107`	`+ }`
	`108`	`+`
	`109`	`+ insertInto = newInsertInto.(*plan.InsertInto)`
	`110`	`+ }`
	`111`	`+`
	`112`	`+ return insertInto, transform.NewTree, nil`
`95`	`113`	`}`
Original file line number	Diff line number	Diff line change
`@@ -217,8 +217,6 @@ var skippedSetupWords = []string{`
`217`	`217`	`}`
`218`	`218`
`219`	`219`	var commentClause = regexp.MustCompile(`(?i)comment '.*?'`)
`220`		-var createIndexStatement = regexp.MustCompile(`(?i)create( unique)? index`)
`221`		-var alterTableStatement = regexp.MustCompile(`(?i)alter table`)
`222`	`220`
`223`	`221`	`// sanitizeQuery strips the query string given of any unsupported constructs without attempting to actually convert`
`224`	`222`	`// to Postgres syntax.`
`@@ -229,13 +227,6 @@ func sanitizeQuery(s string) (bool, string) {`
`229`	`227`	`}`
`230`	`228`	`}`
`231`	`229`
`232`		`- if createIndexStatement.MatchString(s) {`
`233`		`- return false, ""`
`234`		`- }`
`235`		`- if alterTableStatement.MatchString(s) {`
`236`		`- return false, ""`
`237`		`- }`
`238`		`-`
`239`	`230`	`s = commentClause.ReplaceAllString(s, "")`
`240`	`231`	`return true, s`
`241`	`232`	`}`