[MDEV-30828] ORDER BY clause using an integer (positional argument) on a SELECT query involving a pushed down UNION produces incorrect results Created: 2023-03-10  Updated: 2023-11-23  Resolved: 2023-06-30

Status: Closed
Project: MariaDB Server
Component/s: Server
Affects Version/s: 10.6.12
Fix Version/s: 11.1.2, 11.2.1

Type: Bug Priority: Critical
Reporter: Gagan Goel (Inactive) Assignee: Oleg Smirnov
Resolution: Fixed Votes: 0
Labels: None

Issue Links:
Blocks
blocks MCOL-5222 ORDER BY on UNIONs in outer selects d... Stalled
Relates
relates to MDEV-25080 Allow pushdown of queries involving U... Closed

 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;
 
 a
 
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.

 Comments   
Comment by Oleg Smirnov [ 2023-03-23 ] 

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.

Comment by Oleg Smirnov [ 2023-04-03 ]

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.

Comment by Oleg Smirnov [ 2023-04-12 ]

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.

Comment by Oleg Smirnov [ 2023-05-08 ]

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

Comment by Oleksandr Byelkin [ 2023-06-26 ]

OK to push

Generated at Thu Feb 08 10:19:12 UTC 2024 using Jira 8.20.16#820016-sha1:9d11dbea5f4be3d4cc21f03a88dd11d8c8687422.