Re: [Maria-developers] c76ac1e6de8: MDEV-24089 support oracle syntax: rownum
Hi, Michael! On Apr 12, Michael Widenius wrote:
revision-id: c76ac1e6de8 (mariadb-10.5.2-551-gc76ac1e6de8) parent(s): a507eb17708 author: Michael Widenius <michael.widenius@gmail.com> committer: Michael Widenius <michael.widenius@gmail.com> timestamp: 2021-03-24 19:42:24 +0200 message:
diff --git a/mysql-test/main/rownum.test b/mysql-test/main/rownum.test new file mode 100644 index 00000000000..8d89eed2f86 --- /dev/null +++ b/mysql-test/main/rownum.test @@ -0,0 +1,542 @@ +--source include/have_sequence.inc +# +# Test of basic rownum() functionallity +# Test are done with Aria to ensure that row order is stable
what does that mean?
+# + +CREATE OR REPLACE TABLE t1(a int, b int) engine=aria; +CREATE OR REPLACE TABLE t2(a int, b int) engine=aria; +insert into t1 values (1,10),(2,20),(3,30); +insert into t2 values (2,21),(3,31),(4,41); + +--echo # +--echo # Simple selects +--echo # + +select a,b,rownum() from t1; +select a,b,rownum() from t1 where rownum() < 2; +select a,b from t1 where rownum() <= 2; +select a,b from t1 where rownum() > 2; + +--echo # +--echo # Subqueries +--echo # + +select t1.a,rownum(),t3.a,t3.t2_rownum from t1, (select t2.a,rownum() as t2_rownum from t2 where rownum() <=2) t3; +select t1.a, (select t2.b from t2 where t1.a=t2.a and rownum() <= 1) 'b' from t1; +select t1.a, t3.a from t1, (select * from t2 where rownum() <= 2) t3; +select * from (select tt.*,rownum() as id from (select * from t1) tt) t3 where id>2; + +--echo # +--echo # Joins +--echo # + +select t1.a,t1.b,t2.a,t2.b,rownum() from t1,t2 where rownum() <= 4; +select *,rownum() from t1,t2 where t1.a=t2.a and rownum()<=2; +select * from t1 left join t2 on (t2.a=t1.a and rownum()=0); +select * from t1 left join t2 on (t2.a=t1.a and rownum()>1); +select * from t1 left join t2 on (t2.a=t1.a and rownum()<1); +select * from t1 left join t2 on (t2.a=t1.a and rownum()=1); +select * from t1 left join t2 on (t2.a=t1.a and rownum()>=1); + +--echo # +--echo # Union +--echo # + +select * from t1 where rownum() <=2 union select * from t2 where rownum()<= 1; + +--echo # +--echo # Order by +--echo # + +select * from t1 where rownum() <= 2 order by a desc; +explain select * from t1 where rownum() <= 2 order by a desc; +select t1.a,t1.b,rownum() from t1 where rownum() <= 2 order by a desc; +explain select t1.a,t1.b,rownum() from t1 where rownum() <= 2 order by a desc; +select *,rownum() from t1,t2; +select *,rownum() from t1,t2 order by t2.a desc, t1.a desc; +select * from (select * from t1 order by a desc) as t where rownum() <= 2; +select * from t1,t2 where t1.a=t2.a and rownum()<=2 order by t1.a,t2.a; + +--echo # +--echo # Having +--echo # + +select t1.a, sum(t1.b), rownum() as 'r' from t1 group by t1.a having r <= 2; +--error ER_FUNCTION_CANNOT_BE_USED_IN_CLAUSE +select * from t1 having rownum() <= 2; +select t1.a, sum(t1.b), rownum() from t1 group by t1.a; +--error ER_FUNCTION_CANNOT_BE_USED_IN_CLAUSE +select t1.a, sum(t1.b), rownum() as 'r' from t1 group by t1.a having rownum() <= 2; + +--echo # +--echo # Sum functions +--echo # + +select max(rownum()),min(rownum()) from t1; +select sum(rownum()),avg(rownum()) from t1; + +--echo # +--echo # Group by +--echo # + +select t1.a,sum(t1.b) from t1 where rownum() < 2 group by t1.a; +select t1.a,sum(t2.b) from t1 JOIN t2 ON (t1.a=t2.a) where rownum() <= 2 group by t1.a; +select * from (select t1.a,sum(t2.b) from t1 JOIN t2 ON (t1.a=t2.a) group by t1.a) as t where rownum() <= 1; +select t1.a,sum(rownum()),count(*) from t1 where rownum() <= 2 group by t1.a; +select * from (select t1.a,sum(t1.b) from t1 group by t1.a) as t3 where rownum() < 2; + +create table t3 (a int) engine=myisam; +insert into t3 values (3),(5),(5),(3); +select a, max(rownum()) from t3 group by a; +drop table t3; + +CREATE TABLE t3 ( + a int(11) DEFAULT NULL, + b varchar(1024) DEFAULT NULL +); +insert into t3 select mod(seq*3,20)+1, repeat(char(33+mod(seq,90)),mod(seq,10)*100) from seq_1_to_23; +SELECT sq.a,length(sq.f) FROM (SELECT a, GROUP_CONCAT(b,b) AS f FROM t3 GROUP BY a ORDER BY a desc) as sq WHERE ROWNUM() <= 10; +drop table t3; + +--echo # +--echo # Prepared statements +--echo # + +PREPARE stmt1 from "select a,b,rownum() from t1 where rownum() <= 2"; +execute stmt1; +execute stmt1; +deallocate prepare stmt1; + +--echo # +--echo # Views +--echo # + +create view v1 as select t1.a,rownum() from t1; +select * from v1; +select t1.a,v1.* from t1,v1 where t1.a=v1.a; +drop view v1; + +CREATE TABLE t3 (a INT); +INSERT INTO t3 VALUES (1),(2),(3); +CREATE VIEW v1 AS SELECT a FROM t3 WHERE ROWNUM() <= 2; +SELECT * FROM v1; +drop view v1; +drop table t3; + +--echo # +--echo # Reserved words +--echo # + +create table t4 (a int, rownum int); +insert into t4 (a,rownum) values (1,2); +select t4.a,t4.rownum from t4; +drop table t4; + +--echo # +--echo # Test Oracle mode +--echo # + +set SQL_MODE=ORACLE; +select t1.a,rownum from t1 where rownum<=2; +select t1.a,rownum() from t1 where rownum()<=2; +--error ER_PARSE_ERROR +create table t4 (a int, rownum int); + +DELIMITER |; +DECLARE + CURSOR c_cursor + IS select a,b,rownum from t1 where rownum <= 2; + v_a t1.a%TYPE; + v_b t1.b%TYPE; + v_rn t1.a%TYPE; +BEGIN + OPEN c_cursor; + FETCH c_cursor INTO v_a, v_b, v_rn; + WHILE c_cursor%FOUND LOOP + SELECT concat(v_a,'--',v_b,'--',v_rn); + FETCH c_cursor INTO v_a, v_b, v_rn; + END LOOP; + CLOSE c_cursor; +END;| +DELIMITER ;| + +--error ER_FUNCTION_CANNOT_BE_USED_IN_CLAUSE +select a, rownum from t1 group by a, rownum having rownum < 3; +select a, rownum from t1 group by a, rownum having "rownum" < 3;
what's the difference between these two? Why the first is an error and the second is not?
+select a, rownum as r from t1 group by a, rownum having r < 3;
And the this one. Why is it not an error either?
+ +set SQL_MODE=DEFAULT; + +--echo # Cleanup + +drop table t1,t2; + +--echo # +--echo # INSERT +--echo # + +create table t1 (a int not null primary key, b int); +insert into t1 values (1,rownum()),(2,rownum()),(3,rownum()); +select * from t1; +drop table t1; + +--echo # +--echo # INSERT DELAYED +--echo # + +create table t1 (a int not null primary key, b int); +insert delayed into t1 values (1,rownum()),(2,rownum()),(3,rownum()); +select * from t1; +drop table t1; + +--echo # +--echo # INSERT ... RETURNING +--echo # + +create or replace table t1 (a int); +insert into t1 values (1),(2) returning a, rownum(); +drop table t1; + +--echo # +--echo # UPDATE +--echo # + +create table t1 (a int not null primary key, b int); +insert into t1 values (1,1),(2,2),(3,3); +update t1 set b=0; +update t1 set b=rownum()+1; +select * from t1; + +update t1 set b=0; +update t1 set b=rownum() where a < 10 and rownum() < 2; +select * from t1; +drop table t1; + +create table t1 (a int); +insert into t1 values (10),(20),(30); +update t1 set a = rownum(); +select * from t1; +update t1 set a = rownum(); +select * from t1; +drop table t1; + +--echo # +--echo # DELETE +--echo # + +create table t1 (a int not null primary key, b int); +insert into t1 values (1,1),(2,0),(3,0); +delete from t1 where a < 10 and rownum() < 2; +select * from t1; +drop table t1; + +--echo # +--echo # MULTI-TABLE-DELETE +--echo # + +create table t1 (a int not null primary key); +insert into t1 values (1),(2),(3); +create table t2 (a int not null primary key); +insert into t2 values (1),(2),(3); + +delete t1,t2 from t1,t2 where t1.a=t2.a and rownum() <= 2; +select * from t1; +select * from t2; +drop table t1,t2; + +--echo # +--echo # MULTI-TABLE-UPDATE + +CREATE TABLE t1 (ID INT); +CREATE TABLE t2 (ID INT, + s1 TEXT, s2 TEXT, s3 VARCHAR(10), s4 TEXT, s5 VARCHAR(10)); + +INSERT INTO t1 VALUES (1),(2); +INSERT INTO t2 VALUES (1,'test', 'test', 'test', 'test', 'test'), + (2,'test', 'test', 'test', 'test', 'test'); + +SELECT * FROM t1 LEFT JOIN t2 USING(ID); +UPDATE t1 LEFT JOIN t2 USING(ID) SET s1 = 'changed'; +select * from t2; +update t2 set s1=""; +UPDATE t1 LEFT JOIN t2 USING(ID) SET s1 = 'changed' where rownum() <=1; +select * from t2; +drop table t1,t2; + +--echo # +--echo # LOAD DATA +--echo # + +create table t1 (a int, b int, c int); +load data infile '../../std_data/loaddata7.dat' into table t1 fields terminated by ',' lines terminated by "\r\n" (a,b) set c=rownum(); +select * from t1; +drop table t1; + +--echo # +--echo # LIMIT OPTIMIZATION +--echo # + +create table t1 (a int); +insert into t1 select seq from seq_1_to_100; + +flush status; +select * from t1 where rownum() <= 3; +show status like "Rows_read"; +flush status; +flush status; +select * from t1 where rownum() <= 4 and rownum() <= 3; +show status like "Rows_read"; +flush status; +select * from t1 where rownum() < 4 and a > 10; +show status like "Rows_read"; +flush status; +select * from t1 where 3 >= rownum(); +show status like "Rows_read"; +flush status; +select * from t1 where 4 > rownum() and a > 20; +show status like "Rows_read"; +flush status; +select * from t1 where rownum() = 1 and a > 10; +show status like "Rows_read"; +flush status; +select * from t1 where a > 30 && 1 = rownum(); +show status like "Rows_read"; +flush status; + +--echo # No limit optimization + +select * from t1 where rownum() > 10; +show status like "Rows_read"; +flush status; +select * from t1 where 10 < rownum(); +show status like "Rows_read"; +flush status; +select * from t1 where rownum() >= 10; +show status like "Rows_read"; +flush status; +select * from t1 where 10 < rownum(); +show status like "Rows_read"; +flush status; +select * from t1 where 10 <= rownum(); +show status like "Rows_read"; +flush status; +select * from t1 where 2 = rownum(); +show status like "Rows_read"; +flush status; +select * from t1 where rownum() = 2; +show status like "Rows_read"; +flush status; +select * from t1 where rownum() <= 0; +show status like "Rows_read"; +flush status; +select * from t1 where rownum() < 10 limit 4, 4; +show status like "Rows_read"; +flush status; +select * from t1 where rownum() < 10 order by a; +show status like "Rows_read"; +flush status; + + +--echo # rownum and limit + +select * from t1 where rownum() < 4 limit 10; +show status like "Rows_read"; +flush status; +select * from t1 where rownum() < 10 limit 4; +show status like "Rows_read"; + +drop table t1; + +--echo # +--echo # Rownum examples from Woqutech +--echo # + +set SQL_MODE=ORACLE; +create table t1 (c1 int ,c2 varchar(20)) engine=myisam; +insert into t1 values (1, 'aaa'),(2, 'bbb'),(3, 'ccc'),(4, 'ddd'),(5, 'eee'); +update t1 set c2 = 'xxx' where rownum = 2; +select * from t1 where c2='xxx'; +update t1 set c2 = 'xxx' where rownum < 3; +select * from t1 where c2='xxx'; +delete from t1 where rownum = 2; +select count(*) from t1; +delete from t1 where rownum < 3; +select count(*) from t1; +delete from t1 where c1=rownum ; +select count(*) from t1; +delete from t1 where c1=rownum+2 ; +select count(*) from t1; +set SQL_MODE=DEFAULT; +drop table t1; + +--echo # +--echo # Rownum() used in not supported places (returns 0 or gives an error) +--echo # + +set @a=rownum(); +select @a; + +--error ER_GENERATED_COLUMN_FUNCTION_IS_NOT_ALLOWED +create or replace table t (a int, b int as (rownum()) virtual); + +create table t1 (a int); +insert into t1 values (3),(1),(5),(8),(4); +handler t1 open; +--error ER_GENERATED_COLUMN_FUNCTION_IS_NOT_ALLOWED +handler t1 read next where rownum() < 1; +handler t1 close; +drop table t1; + +# rownum() executed in a function will be run in the function context. + +create table t1 (a int not null primary key, b int); +insert into t1 values (1,1),(2,2),(3,3); +create function f() returns int return rownum(); +select a, rownum(), f() from t1; +drop function f; +drop table t1; + +# rownum() executed in a trigger will be run in the function context. + +create or replace table t1 (a int, r int); +create trigger tr before update on t1 for each row set NEW.r = rownum(); +insert into t1 (a) values (1),(2); +select * from t1; +update t1 set a=a+10; +select * from t1; +drop trigger tr; +drop table t1; + +--echo # +--echo # LIMIT optimisation +--echo # + +create table t1 (a int); +insert into t1 values (1),(2),(3),(4),(5); + +let $query= +select * from (select a from t1 where a < 1000) as tt where rownum() <= 2; +flush status; +eval $query; +show status like "Rows_read"; +eval explain extended $query; +eval prepare stmt from "$query"; +flush status; +execute stmt; +show status like "Rows_read"; +flush status; +execute stmt; +show status like "Rows_read"; +deallocate prepare stmt; + + +let $query= +select * from (select a from t1 where a < 1000 group by a) as tt where rownum() <= 2; +flush status; +eval $query; +show status like "Rows_read"; +eval explain extended $query; +eval prepare stmt from "$query"; +execute stmt; +execute stmt; +deallocate prepare stmt; + +let $query= +select * from (select a from t1 where a < 1000 group by a order by 1) as tt where rownum() <= 2; +flush status; +eval $query; +show status like "Rows_read"; +eval explain extended $query; +eval prepare stmt from "$query"; +execute stmt; +execute stmt; +deallocate prepare stmt; + +let $query= +select * from (select a from t1 where a < 1000 union select 10) as tt where rownum() <= 2; +flush status; +eval $query; +show status like "Rows_read"; +eval explain extended $query; +eval prepare stmt from "$query"; +execute stmt; +execute stmt; +deallocate prepare stmt; + +--echo # Other limit + +let $query= +select * from (select a from t1 where a < 1000 group by a order by 1 limit 1) as tt where rownum() <= 2; +eval $query; +eval explain extended $query; +eval prepare stmt from "$query"; +execute stmt; +execute stmt; +deallocate prepare stmt; + +--echo # Other limit less + +let $query= +select * from (select a from t1 where a < 1000 group by a order by 1 limit 10) as tt where rownum() <= 2; +eval $query; +eval explain extended $query; +eval prepare stmt from "$query"; +execute stmt; +execute stmt; +deallocate prepare stmt; + +let $query= +select * from (select a from t1 where a < 1000 union select 10 limit 1) as tt where rownum() <= 2; +eval $query; +eval explain extended $query; +eval prepare stmt from "$query"; +execute stmt; +execute stmt; +deallocate prepare stmt; + +--echo # < rownum + +let $query= +select * from (select a from t1 where a < 1000) as tt where rownum() < 2; +eval $query; +eval explain extended $query; +eval prepare stmt from "$query"; +execute stmt; +execute stmt; +deallocate prepare stmt; + +--echo # Simple expression + +let $query= +select * from (select a from t1 where a < 1000) as tt where rownum() <= 1+1; +eval $query; +eval explain extended $query; +eval prepare stmt from "$query"; +execute stmt; +execute stmt; +deallocate prepare stmt; + +--echo # Simple expression reversed + +let $query= +select * from (select a from t1 where a < 1000) as tt where 1+1 >= rownum(); +eval $query; +eval explain extended $query; +eval prepare stmt from "$query"; +execute stmt; +execute stmt; +deallocate prepare stmt; + +--echo # expensive (no opt) + +let $query= +select * from (select a from t1 where a < 1000) as tt where (select max(a) from t1) >= rownum(); +eval $query; +eval explain extended $query; +eval prepare stmt from "$query"; +execute stmt; +execute stmt; +deallocate prepare stmt; + +drop table t1; diff --git a/mysql-test/main/rownum.result b/mysql-test/main/rownum.result new file mode 100644 index 00000000000..e7ea92e5dde --- /dev/null +++ b/mysql-test/main/rownum.result @@ -0,0 +1,925 @@ +CREATE OR REPLACE TABLE t1(a int, b int) engine=aria; +CREATE OR REPLACE TABLE t2(a int, b int) engine=aria; +insert into t1 values (1,10),(2,20),(3,30); +insert into t2 values (2,21),(3,31),(4,41); +# +# Simple selects +# +select a,b,rownum() from t1; +a b rownum() +1 10 1 +2 20 2 +3 30 3 +select a,b,rownum() from t1 where rownum() < 2; +a b rownum() +1 10 1 +select a,b from t1 where rownum() <= 2; +a b +1 10 +2 20 +select a,b from t1 where rownum() > 2; +a b +# +# Subqueries +# +select t1.a,rownum(),t3.a,t3.t2_rownum from t1, (select t2.a,rownum() as t2_rownum from t2 where rownum() <=2) t3; +a rownum() a t2_rownum +1 1 2 1 +1 2 3 2 +2 3 2 1 +2 4 3 2 +3 5 2 1 +3 6 3 2 +select t1.a, (select t2.b from t2 where t1.a=t2.a and rownum() <= 1) 'b' from t1; +a b +1 NULL +2 21 +3 31 +select t1.a, t3.a from t1, (select * from t2 where rownum() <= 2) t3; +a a +1 2 +1 3 +2 2 +2 3 +3 2 +3 3 +select * from (select tt.*,rownum() as id from (select * from t1) tt) t3 where id>2; +a b id +3 30 3 +# +# Joins +# +select t1.a,t1.b,t2.a,t2.b,rownum() from t1,t2 where rownum() <= 4; +a b a b rownum() +1 10 2 21 1 +2 20 2 21 2 +3 30 2 21 3 +1 10 3 31 4 +select *,rownum() from t1,t2 where t1.a=t2.a and rownum()<=2; +a b a b rownum() +2 20 2 21 1 +3 30 3 31 2 +select * from t1 left join t2 on (t2.a=t1.a and rownum()=0); +a b a b +1 10 NULL NULL +2 20 NULL NULL +3 30 NULL NULL +select * from t1 left join t2 on (t2.a=t1.a and rownum()>1); +a b a b +1 10 NULL NULL +2 20 NULL NULL +3 30 NULL NULL +select * from t1 left join t2 on (t2.a=t1.a and rownum()<1); +a b a b +1 10 NULL NULL +2 20 NULL NULL +3 30 NULL NULL +select * from t1 left join t2 on (t2.a=t1.a and rownum()=1); +a b a b +2 20 2 21 +1 10 NULL NULL +3 30 NULL NULL +select * from t1 left join t2 on (t2.a=t1.a and rownum()>=1); +a b a b +2 20 2 21 +3 30 3 31 +1 10 NULL NULL +# +# Union +# +select * from t1 where rownum() <=2 union select * from t2 where rownum()<= 1; +a b +1 10 +2 20 +2 21 +# +# Order by +# +select * from t1 where rownum() <= 2 order by a desc; +a b +2 20 +1 10 +explain select * from t1 where rownum() <= 2 order by a desc; +id select_type table type possible_keys key key_len ref rows Extra +1 SIMPLE t1 ALL NULL NULL NULL NULL 3 Using where; Using filesort +select t1.a,t1.b,rownum() from t1 where rownum() <= 2 order by a desc; +a b rownum() +2 20 2 +1 10 1 +explain select t1.a,t1.b,rownum() from t1 where rownum() <= 2 order by a desc; +id select_type table type possible_keys key key_len ref rows Extra +1 SIMPLE t1 ALL NULL NULL NULL NULL 3 Using where; Using temporary; Using filesort +select *,rownum() from t1,t2; +a b a b rownum() +1 10 2 21 1 +2 20 2 21 2 +3 30 2 21 3 +1 10 3 31 4 +2 20 3 31 5 +3 30 3 31 6 +1 10 4 41 7 +2 20 4 41 8 +3 30 4 41 9 +select *,rownum() from t1,t2 order by t2.a desc, t1.a desc; +a b a b rownum() +3 30 4 41 9 +2 20 4 41 8 +1 10 4 41 7 +3 30 3 31 6 +2 20 3 31 5 +1 10 3 31 4 +3 30 2 21 3 +2 20 2 21 2 +1 10 2 21 1 +select * from (select * from t1 order by a desc) as t where rownum() <= 2; +a b +3 30 +2 20 +select * from t1,t2 where t1.a=t2.a and rownum()<=2 order by t1.a,t2.a; +a b a b +2 20 2 21 +3 30 3 31 +# +# Having +# +select t1.a, sum(t1.b), rownum() as 'r' from t1 group by t1.a having r <= 2; +a sum(t1.b) r +1 10 1 +2 20 2 +select * from t1 having rownum() <= 2; +ERROR HY000: Function 'rownum()' cannot be used in the HAVING clause +select t1.a, sum(t1.b), rownum() from t1 group by t1.a; +a sum(t1.b) rownum() +1 10 1 +2 20 2 +3 30 3 +select t1.a, sum(t1.b), rownum() as 'r' from t1 group by t1.a having rownum() <= 2; +ERROR HY000: Function 'rownum()' cannot be used in the HAVING clause +# +# Sum functions +# +select max(rownum()),min(rownum()) from t1; +max(rownum()) min(rownum()) +3 1 +select sum(rownum()),avg(rownum()) from t1; +sum(rownum()) avg(rownum()) +6 2.0000 +# +# Group by +# +select t1.a,sum(t1.b) from t1 where rownum() < 2 group by t1.a; +a sum(t1.b) +1 10 +select t1.a,sum(t2.b) from t1 JOIN t2 ON (t1.a=t2.a) where rownum() <= 2 group by t1.a; +a sum(t2.b) +2 21 +3 31 +select * from (select t1.a,sum(t2.b) from t1 JOIN t2 ON (t1.a=t2.a) group by t1.a) as t where rownum() <= 1; +a sum(t2.b) +2 21 +select t1.a,sum(rownum()),count(*) from t1 where rownum() <= 2 group by t1.a; +a sum(rownum()) count(*) +1 1 1 +2 2 1 +select * from (select t1.a,sum(t1.b) from t1 group by t1.a) as t3 where rownum() < 2; +a sum(t1.b) +1 10 +create table t3 (a int) engine=myisam; +insert into t3 values (3),(5),(5),(3); +select a, max(rownum()) from t3 group by a; +a max(rownum()) +3 4 +5 3 +drop table t3; +CREATE TABLE t3 ( +a int(11) DEFAULT NULL, +b varchar(1024) DEFAULT NULL +); +insert into t3 select mod(seq*3,20)+1, repeat(char(33+mod(seq,90)),mod(seq,10)*100) from seq_1_to_23; +SELECT sq.a,length(sq.f) FROM (SELECT a, GROUP_CONCAT(b,b) AS f FROM t3 GROUP BY a ORDER BY a desc) as sq WHERE ROWNUM() <= 10; +a length(sq.f) +20 600 +19 1200 +18 1800 +17 400 +16 1000 +15 1600 +14 200 +13 800 +12 1400 +11 0 +drop table t3; +# +# Prepared statements +# +PREPARE stmt1 from "select a,b,rownum() from t1 where rownum() <= 2"; +execute stmt1; +a b rownum() +1 10 1 +2 20 2 +execute stmt1; +a b rownum() +1 10 1 +2 20 2 +deallocate prepare stmt1; +# +# Views +# +create view v1 as select t1.a,rownum() from t1; +select * from v1; +a rownum() +1 1 +2 2 +3 3 +select t1.a,v1.* from t1,v1 where t1.a=v1.a; +a a rownum() +1 1 1 +2 2 2 +3 3 3 +drop view v1; +CREATE TABLE t3 (a INT); +INSERT INTO t3 VALUES (1),(2),(3); +CREATE VIEW v1 AS SELECT a FROM t3 WHERE ROWNUM() <= 2; +SELECT * FROM v1; +a +1 +2 +drop view v1; +drop table t3; +# +# Reserved words +# +create table t4 (a int, rownum int); +insert into t4 (a,rownum) values (1,2); +select t4.a,t4.rownum from t4; +a rownum +1 2 +drop table t4; +# +# Test Oracle mode +# +set SQL_MODE=ORACLE; +select t1.a,rownum from t1 where rownum<=2; +a rownum +1 1 +2 2 +select t1.a,rownum() from t1 where rownum()<=2; +a rownum() +1 1 +2 2 +create table t4 (a int, rownum int); +ERROR 42000: You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near 'rownum int)' at line 1 +DECLARE +CURSOR c_cursor +IS select a,b,rownum from t1 where rownum <= 2; +v_a t1.a%TYPE; +v_b t1.b%TYPE; +v_rn t1.a%TYPE; +BEGIN +OPEN c_cursor; +FETCH c_cursor INTO v_a, v_b, v_rn; +WHILE c_cursor%FOUND LOOP +SELECT concat(v_a,'--',v_b,'--',v_rn); +FETCH c_cursor INTO v_a, v_b, v_rn; +END LOOP; +CLOSE c_cursor; +END;| +concat(v_a,'--',v_b,'--',v_rn) +1--10--1 +concat(v_a,'--',v_b,'--',v_rn) +2--20--2 +select a, rownum from t1 group by a, rownum having rownum < 3; +ERROR HY000: Function 'rownum()' cannot be used in the HAVING clause +select a, rownum as r from t1 group by a, rownum having r < 3; +a r +1 1 +2 2 +select a, rownum from t1 group by a, rownum having "rownum" < 3; +a rownum +1 1 +2 2 +set SQL_MODE=DEFAULT; +# Cleanup +drop table t1,t2; +# +# INSERT +# +create table t1 (a int not null primary key, b int); +insert into t1 values (1,rownum()),(2,rownum()),(3,rownum()); +select * from t1; +a b +1 1 +2 2 +3 3 +drop table t1; +# +# INSERT DELAYED +# +create table t1 (a int not null primary key, b int); +insert delayed into t1 values (1,rownum()),(2,rownum()),(3,rownum()); +select * from t1; +a b +1 1 +2 2 +3 3 +drop table t1; +# +# INSERT ... RETURNING +# +create or replace table t1 (a int); +insert into t1 values (1),(2) returning a, rownum(); +a rownum() +1 1 +2 2 +drop table t1; +# +# UPDATE +# +create table t1 (a int not null primary key, b int); +insert into t1 values (1,1),(2,2),(3,3); +update t1 set b=0; +update t1 set b=rownum()+1; +select * from t1; +a b +1 2 +2 3 +3 4 +update t1 set b=0; +update t1 set b=rownum() where a < 10 and rownum() < 2; +select * from t1; +a b +1 1 +2 0 +3 0 +drop table t1; +create table t1 (a int); +insert into t1 values (10),(20),(30); +update t1 set a = rownum(); +select * from t1; +a +1 +2 +3 +update t1 set a = rownum(); +select * from t1; +a +1 +2 +3 +drop table t1; +# +# DELETE +# +create table t1 (a int not null primary key, b int); +insert into t1 values (1,1),(2,0),(3,0); +delete from t1 where a < 10 and rownum() < 2; +select * from t1; +a b +2 0 +3 0 +drop table t1; +# +# MULTI-TABLE-DELETE +# +create table t1 (a int not null primary key); +insert into t1 values (1),(2),(3); +create table t2 (a int not null primary key); +insert into t2 values (1),(2),(3); +delete t1,t2 from t1,t2 where t1.a=t2.a and rownum() <= 2; +select * from t1; +a +3 +select * from t2; +a +3 +drop table t1,t2; +# +# MULTI-TABLE-UPDATE +CREATE TABLE t1 (ID INT); +CREATE TABLE t2 (ID INT, +s1 TEXT, s2 TEXT, s3 VARCHAR(10), s4 TEXT, s5 VARCHAR(10)); +INSERT INTO t1 VALUES (1),(2); +INSERT INTO t2 VALUES (1,'test', 'test', 'test', 'test', 'test'), +(2,'test', 'test', 'test', 'test', 'test'); +SELECT * FROM t1 LEFT JOIN t2 USING(ID); +ID s1 s2 s3 s4 s5 +1 test test test test test +2 test test test test test +UPDATE t1 LEFT JOIN t2 USING(ID) SET s1 = 'changed'; +select * from t2; +ID s1 s2 s3 s4 s5 +1 changed test test test test +2 changed test test test test +update t2 set s1=""; +UPDATE t1 LEFT JOIN t2 USING(ID) SET s1 = 'changed' where rownum() <=1; +select * from t2; +ID s1 s2 s3 s4 s5 +1 changed test test test test +2 test test test test +drop table t1,t2; +# +# LOAD DATA +# +create table t1 (a int, b int, c int); +load data infile '../../std_data/loaddata7.dat' into table t1 fields terminated by ',' lines terminated by "\r\n" (a,b) set c=rownum(); +select * from t1; +a b c +2 2 1 +3 3 2 +4 4 3 +5 5 4 +6 6 5 +drop table t1; +# +# LIMIT OPTIMIZATION +# +create table t1 (a int); +insert into t1 select seq from seq_1_to_100; +flush status; +select * from t1 where rownum() <= 3; +a +1 +2 +3 +show status like "Rows_read"; +Variable_name Value +Rows_read 3 +flush status; +flush status;
why twice?
+select * from t1 where rownum() <= 4 and rownum() <= 3; +a +1 +2 +3 +show status like "Rows_read"; +Variable_name Value +Rows_read 3 +flush status; +select * from t1 where rownum() < 4 and a > 10; +a +11 +12 +13 +show status like "Rows_read"; +Variable_name Value +Rows_read 13 +flush status; +select * from t1 where 3 >= rownum(); +a +1 +2 +3 +show status like "Rows_read"; +Variable_name Value +Rows_read 3 +flush status; +select * from t1 where 4 > rownum() and a > 20; +a +21 +22 +23 +show status like "Rows_read"; +Variable_name Value +Rows_read 23 +flush status; +select * from t1 where rownum() = 1 and a > 10; +a +11 +show status like "Rows_read"; +Variable_name Value +Rows_read 11 +flush status; +select * from t1 where a > 30 && 1 = rownum(); +a +31 +show status like "Rows_read"; +Variable_name Value +Rows_read 31 +flush status; +# No limit optimization +select * from t1 where rownum() > 10; +a +show status like "Rows_read"; +Variable_name Value +Rows_read 100
You could've optimized it to `limit 0` but I guess it's not a particularly common use case
+flush status; +select * from t1 where 10 < rownum(); +a +show status like "Rows_read"; +Variable_name Value +Rows_read 100 +flush status; +select * from t1 where rownum() >= 10; +a +show status like "Rows_read"; +Variable_name Value +Rows_read 100 +flush status; +select * from t1 where 10 < rownum(); +a +show status like "Rows_read"; +Variable_name Value +Rows_read 100 +flush status; +select * from t1 where 10 <= rownum(); +a +show status like "Rows_read"; +Variable_name Value +Rows_read 100 +flush status; +select * from t1 where 2 = rownum(); +a +show status like "Rows_read"; +Variable_name Value +Rows_read 100 +flush status; +select * from t1 where rownum() = 2; +a +show status like "Rows_read"; +Variable_name Value +Rows_read 100 +flush status; +select * from t1 where rownum() <= 0; +a +show status like "Rows_read"; +Variable_name Value +Rows_read 100 +flush status; +select * from t1 where rownum() < 10 limit 4, 4;
can you add a test for `select a, rownum() from t1 limit 4, 4` please ?
+a +5 +6 +7 +8 +show status like "Rows_read"; +Variable_name Value +Rows_read 8 +flush status; +select * from t1 where rownum() < 10 order by a; +a +1 +2 +3 +4 +5 +6 +7 +8 +9 +show status like "Rows_read"; +Variable_name Value +Rows_read 100 +flush status; +# rownum and limit +select * from t1 where rownum() < 4 limit 10; +a +1 +2 +3 +show status like "Rows_read"; +Variable_name Value +Rows_read 3 +flush status; +select * from t1 where rownum() < 10 limit 4; +a +1 +2 +3 +4 +show status like "Rows_read"; +Variable_name Value +Rows_read 4 +drop table t1; +# +# Rownum examples from Woqutech +# +set SQL_MODE=ORACLE; +create table t1 (c1 int ,c2 varchar(20)) engine=myisam; +insert into t1 values (1, 'aaa'),(2, 'bbb'),(3, 'ccc'),(4, 'ddd'),(5, 'eee'); +update t1 set c2 = 'xxx' where rownum = 2; +select * from t1 where c2='xxx'; +c1 c2 +update t1 set c2 = 'xxx' where rownum < 3; +select * from t1 where c2='xxx'; +c1 c2 +1 xxx +2 xxx +delete from t1 where rownum = 2; +select count(*) from t1; +count(*) +5 +delete from t1 where rownum < 3; +select count(*) from t1; +count(*) +3 +delete from t1 where c1=rownum ; +select count(*) from t1; +count(*) +3 +delete from t1 where c1=rownum+2 ; +select count(*) from t1; +count(*) +0 +set SQL_MODE=DEFAULT; +drop table t1; +# +# Rownum() used in not supported places (returns 0 or gives an error) +# +set @a=rownum(); +select @a; +@a +0 +create or replace table t (a int, b int as (rownum()) virtual); +ERROR HY000: Function or expression 'rownum()' cannot be used in the GENERATED ALWAYS AS clause of `b` +create table t1 (a int); +insert into t1 values (3),(1),(5),(8),(4); +handler t1 open; +handler t1 read next where rownum() < 1; +ERROR HY000: Function or expression 'rownum()' cannot be used in the WHERE clause of `HANDLER`
why is that? `handler t1 read next limit 2` works just fine.
+handler t1 close; +drop table t1; +create table t1 (a int not null primary key, b int); +insert into t1 values (1,1),(2,2),(3,3); +create function f() returns int return rownum(); +select a, rownum(), f() from t1; +a rownum() f() +1 1 0 +2 2 0 +3 3 0 +drop function f; +drop table t1; +create or replace table t1 (a int, r int); +create trigger tr before update on t1 for each row set NEW.r = rownum(); +insert into t1 (a) values (1),(2); +select * from t1; +a r +1 NULL +2 NULL +update t1 set a=a+10; +select * from t1; +a r +11 0 +12 0 +drop trigger tr; +drop table t1; +# +# LIMIT optimisation +# +create table t1 (a int); +insert into t1 values (1),(2),(3),(4),(5); +flush status; +select * from (select a from t1 where a < 1000) as tt where rownum() <= 2; +a +1 +2 +show status like "Rows_read"; +Variable_name Value +Rows_read 2 +explain extended select * from (select a from t1 where a < 1000) as tt where rownum() <= 2; +id select_type table type possible_keys key key_len ref rows filtered Extra +1 PRIMARY <derived2> ALL NULL NULL NULL NULL 2 100.00 Using where +2 DERIVED t1 ALL NULL NULL NULL NULL 5 100.00 Using where +Warnings: +Note 1003 /* select#1 */ select `tt`.`a` AS `a` from (/* select#2 */ select `test`.`t1`.`a` AS `a` from `test`.`t1` where `test`.`t1`.`a` < 1000 limit 2) `tt` where rownum() <= 2 +prepare stmt from "select * from (select a from t1 where a < 1000) as tt where rownum() <= 2"; +flush status; +execute stmt; +a +1 +2 +show status like "Rows_read"; +Variable_name Value +Rows_read 2 +flush status; +execute stmt; +a +1 +2 +show status like "Rows_read"; +Variable_name Value +Rows_read 2 +deallocate prepare stmt; +flush status; +select * from (select a from t1 where a < 1000 group by a) as tt where rownum() <= 2; +a +1 +2 +show status like "Rows_read"; +Variable_name Value +Rows_read 5 +explain extended select * from (select a from t1 where a < 1000 group by a) as tt where rownum() <= 2; +id select_type table type possible_keys key key_len ref rows filtered Extra +1 PRIMARY <derived2> ALL NULL NULL NULL NULL 2 100.00 Using where +2 DERIVED t1 ALL NULL NULL NULL NULL 5 100.00 Using where; Using temporary; Using filesort +Warnings: +Note 1003 /* select#1 */ select `tt`.`a` AS `a` from (/* select#2 */ select `test`.`t1`.`a` AS `a` from `test`.`t1` where `test`.`t1`.`a` < 1000 group by `test`.`t1`.`a` limit 2) `tt` where rownum() <= 2 +prepare stmt from "select * from (select a from t1 where a < 1000 group by a) as tt where rownum() <= 2"; +execute stmt; +a +1 +2 +execute stmt; +a +1 +2 +deallocate prepare stmt; +flush status; +select * from (select a from t1 where a < 1000 group by a order by 1) as tt where rownum() <= 2; +a +1 +2 +show status like "Rows_read"; +Variable_name Value +Rows_read 5 +explain extended select * from (select a from t1 where a < 1000 group by a order by 1) as tt where rownum() <= 2; +id select_type table type possible_keys key key_len ref rows filtered Extra +1 PRIMARY <derived2> ALL NULL NULL NULL NULL 2 100.00 Using where +2 DERIVED t1 ALL NULL NULL NULL NULL 5 100.00 Using where; Using temporary; Using filesort +Warnings: +Note 1003 /* select#1 */ select `tt`.`a` AS `a` from (/* select#2 */ select `test`.`t1`.`a` AS `a` from `test`.`t1` where `test`.`t1`.`a` < 1000 group by `test`.`t1`.`a` order by 1 limit 2) `tt` where rownum() <= 2 +prepare stmt from "select * from (select a from t1 where a < 1000 group by a order by 1) as tt where rownum() <= 2"; +execute stmt; +a +1 +2 +execute stmt; +a +1 +2 +deallocate prepare stmt; +flush status; +select * from (select a from t1 where a < 1000 union select 10) as tt where rownum() <= 2; +a +1 +2 +show status like "Rows_read"; +Variable_name Value +Rows_read 5 +explain extended select * from (select a from t1 where a < 1000 union select 10) as tt where rownum() <= 2; +id select_type table type possible_keys key key_len ref rows filtered Extra +1 PRIMARY <derived2> ALL NULL NULL NULL NULL 2 100.00 Using where +2 DERIVED t1 ALL NULL NULL NULL NULL 5 100.00 Using where +3 UNION NULL NULL NULL NULL NULL NULL NULL NULL No tables used +NULL UNION RESULT <union2,3> ALL NULL NULL NULL NULL NULL NULL +Warnings: +Note 1003 /* select#1 */ select `tt`.`a` AS `a` from (/* select#2 */ select `test`.`t1`.`a` AS `a` from `test`.`t1` where `test`.`t1`.`a` < 1000 union /* select#3 */ select 10 AS `10` limit 2) `tt` where rownum() <= 2 +prepare stmt from "select * from (select a from t1 where a < 1000 union select 10) as tt where rownum() <= 2"; +execute stmt; +a +1 +2 +execute stmt; +a +1 +2 +deallocate prepare stmt; +# Other limit +select * from (select a from t1 where a < 1000 group by a order by 1 limit 1) as tt where rownum() <= 2; +a +1 +explain extended select * from (select a from t1 where a < 1000 group by a order by 1 limit 1) as tt where rownum() <= 2; +id select_type table type possible_keys key key_len ref rows filtered Extra +1 PRIMARY <derived2> ALL NULL NULL NULL NULL 2 100.00 Using where +2 DERIVED t1 ALL NULL NULL NULL NULL 5 100.00 Using where; Using temporary; Using filesort +Warnings: +Note 1003 /* select#1 */ select `tt`.`a` AS `a` from (/* select#2 */ select `test`.`t1`.`a` AS `a` from `test`.`t1` where `test`.`t1`.`a` < 1000 group by `test`.`t1`.`a` order by 1 limit 1) `tt` where rownum() <= 2 +prepare stmt from "select * from (select a from t1 where a < 1000 group by a order by 1 limit 1) as tt where rownum() <= 2"; +execute stmt; +a +1 +execute stmt; +a +1 +deallocate prepare stmt; +# Other limit less +select * from (select a from t1 where a < 1000 group by a order by 1 limit 10) as tt where rownum() <= 2; +a +1 +2 +explain extended select * from (select a from t1 where a < 1000 group by a order by 1 limit 10) as tt where rownum() <= 2; +id select_type table type possible_keys key key_len ref rows filtered Extra +1 PRIMARY <derived2> ALL NULL NULL NULL NULL 2 100.00 Using where +2 DERIVED t1 ALL NULL NULL NULL NULL 5 100.00 Using where; Using temporary; Using filesort +Warnings: +Note 1003 /* select#1 */ select `tt`.`a` AS `a` from (/* select#2 */ select `test`.`t1`.`a` AS `a` from `test`.`t1` where `test`.`t1`.`a` < 1000 group by `test`.`t1`.`a` order by 1 limit 2) `tt` where rownum() <= 2 +prepare stmt from "select * from (select a from t1 where a < 1000 group by a order by 1 limit 10) as tt where rownum() <= 2"; +execute stmt; +a +1 +2 +execute stmt; +a +1 +2 +deallocate prepare stmt; +select * from (select a from t1 where a < 1000 union select 10 limit 1) as tt where rownum() <= 2; +a +1 +explain extended select * from (select a from t1 where a < 1000 union select 10 limit 1) as tt where rownum() <= 2; +id select_type table type possible_keys key key_len ref rows filtered Extra +1 PRIMARY <derived2> ALL NULL NULL NULL NULL 5 100.00 Using where +2 DERIVED t1 ALL NULL NULL NULL NULL 5 100.00 Using where +3 UNION NULL NULL NULL NULL NULL NULL NULL NULL No tables used +NULL UNION RESULT <union2,3> ALL NULL NULL NULL NULL NULL NULL +Warnings: +Note 1003 /* select#1 */ select `tt`.`a` AS `a` from (/* select#2 */ select `test`.`t1`.`a` AS `a` from `test`.`t1` where `test`.`t1`.`a` < 1000 union /* select#3 */ select 10 AS `10` limit 1) `tt` where rownum() <= 2 +prepare stmt from "select * from (select a from t1 where a < 1000 union select 10 limit 1) as tt where rownum() <= 2"; +execute stmt; +a +1 +execute stmt; +a +1 +deallocate prepare stmt; +# < rownum +select * from (select a from t1 where a < 1000) as tt where rownum() < 2; +a +1 +explain extended select * from (select a from t1 where a < 1000) as tt where rownum() < 2; +id select_type table type possible_keys key key_len ref rows filtered Extra +1 PRIMARY <derived2> ALL NULL NULL NULL NULL 2 100.00 Using where +2 DERIVED t1 ALL NULL NULL NULL NULL 5 100.00 Using where +Warnings: +Note 1003 /* select#1 */ select `tt`.`a` AS `a` from (/* select#2 */ select `test`.`t1`.`a` AS `a` from `test`.`t1` where `test`.`t1`.`a` < 1000 limit 1) `tt` where rownum() < 2 +prepare stmt from "select * from (select a from t1 where a < 1000) as tt where rownum() < 2"; +execute stmt; +a +1 +execute stmt; +a +1 +deallocate prepare stmt; +# Simple expression +select * from (select a from t1 where a < 1000) as tt where rownum() <= 1+1; +a +1 +2 +explain extended select * from (select a from t1 where a < 1000) as tt where rownum() <= 1+1; +id select_type table type possible_keys key key_len ref rows filtered Extra +1 PRIMARY <derived2> ALL NULL NULL NULL NULL 2 100.00 Using where +2 DERIVED t1 ALL NULL NULL NULL NULL 5 100.00 Using where +Warnings: +Note 1003 /* select#1 */ select `tt`.`a` AS `a` from (/* select#2 */ select `test`.`t1`.`a` AS `a` from `test`.`t1` where `test`.`t1`.`a` < 1000 limit 2) `tt` where rownum() <= <cache>(1 + 1) +prepare stmt from "select * from (select a from t1 where a < 1000) as tt where rownum() <= 1+1"; +execute stmt; +a +1 +2 +execute stmt; +a +1 +2 +deallocate prepare stmt; +# Simple expression reversed +select * from (select a from t1 where a < 1000) as tt where 1+1 >= rownum(); +a +1 +2 +explain extended select * from (select a from t1 where a < 1000) as tt where 1+1 >= rownum(); +id select_type table type possible_keys key key_len ref rows filtered Extra +1 PRIMARY <derived2> ALL NULL NULL NULL NULL 2 100.00 Using where +2 DERIVED t1 ALL NULL NULL NULL NULL 5 100.00 Using where +Warnings: +Note 1003 /* select#1 */ select `tt`.`a` AS `a` from (/* select#2 */ select `test`.`t1`.`a` AS `a` from `test`.`t1` where `test`.`t1`.`a` < 1000 limit 2) `tt` where <cache>(1 + 1) >= rownum() +prepare stmt from "select * from (select a from t1 where a < 1000) as tt where 1+1 >= rownum()"; +execute stmt; +a +1 +2 +execute stmt; +a +1 +2 +deallocate prepare stmt; +# expensive (no opt) +select * from (select a from t1 where a < 1000) as tt where (select max(a) from t1) >= rownum(); +a +1 +2 +3 +4 +5 +explain extended select * from (select a from t1 where a < 1000) as tt where (select max(a) from t1) >= rownum(); +id select_type table type possible_keys key key_len ref rows filtered Extra +1 PRIMARY <derived2> ALL NULL NULL NULL NULL 5 100.00 Using where +3 SUBQUERY t1 ALL NULL NULL NULL NULL 5 100.00 +2 DERIVED t1 ALL NULL NULL NULL NULL 5 100.00 Using where +Warnings: +Note 1003 /* select#1 */ select `tt`.`a` AS `a` from (/* select#2 */ select `test`.`t1`.`a` AS `a` from `test`.`t1` where `test`.`t1`.`a` < 1000) `tt` where (/* select#3 */ select max(`test`.`t1`.`a`) from `test`.`t1`) >= rownum() +prepare stmt from "select * from (select a from t1 where a < 1000) as tt where (select max(a) from t1) >= rownum()"; +execute stmt; +a +1 +2 +3 +4 +5 +execute stmt; +a +1 +2 +3 +4 +5 +deallocate prepare stmt; +drop table t1; diff --git a/sql/filesort.h b/sql/filesort.h index 29ae5e20cc6..806a4a8b027 100644 --- a/sql/filesort.h +++ b/sql/filesort.h @@ -44,24 +44,21 @@ class Filesort: public Sql_alloc /** Number of records to return */ ha_rows limit; /** ORDER BY list with some precalculated info for filesort */ + ha_rows *accepted_rows; /* For ROWNUM */ SORT_FIELD *sortorder;
first, I suspect that you want to put your accepted_rows declaration before the comment that applies to sortorder. second, could you do a bit more than just "For ROWNUM"? why do you need something "for ROWNUM" inside Filesort?
/** select to use for getting records */ SQL_SELECT *select; + /** TRUE <=> free select on destruction */ bool own_select; - /** true means we are using Priority Queue for order by with limit. */ + /** TRUE means we are using Priority Queue for order by with limit. */ bool using_pq; - /* TRUE means sort operation must produce table rowids. FALSE means that it halso has an option of producing {sort_key, addon_fields} pairs. */ bool sort_positions; - - Filesort_tracker *tracker; - Sort_keys *sort_keys; - /* TRUE means all the fields of table of whose bitmap read_set is set need to be read while reading records in the sort buffer. diff --git a/sql/item_func.h b/sql/item_func.h index 53c236bbce7..c237fcd9ecc 100644 --- a/sql/item_func.h +++ b/sql/item_func.h @@ -2125,6 +2125,63 @@ class Item_func_rand :public Item_real_func };
+class Item_func_rownum :public Item_longlong_func +{ + /* + This points to a variable that contains the number of rows + accpted so far in the result set + */ + ha_rows *accepted_rows; + SELECT_LEX *select; +public: + Item_func_rownum(THD *thd); + longlong val_int() override; + LEX_CSTRING func_name_cstring() const override + { + static LEX_CSTRING name= {STRING_WITH_LEN("rownum") }; + return name; + } + enum Functype functype() const override { return ROWNUM_FUNC; } + void update_used_tables() override {} + bool const_item() const override { return 0; } + void fix_after_optimize(THD *thd) override; + bool fix_fields(THD* thd, Item **ref) override; + bool fix_length_and_dec() override + { + unsigned_flag= 1; + used_tables_cache= RAND_TABLE_BIT; + const_item_cache=0; + set_maybe_null();
how can rownum be NULL?
+ return FALSE; + } + void cleanup() override + { + Item_longlong_func::cleanup(); + /* Ensure we don't point to freed memory */ + accepted_rows= 0; + } + bool check_vcol_func_processor(void *arg) override + { + return mark_unsupported_function(func_name(), "()", arg, + VCOL_IMPOSSIBLE); + } + bool check_handler_func_processor(void *arg) override + { + return mark_unsupported_function(func_name(), "()", arg, + VCOL_IMPOSSIBLE);
first, as I wrote above, I think that there's no logical reason why rownum should not with with handler. but even if it doesn't, it's much better to check for SQLCOM_HA_READ from Item_func_rownum::fix_fields instead of traversing the whole item tree for every single HANDLER READ command looking for Item_func_rownum that nobody (literally, not a single user out of all our millions) will use there.
+ } + Item *get_copy(THD *thd) override + { + return this;
It looks wrong. We don't have any other Item that returns `this` here. I think rownum cannot be pushed down because of RAND_TABLE_BIT, So better return NULL here, meaning "cannot be pushed down". Instead of `this` meaning "if you push it down, it'll surreptitiously start corrupting the memory"
+ } + /* This function is used in insert, update and delete */ + void store_pointer_to_row_counter(ha_rows *row_counter)
So this isn't a virtual method, and to use it you downcast after checking type() == FUNC_ITEM and functype() == ROWNUM_FUNC. To avoid adding a new virtual method thus blowing up the vtable, I presume. And fix_after_optimize() is virtual. To avoid error-prone downcasting, would be my guess. Can you explain when you use what approach, please?
+ { + accepted_rows= row_counter; + } +}; + + class Item_func_sign :public Item_long_func { bool check_arguments() const override diff --git a/sql/item_func.cc b/sql/item_func.cc index 25714dda03c..1165b87356a 100644 --- a/sql/item_func.cc +++ b/sql/item_func.cc @@ -7206,3 +7206,72 @@ void Item_func_setval::print(String *str, enum_query_type query_type) str->append_ulonglong(round); str->append(')'); } + + +/* + Return how many row combinations has accepted so far + 1 + + The + 1 is to ensure that 'WHERE ROWNUM <=1' returns one row
this is a very weird explanation, it sounds as if it's a heuristical a hack for ROWNUM <=1 to work Why not to write "+1 takes into account the currently computed row"
+*/ + +longlong Item_func_rownum::val_int() +{ + if (!accepted_rows) + { + /* + Rownum is not properly set up. Probably used in wrong context when + it should not be used. In this case returning 0 is probably the best + solution. + */
DBUG_ASSERT(0); here?
+ return 0; + } + return (longlong) *accepted_rows+1; +} + + +Item_func_rownum::Item_func_rownum(THD *thd): + Item_longlong_func(thd),accepted_rows(0) +{ + /* + Remember the select context. + Add the function to the list fix_after_optimize in the select context + so that we can easily initializef all rownum functions with the pointers + to the row counters. + */ + select= thd->lex->current_select; + select->fix_after_optimize.push_back(this, thd->mem_root); + + /* + Mark that query is using rownum() and ensure that this select is + not merged with other selects + */ + select->with_rownum= 1; + thd->lex->with_rownum= 1; + thd->lex->uncacheable(UNCACHEABLE_RAND);
Why? ROWNUM doesn't make a query uncacheable, it's perfectly deterministic and repeatable.
+ + /* If this command changes data, mark it as unsafe for statement logging */ + if (sql_command_flags[thd->lex->sql_command] & + (CF_UPDATES_DATA | CF_DELETES_DATA)) + thd->lex->set_stmt_unsafe(LEX::BINLOG_STMT_UNSAFE_SYSTEM_FUNCTION);
Why? It's the same as LIMIT. unsafe without ORDER BY and perfectly safe with.
+} + + +bool Item_func_rownum::fix_fields(THD* thd, Item **ref) +{ + if (select->having_fix_field) + { + my_error(ER_FUNCTION_CANNOT_BE_USED_IN_CLAUSE, MYF(0), + "rownum()", "HAVING"); + return 1; + } + return Item_longlong_func::fix_fields(thd, ref); +} + +/* + Store a reference to the variable that contains number of accepted rows +*/ + +void Item_func_rownum::fix_after_optimize(THD *thd) +{ + accepted_rows= &select->join->accepted_rows;
why do you need both that and fix_rownum_pointers()? I'd think that the latter alone would be sufficient
+} diff --git a/sql/share/errmsg-utf8.txt b/sql/share/errmsg-utf8.txt index badaaa17716..aecb00563f7 100644 --- a/sql/share/errmsg-utf8.txt +++ b/sql/share/errmsg-utf8.txt @@ -7973,3 +7973,5 @@ ER_DATA_WAS_COMMITED_UNDER_ROLLBACK eng "Engine %s does not support rollback. Changes were committed during rollback call" ER_PK_INDEX_CANT_BE_IGNORED eng "A primary key cannot be marked as IGNORE" +ER_FUNCTION_CANNOT_BE_USED_IN_CLAUSE + eng "Function '%s' cannot be used in the %s clause"
Is it the first function we've got that cannot be used in HAVING?
diff --git a/sql/sql_derived.cc b/sql/sql_derived.cc index 35b4294707f..da6c567b87c 100644 --- a/sql/sql_derived.cc +++ b/sql/sql_derived.cc @@ -656,14 +656,22 @@ static bool mysql_derived_prepare(THD *thd, LEX *lex, TABLE_LIST *derived) { SELECT_LEX_UNIT *unit= derived->get_unit(); - bool res= FALSE; + SELECT_LEX *first_select; + bool res= FALSE, keep_row_order; DBUG_ENTER("mysql_derived_prepare"); DBUG_PRINT("enter", ("unit: %p table_list: %p alias: '%s'", unit, derived, derived->alias.str)); if (!unit) DBUG_RETURN(FALSE);
- SELECT_LEX *first_select= unit->first_select(); + first_select= unit->first_select(); + /* + If rownum() is used we have to preserve the insert row order + to make group by with filesort to work.
it'd help if you'd add an example of a SELECT here
+ */ + keep_row_order= (thd->lex->with_rownum && + (first_select->group_list.elements || + first_select->order_list.elements));
if (derived->is_recursive_with_table() && !derived->is_with_table_recursive_reference() && diff --git a/sql/sql_insert.cc b/sql/sql_insert.cc index 806049b4465..5c4dff33914 100644 --- a/sql/sql_insert.cc +++ b/sql/sql_insert.cc @@ -746,6 +746,9 @@ bool mysql_insert(THD *thd, TABLE_LIST *table_list, DBUG_RETURN(TRUE); } + + if (thd->lex->with_rownum && table_list->lock_type == TL_WRITE_DELAYED) + table_list->lock_type= TL_WRITE;
why? do you have a test case when it's needed?
if (table_list->lock_type == TL_WRITE_DELAYED) { if (open_and_lock_for_insert_delayed(thd, table_list)) @@ -968,11 +971,16 @@ bool mysql_insert(THD *thd, TABLE_LIST *table_list, */ if (returning && result->send_result_set_metadata(returning->item_list, - Protocol::SEND_NUM_ROWS | Protocol::SEND_EOF)) + Protocol::SEND_NUM_ROWS | + Protocol::SEND_EOF)) goto values_loop_end;
THD_STAGE_INFO(thd, stage_update); thd->decide_logging_format_low(table); + fix_rownum_pointers(thd, thd->lex->current_select, &info.copied); + if (returning) + fix_rownum_pointers(thd, thd->lex->returning(), &info.copied); +
Note that info.copied can be incremented more than once per row (in INSERT...ON DUPLICATE KEY UPDATE, for example). This will probably break rownum logic.
do { DBUG_PRINT("info", ("iteration %llu", iteration)); @@ -2133,8 +2141,14 @@ int write_record(THD *thd, TABLE *table, COPY_INFO *info, select_result *sink) autoinc values (generated inside the handler::ha_write()) and values updated in ON DUPLICATE KEY UPDATE. */ - if (sink && sink->send_data(thd->lex->returning()->item_list) < 0) - trg_error= 1; + if (sink) + { + /* We have to adjust 'copied' to make ROWNUM() work in RETURNING */ + info->copied--; + if (sink->send_data(thd->lex->returning()->item_list) < 0) + trg_error= 1; + info->copied++;
this is quite silly. Can you simply increment info->copied after this sink->send_data call?
+ }
after_trg_or_ignored_err: if (key) diff --git a/sql/sql_limit.h b/sql/sql_limit.h index a4fcedac14a..62fd0546af0 100644 --- a/sql/sql_limit.h +++ b/sql/sql_limit.h @@ -67,6 +67,15 @@ class Select_limit_counters { return select_limit_cnt; } ha_rows get_offset_limit() { return offset_limit_cnt; } + + void set_limit_if_lower(ha_rows limit) + { + if (!offset_limit_cnt)
I suppose, alternatively, you could've had if (offset_limit_cnt + select_limit_cnt > limit) select_limit_cnt= limit - offset_limit_cnt;
+ { + if (select_limit_cnt > limit) + select_limit_cnt= limit; + } + } };
#endif // INCLUDES_MARIADB_SQL_LIMIT_H diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy index 2bd2575b882..57ba9df42c0 100644 --- a/sql/sql_yacc.yy +++ b/sql/sql_yacc.yy @@ -10286,6 +10287,22 @@ function_call_nonkeyword: if (unlikely($$ == NULL)) MYSQL_YYABORT; } +/* Start SQL_MODE_ORACLE_SPECIFIC + | ROWNUM_SYM optional_braces + { + $$= new (thd->mem_root) Item_func_rownum(thd); + if (unlikely($$ == NULL)) + MYSQL_YYABORT; + } +End SQL_MODE_ORACLE_SPECIFIC */ +/* Start SQL_MODE_DEFAULT_SPECIFIC */ + | ROWNUM_SYM '(' ')' + { + $$= new (thd->mem_root) Item_func_rownum(thd); + if (unlikely($$ == NULL)) + MYSQL_YYABORT; + } +/* End SQL_MODE_DEFAULT_SPECIFIC */
You have two nearly identical rules compiled conditionally. I can pretty much guarantee that they'll go out of sync eventually. Better do something like /* Start SQL_MODE_ORACLE_SPECIFIC oracle_optional_braces: optional_braces ; End SQL_MODE_ORACLE_SPECIFIC */ /* Start SQL_MODE_DEFAULT_SPECIFIC */ oracle_optional_braces: '(' ')' ; /* End SQL_MODE_DEFAULT_SPECIFIC */ or even | ROWNUM_SYM /* Start SQL_MODE_ORACLE_SPECIFIC optional_braces End SQL_MODE_ORACLE_SPECIFIC */ /* Start SQL_MODE_DEFAULT_SPECIFIC */ '(' ')' /* End SQL_MODE_DEFAULT_SPECIFIC */ { $$= new (thd->mem_root) Item_func_rownum(thd); if (unlikely($$ == NULL)) MYSQL_YYABORT; }
| SUBDATE_SYM '(' expr ',' expr ')' { $$= new (thd->mem_root) Item_date_add_interval(thd, $3, $5, diff --git a/sql/table.cc b/sql/table.cc index 7f3a6468aa2..100813e1f95 100644 --- a/sql/table.cc +++ b/sql/table.cc @@ -9331,6 +9331,10 @@ bool TABLE_LIST::init_derived(THD *thd, bool init_view) { /* A subquery might be forced to be materialized due to a side-effect. */ if (!is_materialized_derived() && first_select->is_mergeable() && + (unit->outer_select() && !unit->outer_select()->with_rownum) &&
no merging if *outer select* uses rownum? why? example?
+ (!thd->lex->with_rownum || + (!first_select->group_list.elements && + !first_select->order_list.elements)) &&
no merging if no rownum but there's group by or order by? group by - this is obvious, and is already checked earlier in this if() order by - why?
optimizer_flag(thd, OPTIMIZER_SWITCH_DERIVED_MERGE) && !thd->lex->can_not_use_merged(1) && !is_recursive_with_table()) diff --git a/sql/sql_select.cc b/sql/sql_select.cc index 4bb0bc661bd..b002e27799b 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -1789,10 +1803,18 @@ JOIN::optimize_inner() { DBUG_ENTER("JOIN::optimize_inner"); subq_exit_fl= false; - do_send_rows = (unit->lim.get_select_limit()) ? 1 : 0;
DEBUG_SYNC(thd, "before_join_optimize");
+ // rownum used somewhere in query, no limits and it is derived + if (unlikely(thd->lex->with_rownum && + select_lex->first_cond_optimization && + select_lex->master_unit()->derived)) + optimize_upper_rownum_func(); + + do_send_rows = (unit->lim.get_select_limit()) ? 1 : 0;
Is there any particular reason why you put that between DEBUG_SYNC(thd, "before_join_optimize"); and THD_STAGE_INFO(thd, stage_optimizing); ? I'd expect them logically to go directly one after the other
+ + THD_STAGE_INFO(thd, stage_optimizing);
set_allowed_join_cache_types(); @@ -14210,7 +14236,24 @@ static ORDER * remove_const(JOIN *join,ORDER *first_order, COND *cond, bool change_list, bool *simple_order) { - *simple_order= join->rollup.state == ROLLUP::STATE_NONE; + /* + We can't do ORDER BY using filesort if the select list contains a non + deterministic value like RAND() or ROWNUM(). + For example: + SELECT RAND() as 'r' from t1 order by r;
This one uses temporary for me. Can you put here a correct example please?
+ + If we would first sort the table 't1', the RAND() column would be generated + during end_send() and the order would be wrong. + + Previously we had here also a test of ROLLUP: + 'join->rollup.state == ROLLUP::STATE_NONE' + I deleted this because + the ROLLUP was never enforced because of a bug where the inital value + of simple_order was ignored. Having ROLLUP tested now when the code + is fixed causes many test failure and some wrong results so better to + leave the code as it was related to ROLLUP.
No need to explain in a comment why something that is no longer here was removed. Use git for that, commit comments. The code comment should explain the code as it is.
+ */ + *simple_order= !join->rand_table_in_field_list;
why do you need rand_table_in_field_list instead of writing *simple_order= join->select_lex->select_list_tables & RAND_TABLE_BIT; ?
if (join->only_const_tables()) return change_list ? 0 : first_order; // No need to sort
@@ -29587,6 +29640,273 @@ void unpack_to_base_table_fields(TABLE *table) (*cp->do_copy)(cp); }
+/* + Call item->fix_after_optimize for all items registered in + lex->fix_after_optimize + + This is needed for items like ROWNUM(), which needs access to structures + created by the early optimizer pass, like JOIN +*/ + +static void fix_items_after_optimize(THD *thd, SELECT_LEX *select_lex) +{ + List_iterator<Item> li(select_lex->fix_after_optimize); + + while (Item *item= li++) + item->fix_after_optimize(thd); +} + +
here you could've had a comment, like /* callbacks for process_direct_rownum_comparison() */
+static bool is_const(Item *item) +{ + /* + Constant expression in WHERE can be also subquery or other expensive + thing we shoud not evaluate on OPTIMIZATION phase + */ + return item->const_item() && !item->is_expensive(); +} + + +// Check if this is an unexpensive functional expression over basic constants + +static bool is_basic_const(Item *item) +{ + if (item->basic_const_item()) + return true; + if (!item->const_item() || item->is_expensive()) + return false; + if (item->type() == Item::FUNC_ITEM) + { + Item_func *func= ((Item_func *)item); + if (func->argument_count() == 0) + return false; + bool result= true; + Item **args= func->arguments(); + for (uint i= 0; i < func->argument_count() && result; i++) + result= result && is_basic_const(args[i]); + return result; + } + return false; +}
is_basic_const() is a misleading name, a function of basic consts isn't a basic const itself. what's an example of an expression has is_const() == true, but is_basic_const() == false?
+ + +inline bool is_simple_rownum_exp(Item *rownum, Item *constant)
unused
+{ + return (constant->basic_const_item() && + rownum->type() == Item::FUNC_ITEM && + ((Item_func*) rownum)->functype() == Item_func::ROWNUM_FUNC); +} + + +static bool temporary_set_limit(THD *thd __attribute__((unused)), + SELECT_LEX_UNIT *unit, + ha_rows lim) +{ + unit->set_limit_if_lower(lim);
why is it temporary? in the PS/SP sense, just for one execution?
+ return false; +} + + +static bool permanent_limit(THD *thd, SELECT_LEX_UNIT *unit, ha_rows lim) +{ + SELECT_LEX *gpar= unit->global_parameters(); + if (gpar->select_limit != 0 && + // limit can not be an expression but can be parameter + (!gpar->select_limit->basic_const_item() || + ((ha_rows)gpar->select_limit->val_int()) < lim)) + return false; + + Query_arena *arena, backup; + arena= thd->activate_stmt_arena_if_needed(&backup); + + gpar->select_limit= + new (thd->mem_root)Item_int(thd, lim, 20 /* max 64 bit length*/ );
we have defines for that, MAX_BIGINT_WIDTH, MY_INT64_NUM_DECIMAL_DIGITS, LONGLONG_BUFFER_SIZE - whatever you prefer.
+ if (gpar->select_limit == 0) + return true; // EOM + + unit->set_limit(gpar); + + gpar->explicit_limit= true; // to show in EXPLAIN + + if (arena) + thd->restore_active_arena(arena, &backup); + + return false; +} + + +/** + Check posibility of LIMIT setting by rownum() of upper SELECT and do it + + @note Ideal is to convert something like + SELECT ... + FROM (SELECT ...) table + WHERE rownum() < <CONSTANT>; + to + SELECT ... + FROM (SELECT ... LIMIT <CONSTANT>) table + WHERE rownum() < <CONSTANT>;
1. what if the outer select uses LIMIT, not ROWNUM? why not to push that down too? 2. why are you not removing rownum from the outer select? if you remove rownum and reset with_rownum=false, it'll generally give optimizer more freedom.
+ + @retval true EOM + @retval false no errors +*/ + +bool JOIN::optimize_upper_rownum_func() +{ + DBUG_ASSERT(select_lex->master_unit()->derived); + + if (select_lex->master_unit()->first_select() != select_lex) + return false; // first will set parameter + + if (select_lex->master_unit()->global_parameters()->offset_limit != NULL) + return false; // offset is set, we cannot set limit + + SELECT_LEX *outer_select= select_lex->master_unit()->outer_select(); + /* + Check that the outer query (the one that has 'WHERE rownum()...') is using + to see if the can be use it to limit this sub query + */ + if (outer_select == NULL || + !outer_select->with_rownum || + (outer_select->options & SELECT_DISTINCT) || + outer_select->table_list.elements != 1 || + outer_select->where == NULL || + outer_select->where->type() != Item::FUNC_ITEM) + return false; + + return process_direct_rownum_comparison(thd, unit, outer_select->where, + &is_basic_const, + &permanent_limit); +} + +/** + Test if the predicate compares rownum() with a constant + + @return 1 No or invalid rownum() compare + @return 0 rownum() is compared with a constant. + In this case *args contains the constant and + *inv_order constains 1 if the rownum() was the right + argument, like in 'WHERE 2 >= rownum()'. +*/ + +static bool check_rownum_usage(Item_func *func_item, longlong *limit, + bool *inv_order, const_probe probe) +{ + Item *arg1, *arg2; + *inv_order= 0; + DBUG_ASSERT(func_item->argument_count() == 2); + + /* 'rownum op const' or 'const op field' */ + arg1= func_item->arguments()[0]->real_item(); + if (arg1->type() == Item::FUNC_ITEM && + ((Item_func*) arg1)->functype() == Item_func::ROWNUM_FUNC) + { + arg2= func_item->arguments()[1]->real_item(); + if ((*probe)(arg2)) + { + *limit= arg2->val_int(); + return *limit <= 0 || (ulonglong) *limit >= HA_POS_ERROR; + } + } + else if ((*probe)(arg1)) + { + arg2= func_item->arguments()[1]->real_item();
why not to assign arg2 outside of the if() ?
+ if (arg2->type() == Item::FUNC_ITEM && + ((Item_func*) arg2)->functype() == Item_func::ROWNUM_FUNC) + { + *limit= arg1->val_int(); + *inv_order= 1; + return *limit <= 0 || (ulonglong) *limit >= HA_POS_ERROR; + } + } + return 1; +} + + +/* + Limit optimization for ROWNUM() + + Go through the WHERE clause and find out if there are any of the following + constructs on the top level: + rownum() <= integer_constant + rownum() < integer_constant + rownum() = 1 + + If yes, then threat the select as if 'LIMIT integer_constant' would + have been used +*/ + +static void optimize_rownum(THD *thd, SELECT_LEX_UNIT *unit, Item *cond) +{ + DBUG_ENTER("optimize_rownum"); + + if (cond->type() == Item::COND_ITEM) + { + if (((Item_cond*) cond)->functype() == Item_func::COND_AND_FUNC) + { + List_iterator<Item> li(*((Item_cond*) cond)->argument_list()); + Item *item; + while ((item= li++)) + optimize_rownum(thd, unit, item); + } + DBUG_VOID_RETURN; + } + + process_direct_rownum_comparison(thd, unit, cond, &is_const, + &temporary_set_limit);
please, explain why it's is_const/temporary_set_limit here and is_basic_const/permanent_limit in optimize_upper_rownum_func().
+ DBUG_VOID_RETURN; +} + + +static bool process_direct_rownum_comparison(THD *thd, + SELECT_LEX_UNIT *unit, + Item *cond, + const_probe probe, + set_limit_callback set_limit) +{ + DBUG_ENTER("process_direct_rownum_comparison"); + if (cond->real_type() == Item::FUNC_ITEM) + { + Item_func *pred= (Item_func*) cond; + longlong limit; + bool inv; + + if (pred->argument_count() != 2) + DBUG_RETURN(false); // Not a compare functions + if (check_rownum_usage(pred, &limit, &inv, probe)) + DBUG_RETURN(false); + + Item_func::Functype pred_type= pred->functype(); + + if (inv && pred_type != Item_func::EQ_FUNC) + { + if (pred_type == Item_func::GT_FUNC) // # > rownum() + pred_type= Item_func::LT_FUNC; + else if (pred_type == Item_func::GE_FUNC) // # >= rownum() + pred_type= Item_func::LE_FUNC; + else + DBUG_RETURN(false); + } + switch (pred_type) { + case Item_func::LT_FUNC: // rownum() < # + { + if (limit <= 0) + DBUG_RETURN(false); + DBUG_RETURN((*set_limit)(thd, unit, limit - 1)); + case Item_func::LE_FUNC: + DBUG_RETURN((*set_limit)(thd, unit, limit)); + case Item_func::EQ_FUNC: + if (limit == 1) + DBUG_RETURN((*set_limit)(thd, unit, limit)); + break; + default: + break; + } + }
what if it's rownum() > 5 ? why not to mark it as always false right away?
+ } + DBUG_RETURN(false); +} +
/** @} (end of group Query_Optimizer)
Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org
On Mon, Apr 12, 2021 at 7:20 PM Sergei Golubchik <serg@mariadb.org> wrote:
Hi, Michael!
On Apr 12, Michael Widenius wrote:
revision-id: c76ac1e6de8 (mariadb-10.5.2-551-gc76ac1e6de8) parent(s): a507eb17708 author: Michael Widenius <michael.widenius@gmail.com> committer: Michael Widenius <michael.widenius@gmail.com> timestamp: 2021-03-24 19:42:24 +0200 message:
Please include always the first row in the commit message so that I can know which commit you are reviewing. (revision id's are often useless for reviews)
diff --git a/mysql-test/main/rownum.test b/mysql-test/main/rownum.test new file mode 100644 index 00000000000..8d89eed2f86 --- /dev/null +++ b/mysql-test/main/rownum.test @@ -0,0 +1,542 @@ +--source include/have_sequence.inc +# +# Test of basic rownum() functionallity +# Test are done with Aria to ensure that row order is stable
what does that mean?
It means that SELECT * will return the rows in the same order for the test. (There is normally no guaranteed that insert and select order are the same). Please do not include things that your not commenting upon, to make it easier to find your comments.
+ OPEN c_cursor; + FETCH c_cursor INTO v_a, v_b, v_rn; + WHILE c_cursor%FOUND LOOP + SELECT concat(v_a,'--',v_b,'--',v_rn); + FETCH c_cursor INTO v_a, v_b, v_rn; + END LOOP; + CLOSE c_cursor; +END;| +DELIMITER ;| + +--error ER_FUNCTION_CANNOT_BE_USED_IN_CLAUSE +select a, rownum from t1 group by a, rownum having rownum < 3; +select a, rownum from t1 group by a, rownum having "rownum" < 3;
what's the difference between these two? Why the first is an error and the second is not?
There are quotes around rownum in the second version. In Oracle mode that is an error, in MariaDB mode it is not.
+select a, rownum as r from t1 group by a, rownum having r < 3;
And the this one. Why is it not an error either?
Because 'r' is a reference, not a function. 'r' works on the result set. rownum cannot be executed in the having clause (which is executed after all rows are returned).
diff --git a/sql/filesort.h b/sql/filesort.h index 29ae5e20cc6..806a4a8b027 100644 --- a/sql/filesort.h +++ b/sql/filesort.h @@ -44,24 +44,21 @@ class Filesort: public Sql_alloc /** Number of records to return */ ha_rows limit; /** ORDER BY list with some precalculated info for filesort */ + ha_rows *accepted_rows; /* For ROWNUM */ SORT_FIELD *sortorder;
first, I suspect that you want to put your accepted_rows declaration before the comment that applies to sortorder.
Fixed.
second, could you do a bit more than just "For ROWNUM"? why do you need something "for ROWNUM" inside Filesort?
Because we excute the WHERE clause under filesort. and then rownum function needs to know how many rows we have found so far. I added /* Used with ROWNUM. Contains the number of rows filesort has found so far */
index 53c236bbce7..c237fcd9ecc 100644
--- a/sql/item_func.h +++ b/sql/item_func.h @@ -2125,6 +2125,63 @@ class Item_func_rand :public Item_real_func };
+class Item_func_rownum :public Item_longlong_func +{ + /* + This points to a variable that contains the number of rows + accpted so far in the result set + */ + ha_rows *accepted_rows; + SELECT_LEX *select; +public: + Item_func_rownum(THD *thd); + longlong val_int() override; + LEX_CSTRING func_name_cstring() const override + { + static LEX_CSTRING name= {STRING_WITH_LEN("rownum") }; + return name; + } + enum Functype functype() const override { return ROWNUM_FUNC; } + void update_used_tables() override {} + bool const_item() const override { return 0; } + void fix_after_optimize(THD *thd) override; + bool fix_fields(THD* thd, Item **ref) override; + bool fix_length_and_dec() override + { + unsigned_flag= 1; + used_tables_cache= RAND_TABLE_BIT; + const_item_cache=0; + set_maybe_null();
how can rownum be NULL?
In context where there are no rows. like SET @A=rownum;
+ return FALSE; + } + void cleanup() override + { + Item_longlong_func::cleanup(); + /* Ensure we don't point to freed memory */ + accepted_rows= 0; + } + bool check_vcol_func_processor(void *arg) override + { + return mark_unsupported_function(func_name(), "()", arg, + VCOL_IMPOSSIBLE); + } + bool check_handler_func_processor(void *arg) override + { + return mark_unsupported_function(func_name(), "()", arg, + VCOL_IMPOSSIBLE);
first, as I wrote above, I think that there's no logical reason why rownum should not with with handler.
Please read the patch. It cannot be in the handler for multiple reasons: - INSERT, UPDATE, LOAD DATA - handler does not know if the row is accepted - joins etc.
but even if it doesn't, it's much better to check for SQLCOM_HA_READ from Item_func_rownum::fix_fields instead of traversing the whole item tree for every single HANDLER READ command looking for Item_func_rownum that nobody (literally, not a single user out of all our millions) will use there.
There is only a traverse of the tree looking for rownum if the rownum function is used. Why do you think there is some extra travelling of the tree for HANDLER_READ ?
+ } + Item *get_copy(THD *thd) override + { + return this;
It looks wrong. We don't have any other Item that returns `this` here. I think rownum cannot be pushed down because of RAND_TABLE_BIT, So better return NULL here, meaning "cannot be pushed down". Instead of `this` meaning "if you push it down, it'll surreptitiously start corrupting the memory"
I don't think it can trash memory even if it would be pushed down. As long as the item exists, it should be safe to use anywhere. I also don't trust the code to be able to cope with returning null in get_copy.
+ } + /* This function is used in insert, update and delete */ + void store_pointer_to_row_counter(ha_rows *row_counter)
So this isn't a virtual method, and to use it you downcast after checking type() == FUNC_ITEM and functype() == ROWNUM_FUNC. To avoid adding a new virtual method thus blowing up the vtable, I presume.
Yes, it is not a virtual and should not be. And yes, we do not want to add any new virtual functions to items when we have close to 600+ items in the system... It is also much faster to do an if on upper level than doing a function call.
And fix_after_optimize() is virtual. To avoid error-prone downcasting, would be my guess.
Yes. This is something we may need for other items in the future, which is why I made it virtaul.
Can you explain when you use what approach, please?
See above.
--- a/sql/item_func.cc +++ b/sql/item_func.cc @@ -7206,3 +7206,72 @@ void Item_func_setval::print(String *str, enum_query_type query_type) str->append_ulonglong(round); str->append(')'); } + + +/* + Return how many row combinations has accepted so far + 1 + + The + 1 is to ensure that 'WHERE ROWNUM <=1' returns one row
this is a very weird explanation, it sounds as if it's a heuristical a hack for ROWNUM <=1 to work
Why not to write "+1 takes into account the currently computed row"
Because the row is not yet computed or accepted. We can call rownum any time, after a row is accpted, before etc. I personally prefer the current comment as it is more well defined.
+*/ + +longlong Item_func_rownum::val_int() +{ + if (!accepted_rows) + { + /* + Rownum is not properly set up. Probably used in wrong context when + it should not be used. In this case returning 0 is probably the best + solution. + */
DBUG_ASSERT(0); here?
No. One can use rownum in a lot of context where it does makes sence. Instead of trying to generate an error for every of these cases, I decided to return 0 for these. In the future maybe some of these will be valid and then things can change. I do not want to see asserts in test cases people add for things like this.
+ select->with_rownum= 1; + thd->lex->with_rownum= 1; + thd->lex->uncacheable(UNCACHEABLE_RAND);
Why? ROWNUM doesn't make a query uncacheable, it's perfectly deterministic and repeatable.
Not necessartly. Row order may be different from the engine on subsequent reads for example.
+ + /* If this command changes data, mark it as unsafe for statement logging */ + if (sql_command_flags[thd->lex->sql_command] & + (CF_UPDATES_DATA | CF_DELETES_DATA)) + thd->lex->set_stmt_unsafe(LEX::BINLOG_STMT_UNSAFE_SYSTEM_FUNCTION);
Why? It's the same as LIMIT. unsafe without ORDER BY and perfectly safe with.
It is much worse. rownum is not safe to use anytime, not even with ORDER BY etc. (Except maybe with INSERT).. So better to give an error if it is used in any context.
+ Store a reference to the variable that contains number of accepted rows +*/ + +void Item_func_rownum::fix_after_optimize(THD *thd) +{ + accepted_rows= &select->join->accepted_rows;
why do you need both that and fix_rownum_pointers()? I'd think that the latter alone would be sufficient
select is not calling fix_rownum_pointers(). Also, fix_after_optimize() is much faster as it is ONLY called on functions that require this call; No loop required over all existing items, like with insert/update etc.
diff --git a/sql/share/errmsg-utf8.txt b/sql/share/errmsg-utf8.txt index badaaa17716..aecb00563f7 100644 --- a/sql/share/errmsg-utf8.txt +++ b/sql/share/errmsg-utf8.txt @@ -7973,3 +7973,5 @@ ER_DATA_WAS_COMMITED_UNDER_ROLLBACK eng "Engine %s does not support rollback. Changes were committed during rollback call" ER_PK_INDEX_CANT_BE_IGNORED eng "A primary key cannot be marked as IGNORE" +ER_FUNCTION_CANNOT_BE_USED_IN_CLAUSE + eng "Function '%s' cannot be used in the %s clause"
Is it the first function we've got that cannot be used in HAVING?
At least I could not find any similar error. There is no 'having' text string at all in errmsg-utf8.txt. The only reference in the code we have to the "HAVING" string is: my_error(ER_NON_GROUPING_FIELD_USED, MYF(0), ref->name.str, "HAVING"); But the error: my_error(ER_NON_GROUPING_FIELD_USED, MYF(0), ref->name.str, "HAVING"); eng "Non-grouping field '%-.192s' is used in %-.64s clause" Is not really usable in this context.
diff --git a/sql/sql_derived.cc b/sql/sql_derived.cc index 35b4294707f..da6c567b87c 100644 --- a/sql/sql_derived.cc +++ b/sql/sql_derived.cc @@ -656,14 +656,22 @@ static bool mysql_derived_prepare(THD *thd, LEX *lex, TABLE_LIST *derived) { SELECT_LEX_UNIT *unit= derived->get_unit(); - bool res= FALSE; + SELECT_LEX *first_select; + bool res= FALSE, keep_row_order; DBUG_ENTER("mysql_derived_prepare"); DBUG_PRINT("enter", ("unit: %p table_list: %p alias: '%s'", unit, derived, derived->alias.str)); if (!unit) DBUG_RETURN(FALSE);
- SELECT_LEX *first_select= unit->first_select(); + first_select= unit->first_select(); + /* + If rownum() is used we have to preserve the insert row order + to make group by with filesort to work.
it'd help if you'd add an example of a SELECT here
Any FILESORT: SELECT a from t1 GROUP BY a;
+ */ + keep_row_order= (thd->lex->with_rownum && + (first_select->group_list.elements || + first_select->order_list.elements));
I think that the above test is self described. If there is any ORDER BY or GROUP BY use we have to keep the insert order. Anyway, a better comment is of course good: /* If rownum() is used we have to preserve the insert row order to make GROUP BY and ORDER BY with filesort work. SELECT * from (SELECT a,b from t1 ORDER BY a)) WHERE rownum <= 0; When rownum is not used the optimizer will skip the ORDER BY clause. With rownum we have to keep the ORDER BY as this is what is expected. We also have to create any sort result temporary table in such a way that the inserted row order is maintained. */
if (derived->is_recursive_with_table() && !derived->is_with_table_recursive_reference() && diff --git a/sql/sql_insert.cc b/sql/sql_insert.cc index 806049b4465..5c4dff33914 100644 --- a/sql/sql_insert.cc +++ b/sql/sql_insert.cc @@ -746,6 +746,9 @@ bool mysql_insert(THD *thd, TABLE_LIST *table_list, DBUG_RETURN(TRUE); } + + if (thd->lex->with_rownum && table_list->lock_type == TL_WRITE_DELAYED) + table_list->lock_type= TL_WRITE;
why? do you have a test case when it's needed?
Because we cannot calculate row number inside insert delayed... (We do not know if an row is accepted or not).
if (table_list->lock_type == TL_WRITE_DELAYED) { if (open_and_lock_for_insert_delayed(thd, table_list)) @@ -968,11 +971,16 @@ bool mysql_insert(THD *thd, TABLE_LIST *table_list, */ if (returning && result->send_result_set_metadata(returning->item_list, - Protocol::SEND_NUM_ROWS | Protocol::SEND_EOF)) + Protocol::SEND_NUM_ROWS | + Protocol::SEND_EOF)) goto values_loop_end;
THD_STAGE_INFO(thd, stage_update); thd->decide_logging_format_low(table); + fix_rownum_pointers(thd, thd->lex->current_select, &info.copied); + if (returning) + fix_rownum_pointers(thd, thd->lex->returning(), &info.copied); +
Note that info.copied can be incremented more than once per row (in INSERT...ON DUPLICATE KEY UPDATE, for example). This will probably break rownum logic.
I am not sure how it should work in case of duplicate. I do not know how the logic in Oracle is for this (or if it even supports on duplicate key update) and I do not want to speculate on this too much. I do not think this is right as versioning should be invisible for the end user. The current logic for rownum is that rownum is incremented for any accepted rows, either for insert or update. In the current code we seam to do that, execpt for versioning where it is counted twice. I think this is a bug as versioning should be undetectable for a normal user and users may use the return values to check what happened with the row (duplicate or not). Versioning will make any such test unusable. What do you suggest? - Only count the true inserts we are doing? - Remove the copied++ for versioning, in which case the rownum logic should work. As far as I understands, one can only safely use rownum when inserting into an empty table, so do not know how important this is.
do { DBUG_PRINT("info", ("iteration %llu", iteration)); @@ -2133,8 +2141,14 @@ int write_record(THD *thd, TABLE *table, COPY_INFO *info, select_result *sink) autoinc values (generated inside the handler::ha_write()) and values updated in ON DUPLICATE KEY UPDATE. */ - if (sink && sink->send_data(thd->lex->returning()->item_list) < 0) - trg_error= 1; + if (sink) + { + /* We have to adjust 'copied' to make ROWNUM() work in RETURNING */ + info->copied--; + if (sink->send_data(thd->lex->returning()->item_list) < 0) + trg_error= 1; + info->copied++;
this is quite silly. Can you simply increment info->copied after this sink->send_data call?
No, as this leads to wrong resutls. The problem is that for 'returning' the row is already accepted while in all other code it is called before the row is accepted.
+ }
after_trg_or_ignored_err: if (key) diff --git a/sql/sql_limit.h b/sql/sql_limit.h index a4fcedac14a..62fd0546af0 100644 --- a/sql/sql_limit.h +++ b/sql/sql_limit.h @@ -67,6 +67,15 @@ class Select_limit_counters { return select_limit_cnt; } ha_rows get_offset_limit() { return offset_limit_cnt; } + + void set_limit_if_lower(ha_rows limit) + { + if (!offset_limit_cnt)
I suppose, alternatively, you could've had
if (offset_limit_cnt + select_limit_cnt > limit) select_limit_cnt= limit - offset_limit_cnt;
+ { + if (select_limit_cnt > limit) + select_limit_cnt= limit; + } + } };
In some cases LIMIT is absolute. With rownum it only applies if limit is higher. I think it is good to separate those tests.
#endif // INCLUDES_MARIADB_SQL_LIMIT_H diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy index 2bd2575b882..57ba9df42c0 100644 --- a/sql/sql_yacc.yy +++ b/sql/sql_yacc.yy @@ -10286,6 +10287,22 @@ function_call_nonkeyword: if (unlikely($$ == NULL)) MYSQL_YYABORT; } +/* Start SQL_MODE_ORACLE_SPECIFIC + | ROWNUM_SYM optional_braces + { + $$= new (thd->mem_root) Item_func_rownum(thd); + if (unlikely($$ == NULL)) + MYSQL_YYABORT; + } +End SQL_MODE_ORACLE_SPECIFIC */ +/* Start SQL_MODE_DEFAULT_SPECIFIC */ + | ROWNUM_SYM '(' ')' + { + $$= new (thd->mem_root) Item_func_rownum(thd); + if (unlikely($$ == NULL)) + MYSQL_YYABORT; + } +/* End SQL_MODE_DEFAULT_SPECIFIC */
You have two nearly identical rules compiled conditionally. I can pretty much guarantee that they'll go out of sync eventually.
This was what bar suggested we should use.
Better do something like
/* Start SQL_MODE_ORACLE_SPECIFIC oracle_optional_braces: optional_braces ; End SQL_MODE_ORACLE_SPECIFIC */ /* Start SQL_MODE_DEFAULT_SPECIFIC */ oracle_optional_braces: '(' ')' ; /* End SQL_MODE_DEFAULT_SPECIFIC */
Which is totally unreadable.
or even
| ROWNUM_SYM /* Start SQL_MODE_ORACLE_SPECIFIC optional_braces End SQL_MODE_ORACLE_SPECIFIC */ /* Start SQL_MODE_DEFAULT_SPECIFIC */ '(' ')' /* End SQL_MODE_DEFAULT_SPECIFIC */ { $$= new (thd->mem_root) Item_func_rownum(thd); if (unlikely($$ == NULL)) MYSQL_YYABORT; }
A little better, but not much. Still not readable. <cut>
diff --git a/sql/table.cc b/sql/table.cc index 7f3a6468aa2..100813e1f95 100644 --- a/sql/table.cc +++ b/sql/table.cc @@ -9331,6 +9331,10 @@ bool TABLE_LIST::init_derived(THD *thd, bool init_view) { /* A subquery might be forced to be materialized due to a side-effect. */ if (!is_materialized_derived() && first_select->is_mergeable() && + (unit->outer_select() && !unit->outer_select()->with_rownum) &&
no merging if *outer select* uses rownum? why? example?
We cannot merge sub queries with rownum.
+ (!thd->lex->with_rownum || + (!first_select->group_list.elements && + !first_select->order_list.elements)) &&
no merging if no rownum but there's group by or order by? group by - this is obvious, and is already checked earlier in this if() order by - why?
Becase we have to do the order by. A merge would destroy that. (rownum requires original row order)
diff --git a/sql/sql_select.cc b/sql/sql_select.cc + // rownum used somewhere in query, no limits and it is derived + if (unlikely(thd->lex->with_rownum && + select_lex->first_cond_optimization && + select_lex->master_unit()->derived)) + optimize_upper_rownum_func(); + + do_send_rows = (unit->lim.get_select_limit()) ? 1 : 0;
Is there any particular reason why you put that between
DEBUG_SYNC(thd, "before_join_optimize");
and
THD_STAGE_INFO(thd, stage_optimizing);
? I'd expect them logically to go directly one after the other
No particualr reason. I did not see the relationship between the above calls, especially as there was a blank line in between, but I see the point now. Will move the code.
+ + THD_STAGE_INFO(thd, stage_optimizing);
set_allowed_join_cache_types(); @@ -14210,7 +14236,24 @@ static ORDER * remove_const(JOIN *join,ORDER *first_order, COND *cond, bool change_list, bool *simple_order) { - *simple_order= join->rollup.state == ROLLUP::STATE_NONE; + /* + We can't do ORDER BY using filesort if the select list contains a non + deterministic value like RAND() or ROWNUM(). + For example: + SELECT RAND() as 'r' from t1 order by r;
This one uses temporary for me. Can you put here a correct example please?
It is a correct example and probably the best possible example. I do not understand your comment (does not make any sense). Could you please provide a correct comment? Also, try the example and try to figure out why it works. Check also the comment below.
+ + If we would first sort the table 't1', the RAND() column would be generated + during end_send() and the order would be wrong. + + Previously we had here also a test of ROLLUP: + 'join->rollup.state == ROLLUP::STATE_NONE' + I deleted this because + the ROLLUP was never enforced because of a bug where the inital value + of simple_order was ignored. Having ROLLUP tested now when the code + is fixed causes many test failure and some wrong results so better to + leave the code as it was related to ROLLUP.
No need to explain in a comment why something that is no longer here was removed. Use git for that, commit comments. The code comment should explain the code as it is.
No that is not the right approach as otherwise someone would add a similar code back! The comment explains what was removed and why, which is correct thing to do in the code if there is a risk someone would try to add it back.
+ */ + *simple_order= !join->rand_table_in_field_list;
why do you need rand_table_in_field_list instead of writing
*simple_order= join->select_lex->select_list_tables & RAND_TABLE_BIT;
I assume the reason was that originally select_lex was an argument to JOIN::prepare() and someone thought the bit could be reset later or we could not reliable access it from join remove_const(). I agree that it is proably safe to remove the variable and do the test as above. I will test this and do the change if the test passes (I do not see any reason why they shouldn't)
+static void fix_items_after_optimize(THD *thd, SELECT_LEX *select_lex) +{ + List_iterator<Item> li(select_lex->fix_after_optimize); + + while (Item *item= li++) + item->fix_after_optimize(thd); +} + +
here you could've had a comment, like
/* callbacks for process_direct_rownum_comparison() */
+static bool is_const(Item *item) +{ + /* + Constant expression in WHERE can be also subquery or other expensive + thing we shoud not evaluate on OPTIMIZATION phase + */ + return item->const_item() && !item->is_expensive(); +} + + +// Check if this is an unexpensive functional expression over basic constants + +static bool is_basic_const(Item *item) +{ + if (item->basic_const_item()) + return true; + if (!item->const_item() || item->is_expensive()) + return false; + if (item->type() == Item::FUNC_ITEM) + { + Item_func *func= ((Item_func *)item); + if (func->argument_count() == 0) + return false; + bool result= true; + Item **args= func->arguments(); + for (uint i= 0; i < func->argument_count() && result; i++) + result= result && is_basic_const(args[i]); + return result; + } + return false; +}
is_basic_const() is a misleading name, a function of basic consts isn't a basic const itself.
This is how bar has named the item and classes. Do a grep after basic_const..
what's an example of an expression has is_const() == true, but is_basic_const() == false?
"1+1" THis is the sum of two basic constants.
+ + +inline bool is_simple_rownum_exp(Item *rownum, Item *constant)
unused
+{ + return (constant->basic_const_item() && + rownum->type() == Item::FUNC_ITEM && + ((Item_func*) rownum)->functype() == Item_func::ROWNUM_FUNC); +} + + +static bool temporary_set_limit(THD *thd __attribute__((unused)), + SELECT_LEX_UNIT *unit, + ha_rows lim) +{ + unit->set_limit_if_lower(lim);
why is it temporary? in the PS/SP sense, just for one execution?
You have to ask Sanja that. I assume it is just for one exection as limits may be parameters and constant can change. After all, we have also permanent limit and we have to distingush these functions somehow. One different is that permanent limit is show in explain.
+ return false; +} + + +static bool permanent_limit(THD *thd, SELECT_LEX_UNIT *unit, ha_rows lim) +{ + SELECT_LEX *gpar= unit->global_parameters(); + if (gpar->select_limit != 0 && + // limit can not be an expression but can be parameter + (!gpar->select_limit->basic_const_item() || + ((ha_rows)gpar->select_limit->val_int()) < lim)) + return false; + + Query_arena *arena, backup; + arena= thd->activate_stmt_arena_if_needed(&backup); + + gpar->select_limit= + new (thd->mem_root)Item_int(thd, lim, 20 /* max 64 bit length*/ );
we have defines for that, MAX_BIGINT_WIDTH, MY_INT64_NUM_DECIMAL_DIGITS, LONGLONG_BUFFER_SIZE - whatever you prefer.
This come from Sanja, but I can change it. (I also updated the comments to be "more english")
+ if (gpar->select_limit == 0) + return true; // EOM + + unit->set_limit(gpar); + + gpar->explicit_limit= true; // to show in EXPLAIN + + if (arena) + thd->restore_active_arena(arena, &backup); + + return false; +} + + +/** + Check posibility of LIMIT setting by rownum() of upper SELECT and do it + + @note Ideal is to convert something like + SELECT ... + FROM (SELECT ...) table + WHERE rownum() < <CONSTANT>; + to + SELECT ... + FROM (SELECT ... LIMIT <CONSTANT>) table + WHERE rownum() < <CONSTANT>;
1. what if the outer select uses LIMIT, not ROWNUM? why not to push that down too?
We already have code for that.
2. why are you not removing rownum from the outer select?
Because it may not be safe to do in some cases. Better safe than sorry..
if you remove rownum and reset with_rownum=false, it'll generally give optimizer more freedom.
Has no effect on our optimizer in this particular case.
+static bool check_rownum_usage(Item_func *func_item, longlong *limit, + bool *inv_order, const_probe probe) +{ + Item *arg1, *arg2; + *inv_order= 0; + DBUG_ASSERT(func_item->argument_count() == 2); + + /* 'rownum op const' or 'const op field' */ + arg1= func_item->arguments()[0]->real_item(); + if (arg1->type() == Item::FUNC_ITEM && + ((Item_func*) arg1)->functype() == Item_func::ROWNUM_FUNC) + { + arg2= func_item->arguments()[1]->real_item(); + if ((*probe)(arg2)) + { + *limit= arg2->val_int(); + return *limit <= 0 || (ulonglong) *limit >= HA_POS_ERROR; + } + } + else if ((*probe)(arg1)) + { + arg2= func_item->arguments()[1]->real_item();
why not to assign arg2 outside of the if() ?
Because we should not evaluate it if probe is not true. It can be expensive to evalute it or it can have side effects that we may want to avoid. For example, it could be a sub query and probe is there to ensure we don't evaluate it here.
+ if (arg2->type() == Item::FUNC_ITEM && + ((Item_func*) arg2)->functype() == Item_func::ROWNUM_FUNC) + { + *limit= arg1->val_int(); + *inv_order= 1; + return *limit <= 0 || (ulonglong) *limit >= HA_POS_ERROR; + } + } + return 1; +} + + +/* + Limit optimization for ROWNUM() + + Go through the WHERE clause and find out if there are any of the following + constructs on the top level: + rownum() <= integer_constant + rownum() < integer_constant + rownum() = 1 + + If yes, then threat the select as if 'LIMIT integer_constant' would + have been used +*/ + +static void optimize_rownum(THD *thd, SELECT_LEX_UNIT *unit, Item *cond) +{ + DBUG_ENTER("optimize_rownum"); + + if (cond->type() == Item::COND_ITEM) + { + if (((Item_cond*) cond)->functype() == Item_func::COND_AND_FUNC) + { + List_iterator<Item> li(*((Item_cond*) cond)->argument_list()); + Item *item; + while ((item= li++)) + optimize_rownum(thd, unit, item); + } + DBUG_VOID_RETURN; + } + + process_direct_rownum_comparison(thd, unit, cond, &is_const, + &temporary_set_limit);
please, explain why it's is_const/temporary_set_limit here and is_basic_const/permanent_limit in optimize_upper_rownum_func().
Please ask Sanja. I think the short version is that const/temporary is for the current select why is_basic_const/permanent is for pushing limit from an outer select to an inner select and there is more restrictions for that. <cut>
+ switch (pred_type) { + case Item_func::LT_FUNC: // rownum() < # + { + if (limit <= 0) + DBUG_RETURN(false); + DBUG_RETURN((*set_limit)(thd, unit, limit - 1)); + case Item_func::LE_FUNC: + DBUG_RETURN((*set_limit)(thd, unit, limit)); + case Item_func::EQ_FUNC: + if (limit == 1) + DBUG_RETURN((*set_limit)(thd, unit, limit)); + break; + default: + break; + } + }
what if it's rownum() > 5 ? why not to mark it as always false right away?
We decided to not do this optimization at this point in time Would require a lot of more work and testing to ensure detection of wrong rownum usage would always work. Not important for the real world. Regards, Monty
Hi, Monty! On Apr 18, Michael Widenius wrote:
diff --git a/mysql-test/main/rownum.test b/mysql-test/main/rownum.test new file mode 100644 index 00000000000..8d89eed2f86 --- /dev/null +++ b/mysql-test/main/rownum.test @@ -0,0 +1,542 @@ +--source include/have_sequence.inc +# +# Test of basic rownum() functionallity +# Test are done with Aria to ensure that row order is stable
what does that mean?
It means that SELECT * will return the rows in the same order for the test. (There is normally no guaranteed that insert and select order are the same).
But both MyISAM and InnoDB guarantee that too. What's so special with Aria?
+ OPEN c_cursor; + FETCH c_cursor INTO v_a, v_b, v_rn; + WHILE c_cursor%FOUND LOOP + SELECT concat(v_a,'--',v_b,'--',v_rn); + FETCH c_cursor INTO v_a, v_b, v_rn; + END LOOP; + CLOSE c_cursor; +END;| +DELIMITER ;| + +--error ER_FUNCTION_CANNOT_BE_USED_IN_CLAUSE +select a, rownum from t1 group by a, rownum having rownum < 3; +select a, rownum from t1 group by a, rownum having "rownum" < 3;
what's the difference between these two? Why the first is an error and the second is not?
There are quotes around rownum in the second version. In Oracle mode that is an error, in MariaDB mode it is not.
I can see the quotes, that's what I asked about. Why *in Oracle mode* the first is an error and the second is not? I'd expect them to be identical.
+select a, rownum as r from t1 group by a, rownum having r < 3;
And the this one. Why is it not an error either?
Because 'r' is a reference, not a function. 'r' works on the result set. rownum cannot be executed in the having clause (which is executed after all rows are returned).
I don't understand it. HAVING is not executed *after* all rows are returned, it's executed *while* they're returned. What is the difference between rownum directly in HAVING and referring to it by an alias?
index 53c236bbce7..c237fcd9ecc 100644
--- a/sql/item_func.h +++ b/sql/item_func.h @@ -2125,6 +2125,63 @@ class Item_func_rand :public Item_real_func };
+class Item_func_rownum :public Item_longlong_func +{ + /* + This points to a variable that contains the number of rows + accpted so far in the result set + */ + ha_rows *accepted_rows; + SELECT_LEX *select; +public: + Item_func_rownum(THD *thd); + longlong val_int() override; + LEX_CSTRING func_name_cstring() const override + { + static LEX_CSTRING name= {STRING_WITH_LEN("rownum") }; + return name; + } + enum Functype functype() const override { return ROWNUM_FUNC; } + void update_used_tables() override {} + bool const_item() const override { return 0; } + void fix_after_optimize(THD *thd) override; + bool fix_fields(THD* thd, Item **ref) override; + bool fix_length_and_dec() override + { + unsigned_flag= 1; + used_tables_cache= RAND_TABLE_BIT; + const_item_cache=0; + set_maybe_null();
how can rownum be NULL?
In context where there are no rows. like SET @A=rownum;
Do you have test cases for that?
+ return FALSE; + } + void cleanup() override + { + Item_longlong_func::cleanup(); + /* Ensure we don't point to freed memory */ + accepted_rows= 0; + } + bool check_vcol_func_processor(void *arg) override + { + return mark_unsupported_function(func_name(), "()", arg, + VCOL_IMPOSSIBLE); + } + bool check_handler_func_processor(void *arg) override + { + return mark_unsupported_function(func_name(), "()", arg, + VCOL_IMPOSSIBLE);
first, as I wrote above, I think that there's no logical reason why rownum should not work with handler.
Please read the patch. It cannot be in the handler for multiple reasons: - INSERT, UPDATE, LOAD DATA - handler does not know if the row is accepted - joins etc.
I mean the HANDLER statement, that's why my comment is just below check_handler_func_processor() method.
but even if it doesn't, it's much better to check for SQLCOM_HA_READ from Item_func_rownum::fix_fields instead of traversing the whole item tree for every single HANDLER READ command looking for Item_func_rownum that nobody (literally, not a single user out of all our millions) will use there.
There is only a traverse of the tree looking for rownum if the rownum function is used.
Why do you think there is some extra travelling of the tree for HANDLER_READ ?
Because, see above, this comment refers to check_handler_func_processor() method this is called on every item by walrking the item tree. As there is only one item weird enough to possibly need it, I'd rather do the check in the item directly.
+ } + Item *get_copy(THD *thd) override + { + return this;
It looks wrong. We don't have any other Item that returns `this` here. I think rownum cannot be pushed down because of RAND_TABLE_BIT, So better return NULL here, meaning "cannot be pushed down". Instead of `this` meaning "if you push it down, it'll surreptitiously start corrupting the memory"
I don't think it can trash memory even if it would be pushed down. As long as the item exists, it should be safe to use anywhere. I also don't trust the code to be able to cope with returning null in get_copy.
Quite the contrary. There are other items that return NULL from get_copy(). It's expected and normal, the code is designed to support it, it was part of the original implementation. No get_copy() returns this, the code is not designed to handle it.
+ } + /* This function is used in insert, update and delete */ + void store_pointer_to_row_counter(ha_rows *row_counter)
So this isn't a virtual method, and to use it you downcast after checking type() == FUNC_ITEM and functype() == ROWNUM_FUNC. To avoid adding a new virtual method thus blowing up the vtable, I presume.
Yes, it is not a virtual and should not be. And yes, we do not want to add any new virtual functions to items when we have close to 600+ items in the system...
It is also much faster to do an if on upper level than doing a function call.
And fix_after_optimize() is virtual. To avoid error-prone downcasting, would be my guess.
Yes. This is something we may need for other items in the future, which is why I made it virtaul.
Do you have in mind any other items that will need it any time soon? If not, then I agree with what you wrote above, we do not want to add any new virtual functions to items when we have close to 600+ items in the system.
--- a/sql/item_func.cc +++ b/sql/item_func.cc @@ -7206,3 +7206,72 @@ void Item_func_setval::print(String *str, enum_query_type query_type) str->append_ulonglong(round); str->append(')'); } + + +/* + Return how many row combinations has accepted so far + 1 + + The + 1 is to ensure that 'WHERE ROWNUM <=1' returns one row
this is a very weird explanation, it sounds as if it's a heuristical a hack for ROWNUM <=1 to work
Why not to write "+1 takes into account the currently computed row"
Because the row is not yet computed or accepted. We can call rownum any time, after a row is accpted, before etc.
I personally prefer the current comment as it is more well defined.
So, you mean, +1 is not needed for WHERE ROWNUM <=2 to return two rows? Or for WHERE ROWNUM<=100 to return 100 rows? You pick one very specific case as an explanation, this implicitly assumes it's the main reason. Which it is not, even if we'll only consider exactly the same condition template, there are still 18446744073709551614 other reasons :) +1 is, exactly, to take into account the current row, the one being processed right now. By the way, instead of +1 here, you could simply initialize your accepted_rows with 1, not with 0.
+*/ + +longlong Item_func_rownum::val_int() +{ + if (!accepted_rows) + { + /* + Rownum is not properly set up. Probably used in wrong context when + it should not be used. In this case returning 0 is probably the best + solution. + */
DBUG_ASSERT(0); here?
No. One can use rownum in a lot of context where it does makes sence. Instead of trying to generate an error for every of these cases, I decided to return 0 for these. In the future maybe some of these will be valid and then things can change. I do not want to see asserts in test cases people add for things like this.
Do you have test cases for that?
+ select->with_rownum= 1; + thd->lex->with_rownum= 1; + thd->lex->uncacheable(UNCACHEABLE_RAND);
Why? ROWNUM doesn't make a query uncacheable, it's perfectly deterministic and repeatable.
Not necessartly. Row order may be different from the engine on subsequent reads for example.
This is the same with LIMIT. Even if the row order might be different, the first result is still correct, so it can be cached and reused. LIMIT doesn't make a query uncacheable.
+ + /* If this command changes data, mark it as unsafe for statement logging */ + if (sql_command_flags[thd->lex->sql_command] & + (CF_UPDATES_DATA | CF_DELETES_DATA)) + thd->lex->set_stmt_unsafe(LEX::BINLOG_STMT_UNSAFE_SYSTEM_FUNCTION);
Why? It's the same as LIMIT. unsafe without ORDER BY and perfectly safe with.
It is much worse. rownum is not safe to use anytime, not even with ORDER BY etc. (Except maybe with INSERT).. So better to give an error if it is used in any context.
example?
Anyway, a better comment is of course good:
/* If rownum() is used we have to preserve the insert row order to make GROUP BY and ORDER BY with filesort work.
SELECT * from (SELECT a,b from t1 ORDER BY a)) WHERE rownum <= 0;
When rownum is not used the optimizer will skip the ORDER BY clause. With rownum we have to keep the ORDER BY as this is what is expected. We also have to create any sort result temporary table in such a way that the inserted row order is maintained. */
Very good, thanks!
if (derived->is_recursive_with_table() && !derived->is_with_table_recursive_reference() && diff --git a/sql/sql_insert.cc b/sql/sql_insert.cc index 806049b4465..5c4dff33914 100644 --- a/sql/sql_insert.cc +++ b/sql/sql_insert.cc @@ -746,6 +746,9 @@ bool mysql_insert(THD *thd, TABLE_LIST *table_list, DBUG_RETURN(TRUE); } + + if (thd->lex->with_rownum && table_list->lock_type == TL_WRITE_DELAYED) + table_list->lock_type= TL_WRITE;
why? do you have a test case when it's needed?
Because we cannot calculate row number inside insert delayed... (We do not know if an row is accepted or not).
"Not accepted"? The only possibility for a row to be not accepted is INSERT IGNORE and a unique key violation, right? WHERE clause is done before the delayed thread. I don't see any tests for INSERT IGNORE in your patch, so I cannot say what the intended behavior should be. Could you please add a test for it? For example, create table t1 (a int not null primary key, b int); insert ignore t1 values (1,rownum()),(2,rownum()),(2,rownum()),(3,rownum());
@@ -2133,8 +2141,14 @@ int write_record(THD *thd, TABLE *table, COPY_INFO *info, select_result *sink) autoinc values (generated inside the handler::ha_write()) and values updated in ON DUPLICATE KEY UPDATE. */ - if (sink && sink->send_data(thd->lex->returning()->item_list) < 0) - trg_error= 1; + if (sink) + { + /* We have to adjust 'copied' to make ROWNUM() work in RETURNING */ + info->copied--; + if (sink->send_data(thd->lex->returning()->item_list) < 0) + trg_error= 1; + info->copied++;
this is quite silly. Can you simply increment info->copied after this sink->send_data call?
No, as this leads to wrong resutls. The problem is that for 'returning' the row is already accepted while in all other code it is called before the row is accepted.
Sorry, I don't understand. Why it would lead to wrong results?
+ }
after_trg_or_ignored_err: if (key) diff --git a/sql/sql_limit.h b/sql/sql_limit.h index a4fcedac14a..62fd0546af0 100644 --- a/sql/sql_limit.h +++ b/sql/sql_limit.h @@ -67,6 +67,15 @@ class Select_limit_counters { return select_limit_cnt; } ha_rows get_offset_limit() { return offset_limit_cnt; } + + void set_limit_if_lower(ha_rows limit) + { + if (!offset_limit_cnt)
I suppose, alternatively, you could've had
if (offset_limit_cnt + select_limit_cnt > limit) select_limit_cnt= limit - offset_limit_cnt;
+ { + if (select_limit_cnt > limit) + select_limit_cnt= limit; + } + } };
In some cases LIMIT is absolute. With rownum it only applies if limit is higher. I think it is good to separate those tests.
What do you mean "LIMIT is absolute"? I only suggested that LIMIT can take ROWNUM into account also when OFFSET is non-zero. In your case you only adjust LIMIT if there is no OFFSET. It's not a particularly common case, so not a very important optimization, of course. On the other hand, the suggested change is quite simple.
#endif // INCLUDES_MARIADB_SQL_LIMIT_H diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy index 2bd2575b882..57ba9df42c0 100644 --- a/sql/sql_yacc.yy +++ b/sql/sql_yacc.yy @@ -10286,6 +10287,22 @@ function_call_nonkeyword: if (unlikely($$ == NULL)) MYSQL_YYABORT; } +/* Start SQL_MODE_ORACLE_SPECIFIC + | ROWNUM_SYM optional_braces + { + $$= new (thd->mem_root) Item_func_rownum(thd); + if (unlikely($$ == NULL)) + MYSQL_YYABORT; + } +End SQL_MODE_ORACLE_SPECIFIC */ +/* Start SQL_MODE_DEFAULT_SPECIFIC */ + | ROWNUM_SYM '(' ')' + { + $$= new (thd->mem_root) Item_func_rownum(thd); + if (unlikely($$ == NULL)) + MYSQL_YYABORT; + } +/* End SQL_MODE_DEFAULT_SPECIFIC */
You have two nearly identical rules compiled conditionally. I can pretty much guarantee that they'll go out of sync eventually.
Better do something like
| ROWNUM_SYM /* Start SQL_MODE_ORACLE_SPECIFIC optional_braces End SQL_MODE_ORACLE_SPECIFIC */ /* Start SQL_MODE_DEFAULT_SPECIFIC */ '(' ')' /* End SQL_MODE_DEFAULT_SPECIFIC */ { $$= new (thd->mem_root) Item_func_rownum(thd); if (unlikely($$ == NULL)) MYSQL_YYABORT; }
A little better, but not much. Still not readable.
This whole work of removing sql_yacc_ora.yy and using comments for parts of the grammar that differ - this was done *exactly* to avoid two nearly identical grammars that always needs to be modified in sync. Which is very error-prone. By literally copying a part of the grammar you're reintroducing the old problem of two copies. Not completely, of course, by bit by bit we'll have two copies to maintain again. Please, either use the "little better" grammar that I suggested or rewrite it any way you want that doesn't involve two identical copies of the same rule.
<cut>
diff --git a/sql/sql_select.cc b/sql/sql_select.cc @@ -14210,7 +14236,24 @@ static ORDER * remove_const(JOIN *join,ORDER *first_order, COND *cond, bool change_list, bool *simple_order) { - *simple_order= join->rollup.state == ROLLUP::STATE_NONE; + /* + We can't do ORDER BY using filesort if the select list contains a non + deterministic value like RAND() or ROWNUM(). + For example: + SELECT RAND() as 'r' from t1 order by r;
This one uses temporary for me. Can you put here a correct example please?
It is a correct example and probably the best possible example. I do not understand your comment (does not make any sense). Could you please provide a correct comment?
I tried this in the 10.5 branch: MariaDB [test]> explain select rand() as r from mysql.user order by r\G *************************** 1. row *************************** id: 1 select_type: SIMPLE table: user type: index possible_keys: NULL key: PRIMARY key_len: 228 ref: NULL rows: 3 Extra: Using index; Using temporary; Using filesort 1 row in set (0.002 sec) as you see, it uses temporary, so there is no need to do your + *simple_order= !join->rand_table_in_field_list; because there is no bug here. At least for the example that you've used. This is why I asked for an example of a query that indeed behaves incorrectly without this fix of yours.
+static bool is_const(Item *item) +{ + /* + Constant expression in WHERE can be also subquery or other expensive + thing we shoud not evaluate on OPTIMIZATION phase + */ + return item->const_item() && !item->is_expensive(); +} + + +// Check if this is an unexpensive functional expression over basic constants + +static bool is_basic_const(Item *item) +{ + if (item->basic_const_item()) + return true; + if (!item->const_item() || item->is_expensive()) + return false; + if (item->type() == Item::FUNC_ITEM) + { + Item_func *func= ((Item_func *)item); + if (func->argument_count() == 0) + return false; + bool result= true; + Item **args= func->arguments(); + for (uint i= 0; i < func->argument_count() && result; i++) + result= result && is_basic_const(args[i]); + return result; + } + return false; +}
is_basic_const() is a misleading name, a function of basic consts isn't a basic const itself.
This is how bar has named the item and classes. Do a grep after basic_const..
I know, that was not my question. See below
what's an example of an expression has is_const() == true, but is_basic_const() == false?
"1+1" This is the sum of two basic constants.
Yes, this is an example of the item, where item->const_item() == true and item->basic_const() is false. This is not what I asked. I asked for an example, where is_const(item) is true and is_basic_const(item) is false. The question was not about old items, but about your new code. Because your is_basic_const() is not just item->basic_const(), so I asked for an example, to understand a difference.
+{ + return (constant->basic_const_item() && + rownum->type() == Item::FUNC_ITEM && + ((Item_func*) rownum)->functype() == Item_func::ROWNUM_FUNC); +} + +
+ return false; +} + + +static bool permanent_limit(THD *thd, SELECT_LEX_UNIT *unit, ha_rows lim) +{ + SELECT_LEX *gpar= unit->global_parameters(); + if (gpar->select_limit != 0 && + // limit can not be an expression but can be parameter + (!gpar->select_limit->basic_const_item() || + ((ha_rows)gpar->select_limit->val_int()) < lim)) + return false; + + Query_arena *arena, backup; + arena= thd->activate_stmt_arena_if_needed(&backup); + + gpar->select_limit= + new (thd->mem_root)Item_int(thd, lim, 20 /* max 64 bit length*/ );
we have defines for that, MAX_BIGINT_WIDTH, MY_INT64_NUM_DECIMAL_DIGITS, LONGLONG_BUFFER_SIZE - whatever you prefer.
This come from Sanja, but I can change it. (I also updated the comments to be "more english")
Thanks
+/** + Check posibility of LIMIT setting by rownum() of upper SELECT and do it + + @note Ideal is to convert something like + SELECT ... + FROM (SELECT ...) table + WHERE rownum() < <CONSTANT>; + to + SELECT ... + FROM (SELECT ... LIMIT <CONSTANT>) table + WHERE rownum() < <CONSTANT>;
1. what if the outer select uses LIMIT, not ROWNUM? why not to push that down too?
We already have code for that.
But then you don't need to push ROWNUM into a subquery. You have code to convert ROWNUM into a LIMIT, and you just said that we already have code to push LIMIT. So, you do not need to push ROWNUM specifically, it'll happen automatically, won't it?
2. why are you not removing rownum from the outer select?
Because it may not be safe to do in some cases. Better safe than sorry..
if you remove rownum and reset with_rownum=false, it'll generally give optimizer more freedom.
Has no effect on our optimizer in this particular case.
I'm not sure about it. ROWNUM cannot be pushed down into a subquery. LIMIT can be, as you said, so if you convert a ROWNUM into a LIMIT and remove ROWNUM from WHERE, then the whole WHERE can be pushed into a subquery, and a LIMIT can be too. So SELECT * FROM (SELECT col1, col2 FROM tables WHERE cond ORDER BY something) WHERE ROWNUM <=100 AND col1 = 10; can first be changed to SELECT * FROM (SELECT col1, col2 FROM tables WHERE cond ORDER BY something) WHERE col1 = 10 LIMIT 100; and then to SELECT * FROM (SELECT col1, col2 FROM tables WHERE cond AND col1 = 10 ORDER BY something LIMIT 100); and for that you don't even need any special code for pushing ROWNUM into a subquery as a limit, you only need to convert ROWNUM to a LIMIT on the same level of select.
+static bool check_rownum_usage(Item_func *func_item, longlong *limit, + bool *inv_order, const_probe probe) +{ + Item *arg1, *arg2; + *inv_order= 0; + DBUG_ASSERT(func_item->argument_count() == 2); + + /* 'rownum op const' or 'const op field' */ + arg1= func_item->arguments()[0]->real_item(); + if (arg1->type() == Item::FUNC_ITEM && + ((Item_func*) arg1)->functype() == Item_func::ROWNUM_FUNC) + { + arg2= func_item->arguments()[1]->real_item(); + if ((*probe)(arg2)) + { + *limit= arg2->val_int(); + return *limit <= 0 || (ulonglong) *limit >= HA_POS_ERROR; + } + } + else if ((*probe)(arg1)) + { + arg2= func_item->arguments()[1]->real_item();
why not to assign arg2 outside of the if() ?
Because we should not evaluate it if probe is not true. It can be expensive to evalute it or it can have side effects that we may want to avoid. For example, it could be a sub query and probe is there to ensure we don't evaluate it here.
Sure, but func_item->arguments()[1]->real_item() is not evaluating anything. I only mean arg2= line, that's identical in both branches. I didn't mean doing val_int() unconditionally.
+ if (arg2->type() == Item::FUNC_ITEM && + ((Item_func*) arg2)->functype() == Item_func::ROWNUM_FUNC) + { + *limit= arg1->val_int(); + *inv_order= 1; + return *limit <= 0 || (ulonglong) *limit >= HA_POS_ERROR; + } + } + return 1; +} + + +/* + Limit optimization for ROWNUM() + + Go through the WHERE clause and find out if there are any of the following + constructs on the top level: + rownum() <= integer_constant + rownum() < integer_constant + rownum() = 1 + + If yes, then threat the select as if 'LIMIT integer_constant' would + have been used +*/ + +static void optimize_rownum(THD *thd, SELECT_LEX_UNIT *unit, Item *cond) +{ + DBUG_ENTER("optimize_rownum"); + + if (cond->type() == Item::COND_ITEM) + { + if (((Item_cond*) cond)->functype() == Item_func::COND_AND_FUNC) + { + List_iterator<Item> li(*((Item_cond*) cond)->argument_list()); + Item *item; + while ((item= li++)) + optimize_rownum(thd, unit, item); + } + DBUG_VOID_RETURN; + } + + process_direct_rownum_comparison(thd, unit, cond, &is_const, + &temporary_set_limit);
please, explain why it's is_const/temporary_set_limit here and is_basic_const/permanent_limit in optimize_upper_rownum_func().
Please ask Sanja. I think the short version is that const/temporary is for the current select why is_basic_const/permanent is for pushing limit from an outer select to an inner select and there is more restrictions for that.
Sanja said that it's your code and if it were up to him he'd do both permanent. Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org
Hi! On Mon, Apr 19, 2021 at 10:52 PM Sergei Golubchik <serg@mariadb.org> wrote:
Hi, Monty!
On Apr 18, Michael Widenius wrote:
diff --git a/mysql-test/main/rownum.test b/mysql-test/main/rownum.test new file mode 100644 index 00000000000..8d89eed2f86 --- /dev/null +++ b/mysql-test/main/rownum.test @@ -0,0 +1,542 @@ +--source include/have_sequence.inc +# +# Test of basic rownum() functionallity +# Test are done with Aria to ensure that row order is stable
what does that mean?
It means that SELECT * will return the rows in the same order for the test. (There is normally no guaranteed that insert and select order are the same).
But both MyISAM and InnoDB guarantee that too. What's so special with Aria?
MyISAM does guarantee that, but not InnoDB (returns rows in primary key order) and Aria with page storage will not do it either.
+ OPEN c_cursor; + FETCH c_cursor INTO v_a, v_b, v_rn; + WHILE c_cursor%FOUND LOOP + SELECT concat(v_a,'--',v_b,'--',v_rn); + FETCH c_cursor INTO v_a, v_b, v_rn; + END LOOP; + CLOSE c_cursor; +END;| +DELIMITER ;| + +--error ER_FUNCTION_CANNOT_BE_USED_IN_CLAUSE +select a, rownum from t1 group by a, rownum having rownum < 3; +select a, rownum from t1 group by a, rownum having "rownum" < 3;
what's the difference between these two? Why the first is an error and the second is not?
There are quotes around rownum in the second version. In Oracle mode that is an error, in MariaDB mode it is not.
I can see the quotes, that's what I asked about. Why *in Oracle mode* the first is an error and the second is not? I'd expect them to be identical.
In Oracle you cannot use rownum function in having. You can however, thanks to MariaDB, refer to a column in the select part.
+select a, rownum as r from t1 group by a, rownum having r < 3;
And the this one. Why is it not an error either?
Because 'r' is a reference, not a function. 'r' works on the result set. rownum cannot be executed in the having clause (which is executed after all rows are returned).
I don't understand it. HAVING is not executed *after* all rows are returned, it's executed *while* they're returned. What is the difference between rownum directly in HAVING and referring to it by an alias?
having rownum < 0 is NOT using an alias. You can not use a row function on a group result. The group does NOT have a rownum. Each group consist of a set of rows each having it's own rownum. Which rownum should the rownum usage in having get? To put things simple, we are doing exactly what Oracle is doing, which is the purpose of this patch. <cut>
how can rownum be NULL?
In context where there are no rows. like SET @A=rownum;
Do you have test cases for that?
Yes, of course. Was part of the patch: --echo # --echo # Rownum() used in not supported places (returns 0 or gives an error) --echo # set @a=rownum(); select @a;
+ return FALSE; + } + void cleanup() override + { + Item_longlong_func::cleanup(); + /* Ensure we don't point to freed memory */ + accepted_rows= 0; + } + bool check_vcol_func_processor(void *arg) override + { + return mark_unsupported_function(func_name(), "()", arg, + VCOL_IMPOSSIBLE); + } + bool check_handler_func_processor(void *arg) override + { + return mark_unsupported_function(func_name(), "()", arg, + VCOL_IMPOSSIBLE);
first, as I wrote above, I think that there's no logical reason why rownum should not work with handler.
Please read the patch. It cannot be in the handler for multiple reasons: - INSERT, UPDATE, LOAD DATA - handler does not know if the row is accepted - joins etc.
I mean the HANDLER statement, that's why my comment is just below check_handler_func_processor() method.
Oracle does not support HANDLER, so there is no point in even trying. Also with handler, you have full control of the rows that you are getting, so there is no extra benefit of having rownum.
but even if it doesn't, it's much better to check for SQLCOM_HA_READ from Item_func_rownum::fix_fields instead of traversing the whole item tree for every single HANDLER READ command looking for Item_func_rownum that nobody (literally, not a single user out of all our millions) will use there.
There is only a traverse of the tree looking for rownum if the rownum function is used.
Why do you think there is some extra travelling of the tree for HANDLER_READ ?
Because, see above, this comment refers to check_handler_func_processor() method this is called on every item by walrking the item tree.
We do not do this for every item, we only do it for items in the where condition which are in most cases very few and thus almost no overhead. I do not think it would be good to have switch over all possible items that cannot be used in the handler and test for those in the caller. Generally, I don't think items should have a list of command where they cannot be used. Normally it is the caller that should do the checking.
As there is only one item weird enough to possibly need it, I'd rather do the check in the item directly.
And when we flag more items to not be used for handler, or other cases we whould add a list of commands that we check against in all of them? I do not think it is worth the trouble for doing it in this case as I am sure that there are other items that we should also prohibit with handler read (like sub queries for example).
+ } + Item *get_copy(THD *thd) override + { + return this;
It looks wrong. We don't have any other Item that returns `this` here. I think rownum cannot be pushed down because of RAND_TABLE_BIT, So better return NULL here, meaning "cannot be pushed down". Instead of `this` meaning "if you push it down, it'll surreptitiously start corrupting the memory"
I don't think it can trash memory even if it would be pushed down. As long as the item exists, it should be safe to use anywhere. I also don't trust the code to be able to cope with returning null in get_copy.
Quite the contrary. There are other items that return NULL from get_copy(). It's expected and normal, the code is designed to support it, it was part of the original implementation. No get_copy() returns this, the code is not designed to handle it.
Actually, originally we had it in many places and it did work and thus we supported it. There is no reason to assume that this would not work in this case. The reason this works is that Item_rownum does not change it null state between copies and does not store it's value in itself. That is the critera for an Item that can return itself.
+ /* This function is used in insert, update and delete */ + void store_pointer_to_row_counter(ha_rows *row_counter)
So this isn't a virtual method, and to use it you downcast after checking type() == FUNC_ITEM and functype() == ROWNUM_FUNC. To avoid adding a new virtual method thus blowing up the vtable, I presume.
Yes, it is not a virtual and should not be. And yes, we do not want to add any new virtual functions to items when we have close to 600+ items in the system...
It is also much faster to do an if on upper level than doing a function call.
And fix_after_optimize() is virtual. To avoid error-prone downcasting, would be my guess.
Yes. This is something we may need for other items in the future, which is why I made it virtaul.
Do you have in mind any other items that will need it any time soon? If not, then I agree with what you wrote above, we do not want to add any new virtual functions to items when we have close to 600+ items in the system.
I don't know just now of any items that would need it, except maybe Item_equal, that is currently sub optimal. However, I will know much more after I start working full time on the optimizer.
--- a/sql/item_func.cc +++ b/sql/item_func.cc @@ -7206,3 +7206,72 @@ void Item_func_setval::print(String *str, enum_query_type query_type) str->append_ulonglong(round); str->append(')'); } + + +/* + Return how many row combinations has accepted so far + 1 + + The + 1 is to ensure that 'WHERE ROWNUM <=1' returns one row
this is a very weird explanation, it sounds as if it's a heuristical a hack for ROWNUM <=1 to work
Why not to write "+1 takes into account the currently computed row"
Because the row is not yet computed or accepted. We can call rownum any time, after a row is accpted, before etc.
I personally prefer the current comment as it is more well defined.
So, you mean, +1 is not needed for WHERE ROWNUM <=2 to return two rows? Or for WHERE ROWNUM<=100 to return 100 rows?
You pick one very specific case as an explanation, this implicitly assumes it's the main reason. Which it is not, even if we'll only consider exactly the same condition template, there are still 18446744073709551614 other reasons :)
Sorry, I neither understand your reasoning or logic here. The comment is correct and explains what is going on in a clear manner. The <= 1 is just an example. You can replace 1 with any number and 'one' with same number verbatim and the comment still works.
+1 is, exactly, to take into account the current row, the one being processed right now.
By the way, instead of +1 here, you could simply initialize your accepted_rows with 1, not with 0.
Which would be even harder to explain. Why would we have one accepted row before we even run the query?
+*/ + +longlong Item_func_rownum::val_int() +{ + if (!accepted_rows) + { + /* + Rownum is not properly set up. Probably used in wrong context when + it should not be used. In this case returning 0 is probably the best + solution. + */
DBUG_ASSERT(0); here?
No. One can use rownum in a lot of context where it does makes sence. Instead of trying to generate an error for every of these cases, I decided to return 0 for these. In the future maybe some of these will be valid and then things can change. I do not want to see asserts in test cases people add for things like this.
Do you have test cases for that?
Yes, we it is in the test suite (We already discussed this before).
+ select->with_rownum= 1; + thd->lex->with_rownum= 1; + thd->lex->uncacheable(UNCACHEABLE_RAND);
Why? ROWNUM doesn't make a query uncacheable, it's perfectly deterministic and repeatable.
Not necessartly. Row order may be different from the engine on subsequent reads for example.
This is the same with LIMIT. Even if the row order might be different, the first result is still correct, so it can be cached and reused. LIMIT doesn't make a query uncacheable.
I think you are missing the point. marking things uncacheable() means that we cannot cache the value internally, for example in sub queries and into item cache. You can verify this by removing the above line and running mtr rownum and verify that it is failing.
diff --git a/sql/sql_insert.cc b/sql/sql_insert.cc index 806049b4465..5c4dff33914 100644 --- a/sql/sql_insert.cc +++ b/sql/sql_insert.cc @@ -746,6 +746,9 @@ bool mysql_insert(THD *thd, TABLE_LIST *table_list, DBUG_RETURN(TRUE); } + + if (thd->lex->with_rownum && table_list->lock_type == TL_WRITE_DELAYED) + table_list->lock_type= TL_WRITE;
why? do you have a test case when it's needed?
Because we cannot calculate row number inside insert delayed... (We do not know if an row is accepted or not).
"Not accepted"? The only possibility for a row to be not accepted is INSERT IGNORE and a unique key violation, right? WHERE clause is done before the delayed thread.
I don't see any tests for INSERT IGNORE in your patch, so I cannot say what the intended behavior should be. Could you please add a test for it? For example,
create table t1 (a int not null primary key, b int); insert ignore t1 values (1,rownum()),(2,rownum()),(2,rownum()),(3,rownum());
Add edit, and it worked perfectly.
@@ -2133,8 +2141,14 @@ int write_record(THD *thd, TABLE *table, COPY_INFO *info, select_result *sink) autoinc values (generated inside the handler::ha_write()) and values updated in ON DUPLICATE KEY UPDATE. */ - if (sink && sink->send_data(thd->lex->returning()->item_list) < 0) - trg_error= 1; + if (sink) + { + /* We have to adjust 'copied' to make ROWNUM() work in RETURNING */ + info->copied--; + if (sink->send_data(thd->lex->returning()->item_list) < 0) + trg_error= 1; + info->copied++;
this is quite silly. Can you simply increment info->copied after this sink->send_data call?
No, as this leads to wrong resutls. The problem is that for 'returning' the row is already accepted while in all other code it is called before the row is accepted.
Sorry, I don't understand. Why it would lead to wrong results?
There is some cases where copied is incremented twice for a row, like in case of versioning. (This could be a bug). In case of "insert into t1 values(1) on duplicate key update f1=1" If the new row is identical to the old, we are not incrementing rownum. This means that we cannot increment it after, as we don't know if it should be incremented 0, 1, or 2 times.
--- a/sql/sql_limit.h +++ b/sql/sql_limit.h @@ -67,6 +67,15 @@ class Select_limit_counters { return select_limit_cnt; } ha_rows get_offset_limit() { return offset_limit_cnt; } + + void set_limit_if_lower(ha_rows limit) + { + if (!offset_limit_cnt)
I suppose, alternatively, you could've had
if (offset_limit_cnt + select_limit_cnt > limit) select_limit_cnt= limit - offset_limit_cnt;
+ { + if (select_limit_cnt > limit) + select_limit_cnt= limit; + } + } };
In some cases LIMIT is absolute. With rownum it only applies if limit is higher. I think it is good to separate those tests.
What do you mean "LIMIT is absolute"?
LIMIT 10
I only suggested that LIMIT can take ROWNUM into account also when OFFSET is non-zero. In your case you only adjust LIMIT if there is no OFFSET.
The problem is that limit and offset counts different things. If we have for example SELECT * FROM (SELECT * from t1 LIMIT 10,1000)) WHERE rownum < 100 In this case we have to ignore that 10 first rows and then check rownum and limit on the rest. This means that we have to adjust select_limit_cnt independent of offset. However in this case: SELECT * from t1 where rownum < 100 LIMIT 10,1000 The rownum will have to be applied before the offset of 10. To avoid this confusion and possible bugs, it was easer to ignore pushing the condition down when offset exists. In practice, very few queries has both rownum and limit, so I don't think it is worth the effort to spend too much time on this.
It's not a particularly common case, so not a very important optimization, of course. On the other hand, the suggested change is quite simple.
Simple, but not trivial.
#endif // INCLUDES_MARIADB_SQL_LIMIT_H diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy index 2bd2575b882..57ba9df42c0 100644 --- a/sql/sql_yacc.yy +++ b/sql/sql_yacc.yy @@ -10286,6 +10287,22 @@ function_call_nonkeyword: if (unlikely($$ == NULL)) MYSQL_YYABORT; } +/* Start SQL_MODE_ORACLE_SPECIFIC + | ROWNUM_SYM optional_braces + { + $$= new (thd->mem_root) Item_func_rownum(thd); + if (unlikely($$ == NULL)) + MYSQL_YYABORT; + } +End SQL_MODE_ORACLE_SPECIFIC */ +/* Start SQL_MODE_DEFAULT_SPECIFIC */ + | ROWNUM_SYM '(' ')' + { + $$= new (thd->mem_root) Item_func_rownum(thd); + if (unlikely($$ == NULL)) + MYSQL_YYABORT; + } +/* End SQL_MODE_DEFAULT_SPECIFIC */
You have two nearly identical rules compiled conditionally. I can pretty much guarantee that they'll go out of sync eventually.
Better do something like
| ROWNUM_SYM /* Start SQL_MODE_ORACLE_SPECIFIC optional_braces End SQL_MODE_ORACLE_SPECIFIC */ /* Start SQL_MODE_DEFAULT_SPECIFIC */ '(' ')' /* End SQL_MODE_DEFAULT_SPECIFIC */ { $$= new (thd->mem_root) Item_func_rownum(thd); if (unlikely($$ == NULL)) MYSQL_YYABORT; }
A little better, but not much. Still not readable.
This whole work of removing sql_yacc_ora.yy and using comments for parts of the grammar that differ - this was done *exactly* to avoid two nearly identical grammars that always needs to be modified in sync. Which is very error-prone.
By literally copying a part of the grammar you're reintroducing the old problem of two copies. Not completely, of course, by bit by bit we'll have two copies to maintain again.
Please, either use the "little better" grammar that I suggested or rewrite it any way you want that doesn't involve two identical copies of the same rule.
There is now way they can go out of sync as we have test cases to detect this. (I don't find the new suggested code readable).
<cut>
diff --git a/sql/sql_select.cc b/sql/sql_select.cc @@ -14210,7 +14236,24 @@ static ORDER * remove_const(JOIN *join,ORDER *first_order, COND *cond, bool change_list, bool *simple_order) { - *simple_order= join->rollup.state == ROLLUP::STATE_NONE; + /* + We can't do ORDER BY using filesort if the select list contains a non + deterministic value like RAND() or ROWNUM(). + For example: + SELECT RAND() as 'r' from t1 order by r;
This one uses temporary for me. Can you put here a correct example please?
It is a correct example and probably the best possible example. I do not understand your comment (does not make any sense). Could you please provide a correct comment?
I tried this in the 10.5 branch:
MariaDB [test]> explain select rand() as r from mysql.user order by r\G *************************** 1. row *************************** id: 1 select_type: SIMPLE table: user type: index possible_keys: NULL key: PRIMARY key_len: 228 ref: NULL rows: 3 Extra: Using index; Using temporary; Using filesort 1 row in set (0.002 sec)
as you see, it uses temporary, so there is no need to do your
+ *simple_order= !join->rand_table_in_field_list;
because there is no bug here. At least for the example that you've used. This is why I asked for an example of a query that indeed behaves incorrectly without this fix of yours.
The purpose of this function is to set simple_order. If we do not have my line, it will be set always false, which is what you are seeing. However always having 'use temporary' for any filesort is of course not a good idea. Here is an example that shows this if I remove my line. MariaDB [test]> explain select a from t1 order by a; +------+-------------+-------+------+---------------+------+---------+------+------+---------------------------------+ | id | select_type | table | type | possible_keys | key | key_len | ref | rows | Extra | +------+-------------+-------+------+---------------+------+---------+------+------+---------------------------------+ | 1 | SIMPLE | t1 | ALL | NULL | NULL | NULL | NULL | 4 | Using temporary; Using filesort | +------+-------------+-------+------+---------------+------+---------+------+------+---------------------------------+ There is no reason to do a temporary table for this query
+static bool is_const(Item *item) +{ + /* + Constant expression in WHERE can be also subquery or other expensive + thing we shoud not evaluate on OPTIMIZATION phase + */ + return item->const_item() && !item->is_expensive(); +} + + +// Check if this is an unexpensive functional expression over basic constants + +static bool is_basic_const(Item *item) +{ + if (item->basic_const_item()) + return true; + if (!item->const_item() || item->is_expensive()) + return false; + if (item->type() == Item::FUNC_ITEM) + { + Item_func *func= ((Item_func *)item); + if (func->argument_count() == 0) + return false; + bool result= true; + Item **args= func->arguments(); + for (uint i= 0; i < func->argument_count() && result; i++) + result= result && is_basic_const(args[i]); + return result; + } + return false; +}
is_basic_const() is a misleading name, a function of basic consts isn't a basic const itself.
This is how bar has named the item and classes. Do a grep after basic_const..
I know, that was not my question. See below
what's an example of an expression has is_const() == true, but is_basic_const() == false?
"1+1" This is the sum of two basic constants.
Yes, this is an example of the item, where item->const_item() == true and item->basic_const() is false. This is not what I asked.
I asked for an example, where is_const(item) is true and is_basic_const(item) is false. The question was not about old items, but about your new code. Because your is_basic_const() is not just item->basic_const(), so I asked for an example, to understand a difference.
You have to ask Sanja (this is his code). Anyway, in some part the answer to your quesiton is the same as asking when Item::basic_const_item() is true and Item::is_const() is true, as the functions are using this as a base for their return value. The function comments explains in quite a detal what is tested. The main point with is_basic_const() is to detect functions where all arguments are basic_consts() like rownum < 1+2 This is true for is_basic_const.
+/** + Check posibility of LIMIT setting by rownum() of upper SELECT and do it + + @note Ideal is to convert something like + SELECT ... + FROM (SELECT ...) table + WHERE rownum() < <CONSTANT>; + to + SELECT ... + FROM (SELECT ... LIMIT <CONSTANT>) table + WHERE rownum() < <CONSTANT>;
1. what if the outer select uses LIMIT, not ROWNUM? why not to push that down too?
We already have code for that.
But then you don't need to push ROWNUM into a subquery. You have code to convert ROWNUM into a LIMIT, and you just said that we already have code to push LIMIT. So, you do not need to push ROWNUM specifically, it'll happen automatically, won't it?
No, as rownum is not regarded as limit in the upper code and does not work the same way We cannot use the same code as limit. In other words, nothing is happening automatically. For example: SELECT a,count(*) from t1 groupt by a where rownum < 100 limit 100; We cannot push the rows to limit here as rownum works on selected rows and limit works on result rows. We can also not do it with order by, because of the same reason. The only case the rownum makes sence to move to limit on the same join group is for simple queries, without order by, group by, joins etc. In this case the current code is as fast as the limit, so there is no reason to push this either.
2. why are you not removing rownum from the outer select?
Because it may not be safe to do in some cases. Better safe than sorry..
if you remove rownum and reset with_rownum=false, it'll generally give optimizer more freedom.
Has no effect on our optimizer in this particular case.
I'm not sure about it. ROWNUM cannot be pushed down into a subquery. LIMIT can be, as you said, so if you convert a ROWNUM into a LIMIT and remove ROWNUM from WHERE, then the whole WHERE can be pushed into a subquery, and a LIMIT can be too. So
SELECT * FROM (SELECT col1, col2 FROM tables WHERE cond ORDER BY something) WHERE ROWNUM <=100 AND col1 = 10;
can first be changed to
SELECT * FROM (SELECT col1, col2 FROM tables WHERE cond ORDER BY something) WHERE col1 = 10 LIMIT 100;
That would not work if there would be an order by on the upper level or there would be a join involved. The code "col1 = 10 LIMIT 100" and "rownum <= 100 and col1 = 10' are equally fast, so there is no notable win here.
SELECT * FROM (SELECT col1, col2 FROM tables WHERE cond AND col1 = 10 ORDER BY something LIMIT 100);
and for that you don't even need any special code for pushing ROWNUM into a subquery as a limit, you only need to convert ROWNUM to a LIMIT on the same level of select.
You cannot convert rownum to a limit on the same level. The only time you can convert rownum to a limit is when you push it down to single inner select, which is what the current code is doing. We also have to consider that when pushing rownum as part of the limit, is not the same as doing limit on the upper level or even limit on the lower level. We also have to ensure we don't remove any underlaying ORDER BY in the sub query. Still this idea has no effect on the optimizer. The only win is the comparison of rownum with a constant once per result row. We also need a lot more code to cover all cases (like multiple use of rownum etc). I understand what you are trying to suggest. I did take a look at our current code and almost all of it would still be needed to handle the conversion of some rownum's to LIMIT and to handle the general case.
+static bool check_rownum_usage(Item_func *func_item, longlong *limit, + bool *inv_order, const_probe probe) +{ + Item *arg1, *arg2; + *inv_order= 0; + DBUG_ASSERT(func_item->argument_count() == 2); + + /* 'rownum op const' or 'const op field' */ + arg1= func_item->arguments()[0]->real_item(); + if (arg1->type() == Item::FUNC_ITEM && + ((Item_func*) arg1)->functype() == Item_func::ROWNUM_FUNC) + { + arg2= func_item->arguments()[1]->real_item(); + if ((*probe)(arg2)) + { + *limit= arg2->val_int(); + return *limit <= 0 || (ulonglong) *limit >= HA_POS_ERROR; + } + } + else if ((*probe)(arg1)) + { + arg2= func_item->arguments()[1]->real_item();
why not to assign arg2 outside of the if() ?
Because we should not evaluate it if probe is not true. It can be expensive to evalute it or it can have side effects that we may want to avoid. For example, it could be a sub query and probe is there to ensure we don't evaluate it here.
Sure, but func_item->arguments()[1]->real_item() is not evaluating anything. I only mean arg2= line, that's identical in both branches. I didn't mean doing val_int() unconditionally.
Good point. Normally I don't like to use functions on items before they are properly checked. However in this function, we know the item is a function with 2 elements, so it is safe to calculate arg2. The only benefit of the curent code is that we avoid the call completely if there is no rownum function, so there is one line more code, but the code is faster as in most cases we don't have to call real_item for arg2. <cut>
+static void optimize_rownum(THD *thd, SELECT_LEX_UNIT *unit, Item *cond) +{ + DBUG_ENTER("optimize_rownum"); + + if (cond->type() == Item::COND_ITEM) + { + if (((Item_cond*) cond)->functype() == Item_func::COND_AND_FUNC) + { + List_iterator<Item> li(*((Item_cond*) cond)->argument_list()); + Item *item; + while ((item= li++)) + optimize_rownum(thd, unit, item); + } + DBUG_VOID_RETURN; + } + + process_direct_rownum_comparison(thd, unit, cond, &is_const, + &temporary_set_limit);
please, explain why it's is_const/temporary_set_limit here and is_basic_const/permanent_limit in optimize_upper_rownum_func().
Please ask Sanja. I think the short version is that const/temporary is for the current select why is_basic_const/permanent is for pushing limit from an outer select to an inner select and there is more restrictions for that.
Sanja said that it's your code and if it were up to him he'd do both permanent.
All code related to pushing down rownum is Sanjas. In the above function, the loop is mine, but the call to process_direct_rownum_comparison(thd, unit, cond, &is_const, &temporary_set_limit); Is Sanjas. The whole idea of temporary set limit was his idea. If I remember correctly, he did only want to have LIMIT pushdown shown in explain extend for one case, but not for another. For example: SELECT * from (SELECT * from a1) as tmp where rownum < 10 In this case the LIMIT is shown in show example SELECT * from t1 where rownum < 10 In this case the limit is now shown (and we are not using a limit anway). Regards, Monty
Hi, Michael! On May 04, Michael Widenius wrote:
On Mon, Apr 19, 2021 at 10:52 PM Sergei Golubchik <serg@mariadb.org> wrote:
On Apr 18, Michael Widenius wrote:
diff --git a/mysql-test/main/rownum.test b/mysql-test/main/rownum.test new file mode 100644 index 00000000000..8d89eed2f86 --- /dev/null +++ b/mysql-test/main/rownum.test @@ -0,0 +1,542 @@ +--source include/have_sequence.inc +# +# Test of basic rownum() functionallity +# Test are done with Aria to ensure that row order is stable
what does that mean?
It means that SELECT * will return the rows in the same order for the test. (There is normally no guaranteed that insert and select order are the same).
But both MyISAM and InnoDB guarantee that too. What's so special with Aria?
MyISAM does guarantee that, but not InnoDB (returns rows in primary key order) and Aria with page storage will not do it either.
Aria with page row format doesn't. But InnoDB does, primary key order is stable - it's not in the order of insertion, like with MyISAM, but it is stable, always the same.
+ OPEN c_cursor; + FETCH c_cursor INTO v_a, v_b, v_rn; + WHILE c_cursor%FOUND LOOP + SELECT concat(v_a,'--',v_b,'--',v_rn); + FETCH c_cursor INTO v_a, v_b, v_rn; + END LOOP; + CLOSE c_cursor; +END;| +DELIMITER ;| + +--error ER_FUNCTION_CANNOT_BE_USED_IN_CLAUSE +select a, rownum from t1 group by a, rownum having rownum < 3; +select a, rownum from t1 group by a, rownum having "rownum" < 3;
Why *in Oracle mode* the first is an error and the second is not? I'd expect them to be identical.
In Oracle you cannot use rownum function in having. You can however, thanks to MariaDB, refer to a column in the select part.
+select a, rownum as r from t1 group by a, rownum having r < 3;
And the this one. Why is it not an error either?
Because 'r' is a reference, not a function. 'r' works on the result set. rownum cannot be executed in the having clause (which is executed after all rows are returned).
I don't understand it. HAVING is not executed *after* all rows are returned, it's executed *while* they're returned. What is the difference between rownum directly in HAVING and referring to it by an alias?
having rownum < 0 is NOT using an alias.
You can not use a row function on a group result. The group does NOT have a rownum. Each group consist of a set of rows each having it's own rownum. Which rownum should the rownum usage in having get?
To put things simple, we are doing exactly what Oracle is doing, which is the purpose of this patch.
This made so little sense to me even after your explanations, that I've tried it on Oracle. The result was *exactly* the opposite. select a, rownum from t1 group by a, rownum having rownum < 3; this worked just fine and produced the correct result. The other two didn't work, because, so it seems, Oracle doesn't allow to use aliases from the SELECT list in HAVING, the error was select a, rownum as r from t1 group by a,rownum having r < 3; --> ORA-00904: "R": invalid identifier But MariaDB allows it, so in MariaDB all three selects must work.
+ bool check_handler_func_processor(void *arg) override + { + return mark_unsupported_function(func_name(), "()", arg, + VCOL_IMPOSSIBLE);
first, as I wrote above, I think that there's no logical reason why rownum should not work with handler.
Oracle does not support HANDLER, so there is no point in even trying. Also with handler, you have full control of the rows that you are getting, so there is no extra benefit of having rownum.
I know. But you've added a special check, a virtual function, a recursive item tree traversal - all to disable a rownum usage that is not very useful and doesn't exist in Oracle, but still perfectly and consistently fits into MariaDB SQL logic. Did you verify that rownum doesn't "just work" in HANDLER? Even if it doesn't, it'd be easier and more consistent to convert it to LIMIT.
but even if it doesn't, it's much better to check for SQLCOM_HA_READ from Item_func_rownum::fix_fields instead of traversing the whole item tree for every single HANDLER READ command looking for Item_func_rownum that nobody (literally, not a single user out of all our millions) will use there.
Generally, I don't think items should have a list of command where they cannot be used. Normally it is the caller that should do the checking.
Yes. But normally and generally all items work in HANDLER, there's no any logical reason why an item wouldn't. You created rownum as a very weird item that breaks all the common sense rules (because it does in Oracle, I know). I sincerely hope this will never become "normal" and "general", so a check inside Item_rownum is very appropriate here. An even better approach would be to remove the check and just let it work in HANDLER, see above.
+ } + Item *get_copy(THD *thd) override + { + return this;
It looks wrong. We don't have any other Item that returns `this` here. I think rownum cannot be pushed down because of RAND_TABLE_BIT, So better return NULL here, meaning "cannot be pushed down". Instead of `this` meaning "if you push it down, it'll surreptitiously start corrupting the memory"
I don't think it can trash memory even if it would be pushed down. As long as the item exists, it should be safe to use anywhere. I also don't trust the code to be able to cope with returning null in get_copy.
Quite the contrary. There are other items that return NULL from get_copy(). It's expected and normal, the code is designed to support it, it was part of the original implementation. No get_copy() returns this, the code is not designed to handle it.
Actually, originally we had it in many places and it did work and thus we supported it. There is no reason to assume that this would not work in this case. The reason this works is that Item_rownum does not change it null state between copies and does not store it's value in itself. That is the critera for an Item that can return itself.
Monty, I know that get_copy was created to do one of the two things: return *a copy of the item* or *return NULL*. This is how it was originally implemented, by design, and this is how it always worked. Frankly, I don't understand what you're arguing about. get_copy is very well defined - it returns a copy or NULL. You break it and say that "there is no reason to assume that it would not work" in this specific case of Item_rownum? What does it even mean? Item_rownum cannot be copied anyway, it has RAND_TABLE_BIT, so get_copy won't be even called. The only effect of this is that some poor soul can copy your get_copy() and break the server.
--- a/sql/item_func.cc +++ b/sql/item_func.cc @@ -7206,3 +7206,72 @@ void Item_func_setval::print(String *str, enum_query_type query_type) str->append_ulonglong(round); str->append(')'); } + + +/* + Return how many row combinations has accepted so far + 1 + + The + 1 is to ensure that 'WHERE ROWNUM <=1' returns one row
this is a very weird explanation, it sounds as if it's a heuristical a hack for ROWNUM <=1 to work
Why not to write "+1 takes into account the currently computed row"
Because the row is not yet computed or accepted. We can call rownum any time, after a row is accpted, before etc.
I personally prefer the current comment as it is more well defined.
So, you mean, +1 is not needed for WHERE ROWNUM <=2 to return two rows? Or for WHERE ROWNUM<=100 to return 100 rows?
You pick one very specific case as an explanation, this implicitly assumes it's the main reason. Which it is not, even if we'll only consider exactly the same condition template, there are still 18446744073709551614 other reasons :)
Sorry, I neither understand your reasoning or logic here. The comment is correct and explains what is going on in a clear manner. The <= 1 is just an example. You can replace 1 with any number and 'one' with same number verbatim and the comment still works.
if it's an example, the comment should say that. "to ensure that, for example, ..." or "to ensure that queries like ..." or something to explain that `WHERE ROWNUM <=1` is not your single only special case that needs +1. If you'd have "for example" there, I wouldn't have been confused and wouldn't have to write about it.
+1 is, exactly, to take into account the current row, the one being processed right now.
By the way, instead of +1 here, you could simply initialize your accepted_rows with 1, not with 0.
Which would be even harder to explain. Why would we have one accepted row before we even run the query?
If you start with 1, then accepted_rows will mean "the number of the row that will be returned next". Currently it means "number of rows returned so far". Both definitions are consistent and easy to explain.
diff --git a/sql/sql_insert.cc b/sql/sql_insert.cc index 806049b4465..5c4dff33914 100644 --- a/sql/sql_insert.cc +++ b/sql/sql_insert.cc @@ -746,6 +746,9 @@ bool mysql_insert(THD *thd, TABLE_LIST *table_list, DBUG_RETURN(TRUE); } + + if (thd->lex->with_rownum && table_list->lock_type == TL_WRITE_DELAYED) + table_list->lock_type= TL_WRITE;
why? do you have a test case when it's needed?
Because we cannot calculate row number inside insert delayed... (We do not know if an row is accepted or not).
"Not accepted"? The only possibility for a row to be not accepted is INSERT IGNORE and a unique key violation, right? WHERE clause is done before the delayed thread.
I don't see any tests for INSERT IGNORE in your patch, so I cannot say what the intended behavior should be. Could you please add a test for it? For example,
create table t1 (a int not null primary key, b int); insert ignore t1 values (1,rownum()),(2,rownum()),(2,rownum()),(3,rownum());
Add edit, and it worked perfectly.
Did you mean "Added"? No, it didn't work perfectly, it worked inconsistently. See, values (1,rownum),(2,rownum),(2,rownum),(3,rownum) is a table value constructor. You use it as a source to insert rows into a table, ignoring duplicate errors. One can argue, whether a rownum result should be 1,2,3 or 1,2,4, but here's a similar example: create table t1 (a int); insert t1 values (1),(2),(2),(3); create table t2 (x int primary key, y int); insert ignore t2 select a,rownum from t1; select * from t2; +---+------+ | x | y | +---+------+ | 1 | 1 | | 2 | 2 | | 3 | 4 | +---+------+ here it clearly shows that rownum applies to the rows of the source result set that will be inserted, not to what was actually inserted. That is insert ignore t2 values (1,rownum),(2,rownum),(2,rownum),(3,rownum); should also result in +---+------+ | x | y | +---+------+ | 1 | 1 | | 2 | 2 | | 3 | 4 | +---+------+ And from that it follows that INSERT DELAYED can perfectly work with ROWNUM, there can be no problem here. And, by the way, rownum doesn't work in table value constructors now: MariaDB [test]> values (1,rownum),(2,rownum); +---+--------+ | 1 | rownum | +---+--------+ | 1 | 0 | | 2 | 0 | +---+--------+ 2 rows in set (0.001 sec)
--- a/sql/sql_limit.h +++ b/sql/sql_limit.h @@ -67,6 +67,15 @@ class Select_limit_counters { return select_limit_cnt; } ha_rows get_offset_limit() { return offset_limit_cnt; } + + void set_limit_if_lower(ha_rows limit) + { + if (!offset_limit_cnt)
I suppose, alternatively, you could've had
if (offset_limit_cnt + select_limit_cnt > limit) select_limit_cnt= limit - offset_limit_cnt;
+ { + if (select_limit_cnt > limit) + select_limit_cnt= limit; + } + } };
In some cases LIMIT is absolute. With rownum it only applies if limit is higher. I think it is good to separate those tests.
What do you mean "LIMIT is absolute"? I only suggested that LIMIT can take ROWNUM into account also when OFFSET is non-zero. In your case you only adjust LIMIT if there is no OFFSET.
In practice, very few queries has both rownum and limit, so I don't think it is worth the effort to spend too much time on this.
Yes, I certainly agree with that.
diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy index 2bd2575b882..57ba9df42c0 100644 --- a/sql/sql_yacc.yy +++ b/sql/sql_yacc.yy @@ -10286,6 +10287,22 @@ function_call_nonkeyword: if (unlikely($$ == NULL)) MYSQL_YYABORT; } +/* Start SQL_MODE_ORACLE_SPECIFIC + | ROWNUM_SYM optional_braces + { + $$= new (thd->mem_root) Item_func_rownum(thd); + if (unlikely($$ == NULL)) + MYSQL_YYABORT; + } +End SQL_MODE_ORACLE_SPECIFIC */ +/* Start SQL_MODE_DEFAULT_SPECIFIC */ + | ROWNUM_SYM '(' ')' + { + $$= new (thd->mem_root) Item_func_rownum(thd); + if (unlikely($$ == NULL)) + MYSQL_YYABORT; + } +/* End SQL_MODE_DEFAULT_SPECIFIC */
You have two nearly identical rules compiled conditionally. I can pretty much guarantee that they'll go out of sync eventually.
Better do something like
| ROWNUM_SYM /* Start SQL_MODE_ORACLE_SPECIFIC optional_braces End SQL_MODE_ORACLE_SPECIFIC */ /* Start SQL_MODE_DEFAULT_SPECIFIC */ '(' ')' /* End SQL_MODE_DEFAULT_SPECIFIC */ { $$= new (thd->mem_root) Item_func_rownum(thd); if (unlikely($$ == NULL)) MYSQL_YYABORT; }
A little better, but not much. Still not readable.
This whole work of removing sql_yacc_ora.yy and using comments for parts of the grammar that differ - this was done *exactly* to avoid two nearly identical grammars that always needs to be modified in sync. Which is very error-prone.
By literally copying a part of the grammar you're reintroducing the old problem of two copies. Not completely, of course, by bit by bit we'll have two copies to maintain again.
Please, either use the "little better" grammar that I suggested or rewrite it any way you want that doesn't involve two identical copies of the same rule.
There is now way they can go out of sync as we have test cases to detect this. (I don't find the new suggested code readable).
It doesn't matter, you've added your perl script specifically to make this code readable. So no matter what the original yy file will be, your perl-processed files will always be as readable as you wanted them. Anyway, I don't want to waste time on arguing that code duplication should be avoided. If you leave two identical code blocks, someone will someday merge them to reduce code duplication.
diff --git a/sql/sql_select.cc b/sql/sql_select.cc @@ -14210,7 +14236,24 @@ static ORDER * remove_const(JOIN *join,ORDER *first_order, COND *cond, bool change_list, bool *simple_order) { - *simple_order= join->rollup.state == ROLLUP::STATE_NONE; + /* + We can't do ORDER BY using filesort if the select list contains a non + deterministic value like RAND() or ROWNUM(). + For example: + SELECT RAND() as 'r' from t1 order by r;
This one uses temporary for me. Can you put here a correct example please?
It is a correct example and probably the best possible example. I do not understand your comment (does not make any sense). Could you please provide a correct comment?
I tried this in the 10.5 branch:
MariaDB [test]> explain select rand() as r from mysql.user order by r\G *************************** 1. row *************************** id: 1 select_type: SIMPLE table: user type: index possible_keys: NULL key: PRIMARY key_len: 228 ref: NULL rows: 3 Extra: Using index; Using temporary; Using filesort 1 row in set (0.002 sec)
as you see, it uses temporary, so there is no need to do your
+ *simple_order= !join->rand_table_in_field_list;
because there is no bug here. At least for the example that you've used. This is why I asked for an example of a query that indeed behaves incorrectly without this fix of yours.
The purpose of this function is to set simple_order. If we do not have my line, it will be set always false, which is what you are seeing. However always having 'use temporary' for any filesort is of course not a good idea.
Here is an example that shows this if I remove my line.
MariaDB [test]> explain select a from t1 order by a; +------+-------------+-------+------+---------------+------+---------+------+------+---------------------------------+ | id | select_type | table | type | possible_keys | key | key_len | ref | rows | Extra | +------+-------------+-------+------+---------------+------+---------+------+------+---------------------------------+ | 1 | SIMPLE | t1 | ALL | NULL | NULL | NULL | NULL | 4 | Using temporary; Using filesort | +------+-------------+-------+------+---------------+------+---------+------+------+---------------------------------+
There is no reason to do a temporary table for this query
how is it relevant? It doesn't use RAND() and you added a line with join->rand_table_in_field_list. Could you *please* provide an example of a query where your line makes a difference?
+static bool is_const(Item *item) +{ + /* + Constant expression in WHERE can be also subquery or other expensive + thing we shoud not evaluate on OPTIMIZATION phase + */ + return item->const_item() && !item->is_expensive(); +} + + +// Check if this is an unexpensive functional expression over basic constants + +static bool is_basic_const(Item *item) +{ + if (item->basic_const_item()) + return true; + if (!item->const_item() || item->is_expensive()) + return false; + if (item->type() == Item::FUNC_ITEM) + { + Item_func *func= ((Item_func *)item); + if (func->argument_count() == 0) + return false; + bool result= true; + Item **args= func->arguments(); + for (uint i= 0; i < func->argument_count() && result; i++) + result= result && is_basic_const(args[i]); + return result; + } + return false; +}
is_basic_const() is a misleading name, a function of basic consts isn't a basic const itself.
This is how bar has named the item and classes. Do a grep after basic_const..
I know, that was not my question. See below
what's an example of an expression has is_const() == true, but is_basic_const() == false?
"1+1" This is the sum of two basic constants.
Yes, this is an example of the item, where item->const_item() == true and item->basic_const() is false. This is not what I asked.
I asked for an example, where is_const(item) is true and is_basic_const(item) is false. The question was not about old items, but about your new code. Because your is_basic_const() is not just item->basic_const(), so I asked for an example, to understand a difference.
You have to ask Sanja (this is his code). Anyway, in some part the answer to your quesiton is the same as asking when Item::basic_const_item() is true and Item::is_const() is true, as the functions are using this as a base for their return value.
I understand that when Item::basic_const_item() is true, then is_basic_const() will be true. I want to understand cases when Item::basic_const_item() is not true, and is_basic_const() is not true but is_const() is true. See below.
The function comments explains in quite a detal what is tested.
The main point with is_basic_const() is to detect functions where all arguments are basic_consts()
like rownum < 1+2
This is true for is_basic_const.
Yes, of course. I saw that. This is a case when Item::basic_const_item() is false, but is_basic_const() is true. is_basic_const(item) is true, if item is an expression, with all arguments being Item::basic_const_item(). I'm asking, when an item is const_item(), but is not an expression with all arguments being Item::basic_const_item()?
2. why are you not removing rownum from the outer select?
Because it may not be safe to do in some cases. Better safe than sorry..
if you remove rownum and reset with_rownum=false, it'll generally give optimizer more freedom.
Has no effect on our optimizer in this particular case.
I'm not sure about it. ROWNUM cannot be pushed down into a subquery. LIMIT can be, as you said, so if you convert a ROWNUM into a LIMIT and remove ROWNUM from WHERE, then the whole WHERE can be pushed into a subquery, and a LIMIT can be too. So
SELECT * FROM (SELECT col1, col2 FROM tables WHERE cond ORDER BY something) WHERE ROWNUM <=100 AND col1 = 10;
can first be changed to
SELECT * FROM (SELECT col1, col2 FROM tables WHERE cond ORDER BY something) WHERE col1 = 10 LIMIT 100;
SELECT * FROM (SELECT col1, col2 FROM tables WHERE cond AND col1 = 10 ORDER BY something LIMIT 100);
and for that you don't even need any special code for pushing ROWNUM into a subquery as a limit, you only need to convert ROWNUM to a LIMIT on the same level of select.
That would not work if there would be an order by on the upper level or there would be a join involved.
You cannot convert rownum to a limit on the same level. The only time you can convert rownum to a limit is when you push it down to single inner select, which is what the current code is doing.
of course. for any optimization I can say "but it cannot be used anymore if I add this or that". It's not a reason not to support it. For example, we have an optimization to convert outer join to an inner join, even though not every outer join can be converted to an inner join.
I understand what you are trying to suggest. I did take a look at our current code and almost all of it would still be needed to handle the conversion of some rownum's to LIMIT and to handle the general case.
I thought the above could be done very easily - just convert ROWNUM to LIMIT on the same level, when possible. It's a small change in one place. And then just let LIMIT optimisations do their job.
+static void optimize_rownum(THD *thd, SELECT_LEX_UNIT *unit, Item *cond) +{ + DBUG_ENTER("optimize_rownum"); + + if (cond->type() == Item::COND_ITEM) + { + if (((Item_cond*) cond)->functype() == Item_func::COND_AND_FUNC) + { + List_iterator<Item> li(*((Item_cond*) cond)->argument_list()); + Item *item; + while ((item= li++)) + optimize_rownum(thd, unit, item); + } + DBUG_VOID_RETURN; + } + + process_direct_rownum_comparison(thd, unit, cond, &is_const, + &temporary_set_limit);
please, explain why it's is_const/temporary_set_limit here and is_basic_const/permanent_limit in optimize_upper_rownum_func().
Please ask Sanja. I think the short version is that const/temporary is for the current select why is_basic_const/permanent is for pushing limit from an outer select to an inner select and there is more restrictions for that.
Sanja said that it's your code and if it were up to him he'd do both permanent.
All code related to pushing down rownum is Sanjas.
In the above function, the loop is mine, but the call to process_direct_rownum_comparison(thd, unit, cond, &is_const, &temporary_set_limit);
Is Sanjas.
The whole idea of temporary set limit was his idea. If I remember correctly, he did only want to have LIMIT pushdown shown in explain extend for one case, but not for another.
okay, then if neither you nor Sanja knows, I'll try to analyze later myself and see what I'll find. Regards, Sergei
participants (2)
-
Michael Widenius
-
Sergei Golubchik