From 5f71687e5ab9d56198fec24286c47b981d18d3ca Mon Sep 17 00:00:00 2001 From: Jason Fulghum Date: Fri, 18 Oct 2024 15:56:41 -0700 Subject: [PATCH 1/2] Changing selectExprNeedsAlias to consider string literal quotes --- sql/planbuilder/project.go | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/sql/planbuilder/project.go b/sql/planbuilder/project.go index 35a42c14a3..512a40906b 100644 --- a/sql/planbuilder/project.go +++ b/sql/planbuilder/project.go @@ -243,5 +243,14 @@ func selectExprNeedsAlias(e *ast.AliasedExpr, expr sql.Expression) bool { } }) - return complex || e.InputExpression != expr.String() + // If the expression's string representation is quoted, trim the quotes before comparing it to the input expression. + // InputExpression is assigned in the Vitess layer, and it always trims quotes at that time, too. + exprString := expr.String() + if strings.HasPrefix(exprString, "'") && strings.HasSuffix(exprString, "'") { + exprString = exprString[1 : len(exprString)-1] + } + + // If the expression's input value matches expr.String(), then we know that it is referenceable and + // does not need an alias. + return complex || e.InputExpression != exprString } From 64ebafee52251f5a5791b4a6dba96b6ed6f05abb Mon Sep 17 00:00:00 2001 From: fulghum Date: Mon, 21 Oct 2024 18:03:05 +0000 Subject: [PATCH 2/2] [ga-format-pr] Run ./format_repo.sh to fix formatting --- enginetest/queries/query_plans.go | 20 ++++++++++---------- sql/plan/str_expr.go | 9 +++++++++ sql/planbuilder/project.go | 9 ++++++--- sql/types/typecheck_test.go | 3 ++- 4 files changed, 27 insertions(+), 14 deletions(-) diff --git a/enginetest/queries/query_plans.go b/enginetest/queries/query_plans.go index 769c87a5f5..15703e2398 100644 --- a/enginetest/queries/query_plans.go +++ b/enginetest/queries/query_plans.go @@ -4695,7 +4695,7 @@ Select * from ( { Query: `SELECT count(*), i, concat(i, i), 123, 'abc', concat('abc', 'def') FROM emptytable;`, ExpectedPlan: "Project\n" + - " ├─ columns: [count(1):0!null as count(*), emptytable.i:1!null, concat(emptytable.i:1!null,emptytable.i:1!null) as concat(i, i), 123 (tinyint), abc (longtext) as abc, concat(abc (longtext),def (longtext)) as concat('abc', 'def')]\n" + + " ├─ columns: [count(1):0!null as count(*), emptytable.i:1!null, concat(emptytable.i:1!null,emptytable.i:1!null) as concat(i, i), 123 (tinyint), abc (longtext), concat(abc (longtext),def (longtext)) as concat('abc', 'def')]\n" + " └─ GroupBy\n" + " ├─ select: COUNT(1 (bigint)), emptytable.i:0!null\n" + " ├─ group: \n" + @@ -4705,7 +4705,7 @@ Select * from ( " └─ columns: [i]\n" + "", ExpectedEstimates: "Project\n" + - " ├─ columns: [count(1) as count(*), emptytable.i, concat(emptytable.i,emptytable.i) as concat(i, i), 123, 'abc' as abc, concat('abc','def') as concat('abc', 'def')]\n" + + " ├─ columns: [count(1) as count(*), emptytable.i, concat(emptytable.i,emptytable.i) as concat(i, i), 123, 'abc', concat('abc','def') as concat('abc', 'def')]\n" + " └─ GroupBy\n" + " ├─ SelectedExprs(COUNT(1), emptytable.i)\n" + " ├─ Grouping()\n" + @@ -4714,7 +4714,7 @@ Select * from ( " └─ columns: [i]\n" + "", ExpectedAnalysis: "Project\n" + - " ├─ columns: [count(1) as count(*), emptytable.i, concat(emptytable.i,emptytable.i) as concat(i, i), 123, 'abc' as abc, concat('abc','def') as concat('abc', 'def')]\n" + + " ├─ columns: [count(1) as count(*), emptytable.i, concat(emptytable.i,emptytable.i) as concat(i, i), 123, 'abc', concat('abc','def') as concat('abc', 'def')]\n" + " └─ GroupBy\n" + " ├─ SelectedExprs(COUNT(1), emptytable.i)\n" + " ├─ Grouping()\n" + @@ -4726,21 +4726,21 @@ Select * from ( { Query: `SELECT count(*), i, concat(i, i), 123, 'abc', concat('abc', 'def') FROM mytable where false;`, ExpectedPlan: "Project\n" + - " ├─ columns: [count(1):0!null as count(*), mytable.i:1!null, concat(mytable.i:1!null,mytable.i:1!null) as concat(i, i), 123 (tinyint), abc (longtext) as abc, concat(abc (longtext),def (longtext)) as concat('abc', 'def')]\n" + + " ├─ columns: [count(1):0!null as count(*), mytable.i:1!null, concat(mytable.i:1!null,mytable.i:1!null) as concat(i, i), 123 (tinyint), abc (longtext), concat(abc (longtext),def (longtext)) as concat('abc', 'def')]\n" + " └─ GroupBy\n" + " ├─ select: COUNT(1 (bigint)), mytable.i:0!null\n" + " ├─ group: \n" + " └─ EmptyTable\n" + "", ExpectedEstimates: "Project\n" + - " ├─ columns: [count(1) as count(*), mytable.i, concat(mytable.i,mytable.i) as concat(i, i), 123, 'abc' as abc, concat('abc','def') as concat('abc', 'def')]\n" + + " ├─ columns: [count(1) as count(*), mytable.i, concat(mytable.i,mytable.i) as concat(i, i), 123, 'abc', concat('abc','def') as concat('abc', 'def')]\n" + " └─ GroupBy\n" + " ├─ SelectedExprs(COUNT(1), mytable.i)\n" + " ├─ Grouping()\n" + " └─ EmptyTable\n" + "", ExpectedAnalysis: "Project\n" + - " ├─ columns: [count(1) as count(*), mytable.i, concat(mytable.i,mytable.i) as concat(i, i), 123, 'abc' as abc, concat('abc','def') as concat('abc', 'def')]\n" + + " ├─ columns: [count(1) as count(*), mytable.i, concat(mytable.i,mytable.i) as concat(i, i), 123, 'abc', concat('abc','def') as concat('abc', 'def')]\n" + " └─ GroupBy\n" + " ├─ SelectedExprs(COUNT(1), mytable.i)\n" + " ├─ Grouping()\n" + @@ -6705,7 +6705,7 @@ inner join pq on true " └─ Project\n" + " ├─ columns: [i:0!null, s:1!null]\n" + " └─ Project\n" + - " ├─ columns: [t1.i:1!null, hello (longtext) as hello]\n" + + " ├─ columns: [t1.i:1!null, hello (longtext)]\n" + " └─ InnerJoin\n" + " ├─ Eq\n" + " │ ├─ t1.i:1!null\n" + @@ -23946,7 +23946,7 @@ WHERE keyless.c0 IN ( Query: `select 1, 2.0, '3', max(x) from xy`, ExpectedPlan: "Limit(1)\n" + " └─ Project\n" + - " ├─ columns: [1 (tinyint), 2 (decimal(2,1)), 3 (longtext) as 3, xy.x:0!null as max(x)]\n" + + " ├─ columns: [1 (tinyint), 2 (decimal(2,1)), 3 (longtext), xy.x:0!null as max(x)]\n" + " └─ IndexedTableAccess(xy)\n" + " ├─ index: [xy.x]\n" + " ├─ static: [{[NULL, ∞)}]\n" + @@ -23959,7 +23959,7 @@ WHERE keyless.c0 IN ( "", ExpectedEstimates: "Limit(1)\n" + " └─ Project\n" + - " ├─ columns: [1, 2.0, '3' as 3, xy.x as max(x)]\n" + + " ├─ columns: [1, 2.0, '3', xy.x as max(x)]\n" + " └─ IndexedTableAccess(xy)\n" + " ├─ index: [xy.x]\n" + " ├─ filters: [{[NULL, ∞)}]\n" + @@ -23968,7 +23968,7 @@ WHERE keyless.c0 IN ( "", ExpectedAnalysis: "Limit(1)\n" + " └─ Project\n" + - " ├─ columns: [1, 2.0, '3' as 3, xy.x as max(x)]\n" + + " ├─ columns: [1, 2.0, '3', xy.x as max(x)]\n" + " └─ IndexedTableAccess(xy)\n" + " ├─ index: [xy.x]\n" + " ├─ filters: [{[NULL, ∞)}]\n" + diff --git a/sql/plan/str_expr.go b/sql/plan/str_expr.go index 2ec1f94984..37660e1105 100644 --- a/sql/plan/str_expr.go +++ b/sql/plan/str_expr.go @@ -2,6 +2,7 @@ package plan import ( "github.com/dolthub/go-mysql-server/sql" + "github.com/dolthub/go-mysql-server/sql/expression" "github.com/dolthub/go-mysql-server/sql/transform" ) @@ -20,6 +21,14 @@ func AliasSubqueryString(e sql.Expression) string { if err != nil { panic(err) } + + // String literal values are quoted when their String() method is called, so to avoid that, we + // check if we're dealing with a string literal and use it's raw value if so. + if literal, ok := e.(*expression.Literal); ok { + if s, ok := literal.Value().(string); ok { + return s + } + } return e.String() } diff --git a/sql/planbuilder/project.go b/sql/planbuilder/project.go index 512a40906b..cc94d0b871 100644 --- a/sql/planbuilder/project.go +++ b/sql/planbuilder/project.go @@ -242,6 +242,9 @@ func selectExprNeedsAlias(e *ast.AliasedExpr, expr sql.Expression) bool { return true } }) + if complex { + return true + } // If the expression's string representation is quoted, trim the quotes before comparing it to the input expression. // InputExpression is assigned in the Vitess layer, and it always trims quotes at that time, too. @@ -250,7 +253,7 @@ func selectExprNeedsAlias(e *ast.AliasedExpr, expr sql.Expression) bool { exprString = exprString[1 : len(exprString)-1] } - // If the expression's input value matches expr.String(), then we know that it is referenceable and - // does not need an alias. - return complex || e.InputExpression != exprString + // If the expression's input value does not match expr.String(), then we know that it is not + // referenceable and will need an alias. + return e.InputExpression != exprString } 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) {