diff --git a/engine.go b/engine.go index 6fe863ea98..5285ec83cd 100644 --- a/engine.go +++ b/engine.go @@ -253,6 +253,23 @@ func (e *Engine) Query(ctx *sql.Context, query string) (sql.Schema, sql.RowIter, return e.QueryWithBindings(ctx, query, nil, nil, nil) } +func clearWarnings(ctx *sql.Context, node sql.Node) { + if ctx == nil || ctx.Session == nil { + return + } + + switch n := node.(type) { + case *plan.Offset, *plan.Limit: + // `show warning limit x offset y` is valid, so we need to recurse + clearWarnings(ctx, n.Children()[0]) + case plan.ShowWarnings: + // ShowWarnings should not clear the warnings, but should still reset the warning count. + ctx.ClearWarningCount() + default: + ctx.ClearWarnings() + } +} + func bindingsToExprs(bindings map[string]*querypb.BindVariable) (map[string]sql.Expression, error) { res := make(map[string]sql.Expression, len(bindings)) for k, v := range bindings { @@ -387,10 +404,18 @@ func (e *Engine) QueryWithBindings(ctx *sql.Context, query string, parsed sqlpar return nil, nil, nil, err } + // planbuilding can produce warnings, so we need to preserve them + numPrevWarnings := len(ctx.Session.Warnings()) bound, qFlags, err := e.bindQuery(ctx, query, parsed, bindings, binder, qFlags) if err != nil { return nil, nil, nil, err } + newWarnings := ctx.Session.Warnings()[numPrevWarnings:] + clearWarnings(ctx, bound) + // restore new warnings (backwards because they are in reverse order) + for i := len(newWarnings) - 1; i >= 0; i-- { + ctx.Session.Warn(newWarnings[i]) + } analyzed, err := e.analyzeNode(ctx, query, bound, qFlags) if err != nil { diff --git a/enginetest/queries/script_queries.go b/enginetest/queries/script_queries.go index c9965a80c6..b6dc976315 100644 --- a/enginetest/queries/script_queries.go +++ b/enginetest/queries/script_queries.go @@ -8809,21 +8809,17 @@ var CreateDatabaseScripts = []ScriptTest{ Expected: []sql.Row{{types.NewOkResult(1)}}, }, { - SkipResultCheckOnServerEngine: true, // tracking issue here, https://github.com/dolthub/dolt/issues/6921. Also for when run with prepares, the warning is added twice - Query: "SHOW WARNINGS /* 1 */", - Expected: []sql.Row{{"Warning", 1235, "Setting CHARACTER SET, COLLATION and ENCRYPTION are not supported yet"}}, + Query: "SHOW WARNINGS /* 1 */", + Expected: []sql.Row{{"Warning", 1235, "Setting CHARACTER SET, COLLATION and ENCRYPTION are not supported yet"}}, }, { Query: "CREATE DATABASE newtest1db DEFAULT COLLATE binary ENCRYPTION='Y'", Expected: []sql.Row{{types.NewOkResult(1)}}, }, { - SkipResultCheckOnServerEngine: true, // tracking issue here, https://github.com/dolthub/dolt/issues/6921. - // TODO: There should only be one warning (the warnings are not clearing for create database query) AND 'PREPARE' statements should not create warning from its query Query: "SHOW WARNINGS /* 2 */", Expected: []sql.Row{ {"Warning", 1235, "Setting CHARACTER SET, COLLATION and ENCRYPTION are not supported yet"}, - {"Warning", 1235, "Setting CHARACTER SET, COLLATION and ENCRYPTION are not supported yet"}, }, }, { @@ -8939,10 +8935,8 @@ var DropDatabaseScripts = []ScriptTest{ Expected: []sql.Row{{types.OkResult{RowsAffected: 1}}}, }, { - // TODO: there should not be warning - // https://github.com/dolthub/dolt/issues/6921 Query: "SHOW WARNINGS", - Expected: []sql.Row{{"Note", 1008, "Can't drop database mydb; database doesn't exist "}}, + Expected: []sql.Row{}, }, { Query: "SELECT DATABASE()", diff --git a/sql/analyzer/analyzer.go b/sql/analyzer/analyzer.go index c33f426ee5..227d3aafd7 100644 --- a/sql/analyzer/analyzer.go +++ b/sql/analyzer/analyzer.go @@ -406,8 +406,7 @@ func NewProcRuleSelector(sel RuleSelector) RuleSelector { // once after default rules should only be run once AutocommitId, TrackProcessId, - parallelizeId, - clearWarningsId: + parallelizeId: return false } return sel(id) diff --git a/sql/analyzer/rule_ids.go b/sql/analyzer/rule_ids.go index 5c5d01b173..dc4ed77194 100644 --- a/sql/analyzer/rule_ids.go +++ b/sql/analyzer/rule_ids.go @@ -90,5 +90,4 @@ const ( AutocommitId // addAutocommit TrackProcessId // trackProcess parallelizeId // parallelize - clearWarningsId // clearWarnings ) diff --git a/sql/analyzer/ruleid_string.go b/sql/analyzer/ruleid_string.go index 0d12dd8320..79186254c9 100755 --- a/sql/analyzer/ruleid_string.go +++ b/sql/analyzer/ruleid_string.go @@ -84,12 +84,11 @@ func _() { _ = x[AutocommitId-73] _ = x[TrackProcessId-74] _ = x[parallelizeId-75] - _ = x[clearWarningsId-76] } -const _RuleId_name = "applyDefaultSelectLimitvalidateOffsetAndLimitvalidateStarExpressionsvalidateCreateTablevalidateAlterTablevalidateExprSemloadStoredProceduresvalidateDropTablesresolveDropConstraintvalidateDropConstraintresolveCreateSelectresolveSubqueriesresolveUnionsresolveDescribeQueryvalidateColumnDefaultsvalidateCreateTriggervalidateCreateProcedurevalidateReadOnlyDatabasevalidateReadOnlyTransactionvalidateDatabaseSetvalidatePrivilegesapplyEventSchedulerflattenTableAliasespushdownSubqueryAliasFiltersvalidateCheckConstraintsreplaceCountStarreplaceCrossJoinsmoveJoinConditionsToFiltersimplifyFilterspushNotFiltershoistOutOfScopeFiltersunnestInSubqueriesunnestExistsSubqueriesfinalizeSubqueriesfinalizeUnionsloadTriggersprocessTruncateresolveAlterColumnstripTableNamesFromColumnDefaultsoptimizeJoinspushFiltersapplyIndexesFromOuterScopepruneTablesassignExecIndexesinlineSubqueryAliasRefseraseProjectionflattenDistinctreplaceAggreplaceIdxSortinsertTopNNodesreplaceIdxOrderByDistanceapplyHashInresolveInsertRowsapplyTriggersapplyProceduresassignRoutinesmodifyUpdateExprsForJoinapplyUpdateAccumulatorswrapWithRollbackapplyForeignKeysvalidateResolvedvalidateOrderByvalidateGroupByvalidateSchemaSourcevalidateIndexCreationvalidateOperandsvalidateIntervalUsagevalidateSubqueryColumnsvalidateUnionSchemasMatchvalidateAggregationsvalidateDeleteFromcacheSubqueryAliasesInJoinsbacktickDefaultColumnValueNamesaddAutocommittrackProcessparallelizeclearWarnings" +const _RuleId_name = "applyDefaultSelectLimitvalidateOffsetAndLimitvalidateStarExpressionsvalidateCreateTablevalidateAlterTablevalidateExprSemloadStoredProceduresvalidateDropTablesresolveDropConstraintvalidateDropConstraintresolveCreateSelectresolveSubqueriesresolveUnionsresolveDescribeQueryvalidateColumnDefaultsvalidateCreateTriggervalidateCreateProcedurevalidateReadOnlyDatabasevalidateReadOnlyTransactionvalidateDatabaseSetvalidatePrivilegesapplyEventSchedulerflattenTableAliasespushdownSubqueryAliasFiltersvalidateCheckConstraintsreplaceCountStarreplaceCrossJoinsmoveJoinConditionsToFiltersimplifyFilterspushNotFiltershoistOutOfScopeFiltersunnestInSubqueriesunnestExistsSubqueriesfinalizeSubqueriesfinalizeUnionsloadTriggersprocessTruncateresolveAlterColumnstripTableNamesFromColumnDefaultsoptimizeJoinspushFiltersapplyIndexesFromOuterScopepruneTablesassignExecIndexesinlineSubqueryAliasRefseraseProjectionflattenDistinctreplaceAggreplaceIdxSortinsertTopNNodesreplaceIdxOrderByDistanceapplyHashInresolveInsertRowsapplyTriggersapplyProceduresassignRoutinesmodifyUpdateExprsForJoinapplyUpdateAccumulatorswrapWithRollbackapplyForeignKeysvalidateResolvedvalidateOrderByvalidateGroupByvalidateSchemaSourcevalidateIndexCreationvalidateOperandsvalidateIntervalUsagevalidateSubqueryColumnsvalidateUnionSchemasMatchvalidateAggregationsvalidateDeleteFromcacheSubqueryAliasesInJoinsbacktickDefaultColumnValueNamesaddAutocommittrackProcessparallelize" -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, 1435} +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} func (i RuleId) String() string { if i < 0 || i >= RuleId(len(_RuleId_index)-1) { diff --git a/sql/analyzer/rules.go b/sql/analyzer/rules.go index 85cd895a34..53b7a01647 100644 --- a/sql/analyzer/rules.go +++ b/sql/analyzer/rules.go @@ -29,7 +29,6 @@ func init() { {AutocommitId, addAutocommit}, {TrackProcessId, trackProcess}, {parallelizeId, parallelize}, - {clearWarningsId, clearWarnings}, } } diff --git a/sql/analyzer/warnings.go b/sql/analyzer/warnings.go deleted file mode 100644 index 87807eb377..0000000000 --- a/sql/analyzer/warnings.go +++ /dev/null @@ -1,42 +0,0 @@ -// Copyright 2020-2021 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" -) - -func clearWarnings(ctx *sql.Context, a *Analyzer, node sql.Node, scope *plan.Scope, sel RuleSelector, qFlags *sql.QueryFlags) (sql.Node, transform.TreeIdentity, error) { - children := node.Children() - if len(children) == 0 { - return node, transform.SameTree, nil - } - - switch ch := children[0].(type) { - case plan.ShowWarnings: - return node, transform.SameTree, nil - case *plan.Offset: - clearWarnings(ctx, a, ch, scope, sel, qFlags) - return node, transform.SameTree, nil - case *plan.Limit: - clearWarnings(ctx, a, ch, scope, sel, qFlags) - return node, transform.SameTree, nil - } - - ctx.ClearWarnings() - return node, transform.SameTree, nil -} diff --git a/sql/base_session.go b/sql/base_session.go index 6a4d6a3207..e08e642325 100644 --- a/sql/base_session.go +++ b/sql/base_session.go @@ -41,7 +41,7 @@ type BaseSession struct { viewReg *ViewRegistry warnings []*Warning warningLock bool - warncnt uint16 + warningCount uint16 // num of warnings from recent query; does not always equal to len(warnings) locks map[string]bool queriedDb string lastQueryInfo map[string]*atomic.Value @@ -331,6 +331,7 @@ func (s *BaseSession) SetConnectionId(id uint32) { // Warn stores the warning in the session. func (s *BaseSession) Warn(warn *Warning) { s.warnings = append(s.warnings, warn) + s.warningCount = uint16(len(s.warnings)) } // Warnings returns a copy of session warnings (from the most recent - the last one) @@ -354,25 +355,25 @@ func (s *BaseSession) UnlockWarnings() { s.warningLock = false } +// ClearWarningCount cleans up session warnings +func (s *BaseSession) ClearWarningCount() { + s.warningCount = 0 +} + // ClearWarnings cleans up session warnings func (s *BaseSession) ClearWarnings() { if s.warningLock { return } - cnt := uint16(len(s.warnings)) - if s.warncnt != cnt { - s.warncnt = cnt - return - } if s.warnings != nil { s.warnings = s.warnings[:0] } - s.warncnt = 0 + s.ClearWarningCount() } // WarningCount returns a number of session warnings func (s *BaseSession) WarningCount() uint16 { - return uint16(len(s.warnings)) + return s.warningCount } // AddLock adds a lock to the set of locks owned by this user which will need to be released if this session terminates diff --git a/sql/session.go b/sql/session.go index f0a4c10ead..ec73bea4c7 100644 --- a/sql/session.go +++ b/sql/session.go @@ -104,6 +104,8 @@ type Session interface { Warn(warn *Warning) // Warnings returns a copy of session warnings (from the most recent). Warnings() []*Warning + // ClearWarningCount clears the warning count without clearing the actual warnings. + ClearWarningCount() // ClearWarnings cleans up session warnings. ClearWarnings() // WarningCount returns a number of session warnings