Skip to content

Commit

Permalink
Restrict LIMIT/OFFSET value to either be a constant or a parameter.
Browse files Browse the repository at this point in the history
Not all expressions are supported on MySQL. Thus restrict to only
constant and placeholder parameters in LIMIT and OFFSET clauses.
This avoids errors while preparing a MySQL query.

Suraj Kharage and Jeevan Chalke.
  • Loading branch information
jeevanchalke committed May 16, 2022
1 parent 3760054 commit 7da2699
Show file tree
Hide file tree
Showing 4 changed files with 129 additions and 14 deletions.
52 changes: 52 additions & 0 deletions expected/limit_offset_pushdown.out
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,58 @@ SELECT * FROM f_test_tbl2 ORDER BY 1 LIMIT 5 OFFSET NULL;
50 | IT | PUNE
(5 rows)

-- Limit with placeholder.
EXPLAIN (VERBOSE, COSTS FALSE)
SELECT * FROM f_test_tbl2 ORDER BY 1 LIMIT (SELECT COUNT(*) FROM f_test_tbl2);
QUERY PLAN
----------------------------------------------------------------------------------------------------------------------
Foreign Scan on public.f_test_tbl2
Output: f_test_tbl2.c1, f_test_tbl2.c2, f_test_tbl2.c3
Remote query: SELECT `c1`, `c2`, `c3` FROM `mysql_fdw_regress`.`test_tbl2` ORDER BY `c1` IS NULL, `c1` ASC LIMIT ?
InitPlan 1 (returns $0)
-> Foreign Scan
Output: (count(*))
Relations: Aggregate on (mysql_fdw_regress.f_test_tbl2)
Remote query: SELECT count(*) FROM `mysql_fdw_regress`.`test_tbl2`
(8 rows)

SELECT * FROM f_test_tbl2 ORDER BY 1 LIMIT (SELECT COUNT(*) FROM f_test_tbl2);
c1 | c2 | c3
----+----------------+----------
10 | DEVELOPMENT | PUNE
20 | ADMINISTRATION | BANGLORE
30 | SALES | MUMBAI
40 | HR | NAGPUR
50 | IT | PUNE
60 | DB SERVER | PUNE
(6 rows)

-- Limit with expression, should not pushdown.
EXPLAIN (VERBOSE, COSTS FALSE)
SELECT * FROM f_test_tbl2 ORDER BY 1 LIMIT (10 - (SELECT COUNT(*) FROM f_test_tbl2));
QUERY PLAN
--------------------------------------------------------------------------------------------------------------------
Limit
Output: f_test_tbl2.c1, f_test_tbl2.c2, f_test_tbl2.c3
InitPlan 1 (returns $0)
-> Foreign Scan
Output: (count(*))
Relations: Aggregate on (mysql_fdw_regress.f_test_tbl2)
Remote query: SELECT count(*) FROM `mysql_fdw_regress`.`test_tbl2`
-> Foreign Scan on public.f_test_tbl2
Output: f_test_tbl2.c1, f_test_tbl2.c2, f_test_tbl2.c3
Remote query: SELECT `c1`, `c2`, `c3` FROM `mysql_fdw_regress`.`test_tbl2` ORDER BY `c1` IS NULL, `c1` ASC
(10 rows)

SELECT * FROM f_test_tbl2 ORDER BY 1 LIMIT (10 - (SELECT COUNT(*) FROM f_test_tbl2));
c1 | c2 | c3
----+----------------+----------
10 | DEVELOPMENT | PUNE
20 | ADMINISTRATION | BANGLORE
30 | SALES | MUMBAI
40 | HR | NAGPUR
(4 rows)

DELETE FROM f_test_tbl2;
DROP FOREIGN TABLE f_test_tbl2;
DROP USER MAPPING FOR public SERVER mysql_svr;
Expand Down
54 changes: 54 additions & 0 deletions expected/limit_offset_pushdown_1.out
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,60 @@ SELECT * FROM f_test_tbl2 ORDER BY 1 LIMIT 5 OFFSET NULL;
50 | IT | PUNE
(5 rows)

-- Limit with placeholder.
EXPLAIN (VERBOSE, COSTS FALSE)
SELECT * FROM f_test_tbl2 ORDER BY 1 LIMIT (SELECT COUNT(*) FROM f_test_tbl2);
QUERY PLAN
--------------------------------------------------------------------------------------------------------------------
Limit
Output: f_test_tbl2.c1, f_test_tbl2.c2, f_test_tbl2.c3
InitPlan 1 (returns $0)
-> Foreign Scan
Output: (count(*))
Relations: Aggregate on (mysql_fdw_regress.f_test_tbl2)
Remote query: SELECT count(*) FROM `mysql_fdw_regress`.`test_tbl2`
-> Foreign Scan on public.f_test_tbl2
Output: f_test_tbl2.c1, f_test_tbl2.c2, f_test_tbl2.c3
Remote query: SELECT `c1`, `c2`, `c3` FROM `mysql_fdw_regress`.`test_tbl2` ORDER BY `c1` IS NULL, `c1` ASC
(10 rows)

SELECT * FROM f_test_tbl2 ORDER BY 1 LIMIT (SELECT COUNT(*) FROM f_test_tbl2);
c1 | c2 | c3
----+----------------+----------
10 | DEVELOPMENT | PUNE
20 | ADMINISTRATION | BANGLORE
30 | SALES | MUMBAI
40 | HR | NAGPUR
50 | IT | PUNE
60 | DB SERVER | PUNE
(6 rows)

-- Limit with expression, should not pushdown.
EXPLAIN (VERBOSE, COSTS FALSE)
SELECT * FROM f_test_tbl2 ORDER BY 1 LIMIT (10 - (SELECT COUNT(*) FROM f_test_tbl2));
QUERY PLAN
--------------------------------------------------------------------------------------------------------------------
Limit
Output: f_test_tbl2.c1, f_test_tbl2.c2, f_test_tbl2.c3
InitPlan 1 (returns $0)
-> Foreign Scan
Output: (count(*))
Relations: Aggregate on (mysql_fdw_regress.f_test_tbl2)
Remote query: SELECT count(*) FROM `mysql_fdw_regress`.`test_tbl2`
-> Foreign Scan on public.f_test_tbl2
Output: f_test_tbl2.c1, f_test_tbl2.c2, f_test_tbl2.c3
Remote query: SELECT `c1`, `c2`, `c3` FROM `mysql_fdw_regress`.`test_tbl2` ORDER BY `c1` IS NULL, `c1` ASC
(10 rows)

SELECT * FROM f_test_tbl2 ORDER BY 1 LIMIT (10 - (SELECT COUNT(*) FROM f_test_tbl2));
c1 | c2 | c3
----+----------------+----------
10 | DEVELOPMENT | PUNE
20 | ADMINISTRATION | BANGLORE
30 | SALES | MUMBAI
40 | HR | NAGPUR
(4 rows)

DELETE FROM f_test_tbl2;
DROP FOREIGN TABLE f_test_tbl2;
DROP USER MAPPING FOR public SERVER mysql_svr;
Expand Down
27 changes: 13 additions & 14 deletions mysql_fdw.c
Original file line number Diff line number Diff line change
Expand Up @@ -4487,32 +4487,31 @@ mysql_add_foreign_final_paths(PlannerInfo *root, RelOptInfo *input_rel,
return;

/*
* Support only Const and Param nodes as expressions are NOT suported.
* MySQL doesn't support LIMIT/OFFSET NULL/ALL syntax, so check for the
* same. If const node is null then do not pushdown limit/offset clause.
*/
if (parse->limitCount && nodeTag(parse->limitCount) == T_Const)
if (parse->limitCount)
{
Const *c = (Const *) parse->limitCount;
if (nodeTag(parse->limitCount) != T_Const &&
nodeTag(parse->limitCount) != T_Param)
return;

if (c->constisnull)
if (nodeTag(parse->limitCount) == T_Const &&
((Const *) parse->limitCount)->constisnull)
return;
}
if (parse->limitOffset && nodeTag(parse->limitOffset) == T_Const)
if (parse->limitOffset)
{
Const *c = (Const *) parse->limitOffset;
if (nodeTag(parse->limitOffset) != T_Const &&
nodeTag(parse->limitOffset) != T_Param)
return;

if (c->constisnull)
if (nodeTag(parse->limitOffset) == T_Const &&
((Const *) parse->limitOffset)->constisnull)
return;
}

/*
* Also, the LIMIT/OFFSET cannot be pushed down, if their expressions are
* not safe to remote.
*/
if (!mysql_is_foreign_expr(root, input_rel, (Expr *) parse->limitOffset, true) ||
!mysql_is_foreign_expr(root, input_rel, (Expr *) parse->limitCount, true))
return;

/* Safe to push down */
fpinfo->pushdown_safe = true;

Expand Down
10 changes: 10 additions & 0 deletions sql/limit_offset_pushdown.sql
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,16 @@ EXPLAIN (VERBOSE, COSTS FALSE)
SELECT * FROM f_test_tbl2 ORDER BY 1 LIMIT 5 OFFSET NULL;
SELECT * FROM f_test_tbl2 ORDER BY 1 LIMIT 5 OFFSET NULL;

-- Limit with placeholder.
EXPLAIN (VERBOSE, COSTS FALSE)
SELECT * FROM f_test_tbl2 ORDER BY 1 LIMIT (SELECT COUNT(*) FROM f_test_tbl2);
SELECT * FROM f_test_tbl2 ORDER BY 1 LIMIT (SELECT COUNT(*) FROM f_test_tbl2);

-- Limit with expression, should not pushdown.
EXPLAIN (VERBOSE, COSTS FALSE)
SELECT * FROM f_test_tbl2 ORDER BY 1 LIMIT (10 - (SELECT COUNT(*) FROM f_test_tbl2));
SELECT * FROM f_test_tbl2 ORDER BY 1 LIMIT (10 - (SELECT COUNT(*) FROM f_test_tbl2));

DELETE FROM f_test_tbl2;
DROP FOREIGN TABLE f_test_tbl2;
DROP USER MAPPING FOR public SERVER mysql_svr;
Expand Down

0 comments on commit 7da2699

Please sign in to comment.