From a97d9944e928396d98beccd6136a9ccafc47530b Mon Sep 17 00:00:00 2001 From: James Cor Date: Thu, 17 Oct 2024 15:33:07 -0700 Subject: [PATCH 01/12] removing transaction committing node --- engine.go | 27 +---------- sql/analyzer/autocommit.go | 34 -------------- sql/analyzer/parallelize.go | 4 -- sql/analyzer/resolve_subqueries.go | 2 - sql/analyzer/rules.go | 1 - sql/plan/transaction_committing_iter.go | 60 ------------------------- sql/rowexec/node_builder.gen.go | 2 - sql/rowexec/transaction.go | 8 ---- sql/rowexec/transaction_iters.go | 8 ++++ 9 files changed, 10 insertions(+), 136 deletions(-) delete mode 100644 sql/analyzer/autocommit.go diff --git a/engine.go b/engine.go index 5285ec83cd..bd43fe244a 100644 --- a/engine.go +++ b/engine.go @@ -446,6 +446,8 @@ func (e *Engine) QueryWithBindings(ctx *sql.Context, query string, parsed sqlpar return nil, nil, nil, err } + + iter = rowexec.AddTransactionCommittingIter(iter, qFlags) iter = rowexec.AddExpressionCloser(analyzed, iter) return analyzed.Schema(), iter, qFlags, nil @@ -722,31 +724,6 @@ func (e *Engine) CloseSession(connID uint32) { e.PreparedDataCache.DeleteSessionData(connID) } -// Count number of BindVars in given tree -func countBindVars(node sql.Node) int { - var bindVars map[string]bool - bindCntFunc := func(e sql.Expression) bool { - if bv, ok := e.(*expression.BindVar); ok { - if bindVars == nil { - bindVars = make(map[string]bool) - } - bindVars[bv.Name] = true - } - return true - } - transform.InspectExpressions(node, bindCntFunc) - - // InsertInto.Source not a child of InsertInto, so also need to traverse those - transform.Inspect(node, func(n sql.Node) bool { - if in, ok := n.(*plan.InsertInto); ok { - transform.InspectExpressions(in.Source, bindCntFunc) - return false - } - return true - }) - return len(bindVars) -} - func (e *Engine) beginTransaction(ctx *sql.Context) error { beginNewTransaction := ctx.GetTransaction() == nil || plan.ReadCommitted(ctx) if beginNewTransaction { diff --git a/sql/analyzer/autocommit.go b/sql/analyzer/autocommit.go deleted file mode 100644 index e1ffbd1aa0..0000000000 --- a/sql/analyzer/autocommit.go +++ /dev/null @@ -1,34 +0,0 @@ -// Copyright 2022 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 analyzer - -import ( - "github.com/dolthub/go-mysql-server/sql" - "github.com/dolthub/go-mysql-server/sql/plan" - "github.com/dolthub/go-mysql-server/sql/transform" -) - -// addAutocommit wraps each query with a TransactionCommittingNode. -func addAutocommit(ctx *sql.Context, a *Analyzer, n sql.Node, scope *plan.Scope, sel RuleSelector, qFlags *sql.QueryFlags) (sql.Node, transform.TreeIdentity, error) { - // TODO: This is a bit of a hack. Need to figure out better relationship between new transaction node and warnings. - if FlagIsSet(qFlags, sql.QFlagShowWarnings) { - return n, transform.SameTree, nil - } - - if !n.Resolved() { - return n, transform.SameTree, nil - } - return plan.NewTransactionCommittingNode(n), transform.NewTree, nil -} diff --git a/sql/analyzer/parallelize.go b/sql/analyzer/parallelize.go index 124e541386..d36c38636d 100644 --- a/sql/analyzer/parallelize.go +++ b/sql/analyzer/parallelize.go @@ -54,10 +54,6 @@ func shouldParallelize(node sql.Node, scope *plan.Scope) bool { return false } - if tc, ok := node.(*plan.TransactionCommittingNode); ok { - return shouldParallelize(tc.Child(), scope) - } - // Do not try to parallelize DDL or descriptive operations return !plan.IsNoRowNode(node) } diff --git a/sql/analyzer/resolve_subqueries.go b/sql/analyzer/resolve_subqueries.go index c774ed6a8c..fcd114bd7d 100644 --- a/sql/analyzer/resolve_subqueries.go +++ b/sql/analyzer/resolve_subqueries.go @@ -296,8 +296,6 @@ func StripPassthroughNodes(n sql.Node) sql.Node { switch tn := n.(type) { case *plan.QueryProcess: n = tn.Child() - case *plan.TransactionCommittingNode: - n = tn.Child() default: nodeIsPassthrough = false } diff --git a/sql/analyzer/rules.go b/sql/analyzer/rules.go index 53b7a01647..221151c1f2 100644 --- a/sql/analyzer/rules.go +++ b/sql/analyzer/rules.go @@ -26,7 +26,6 @@ func init() { {inlineSubqueryAliasRefsId, inlineSubqueryAliasRefs}, {cacheSubqueryAliasesInJoinsId, cacheSubqueryAliasesInJoins}, {backtickDefaulColumnValueNamesId, backtickDefaultColumnValueNames}, - {AutocommitId, addAutocommit}, {TrackProcessId, trackProcess}, {parallelizeId, parallelize}, } diff --git a/sql/plan/transaction_committing_iter.go b/sql/plan/transaction_committing_iter.go index 7e3789ed4d..80eda33487 100644 --- a/sql/plan/transaction_committing_iter.go +++ b/sql/plan/transaction_committing_iter.go @@ -15,7 +15,6 @@ package plan import ( - "fmt" "os" "github.com/dolthub/go-mysql-server/sql" @@ -34,65 +33,6 @@ func init() { } } -// TransactionCommittingNode implements autocommit logic. It wraps relevant queries and ensures the database commits -// the transaction. -type TransactionCommittingNode struct { - UnaryNode -} - -var _ sql.Node = (*TransactionCommittingNode)(nil) -var _ sql.CollationCoercible = (*TransactionCommittingNode)(nil) - -// NewTransactionCommittingNode returns a TransactionCommittingNode. -func NewTransactionCommittingNode(child sql.Node) *TransactionCommittingNode { - return &TransactionCommittingNode{UnaryNode: UnaryNode{Child: child}} -} - -// String implements the sql.Node interface. -func (t *TransactionCommittingNode) String() string { - return t.Child().String() -} - -// DebugString implements the sql.DebugStringer interface. -func (t *TransactionCommittingNode) DebugString() string { - return sql.DebugString(t.Child()) -} - -// Describe implements the sql.Describable interface. -func (t *TransactionCommittingNode) Describe(options sql.DescribeOptions) string { - return sql.Describe(t.Child(), options) -} - -func (t *TransactionCommittingNode) IsReadOnly() bool { - return t.Child().IsReadOnly() -} - -// WithChildren implements the sql.Node interface. -func (t *TransactionCommittingNode) WithChildren(children ...sql.Node) (sql.Node, error) { - if len(children) != 1 { - return nil, fmt.Errorf("ds") - } - - t2 := *t - t2.UnaryNode = UnaryNode{Child: children[0]} - return &t2, nil -} - -// CheckPrivileges implements the sql.Node interface. -func (t *TransactionCommittingNode) CheckPrivileges(ctx *sql.Context, opChecker sql.PrivilegedOperationChecker) bool { - return t.Child().CheckPrivileges(ctx, opChecker) -} - -// CollationCoercibility implements the interface sql.CollationCoercible. -func (*TransactionCommittingNode) CollationCoercibility(ctx *sql.Context) (collation sql.CollationID, coercibility byte) { - return sql.Collation_binary, 7 -} - -// Child implements the sql.UnaryNode interface. -func (t *TransactionCommittingNode) Child() sql.Node { - return t.UnaryNode.Child -} - // IsSessionAutocommit returns true if the current session is using implicit transaction management // through autocommit. func IsSessionAutocommit(ctx *sql.Context) (bool, error) { diff --git a/sql/rowexec/node_builder.gen.go b/sql/rowexec/node_builder.gen.go index 0864df9c10..2bde43b174 100644 --- a/sql/rowexec/node_builder.gen.go +++ b/sql/rowexec/node_builder.gen.go @@ -54,8 +54,6 @@ func (b *BaseBuilder) buildNodeExecNoAnalyze(ctx *sql.Context, n sql.Node, row s return b.buildCreateRole(ctx, n, row) case *plan.Loop: return b.buildLoop(ctx, n, row) - case *plan.TransactionCommittingNode: - return b.buildTransactionCommittingNode(ctx, n, row) case *plan.DropColumn: return b.buildDropColumn(ctx, n, row) case *plan.AnalyzeTable: diff --git a/sql/rowexec/transaction.go b/sql/rowexec/transaction.go index c036712d31..6f1a0829cc 100644 --- a/sql/rowexec/transaction.go +++ b/sql/rowexec/transaction.go @@ -300,11 +300,3 @@ func (b *BaseBuilder) buildExecuteQuery(ctx *sql.Context, n *plan.ExecuteQuery, func (b *BaseBuilder) buildUse(ctx *sql.Context, n *plan.Use, row sql.Row) (sql.RowIter, error) { return n.RowIter(ctx, row) } - -func (b *BaseBuilder) buildTransactionCommittingNode(ctx *sql.Context, n *plan.TransactionCommittingNode, row sql.Row) (sql.RowIter, error) { - iter, err := b.Build(ctx, n.Child(), row) - if err != nil { - return nil, err - } - return &TransactionCommittingIter{childIter: iter}, nil -} diff --git a/sql/rowexec/transaction_iters.go b/sql/rowexec/transaction_iters.go index c60935175a..921f4f0dd1 100644 --- a/sql/rowexec/transaction_iters.go +++ b/sql/rowexec/transaction_iters.go @@ -74,6 +74,14 @@ type TransactionCommittingIter struct { transactionDatabase string } +func AddTransactionCommittingIter(child sql.RowIter, qFlags *sql.QueryFlags) sql.RowIter { + // TODO: This is a bit of a hack. Need to figure out better relationship between new transaction node and warnings. + if qFlags != nil && qFlags.IsSet(sql.QFlagShowWarnings) { + return child + } + return &TransactionCommittingIter{childIter: child} +} + func (t *TransactionCommittingIter) Next(ctx *sql.Context) (sql.Row, error) { return t.childIter.Next(ctx) } From 6053e15413c8204320c9aa34e09f8b9c40c266c3 Mon Sep 17 00:00:00 2001 From: jycor Date: Thu, 17 Oct 2024 22:34:58 +0000 Subject: [PATCH 02/12] [ga-format-pr] Run ./format_repo.sh to fix formatting --- sql/types/typecheck_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sql/types/typecheck_test.go b/sql/types/typecheck_test.go index ea8bdc7305..534af061fa 100644 --- a/sql/types/typecheck_test.go +++ b/sql/types/typecheck_test.go @@ -17,8 +17,9 @@ package types import ( "testing" - "github.com/dolthub/go-mysql-server/sql" "github.com/stretchr/testify/assert" + + "github.com/dolthub/go-mysql-server/sql" ) func TestIsGeometry(t *testing.T) { From e38ae97fb0a31fc7fb11145353ea692ffb0b040a Mon Sep 17 00:00:00 2001 From: James Cor Date: Thu, 17 Oct 2024 15:54:53 -0700 Subject: [PATCH 03/12] stringer --- sql/analyzer/rule_ids.go | 1 - sql/analyzer/ruleid_string.go | 9 ++++----- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/sql/analyzer/rule_ids.go b/sql/analyzer/rule_ids.go index dc4ed77194..84d8da5d11 100644 --- a/sql/analyzer/rule_ids.go +++ b/sql/analyzer/rule_ids.go @@ -87,7 +87,6 @@ const ( // after all cacheSubqueryAliasesInJoinsId // cacheSubqueryAliasesInJoins backtickDefaulColumnValueNamesId // backtickDefaultColumnValueNames - AutocommitId // addAutocommit TrackProcessId // trackProcess parallelizeId // parallelize ) diff --git a/sql/analyzer/ruleid_string.go b/sql/analyzer/ruleid_string.go index 79186254c9..7d1c4d8d8b 100755 --- a/sql/analyzer/ruleid_string.go +++ b/sql/analyzer/ruleid_string.go @@ -81,14 +81,13 @@ func _() { _ = x[validateDeleteFromId-70] _ = x[cacheSubqueryAliasesInJoinsId-71] _ = x[backtickDefaulColumnValueNamesId-72] - _ = x[AutocommitId-73] - _ = x[TrackProcessId-74] - _ = x[parallelizeId-75] + _ = x[TrackProcessId-73] + _ = x[parallelizeId-74] } -const _RuleId_name = "applyDefaultSelectLimitvalidateOffsetAndLimitvalidateStarExpressionsvalidateCreateTablevalidateAlterTablevalidateExprSemloadStoredProceduresvalidateDropTablesresolveDropConstraintvalidateDropConstraintresolveCreateSelectresolveSubqueriesresolveUnionsresolveDescribeQueryvalidateColumnDefaultsvalidateCreateTriggervalidateCreateProcedurevalidateReadOnlyDatabasevalidateReadOnlyTransactionvalidateDatabaseSetvalidatePrivilegesapplyEventSchedulerflattenTableAliasespushdownSubqueryAliasFiltersvalidateCheckConstraintsreplaceCountStarreplaceCrossJoinsmoveJoinConditionsToFiltersimplifyFilterspushNotFiltershoistOutOfScopeFiltersunnestInSubqueriesunnestExistsSubqueriesfinalizeSubqueriesfinalizeUnionsloadTriggersprocessTruncateresolveAlterColumnstripTableNamesFromColumnDefaultsoptimizeJoinspushFiltersapplyIndexesFromOuterScopepruneTablesassignExecIndexesinlineSubqueryAliasRefseraseProjectionflattenDistinctreplaceAggreplaceIdxSortinsertTopNNodesreplaceIdxOrderByDistanceapplyHashInresolveInsertRowsapplyTriggersapplyProceduresassignRoutinesmodifyUpdateExprsForJoinapplyUpdateAccumulatorswrapWithRollbackapplyForeignKeysvalidateResolvedvalidateOrderByvalidateGroupByvalidateSchemaSourcevalidateIndexCreationvalidateOperandsvalidateIntervalUsagevalidateSubqueryColumnsvalidateUnionSchemasMatchvalidateAggregationsvalidateDeleteFromcacheSubqueryAliasesInJoinsbacktickDefaultColumnValueNamesaddAutocommittrackProcessparallelize" +const _RuleId_name = "applyDefaultSelectLimitvalidateOffsetAndLimitvalidateStarExpressionsvalidateCreateTablevalidateAlterTablevalidateExprSemloadStoredProceduresvalidateDropTablesresolveDropConstraintvalidateDropConstraintresolveCreateSelectresolveSubqueriesresolveUnionsresolveDescribeQueryvalidateColumnDefaultsvalidateCreateTriggervalidateCreateProcedurevalidateReadOnlyDatabasevalidateReadOnlyTransactionvalidateDatabaseSetvalidatePrivilegesapplyEventSchedulerflattenTableAliasespushdownSubqueryAliasFiltersvalidateCheckConstraintsreplaceCountStarreplaceCrossJoinsmoveJoinConditionsToFiltersimplifyFilterspushNotFiltershoistOutOfScopeFiltersunnestInSubqueriesunnestExistsSubqueriesfinalizeSubqueriesfinalizeUnionsloadTriggersprocessTruncateresolveAlterColumnstripTableNamesFromColumnDefaultsoptimizeJoinspushFiltersapplyIndexesFromOuterScopepruneTablesassignExecIndexesinlineSubqueryAliasRefseraseProjectionflattenDistinctreplaceAggreplaceIdxSortinsertTopNNodesreplaceIdxOrderByDistanceapplyHashInresolveInsertRowsapplyTriggersapplyProceduresassignRoutinesmodifyUpdateExprsForJoinapplyUpdateAccumulatorswrapWithRollbackapplyForeignKeysvalidateResolvedvalidateOrderByvalidateGroupByvalidateSchemaSourcevalidateIndexCreationvalidateOperandsvalidateIntervalUsagevalidateSubqueryColumnsvalidateUnionSchemasMatchvalidateAggregationsvalidateDeleteFromcacheSubqueryAliasesInJoinsbacktickDefaultColumnValueNamestrackProcessparallelize" -var _RuleId_index = [...]uint16{0, 23, 45, 68, 87, 105, 120, 140, 158, 179, 201, 220, 237, 250, 270, 292, 313, 336, 360, 387, 406, 424, 443, 462, 490, 514, 530, 547, 573, 588, 602, 624, 642, 664, 682, 696, 708, 723, 741, 774, 787, 798, 824, 835, 852, 875, 890, 905, 915, 929, 944, 969, 980, 997, 1010, 1025, 1039, 1063, 1086, 1102, 1118, 1134, 1149, 1164, 1184, 1205, 1221, 1242, 1265, 1290, 1310, 1328, 1355, 1386, 1399, 1411, 1422} +var _RuleId_index = [...]uint16{0, 23, 45, 68, 87, 105, 120, 140, 158, 179, 201, 220, 237, 250, 270, 292, 313, 336, 360, 387, 406, 424, 443, 462, 490, 514, 530, 547, 573, 588, 602, 624, 642, 664, 682, 696, 708, 723, 741, 774, 787, 798, 824, 835, 852, 875, 890, 905, 915, 929, 944, 969, 980, 997, 1010, 1025, 1039, 1063, 1086, 1102, 1118, 1134, 1149, 1164, 1184, 1205, 1221, 1242, 1265, 1290, 1310, 1328, 1355, 1386, 1398, 1409} func (i RuleId) String() string { if i < 0 || i >= RuleId(len(_RuleId_index)-1) { From f4caf160faae00fb432938e347d9dffbf2fdc258 Mon Sep 17 00:00:00 2001 From: James Cor Date: Thu, 17 Oct 2024 16:16:54 -0700 Subject: [PATCH 04/12] fix deferred projections --- server/handler.go | 22 ++++++++++------------ sql/analyzer/analyzer.go | 1 - sql/plan/process.go | 6 ++++++ sql/plan/transaction_committing_iter.go | 21 +-------------------- sql/rowexec/expr_closer.go | 6 ++++++ sql/rowexec/transaction_iters.go | 2 +- 6 files changed, 24 insertions(+), 34 deletions(-) diff --git a/server/handler.go b/server/handler.go index 09d0940e0a..a6e0afd399 100644 --- a/server/handler.go +++ b/server/handler.go @@ -519,28 +519,26 @@ func resultForEmptyIter(ctx *sql.Context, iter sql.RowIter, resultFields []*quer func GetDeferredProjections(iter sql.RowIter) (sql.RowIter, []sql.Expression) { switch i := iter.(type) { case *rowexec.ExprCloserIter: - _, projs := GetDeferredProjections(i.GetIter()) - return i, projs + if newChild, projs := GetDeferredProjections(i.GetIter()); projs != nil { + return i.WithChildIter(newChild), projs + } case *plan.TrackedRowIter: - _, projs := GetDeferredProjections(i.GetIter()) - return i, projs + if newChild, projs := GetDeferredProjections(i.GetIter()); projs != nil { + return i.WithChildIter(newChild), projs + } case *rowexec.TransactionCommittingIter: - newChild, projs := GetDeferredProjections(i.GetIter()) - if projs != nil { - i.WithChildIter(newChild) + if newChild, projs := GetDeferredProjections(i.GetIter()); projs != nil { + return i.WithChildIter(newChild), projs } - return i, projs case *iters.LimitIter: - newChild, projs := GetDeferredProjections(i.ChildIter) - if projs != nil { + if newChild, projs := GetDeferredProjections(i.ChildIter); projs != nil { i.ChildIter = newChild + return i, projs } - return i, projs case *rowexec.ProjectIter: if i.CanDefer() { return i.GetChildIter(), i.GetProjections() } - return i, nil } return iter, nil } diff --git a/sql/analyzer/analyzer.go b/sql/analyzer/analyzer.go index 1ecc17bbad..712df34c3a 100644 --- a/sql/analyzer/analyzer.go +++ b/sql/analyzer/analyzer.go @@ -404,7 +404,6 @@ func NewProcRuleSelector(sel RuleSelector) RuleSelector { unnestInSubqueriesId, // once after default rules should only be run once - AutocommitId, TrackProcessId, parallelizeId: return false diff --git a/sql/plan/process.go b/sql/plan/process.go index 07b62b0fbb..db9e510288 100644 --- a/sql/plan/process.go +++ b/sql/plan/process.go @@ -402,6 +402,12 @@ func (i *TrackedRowIter) updateSessionVars(ctx *sql.Context) { } } +func (i *TrackedRowIter) WithChildIter(childIter sql.RowIter) sql.RowIter { + ni := *i + ni.iter = childIter + return &ni +} + type trackedPartitionIndexKeyValueIter struct { sql.PartitionIndexKeyValueIter OnPartitionDone NamedNotifyFunc diff --git a/sql/plan/transaction_committing_iter.go b/sql/plan/transaction_committing_iter.go index 80eda33487..281543d31c 100644 --- a/sql/plan/transaction_committing_iter.go +++ b/sql/plan/transaction_committing_iter.go @@ -15,24 +15,9 @@ package plan import ( - "os" - - "github.com/dolthub/go-mysql-server/sql" -) - -const ( - fakeReadCommittedEnvVar = "READ_COMMITTED_HACK" + "github.com/dolthub/go-mysql-server/sql" ) -var fakeReadCommitted bool - -func init() { - _, ok := os.LookupEnv(fakeReadCommittedEnvVar) - if ok { - fakeReadCommitted = true - } -} - // IsSessionAutocommit returns true if the current session is using implicit transaction management // through autocommit. func IsSessionAutocommit(ctx *sql.Context) (bool, error) { @@ -48,10 +33,6 @@ func IsSessionAutocommit(ctx *sql.Context) (bool, error) { } func ReadCommitted(ctx *sql.Context) bool { - if !fakeReadCommitted { - return false - } - val, err := ctx.GetSessionVariable(ctx, "transaction_isolation") if err != nil { return false diff --git a/sql/rowexec/expr_closer.go b/sql/rowexec/expr_closer.go index 0b4e43adb0..c37354fd0a 100644 --- a/sql/rowexec/expr_closer.go +++ b/sql/rowexec/expr_closer.go @@ -68,3 +68,9 @@ func (eci *ExprCloserIter) Close(ctx *sql.Context) error { func (eci *ExprCloserIter) GetIter() sql.RowIter { return eci.iter } + +func (eci *ExprCloserIter) WithChildIter(childIter sql.RowIter) sql.RowIter { + neci := *eci + neci.iter = childIter + return &neci + } diff --git a/sql/rowexec/transaction_iters.go b/sql/rowexec/transaction_iters.go index 921f4f0dd1..d9e46b4de6 100644 --- a/sql/rowexec/transaction_iters.go +++ b/sql/rowexec/transaction_iters.go @@ -129,6 +129,6 @@ func (t *TransactionCommittingIter) GetIter() sql.RowIter { func (t *TransactionCommittingIter) WithChildIter(childIter sql.RowIter) sql.RowIter { nt := *t - t.childIter = childIter + nt.childIter = childIter return &nt } From 77561cf793d674fcce25fe70e46c4bceb8fa0561 Mon Sep 17 00:00:00 2001 From: jycor Date: Thu, 17 Oct 2024 23:18:51 +0000 Subject: [PATCH 05/12] [ga-format-pr] Run ./format_repo.sh to fix formatting --- sql/plan/transaction_committing_iter.go | 2 +- sql/rowexec/expr_closer.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/sql/plan/transaction_committing_iter.go b/sql/plan/transaction_committing_iter.go index 281543d31c..6e540b80a0 100644 --- a/sql/plan/transaction_committing_iter.go +++ b/sql/plan/transaction_committing_iter.go @@ -15,7 +15,7 @@ package plan import ( - "github.com/dolthub/go-mysql-server/sql" + "github.com/dolthub/go-mysql-server/sql" ) // IsSessionAutocommit returns true if the current session is using implicit transaction management diff --git a/sql/rowexec/expr_closer.go b/sql/rowexec/expr_closer.go index c37354fd0a..32aff88bc0 100644 --- a/sql/rowexec/expr_closer.go +++ b/sql/rowexec/expr_closer.go @@ -73,4 +73,4 @@ func (eci *ExprCloserIter) WithChildIter(childIter sql.RowIter) sql.RowIter { neci := *eci neci.iter = childIter return &neci - } +} From 2e99c479114e143c4bf25233178a1333e1d31fc3 Mon Sep 17 00:00:00 2001 From: James Cor Date: Thu, 17 Oct 2024 16:51:56 -0700 Subject: [PATCH 06/12] more unused funcs --- enginetest/memory_engine_test.go | 18 +----------------- 1 file changed, 1 insertion(+), 17 deletions(-) diff --git a/enginetest/memory_engine_test.go b/enginetest/memory_engine_test.go index 204296e865..c6888e5d3e 100644 --- a/enginetest/memory_engine_test.go +++ b/enginetest/memory_engine_test.go @@ -30,8 +30,7 @@ import ( "github.com/dolthub/go-mysql-server/memory" "github.com/dolthub/go-mysql-server/sql" "github.com/dolthub/go-mysql-server/sql/expression" - "github.com/dolthub/go-mysql-server/sql/plan" - "github.com/dolthub/go-mysql-server/sql/types" + "github.com/dolthub/go-mysql-server/sql/types" _ "github.com/dolthub/go-mysql-server/sql/variables" ) @@ -196,13 +195,6 @@ func TestSingleQueryPrepared(t *testing.T) { enginetest.TestScriptWithEnginePrepared(t, engine, harness, test) } -func newUpdateResult(matched, updated int) types.OkResult { - return types.OkResult{ - RowsAffected: uint64(updated), - Info: plan.UpdateInfo{Matched: matched, Updated: updated}, - } -} - // Convenience test for debugging a single query. Unskip and set to the desired query. func TestSingleScript(t *testing.T) { t.Skip() @@ -1065,14 +1057,6 @@ func findTable(dbs []sql.Database, tableName string) (sql.Database, sql.Table) { return nil, nil } -func mergeSetupScripts(scripts ...setup.SetupScript) []string { - var all []string - for _, s := range scripts { - all = append(all, s...) - } - return all -} - func TestSQLLogicTests(t *testing.T) { enginetest.TestSQLLogicTests(t, enginetest.NewMemoryHarness("default", 1, testNumPartitions, true, mergableIndexDriver)) } From 6426aeea8caaaf1c05ac8d31346c2ca7e1a167b5 Mon Sep 17 00:00:00 2001 From: James Cor Date: Mon, 21 Oct 2024 10:19:03 -0700 Subject: [PATCH 07/12] place transaction iters under tracked iters --- sql/rowexec/transaction_iters.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/sql/rowexec/transaction_iters.go b/sql/rowexec/transaction_iters.go index d9e46b4de6..ee3ad8da23 100644 --- a/sql/rowexec/transaction_iters.go +++ b/sql/rowexec/transaction_iters.go @@ -79,6 +79,11 @@ func AddTransactionCommittingIter(child sql.RowIter, qFlags *sql.QueryFlags) sql if qFlags != nil && qFlags.IsSet(sql.QFlagShowWarnings) { return child } + // TODO: remove this once trackedRowIter is moved out of planbuilder + // Insert TransactionCommittingIter as child of TrackedRowIter + if trackedRowIter, ok := child.(*plan.TrackedRowIter); ok { + return trackedRowIter.WithChildIter(&TransactionCommittingIter{childIter: trackedRowIter.GetIter()}) + } return &TransactionCommittingIter{childIter: child} } From 8232f84ae7e4a836432f8fb48443702df060b6e4 Mon Sep 17 00:00:00 2001 From: jycor Date: Mon, 21 Oct 2024 17:20:24 +0000 Subject: [PATCH 08/12] [ga-format-pr] Run ./format_repo.sh to fix formatting --- enginetest/memory_engine_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/enginetest/memory_engine_test.go b/enginetest/memory_engine_test.go index c6888e5d3e..1c075f88ed 100644 --- a/enginetest/memory_engine_test.go +++ b/enginetest/memory_engine_test.go @@ -30,7 +30,7 @@ import ( "github.com/dolthub/go-mysql-server/memory" "github.com/dolthub/go-mysql-server/sql" "github.com/dolthub/go-mysql-server/sql/expression" - "github.com/dolthub/go-mysql-server/sql/types" + "github.com/dolthub/go-mysql-server/sql/types" _ "github.com/dolthub/go-mysql-server/sql/variables" ) From 28368a892995fe3a8529735b5644a7ff16009ae6 Mon Sep 17 00:00:00 2001 From: James Cor Date: Mon, 21 Oct 2024 10:54:14 -0700 Subject: [PATCH 09/12] rules --- sql/analyzer/rule_ids.go | 2 +- sql/analyzer/ruleid_string.go | 2 +- sql/analyzer/rules.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/sql/analyzer/rule_ids.go b/sql/analyzer/rule_ids.go index 84d8da5d11..1c37156b98 100644 --- a/sql/analyzer/rule_ids.go +++ b/sql/analyzer/rule_ids.go @@ -86,7 +86,7 @@ const ( // after all cacheSubqueryAliasesInJoinsId // cacheSubqueryAliasesInJoins - backtickDefaulColumnValueNamesId // backtickDefaultColumnValueNames + BacktickDefaulColumnValueNamesId // backtickDefaultColumnValueNames TrackProcessId // trackProcess parallelizeId // parallelize ) diff --git a/sql/analyzer/ruleid_string.go b/sql/analyzer/ruleid_string.go index 7d1c4d8d8b..5446095fdd 100755 --- a/sql/analyzer/ruleid_string.go +++ b/sql/analyzer/ruleid_string.go @@ -80,7 +80,7 @@ func _() { _ = x[validateAggregationsId-69] _ = x[validateDeleteFromId-70] _ = x[cacheSubqueryAliasesInJoinsId-71] - _ = x[backtickDefaulColumnValueNamesId-72] + _ = x[BacktickDefaulColumnValueNamesId-72] _ = x[TrackProcessId-73] _ = x[parallelizeId-74] } diff --git a/sql/analyzer/rules.go b/sql/analyzer/rules.go index 221151c1f2..ac5826485e 100644 --- a/sql/analyzer/rules.go +++ b/sql/analyzer/rules.go @@ -25,7 +25,7 @@ func init() { {wrapWithRollbackId, wrapWithRollback}, {inlineSubqueryAliasRefsId, inlineSubqueryAliasRefs}, {cacheSubqueryAliasesInJoinsId, cacheSubqueryAliasesInJoins}, - {backtickDefaulColumnValueNamesId, backtickDefaultColumnValueNames}, + {BacktickDefaulColumnValueNamesId, backtickDefaultColumnValueNames}, {TrackProcessId, trackProcess}, {parallelizeId, parallelize}, } From 0a4e203b1cb3eeec7452d4e4d166a058dcbf58d0 Mon Sep 17 00:00:00 2001 From: James Cor Date: Mon, 21 Oct 2024 13:54:09 -0700 Subject: [PATCH 10/12] asdfasdf --- sql/types/typecheck_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/sql/types/typecheck_test.go b/sql/types/typecheck_test.go index 534af061fa..ea8bdc7305 100644 --- a/sql/types/typecheck_test.go +++ b/sql/types/typecheck_test.go @@ -17,9 +17,8 @@ package types import ( "testing" - "github.com/stretchr/testify/assert" - "github.com/dolthub/go-mysql-server/sql" + "github.com/stretchr/testify/assert" ) func TestIsGeometry(t *testing.T) { From 94334f66bb338451ca8b61b783207498cf4f13b1 Mon Sep 17 00:00:00 2001 From: James Cor Date: Mon, 21 Oct 2024 14:36:56 -0700 Subject: [PATCH 11/12] finalize everywhere --- engine.go | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/engine.go b/engine.go index bd43fe244a..faee6d5dbb 100644 --- a/engine.go +++ b/engine.go @@ -447,8 +447,7 @@ func (e *Engine) QueryWithBindings(ctx *sql.Context, query string, parsed sqlpar return nil, nil, nil, err } - iter = rowexec.AddTransactionCommittingIter(iter, qFlags) - iter = rowexec.AddExpressionCloser(analyzed, iter) + iter = finalizeIters(analyzed, qFlags, iter) return analyzed.Schema(), iter, qFlags, nil } @@ -482,7 +481,8 @@ func (e *Engine) PrepQueryPlanForExecution(ctx *sql.Context, _ string, plan sql. return nil, nil, nil, err } - iter = rowexec.AddExpressionCloser(plan, iter) + + iter = finalizeIters(plan, nil, iter) return plan.Schema(), iter, nil, nil } @@ -829,7 +829,8 @@ func (e *Engine) executeEvent(ctx *sql.Context, dbName, createEventStatement, us } return err } - iter = rowexec.AddExpressionCloser(definitionNode, iter) + + iter = finalizeIters(definitionNode, nil, iter) // Drain the iterate to execute the event body/definition // NOTE: No row data is returned for an event; we just need to execute the statements @@ -862,3 +863,10 @@ func findCreateEventNode(planTree sql.Node) (*plan.CreateEvent, error) { return createEventNode, nil } + +// finalizeIters applies the final transformations on sql.RowIter before execution. +func finalizeIters(analyzed sql.Node, qFlags *sql.QueryFlags, iter sql.RowIter) sql.RowIter { + iter = rowexec.AddTransactionCommittingIter(iter, qFlags) + iter = rowexec.AddExpressionCloser(analyzed, iter) + return iter +} From 23b6055fc7916e726578f0b790f0d10b8046072c Mon Sep 17 00:00:00 2001 From: jycor Date: Mon, 21 Oct 2024 21:41:33 +0000 Subject: [PATCH 12/12] [ga-format-pr] Run ./format_repo.sh to fix formatting --- sql/types/typecheck_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sql/types/typecheck_test.go b/sql/types/typecheck_test.go index ea8bdc7305..534af061fa 100644 --- a/sql/types/typecheck_test.go +++ b/sql/types/typecheck_test.go @@ -17,8 +17,9 @@ package types import ( "testing" - "github.com/dolthub/go-mysql-server/sql" "github.com/stretchr/testify/assert" + + "github.com/dolthub/go-mysql-server/sql" ) func TestIsGeometry(t *testing.T) {