From 9439a964e0048efd57ed0b21fd74812299875d61 Mon Sep 17 00:00:00 2001 From: Jason Fulghum Date: Thu, 28 Jul 2022 14:52:08 -0700 Subject: [PATCH 1/4] Improving error message when a stored procedure isn't found and there is no database selected --- sql/analyzer/stored_procedures.go | 7 +++- sql/analyzer/stored_procedures_test.go | 44 ++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 1 deletion(-) create mode 100644 sql/analyzer/stored_procedures_test.go diff --git a/sql/analyzer/stored_procedures.go b/sql/analyzer/stored_procedures.go index 25d8156ed5..e317810393 100644 --- a/sql/analyzer/stored_procedures.go +++ b/sql/analyzer/stored_procedures.go @@ -378,7 +378,12 @@ func applyProceduresCall(ctx *sql.Context, a *Analyzer, call *plan.Call, scope * procedure := scope.procedures.Get(ctx.GetCurrentDatabase(), call.Name, len(call.Params)) if procedure == nil { - return nil, transform.SameTree, sql.ErrStoredProcedureDoesNotExist.New(call.Name) + err := sql.ErrStoredProcedureDoesNotExist.New(call.Name) + if ctx.GetCurrentDatabase() == "" { + cause := errors.NewKind("this might be because no database is selected") + err = sql.ErrStoredProcedureDoesNotExist.Wrap(cause.New(), call.Name) + } + return nil, transform.SameTree, err } if procedure.HasVariadicParameter() { procedure = procedure.ExtendVariadic(ctx, len(call.Params)) diff --git a/sql/analyzer/stored_procedures_test.go b/sql/analyzer/stored_procedures_test.go new file mode 100644 index 0000000000..daffb86afa --- /dev/null +++ b/sql/analyzer/stored_procedures_test.go @@ -0,0 +1,44 @@ +// 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 ( + "context" + "github.com/stretchr/testify/require" + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/dolthub/go-mysql-server/memory" + "github.com/dolthub/go-mysql-server/sql" + "github.com/dolthub/go-mysql-server/sql/plan" + "github.com/dolthub/go-mysql-server/sql/transform" +) + +func TestStoredProcedureNotFoundWithNoDatabaseSelected(t *testing.T) { + db := memory.NewDatabase("mydb") + analyzer := NewBuilder(sql.NewDatabaseProvider(db)).Build() + ctx := sql.NewContext(context.Background(), sql.WithSession(sql.NewBaseSession())) + + call := plan.NewCall("non_existent_procedure", []sql.Expression{}) + scope, err := loadStoredProcedures(ctx, analyzer, call, newScope(call), DefaultRuleSelector) + require.NoError(t, err) + + node, identity, err := applyProceduresCall(ctx, analyzer, call, scope, DefaultRuleSelector) + assert.Nil(t, node) + assert.Equal(t, transform.SameTree, identity) + assert.True(t, sql.ErrStoredProcedureDoesNotExist.Is(err)) + assert.Contains(t, err.Error(), "this might be because no database is selected") +} From 5c350c05822b32e5039aed5c98a546f7c0fa8cf9 Mon Sep 17 00:00:00 2001 From: fulghum Date: Thu, 28 Jul 2022 21:56:08 +0000 Subject: [PATCH 2/4] [ga-format-pr] Run ./format_repo.sh to fix formatting --- sql/analyzer/stored_procedures_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sql/analyzer/stored_procedures_test.go b/sql/analyzer/stored_procedures_test.go index daffb86afa..82713da665 100644 --- a/sql/analyzer/stored_procedures_test.go +++ b/sql/analyzer/stored_procedures_test.go @@ -16,10 +16,10 @@ package analyzer import ( "context" - "github.com/stretchr/testify/require" "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/dolthub/go-mysql-server/memory" "github.com/dolthub/go-mysql-server/sql" From db5e15c8c36d3ab842a0d80486fb6e4ea4083a07 Mon Sep 17 00:00:00 2001 From: Jason Fulghum Date: Thu, 28 Jul 2022 15:50:04 -0700 Subject: [PATCH 3/4] Cleaned up error message to be more idiomatic, from Max's feedback --- sql/analyzer/stored_procedures.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/sql/analyzer/stored_procedures.go b/sql/analyzer/stored_procedures.go index e317810393..ee8b4a0190 100644 --- a/sql/analyzer/stored_procedures.go +++ b/sql/analyzer/stored_procedures.go @@ -380,8 +380,7 @@ func applyProceduresCall(ctx *sql.Context, a *Analyzer, call *plan.Call, scope * if procedure == nil { err := sql.ErrStoredProcedureDoesNotExist.New(call.Name) if ctx.GetCurrentDatabase() == "" { - cause := errors.NewKind("this might be because no database is selected") - err = sql.ErrStoredProcedureDoesNotExist.Wrap(cause.New(), call.Name) + return nil, transform.SameTree, fmt.Errorf("%w; this might be because no database is selected", err) } return nil, transform.SameTree, err } From baa47ce1ca286d33960a4efc62eeb537a868aea8 Mon Sep 17 00:00:00 2001 From: Jason Fulghum Date: Thu, 28 Jul 2022 16:00:53 -0700 Subject: [PATCH 4/4] Updating test from error change --- sql/analyzer/stored_procedures_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sql/analyzer/stored_procedures_test.go b/sql/analyzer/stored_procedures_test.go index 82713da665..e75a925622 100644 --- a/sql/analyzer/stored_procedures_test.go +++ b/sql/analyzer/stored_procedures_test.go @@ -39,6 +39,6 @@ func TestStoredProcedureNotFoundWithNoDatabaseSelected(t *testing.T) { node, identity, err := applyProceduresCall(ctx, analyzer, call, scope, DefaultRuleSelector) assert.Nil(t, node) assert.Equal(t, transform.SameTree, identity) - assert.True(t, sql.ErrStoredProcedureDoesNotExist.Is(err)) + assert.Contains(t, err.Error(), "stored procedure \"non_existent_procedure\" does not exist") assert.Contains(t, err.Error(), "this might be because no database is selected") }