Skip to content

Commit 6f8e440

Browse files
author
James Cor
committed
move clear warnings out of analyzer into engine
1 parent 52bb933 commit 6f8e440

File tree

8 files changed

+32
-58
lines changed

8 files changed

+32
-58
lines changed

engine.go

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,23 @@ func (e *Engine) Query(ctx *sql.Context, query string) (sql.Schema, sql.RowIter,
253253
return e.QueryWithBindings(ctx, query, nil, nil, nil)
254254
}
255255

256+
func clearWarnings(ctx *sql.Context, node sql.Node) {
257+
if ctx == nil || ctx.Session == nil {
258+
return
259+
}
260+
261+
switch n := node.(type) {
262+
case *plan.Offset, *plan.Limit:
263+
// `show warning limit x offset y` is valid, so we need to recurse
264+
clearWarnings(ctx, n.Children()[0])
265+
case plan.ShowWarnings:
266+
// ShowWarnings should not clear the warnings, but should still reset the warning count.
267+
ctx.ClearWarningCount()
268+
default:
269+
ctx.ClearWarnings()
270+
}
271+
}
272+
256273
func bindingsToExprs(bindings map[string]*querypb.BindVariable) (map[string]sql.Expression, error) {
257274
res := make(map[string]sql.Expression, len(bindings))
258275
for k, v := range bindings {
@@ -387,10 +404,18 @@ func (e *Engine) QueryWithBindings(ctx *sql.Context, query string, parsed sqlpar
387404
return nil, nil, nil, err
388405
}
389406

407+
// planbuilding can produce warnings, so we need to preserve them
408+
numPrevWarnings := len(ctx.Session.Warnings())
390409
bound, qFlags, err := e.bindQuery(ctx, query, parsed, bindings, binder, qFlags)
391410
if err != nil {
392411
return nil, nil, nil, err
393412
}
413+
newWarnings := ctx.Session.Warnings()[numPrevWarnings:]
414+
clearWarnings(ctx, bound)
415+
// restore new warnings (backwards because they are in reverse order)
416+
for i := len(newWarnings) - 1; i >= 0; i-- {
417+
ctx.Session.Warn(newWarnings[i])
418+
}
394419

395420
analyzed, err := e.analyzeNode(ctx, query, bound, qFlags)
396421
if err != nil {

enginetest/queries/script_queries.go

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8809,7 +8809,7 @@ var CreateDatabaseScripts = []ScriptTest{
88098809
Expected: []sql.Row{{types.NewOkResult(1)}},
88108810
},
88118811
{
8812-
SkipResultCheckOnServerEngine: true, // tracking issue here, https://github.com/dolthub/dolt/issues/6921. Also for when run with prepares, the warning is added twice
8812+
//SkipResultCheckOnServerEngine: true, // tracking issue here, https://github.com/dolthub/dolt/issues/6921. Also for when run with prepares, the warning is added twice
88138813
Query: "SHOW WARNINGS /* 1 */",
88148814
Expected: []sql.Row{{"Warning", 1235, "Setting CHARACTER SET, COLLATION and ENCRYPTION are not supported yet"}},
88158815
},
@@ -8818,12 +8818,11 @@ var CreateDatabaseScripts = []ScriptTest{
88188818
Expected: []sql.Row{{types.NewOkResult(1)}},
88198819
},
88208820
{
8821-
SkipResultCheckOnServerEngine: true, // tracking issue here, https://github.com/dolthub/dolt/issues/6921.
8821+
//SkipResultCheckOnServerEngine: true, // tracking issue here, https://github.com/dolthub/dolt/issues/6921.
88228822
// 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
88238823
Query: "SHOW WARNINGS /* 2 */",
88248824
Expected: []sql.Row{
88258825
{"Warning", 1235, "Setting CHARACTER SET, COLLATION and ENCRYPTION are not supported yet"},
8826-
{"Warning", 1235, "Setting CHARACTER SET, COLLATION and ENCRYPTION are not supported yet"},
88278826
},
88288827
},
88298828
{
@@ -8939,10 +8938,8 @@ var DropDatabaseScripts = []ScriptTest{
89398938
Expected: []sql.Row{{types.OkResult{RowsAffected: 1}}},
89408939
},
89418940
{
8942-
// TODO: there should not be warning
8943-
// https://github.com/dolthub/dolt/issues/6921
89448941
Query: "SHOW WARNINGS",
8945-
Expected: []sql.Row{{"Note", 1008, "Can't drop database mydb; database doesn't exist "}},
8942+
Expected: []sql.Row{},
89468943
},
89478944
{
89488945
Query: "SELECT DATABASE()",

sql/analyzer/analyzer.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -406,8 +406,7 @@ func NewProcRuleSelector(sel RuleSelector) RuleSelector {
406406
// once after default rules should only be run once
407407
AutocommitId,
408408
TrackProcessId,
409-
parallelizeId,
410-
clearWarningsId:
409+
parallelizeId:
411410
return false
412411
}
413412
return sel(id)

sql/analyzer/rule_ids.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,5 +90,4 @@ const (
9090
AutocommitId // addAutocommit
9191
TrackProcessId // trackProcess
9292
parallelizeId // parallelize
93-
clearWarningsId // clearWarnings
9493
)

sql/analyzer/ruleid_string.go

Lines changed: 2 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

sql/analyzer/rules.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@ func init() {
2929
{AutocommitId, addAutocommit},
3030
{TrackProcessId, trackProcess},
3131
{parallelizeId, parallelize},
32-
{clearWarningsId, clearWarnings},
3332
}
3433
}
3534

sql/analyzer/warnings.go

Lines changed: 0 additions & 44 deletions
This file was deleted.

sql/session.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ type Session interface {
104104
Warn(warn *Warning)
105105
// Warnings returns a copy of session warnings (from the most recent).
106106
Warnings() []*Warning
107-
// ClearWarnCount clears the warning count without clearing the actual warnings.
107+
// ClearWarningCount clears the warning count without clearing the actual warnings.
108108
ClearWarningCount()
109109
// ClearWarnings cleans up session warnings.
110110
ClearWarnings()

0 commit comments

Comments
 (0)