From 5267c092bdfdbe8448b0d82dfc07090fe45ccf3e Mon Sep 17 00:00:00 2001 From: James Cor Date: Wed, 26 Mar 2025 11:38:44 -0700 Subject: [PATCH 1/3] checkout group by ordinal range --- enginetest/queries/script_queries.go | 26 ++++++++++++++++++++++++++ sql/planbuilder/aggregates.go | 4 ++++ 2 files changed, 30 insertions(+) diff --git a/enginetest/queries/script_queries.go b/enginetest/queries/script_queries.go index 1956b88f35..0559e0ffc0 100644 --- a/enginetest/queries/script_queries.go +++ b/enginetest/queries/script_queries.go @@ -7782,6 +7782,32 @@ where }, }, }, + { + Name: "group by ordinal tests", + Dialect: "mysql", + SetUpScript: []string{ + "create table t (i int, j int);", + }, + Assertions: []ScriptTestAssertion{ + { + Query: "select 1 from t group by 'abc';", + ExpectedErrStr: "expected integer order by literal", + }, + { + // TODO: this actually works in MySQL + Query: "select 1 from t group by -123;", + ExpectedErrStr: "expected positive integer order by literal", + }, + { + Query: "select 1 from t group by 0;", + ExpectedErrStr: "expected positive integer order by literal", + }, + { + Query: "select 1 from t group by 100;", + ExpectedErrStr: "column ordinal out of range: 100", + }, + }, + }, } var SpatialScriptTests = []ScriptTest{ diff --git a/sql/planbuilder/aggregates.go b/sql/planbuilder/aggregates.go index 8fabe2ff9d..2fd7280f36 100644 --- a/sql/planbuilder/aggregates.go +++ b/sql/planbuilder/aggregates.go @@ -131,8 +131,12 @@ func (b *Builder) buildGroupingCols(fromScope, projScope *scope, groupby ast.Gro b.handleErr(fmt.Errorf("expected integer order by literal")) } if intIdx < 1 { + // TODO: this actually works in MySQL b.handleErr(fmt.Errorf("expected positive integer order by literal")) } + if int(intIdx) > len(selects) { + b.handleErr(fmt.Errorf("column ordinal out of range: %d", intIdx)) + } col = projScope.cols[intIdx-1] default: expr := b.buildScalar(fromScope, e) From e34f2a33ef1db611ebb3faa0c4a5c53f93ae8fc6 Mon Sep 17 00:00:00 2001 From: jycor Date: Wed, 26 Mar 2025 18:41:21 +0000 Subject: [PATCH 2/3] [ga-format-pr] Run ./format_repo.sh to fix formatting --- enginetest/queries/script_queries.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/enginetest/queries/script_queries.go b/enginetest/queries/script_queries.go index 0559e0ffc0..982a83108b 100644 --- a/enginetest/queries/script_queries.go +++ b/enginetest/queries/script_queries.go @@ -7790,20 +7790,20 @@ where }, Assertions: []ScriptTestAssertion{ { - Query: "select 1 from t group by 'abc';", + Query: "select 1 from t group by 'abc';", ExpectedErrStr: "expected integer order by literal", }, { // TODO: this actually works in MySQL - Query: "select 1 from t group by -123;", + Query: "select 1 from t group by -123;", ExpectedErrStr: "expected positive integer order by literal", }, { - Query: "select 1 from t group by 0;", + Query: "select 1 from t group by 0;", ExpectedErrStr: "expected positive integer order by literal", }, { - Query: "select 1 from t group by 100;", + Query: "select 1 from t group by 100;", ExpectedErrStr: "column ordinal out of range: 100", }, }, From ab1155c5002c69f70e9322058fef45ba8b827d88 Mon Sep 17 00:00:00 2001 From: James Cor Date: Wed, 26 Mar 2025 12:06:28 -0700 Subject: [PATCH 3/3] move tests --- enginetest/queries/script_queries.go | 26 -------------------------- sql/planbuilder/parse_test.go | 19 +++++++++++++++++++ 2 files changed, 19 insertions(+), 26 deletions(-) diff --git a/enginetest/queries/script_queries.go b/enginetest/queries/script_queries.go index 0559e0ffc0..1956b88f35 100644 --- a/enginetest/queries/script_queries.go +++ b/enginetest/queries/script_queries.go @@ -7782,32 +7782,6 @@ where }, }, }, - { - Name: "group by ordinal tests", - Dialect: "mysql", - SetUpScript: []string{ - "create table t (i int, j int);", - }, - Assertions: []ScriptTestAssertion{ - { - Query: "select 1 from t group by 'abc';", - ExpectedErrStr: "expected integer order by literal", - }, - { - // TODO: this actually works in MySQL - Query: "select 1 from t group by -123;", - ExpectedErrStr: "expected positive integer order by literal", - }, - { - Query: "select 1 from t group by 0;", - ExpectedErrStr: "expected positive integer order by literal", - }, - { - Query: "select 1 from t group by 100;", - ExpectedErrStr: "column ordinal out of range: 100", - }, - }, - }, } var SpatialScriptTests = []ScriptTest{ diff --git a/sql/planbuilder/parse_test.go b/sql/planbuilder/parse_test.go index a9ca2fd73f..82e1225d5b 100644 --- a/sql/planbuilder/parse_test.go +++ b/sql/planbuilder/parse_test.go @@ -2950,6 +2950,25 @@ func TestPlanBuilderErr(t *testing.T) { Query: "select x + 1 as xx from xy join uv on (x = u) having x = 123;", Err: "column \"x\" could not be found in any table in scope", }, + + // Test GroupBy Ordinals + { + Query: "select 1 from xy group by 'abc';", + Err: "expected integer order by literal", + }, + { + // TODO: this actually works in MySQL + Query: "select 1 from xy group by -123;", + Err: "expected positive integer order by literal", + }, + { + Query: "select 1 from xy group by 0;", + Err: "expected positive integer order by literal", + }, + { + Query: "select 1 from xy group by 100;", + Err: "column ordinal out of range: 100", + }, } db := memory.NewDatabase("mydb")