Skip to content

Commit 5ae2e21

Browse files
fix(plugin): MCOL-6088 This ensures that SELECT col AS alias ... GROUP BY alias works correctly even when col is wrapped in an implicit aggregate wrapper.
1 parent 4f39f6a commit 5ae2e21

File tree

5 files changed

+191
-14
lines changed

5 files changed

+191
-14
lines changed

dbcon/mysql/ha_mcs_execplan.cpp

Lines changed: 80 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5848,8 +5848,11 @@ int processGroupBy(SELECT_LEX& select_lex, gp_walk_info& gwi, const bool withRol
58485848

58495849
if (sc)
58505850
{
5851+
// buildSimpleColumn succeeded - we have a valid table column reference
5852+
// This table column takes precedence over any SELECT aliases (SQL standard behavior)
5853+
// Just check if it's in the SELECT list to set orderPos (optional optimization)
58515854
bool found = false;
5852-
for (uint32_t j = 0; j < gwi.returnedCols.size(); j++)
5855+
for (uint32_t j = 0; !found && j < gwi.returnedCols.size(); j++)
58535856
{
58545857
if (sc->sameColumn(gwi.returnedCols[j].get()))
58555858
{
@@ -5858,25 +5861,95 @@ int processGroupBy(SELECT_LEX& select_lex, gp_walk_info& gwi, const bool withRol
58585861
break;
58595862
}
58605863
}
5861-
for (uint32_t j = 0; !found && j < gwi.returnedCols.size(); j++)
5864+
// Note: We do NOT fall back to alias matching here.
5865+
// The SimpleColumn from buildSimpleColumn is the correct table column to use.
5866+
}
5867+
else if (rc)
5868+
{
5869+
// rc is not a SimpleColumn (e.g., could be an aggregate or other type)
5870+
// Try to match by alias name
5871+
for (uint32_t j = 0; j < gwi.returnedCols.size(); j++)
58625872
{
5863-
if (strcasecmp(sc->alias().c_str(), gwi.returnedCols[j]->alias().c_str()) == 0)
5873+
if (ifp->name.length && strcasecmp(ifp->name.str, gwi.returnedCols[j].get()->alias().c_str()) == 0)
58645874
{
5875+
ReturnedColumn* matched_col = gwi.returnedCols[j].get();
5876+
5877+
// If it's an aggregate column, try to unwrap it to get the underlying column
5878+
AggregateColumn* agg_check = dynamic_cast<AggregateColumn*>(matched_col);
5879+
if (agg_check)
5880+
{
5881+
// Check if the aggregate has parameters and use the first parameter if it's not an aggregate
5882+
// itself
5883+
if (!agg_check->aggParms().empty())
5884+
{
5885+
ReturnedColumn* inner_col = agg_check->aggParms()[0].get();
5886+
AggregateColumn* inner_agg = dynamic_cast<AggregateColumn*>(inner_col);
5887+
5888+
if (!inner_agg)
5889+
{
5890+
// Use the inner non-aggregate column
5891+
matched_col = inner_col;
5892+
}
5893+
else
5894+
{
5895+
// It's a nested aggregate, skip it
5896+
continue;
5897+
}
5898+
}
5899+
else
5900+
{
5901+
// Aggregate with no parameters (like COUNT(*)), skip it
5902+
continue;
5903+
}
5904+
}
5905+
58655906
delete rc;
5866-
rc = gwi.returnedCols[j].get()->clone();
5907+
rc = matched_col->clone();
58675908
rc->orderPos(j);
58685909
break;
58695910
}
58705911
}
58715912
}
58725913
else
58735914
{
5915+
// buildSimpleColumn() returned NULL - this might be due to ambiguity
5916+
// or column not found. Try to find a matching alias in the SELECT list.
58745917
for (uint32_t j = 0; j < gwi.returnedCols.size(); j++)
58755918
{
5876-
if (ifp->name.length && string(ifp->name.str) == gwi.returnedCols[j].get()->alias())
5919+
if (ifp->name.length && strcasecmp(ifp->name.str, gwi.returnedCols[j].get()->alias().c_str()) == 0)
58775920
{
5878-
delete rc;
5879-
rc = gwi.returnedCols[j].get()->clone();
5921+
ReturnedColumn* matched_col = gwi.returnedCols[j].get();
5922+
5923+
// If it's an aggregate column, try to unwrap it to get the underlying column
5924+
AggregateColumn* agg_check = dynamic_cast<AggregateColumn*>(matched_col);
5925+
if (agg_check)
5926+
{
5927+
// Check if the aggregate has parameters and use the first parameter if it's not an aggregate
5928+
// itself
5929+
if (!agg_check->aggParms().empty())
5930+
{
5931+
ReturnedColumn* inner_col = agg_check->aggParms()[0].get();
5932+
AggregateColumn* inner_agg = dynamic_cast<AggregateColumn*>(inner_col);
5933+
5934+
if (!inner_agg)
5935+
{
5936+
// Use the inner non-aggregate column
5937+
matched_col = inner_col;
5938+
}
5939+
else
5940+
{
5941+
// It's a nested aggregate, skip it
5942+
continue;
5943+
}
5944+
}
5945+
else
5946+
{
5947+
// Aggregate with no parameters (like COUNT(*)), skip it
5948+
continue;
5949+
}
5950+
}
5951+
5952+
rc = matched_col->clone();
58805953
rc->orderPos(j);
58815954
break;
58825955
}
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
DROP DATABASE IF EXISTS group_by_ambiguous_test;
2+
CREATE DATABASE group_by_ambiguous_test;
3+
USE group_by_ambiguous_test;
4+
CREATE TABLE `test2` (
5+
`id` INT UNSIGNED NOT NULL,
6+
`GBP` DOUBLE UNSIGNED NOT NULL,
7+
`cat_id` INT UNSIGNED NOT NULL,
8+
`cat_id_level_0` INT UNSIGNED NOT NULL
9+
) ENGINE=Columnstore;
10+
INSERT INTO test2 VALUES (1, 100.5, 10, 20);
11+
INSERT INTO test2 VALUES (2, 200.5, 10, 30);
12+
INSERT INTO test2 VALUES (3, 300.5, 20, 20);
13+
#
14+
# Test case: GROUP BY with ambiguous column reference
15+
# This should not throw a ColumnStore error, but resolve to the table column (SQL standard)
16+
#
17+
SELECT cat_id_level_0 AS entity_id, SUM(GBP) AS spend, cat_id_level_0 AS cat_id
18+
FROM test2
19+
GROUP BY cat_id;
20+
entity_id spend cat_id
21+
20 300.5 20
22+
30 301 30
23+
Warnings:
24+
Warning 1052 Column 'cat_id' in GROUP BY is ambiguous
25+
DROP TABLE test2;
26+
#
27+
# Test case: GROUP BY with alias that matches a column but points to different column
28+
#
29+
CREATE TABLE t1 (col1 int, col2 varchar(5), col_t1 int) ENGINE=Columnstore;
30+
INSERT INTO t1 VALUES(10,'hello',10);
31+
INSERT INTO t1 VALUES(20,'hello',20);
32+
INSERT INTO t1 VALUES(30,'hello',30);
33+
INSERT INTO t1 VALUES(10,'bye',10);
34+
INSERT INTO t1 VALUES(10,'sam',10);
35+
INSERT INTO t1 VALUES(10,'bob',10);
36+
# GROUP BY col2 is ambiguous: matches both table column and SELECT alias
37+
# SQL standard: prefers table column col2 (varchar) over alias
38+
SELECT SUM(col1) AS col2 FROM t1 GROUP BY col2 ORDER BY col2;
39+
col2
40+
10
41+
10
42+
10
43+
60
44+
Warnings:
45+
Warning 1052 Column 'col2' in GROUP BY is ambiguous
46+
#
47+
# Cleanup
48+
#
49+
DROP TABLE t1;
50+
DROP DATABASE group_by_ambiguous_test;

mysql-test/columnstore/basic/r/mcol-4525.result

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -77,18 +77,22 @@ INSERT INTO t1 VALUES(10,'bye',10);
7777
INSERT INTO t1 VALUES(10,'sam',10);
7878
INSERT INTO t1 VALUES(10,'bob',10);
7979
SET columnstore_select_handler=ON;
80-
SELECT SUM(col1) AS col2 FROM t1 GROUP BY col2;
81-
ERROR 42000: The storage engine for the table doesn't support MCS-2016: Non supported item 'col2' on the GROUP BY list.
80+
SELECT SUM(col1) AS col2 FROM t1 GROUP BY col2 ORDER BY col2;
81+
col2
82+
10
83+
10
84+
10
85+
60
8286
SELECT col1 c FROM t1 ORDER BY AVG(col1);
8387
c
8488
10
8589
SET columnstore_select_handler=AUTO;
86-
SELECT SUM(col1) AS col2 FROM t1 GROUP BY col2;
90+
SELECT SUM(col1) AS col2 FROM t1 GROUP BY col2 ORDER BY col2;
8791
col2
8892
10
8993
10
90-
60
9194
10
95+
60
9296
SELECT col1 c FROM t1 ORDER BY AVG(col1);
9397
c
9498
10
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
--source ../include/have_columnstore.inc
2+
3+
--disable_warnings
4+
DROP DATABASE IF EXISTS group_by_ambiguous_test;
5+
--enable_warnings
6+
7+
CREATE DATABASE group_by_ambiguous_test;
8+
USE group_by_ambiguous_test;
9+
10+
CREATE TABLE `test2` (
11+
`id` INT UNSIGNED NOT NULL,
12+
`GBP` DOUBLE UNSIGNED NOT NULL,
13+
`cat_id` INT UNSIGNED NOT NULL,
14+
`cat_id_level_0` INT UNSIGNED NOT NULL
15+
) ENGINE=Columnstore;
16+
17+
INSERT INTO test2 VALUES (1, 100.5, 10, 20);
18+
INSERT INTO test2 VALUES (2, 200.5, 10, 30);
19+
INSERT INTO test2 VALUES (3, 300.5, 20, 20);
20+
21+
--echo #
22+
--echo # Test case: GROUP BY with ambiguous column reference
23+
--echo # This should not throw a ColumnStore error, but resolve to the table column (SQL standard)
24+
--echo #
25+
26+
SELECT cat_id_level_0 AS entity_id, SUM(GBP) AS spend, cat_id_level_0 AS cat_id
27+
FROM test2
28+
GROUP BY cat_id;
29+
DROP TABLE test2;
30+
31+
--echo #
32+
--echo # Test case: GROUP BY with alias that matches a column but points to different column
33+
--echo #
34+
35+
CREATE TABLE t1 (col1 int, col2 varchar(5), col_t1 int) ENGINE=Columnstore;
36+
INSERT INTO t1 VALUES(10,'hello',10);
37+
INSERT INTO t1 VALUES(20,'hello',20);
38+
INSERT INTO t1 VALUES(30,'hello',30);
39+
INSERT INTO t1 VALUES(10,'bye',10);
40+
INSERT INTO t1 VALUES(10,'sam',10);
41+
INSERT INTO t1 VALUES(10,'bob',10);
42+
43+
--echo # GROUP BY col2 is ambiguous: matches both table column and SELECT alias
44+
--echo # SQL standard: prefers table column col2 (varchar) over alias
45+
SELECT SUM(col1) AS col2 FROM t1 GROUP BY col2 ORDER BY col2;
46+
47+
--echo #
48+
--echo # Cleanup
49+
--echo #
50+
DROP TABLE t1;
51+
DROP DATABASE group_by_ambiguous_test;

mysql-test/columnstore/basic/t/mcol-4525.test

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -83,11 +83,10 @@ INSERT INTO t1 VALUES(10,'bye',10);
8383
INSERT INTO t1 VALUES(10,'sam',10);
8484
INSERT INTO t1 VALUES(10,'bob',10);
8585
SET columnstore_select_handler=ON;
86-
--error 1178
87-
SELECT SUM(col1) AS col2 FROM t1 GROUP BY col2;
86+
SELECT SUM(col1) AS col2 FROM t1 GROUP BY col2 ORDER BY col2;
8887
SELECT col1 c FROM t1 ORDER BY AVG(col1);
8988
SET columnstore_select_handler=AUTO;
90-
SELECT SUM(col1) AS col2 FROM t1 GROUP BY col2;
89+
SELECT SUM(col1) AS col2 FROM t1 GROUP BY col2 ORDER BY col2;
9190
SELECT col1 c FROM t1 ORDER BY AVG(col1);
9291
DROP TABLE t1;
9392

0 commit comments

Comments
 (0)