Skip to content

Commit

Permalink
Check the shippability of sort clauses properly.
Browse files Browse the repository at this point in the history 
We were pushing the ORDER BY clauses to the remote side without
verifying whether the sort operator was safe to ship.  It resulted
in a wrong output when the sort operator isn't default for the sort
expression's type.  Fix the same.

FDW-564, Vaibhav Dalvi, reviewed by Suraj Kharage and Jeevan Chalke.
  • Loading branch information
@jeevanchalke
jeevanchalke committed Nov 29, 2022
1 parent f1a6b44 commit 234c8cf
Show file tree
Hide file tree
Showing 6 changed files with 336 additions and 39 deletions.
70 changes: 70 additions & 0 deletions deparse.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
#include "mongo.h"
#endif
#include "mongo_query.h"
#include "nodes/nodeFuncs.h"
#if PG_VERSION_NUM < 120000
#include "nodes/relation.h"
#include "optimizer/var.h"
Expand Down Expand Up @@ -623,3 +624,72 @@ mongo_add_null_check(Var *column, BSON *expr, pipeline_cxt *context)
bsonAppendNull(&ne_expr, "1");
bsonAppendFinishArray(expr, &ne_expr);
}

/*
* mongo_is_foreign_pathkey
* Returns true if it's safe to push down the sort expression described by
* 'pathkey' to the foreign server.
*/
bool
mongo_is_foreign_pathkey(PlannerInfo *root, RelOptInfo *baserel,
PathKey *pathkey)
{
EquivalenceMember *em;
EquivalenceClass *pathkey_ec = pathkey->pk_eclass;
Expr *em_expr;

/*
* mongo_is_foreign_expr would detect volatile expressions as well,
* but checking ec_has_volatile here saves some cycles.
*/
if (pathkey_ec->ec_has_volatile)
return false;

/* can push if a suitable EC member exists */
if (!(em = mongo_find_em_for_rel(root, pathkey_ec, baserel)))
return false;

/* Ignore binary-compatible relabeling */
em_expr = em->em_expr;
while (em_expr && IsA(em_expr, RelabelType))
em_expr = ((RelabelType *) em_expr)->arg;

/* Only Vars are allowed per MongoDB. */
if (!IsA(em_expr, Var))
return false;

/* Check for sort operator pushability. */
if (!mongo_is_default_sort_operator(em, pathkey))
return false;

return true;
}

/*
* mongo_is_builtin
* Return true if given object is one of PostgreSQL's built-in objects.
*
* We use FirstBootstrapObjectId as the cutoff, so that we only consider
* objects with hand-assigned OIDs to be "built in", not for instance any
* function or type defined in the information_schema.
*
* Our constraints for dealing with types are tighter than they are for
* functions or operators: we want to accept only types that are in pg_catalog,
* else format_type might incorrectly fail to schema-qualify their names.
* (This could be fixed with some changes to format_type, but for now there's
* no need.) Thus we must exclude information_schema types.
*
* XXX there is a problem with this, which is that the set of built-in
* objects expands over time. Something that is built-in to us might not
* be known to the remote server, if it's of an older version. But keeping
* track of that would be a huge exercise.
*/
bool
mongo_is_builtin(Oid oid)
{
#if PG_VERSION_NUM >= 120000
return (oid < FirstGenbkiObjectId);
#else
return (oid < FirstBootstrapObjectId);
#endif
}
62 changes: 62 additions & 0 deletions expected/pushdown.out
View file
Original file line number Diff line number Diff line change
Expand Up @@ -700,6 +700,62 @@ SELECT _id, a01, a31, a32, a33, a34, a35 FROM f_test_large ORDER BY
2 | 1 | 31 | 132 | 133 | 134 | 135
(5 rows)

-- FDW-564: Test ORDER BY with user defined operators. Create the operator
-- family required for the test.
CREATE OPERATOR PUBLIC.<^ (
LEFTARG = INT4,
RIGHTARG = INT4,
PROCEDURE = INT4EQ
);
CREATE OPERATOR PUBLIC.=^ (
LEFTARG = INT4,
RIGHTARG = INT4,
PROCEDURE = INT4LT
);
CREATE OPERATOR PUBLIC.>^ (
LEFTARG = INT4,
RIGHTARG = INT4,
PROCEDURE = INT4GT
);
CREATE OPERATOR FAMILY my_op_family USING btree;
CREATE FUNCTION MY_OP_CMP(A INT, B INT) RETURNS INT AS
$$ BEGIN RETURN BTINT4CMP(A, B); END $$ LANGUAGE PLPGSQL;
CREATE OPERATOR CLASS my_op_class FOR TYPE INT USING btree FAMILY my_op_family AS
OPERATOR 1 PUBLIC.<^,
OPERATOR 3 PUBLIC.=^,
OPERATOR 5 PUBLIC.>^,
FUNCTION 1 my_op_cmp(INT, INT);
-- FDW-564: User defined operators are not pushed down.
EXPLAIN (COSTS FALSE, VERBOSE)
SELECT * FROM f_mongo_test ORDER BY a USING OPERATOR(public.<^);
QUERY PLAN
---------------------------------------------------------
Sort
Output: _id, a, b
Sort Key: f_mongo_test.a USING <^
-> Foreign Scan on public.f_mongo_test
Output: _id, a, b
Foreign Namespace: mongo_fdw_regress.mongo_test
(6 rows)

EXPLAIN (COSTS FALSE, VERBOSE)
SELECT MIN(a) FROM f_mongo_test ORDER BY 1 USING OPERATOR(public.<^);
QUERY PLAN
-----------------------------------------------------------------
Sort
Output: ($0)
Sort Key: ($0) USING <^
InitPlan 1 (returns $0)
-> Limit
Output: f_mongo_test.a
-> Foreign Scan on public.f_mongo_test
Output: f_mongo_test.a
Filter: (f_mongo_test.a IS NOT NULL)
Foreign Namespace: mongo_fdw_regress.mongo_test
-> Result
Output: $0
(12 rows)

-- Cleanup
DELETE FROM f_mongo_test WHERE a != 0;
DELETE FROM f_test_tbl2 WHERE c1 IS NULL;
Expand All @@ -708,6 +764,12 @@ DROP FOREIGN TABLE f_test_tbl1;
DROP FOREIGN TABLE f_test_tbl2;
DROP FOREIGN TABLE f_test_tbl3;
DROP FOREIGN TABLE f_test_large;
DROP OPERATOR CLASS my_op_class USING btree;
DROP FUNCTION my_op_cmp(a INT, b INT);
DROP OPERATOR FAMILY my_op_family USING btree;
DROP OPERATOR public.>^(INT, INT);
DROP OPERATOR public.=^(INT, INT);
DROP OPERATOR public.<^(INT, INT);
DROP USER MAPPING FOR public SERVER mongo_server;
DROP SERVER mongo_server;
DROP EXTENSION mongo_fdw;
58 changes: 58 additions & 0 deletions expected/pushdown_1.out
View file
Original file line number Diff line number Diff line change
Expand Up @@ -744,6 +744,58 @@ SELECT _id, a01, a31, a32, a33, a34, a35 FROM f_test_large ORDER BY
2 | 1 | 31 | 132 | 133 | 134 | 135
(5 rows)

-- FDW-564: Test ORDER BY with user defined operators. Create the operator
-- family required for the test.
CREATE OPERATOR PUBLIC.<^ (
LEFTARG = INT4,
RIGHTARG = INT4,
PROCEDURE = INT4EQ
);
CREATE OPERATOR PUBLIC.=^ (
LEFTARG = INT4,
RIGHTARG = INT4,
PROCEDURE = INT4LT
);
CREATE OPERATOR PUBLIC.>^ (
LEFTARG = INT4,
RIGHTARG = INT4,
PROCEDURE = INT4GT
);
CREATE OPERATOR FAMILY my_op_family USING btree;
CREATE FUNCTION MY_OP_CMP(A INT, B INT) RETURNS INT AS
$$ BEGIN RETURN BTINT4CMP(A, B); END $$ LANGUAGE PLPGSQL;
CREATE OPERATOR CLASS my_op_class FOR TYPE INT USING btree FAMILY my_op_family AS
OPERATOR 1 PUBLIC.<^,
OPERATOR 3 PUBLIC.=^,
OPERATOR 5 PUBLIC.>^,
FUNCTION 1 my_op_cmp(INT, INT);
-- FDW-564: User defined operators are not pushed down.
EXPLAIN (COSTS FALSE, VERBOSE)
SELECT * FROM f_mongo_test ORDER BY a USING OPERATOR(public.<^);
QUERY PLAN
---------------------------------------------------------
Sort
Output: _id, a, b
Sort Key: f_mongo_test.a USING <^
-> Foreign Scan on public.f_mongo_test
Output: _id, a, b
Foreign Namespace: mongo_fdw_regress.mongo_test
(6 rows)

EXPLAIN (COSTS FALSE, VERBOSE)
SELECT MIN(a) FROM f_mongo_test ORDER BY 1 USING OPERATOR(public.<^);
QUERY PLAN
---------------------------------------------------------------
Sort
Output: (min(a))
Sort Key: (min(f_mongo_test.a)) USING <^
-> Aggregate
Output: min(a)
-> Foreign Scan on public.f_mongo_test
Output: _id, a, b
Foreign Namespace: mongo_fdw_regress.mongo_test
(8 rows)

-- Cleanup
DELETE FROM f_mongo_test WHERE a != 0;
DELETE FROM f_test_tbl2 WHERE c1 IS NULL;
Expand All @@ -752,6 +804,12 @@ DROP FOREIGN TABLE f_test_tbl1;
DROP FOREIGN TABLE f_test_tbl2;
DROP FOREIGN TABLE f_test_tbl3;
DROP FOREIGN TABLE f_test_large;
DROP OPERATOR CLASS my_op_class USING btree;
DROP FUNCTION my_op_cmp(a INT, b INT);
DROP OPERATOR FAMILY my_op_family USING btree;
DROP OPERATOR public.>^(INT, INT);
DROP OPERATOR public.=^(INT, INT);
DROP OPERATOR public.<^(INT, INT);
DROP USER MAPPING FOR public SERVER mongo_server;
DROP SERVER mongo_server;
DROP EXTENSION mongo_fdw;
Loading

0 comments on commit 234c8cf

Please sign in to comment.