[MDEV-30828] ORDER BY clause using an integer (positional argument) on a SELECT query involving a pushed down UNION produces incorrect results - Jira

Details

Type: Bug
Status: Closed (View Workflow)
Priority: Critical
Resolution: Fixed
Affects Version/s: 10.6.12
Fix Version/s: 11.1.2, 11.2.1
Component/s: Server
Labels:
None

Description

The below diff shows incorrect outputs on the federatedx storage engine:

diff --git a/mysql-test/suite/federated/federatedx_create_handlers.result b/mysql-test/suite/federated/federatedx_create_handlers.result

index 044f64c796f..8da86fdd016 100644

--- a/mysql-test/suite/federated/federatedx_create_handlers.result

+++ b/mysql-test/suite/federated/federatedx_create_handlers.result

@@ -550,6 +550,36 @@ CREATE TABLE t3 (a varchar(30)) ENGINE=MyISAM;

 CREATE TABLE t4 (a varchar(30)) ENGINE=MyISAM;

 INSERT INTO t3 VALUES ('t3_myisam1'), ('t3_myisam2'), ('t3_myisam3');

 INSERT INTO t4 VALUES ('t4_myisam1'), ('t4_myisam2'), ('t4_myisam3');

+SELECT 1, a FROM federated.t1 UNION ALL SELECT 2, a FROM federated.t2 ORDER BY 3;

+1      a

+1      abc

+1      bcd

+1      cde

+2      abc

+2      bcd

+2      cde

+2      def

+2      efg

+SELECT a FROM federated.t1 UNION ALL SELECT a FROM federated.t2 ORDER BY 2;

+a

+abc

+bcd

+cde

+abc

+bcd

+cde

+def

+efg

+SELECT a FROM federated.t1 UNION ALL SELECT a FROM federated.t2 ORDER BY 1;

+a

+abc

+bcd

+cde

+abc

+bcd

+cde

+def

+efg

 # Pushdown of the whole UNION

 SELECT * from federated.t1 UNION SELECT * from federated.t2;

diff --git a/mysql-test/suite/federated/federatedx_create_handlers.test b/mysql-test/suite/federated/federatedx_create_handlers.test

index 8ad3172b35d..f9bdb9cf04e 100644

--- a/mysql-test/suite/federated/federatedx_create_handlers.test

+++ b/mysql-test/suite/federated/federatedx_create_handlers.test

@@ -421,6 +421,10 @@ CREATE TABLE t4 (a varchar(30)) ENGINE=MyISAM;

 INSERT INTO t3 VALUES ('t3_myisam1'), ('t3_myisam2'), ('t3_myisam3');

 INSERT INTO t4 VALUES ('t4_myisam1'), ('t4_myisam2'), ('t4_myisam3');

+SELECT 1, a FROM federated.t1 UNION ALL SELECT 2, a FROM federated.t2 ORDER BY 3;

+SELECT a FROM federated.t1 UNION ALL SELECT a FROM federated.t2 ORDER BY 2;

+SELECT a FROM federated.t1 UNION ALL SELECT a FROM federated.t2 ORDER BY 1;

 --echo # Pushdown of the whole UNION

 SELECT * from federated.t1 UNION SELECT * from federated.t2;

 EXPLAIN SELECT * from federated.t1 UNION SELECT * from federated.t2;

The expected output for the above 3 queries should be as follows (here i1 and i2 are equivalent InnoDB tables):

MariaDB [test]> SELECT 1, a FROM i1 UNION ALL SELECT 2, a FROM i2 ORDER BY 3;

ERROR 1054 (42S22): Unknown column '3' in 'order clause'

MariaDB [test]> SELECT a FROM i1 UNION ALL SELECT a FROM i2 ORDER BY 2;

ERROR 1054 (42S22): Unknown column '2' in 'order clause'

MariaDB [test]> SELECT a FROM i1 UNION ALL SELECT a FROM i2 ORDER BY 1;

+------+

| a    |

+------+

| abc  |

| abc  |

| bcd  |

| bcd  |

| cde  |

| cde  |

| def  |

| efg  |

+------+

8 rows in set (0.003 sec)

It is possible query 3 is currently not supported by the federatedx storage engine. But for queries 1 and 2, the server should error out (as is the case with InnoDB) before attempting to pushdown the query to the foreign engine.

Attachments

Issue Links

blocks

MCOL-5222 ORDER BY on UNIONs in outer selects does not work

Stalled

relates to

MDEV-25080 Allow pushdown of queries involving UNIONs in outer select to foreign engines

Closed

Activity

Ascending order - Click to sort in descending order

Oleg Smirnov added a comment - 2023-03-23 13:11

select a from t1 union all select a from t2 order by 2;

>> Server execution not reaching find_order_in_list() in sql/sql_select.cc is the problem/bug here
Such a union is processed in MariaDB in the following way (when there is no pushdown).
First results from parts of the union are stored into a temporary table (select a from t1 union select a from t2). Then a special structure fake_select_lex is created which is responsible for retrieving data from that temporary table applying ORDER BY. So the error about wrong ORDER BY appears only at the stage of fake_select_lex processing.
In the case of pushdown fake_select_lex is not involved in processing that's why there is no error on MariaDB side either.

find_order_in_list() is called for a JOIN corresponding to the execution of fake_select_lex mentioned above. When fake_select_lex isn't employed there's no call to find_order_in_list().
So it's more an architectural drawback than a bug. We'll think whether this can be solved without significant rework of unions processing.

Oleg Smirnov added a comment - 2023-03-23 13:11 select a from t1 union all select a from t2 order by 2; >> Server execution not reaching find_order_in_list() in sql/sql_select.cc is the problem/bug here Such a union is processed in MariaDB in the following way (when there is no pushdown). First results from parts of the union are stored into a temporary table (select a from t1 union select a from t2). Then a special structure fake_select_lex is created which is responsible for retrieving data from that temporary table applying ORDER BY. So the error about wrong ORDER BY appears only at the stage of fake_select_lex processing. In the case of pushdown fake_select_lex is not involved in processing that's why there is no error on MariaDB side either. find_order_in_list() is called for a JOIN corresponding to the execution of fake_select_lex mentioned above. When fake_select_lex isn't employed there's no call to find_order_in_list() . So it's more an architectural drawback than a bug. We'll think whether this can be solved without significant rework of unions processing.

Oleg Smirnov added a comment - 2023-04-03 11:37

tntnatbry, I've pushed 10.6-mdev-30828 to the ES which addresses the first issue: pushing down SQL with incorrect ORDER BY. Now such statements will error out on the MariaDB side. There is another issue with FederatedX producing unordered results which I'm planning to fix next.

Regarding the bad side effect: statements

select a from t1 order by a;

and

select a from t1 union all select a from t2 order by a;

are different in terms of column "a" meaning. The "order by a" clause in the case of union relates to the result of the union and not to the particular table column. So the statement can be represented as

(select a from t1 union all select a from t2) order by a;

There can even be a statement like this:

select b as a from t1 union select c as a from t2 order by a;

It's not possible to have an Item* referring to "t1.a" since there's no such column in t1. "a" in the ORDER BY must probably refer to a temporary table containing results of the UNION.

Oleg Smirnov added a comment - 2023-04-03 11:37 tntnatbry , I've pushed 10.6-mdev-30828 to the ES which addresses the first issue: pushing down SQL with incorrect ORDER BY. Now such statements will error out on the MariaDB side. There is another issue with FederatedX producing unordered results which I'm planning to fix next. Regarding the bad side effect: statements select a from t1 order by a; and select a from t1 union all select a from t2 order by a; are different in terms of column "a" meaning. The "order by a" clause in the case of union relates to the result of the union and not to the particular table column. So the statement can be represented as (select a from t1 union all select a from t2) order by a; There can even be a statement like this: select b as a from t1 union select c as a from t2 order by a; It's not possible to have an Item* referring to "t1.a" since there's no such column in t1. "a" in the ORDER BY must probably refer to a temporary table containing results of the UNION.

Oleg Smirnov added a comment - 2023-04-12 12:21

It turned out to be not so easy to implement global ORDER BY (i.e. related to the whole UNION/EXCEPT/INTERSECT) validation before the pushdown. The problem is we select and initialize the pushdown handler in st_select_unit::prepare(). The structure responsible for global ORDER BY is fake_select_lex. But fake_select_lex is not validated completely at prepare(), some validation steps are performed later, during the execution phase at st_select_unit::exec_inner(), when calling mysql_select(). I haven't succeded with trying to add the complete fake_select_lex validation to st_select_unit::prepare(). A lot of test cases start failing in different places of code since it's an architectural change.

My suggestion is to restrict pushing down units having global ORDER BY and/or GROUP BY.

Oleg Smirnov added a comment - 2023-04-12 12:21 It turned out to be not so easy to implement global ORDER BY (i.e. related to the whole UNION/EXCEPT/INTERSECT) validation before the pushdown. The problem is we select and initialize the pushdown handler in st_select_unit::prepare(). The structure responsible for global ORDER BY is fake_select_lex. But fake_select_lex is not validated completely at prepare(), some validation steps are performed later, during the execution phase at st_select_unit::exec_inner(), when calling mysql_select(). I haven't succeded with trying to add the complete fake_select_lex validation to st_select_unit::prepare(). A lot of test cases start failing in different places of code since it's an architectural change. My suggestion is to restrict pushing down units having global ORDER BY and/or GROUP BY.

Oleg Smirnov added a comment - 2023-05-08 09:27

The fix is pushed to ES 10.6-mdev-30828.

Oleg Smirnov added a comment - 2023-05-08 09:27 The fix is pushed to ES 10.6-mdev-30828.

Oleksandr Byelkin added a comment - 2023-06-26 11:06

OK to push

Oleksandr Byelkin added a comment - 2023-06-26 11:06 OK to push

MariaDB Server

ORDER BY clause using an integer (positional argument) on a SELECT query involving a pushed down UNION produces incorrect results

Details

Description

Attachments

Issue Links

Activity

People

Dates

Git Integration