diff --git a/src/backend/executor/nodeIndexscan.c b/src/backend/executor/nodeIndexscan.c index 96d34beaf438f40ead0de15ec33947342b07bfcb..d79c1aa8ca98b029c78d25c4d6f84eb1b4d307da 100644 --- a/src/backend/executor/nodeIndexscan.c +++ b/src/backend/executor/nodeIndexscan.c @@ -30,6 +30,7 @@ #include "executor/execdebug.h" #include "executor/nodeIndexscan.h" #include "lib/pairingheap.h" +#include "nodes/nodeFuncs.h" #include "optimizer/clauses.h" #include "utils/array.h" #include "utils/datum.h" @@ -159,9 +160,18 @@ IndexNextWithReorder(IndexScanState *node) bool *lastfetched_nulls; int cmp; - /* only forward scan is supported with reordering. */ + /* + * Only forward scan is supported with reordering. Note: we can get away + * with just Asserting here because the system will not try to run the + * plan backwards if ExecSupportsBackwardScan() says it won't work. + * Currently, that is guaranteed because no index AMs support both + * amcanorderbyop and amcanbackward; if any ever do, + * ExecSupportsBackwardScan() will need to consider indexorderbys + * explicitly. + */ Assert(!ScanDirectionIsBackward(((IndexScan *) node->ss.ps.plan)->indexorderdir)); Assert(ScanDirectionIsForward(node->ss.ps.state->es_direction)); + scandesc = node->iss_ScanDesc; econtext = node->ss.ps.ps_ExprContext; slot = node->ss.ss_ScanTupleSlot; @@ -370,7 +380,11 @@ cmp_orderbyvals(const Datum *adist, const bool *anulls, { SortSupport ssup = &node->iss_SortSupport[i]; - /* Handle nulls. We only support NULLS LAST. */ + /* + * Handle nulls. We only need to support NULLS LAST ordering, because + * match_pathkeys_to_index() doesn't consider indexorderby + * implementation otherwise. + */ if (anulls[i] && !bnulls[i]) return 1; else if (!anulls[i] && bnulls[i]) @@ -388,7 +402,7 @@ cmp_orderbyvals(const Datum *adist, const bool *anulls, /* * Pairing heap provides getting topmost (greatest) element while KNN provides - * ascending sort. That's why we inverse the sort order. + * ascending sort. That's why we invert the sort order. */ static int reorderqueue_cmp(const pairingheap_node *a, const pairingheap_node *b, @@ -917,35 +931,30 @@ ExecInitIndexScan(IndexScan *node, EState *estate, int eflags) { int numOrderByKeys = indexstate->iss_NumOrderByKeys; int i; - ListCell *lc; + ListCell *lco; + ListCell *lcx; /* - * Prepare sort support, and look up the distance type for each ORDER - * BY expression. + * Prepare sort support, and look up the data type for each ORDER BY + * expression. */ Assert(numOrderByKeys == list_length(node->indexorderbyops)); - indexstate->iss_SortSupport = + Assert(numOrderByKeys == list_length(node->indexorderbyorig)); + indexstate->iss_SortSupport = (SortSupportData *) palloc0(numOrderByKeys * sizeof(SortSupportData)); - indexstate->iss_OrderByTypByVals = + indexstate->iss_OrderByTypByVals = (bool *) palloc(numOrderByKeys * sizeof(bool)); - indexstate->iss_OrderByTypLens = + indexstate->iss_OrderByTypLens = (int16 *) palloc(numOrderByKeys * sizeof(int16)); i = 0; - foreach(lc, node->indexorderbyops) + forboth(lco, node->indexorderbyops, lcx, node->indexorderbyorig) { - Oid orderbyop = lfirst_oid(lc); - Oid orderbyType; - Oid opfamily; - int16 strategy; + Oid orderbyop = lfirst_oid(lco); + Node *orderbyexpr = (Node *) lfirst(lcx); + Oid orderbyType = exprType(orderbyexpr); PrepareSortSupportFromOrderingOp(orderbyop, &indexstate->iss_SortSupport[i]); - - if (!get_ordering_op_properties(orderbyop, - &opfamily, &orderbyType, &strategy)) - elog(ERROR, "operator %u is not a valid ordering operator", - orderbyop); - get_typlenbyval(orderbyType, &indexstate->iss_OrderByTypLens[i], &indexstate->iss_OrderByTypByVals[i]); @@ -953,10 +962,10 @@ ExecInitIndexScan(IndexScan *node, EState *estate, int eflags) } /* allocate arrays to hold the re-calculated distances */ - indexstate->iss_OrderByValues = - palloc(indexstate->iss_NumOrderByKeys * sizeof(Datum)); - indexstate->iss_OrderByNulls = - palloc(indexstate->iss_NumOrderByKeys * sizeof(bool)); + indexstate->iss_OrderByValues = (Datum *) + palloc(numOrderByKeys * sizeof(Datum)); + indexstate->iss_OrderByNulls = (bool *) + palloc(numOrderByKeys * sizeof(bool)); /* and initialize the reorder queue */ indexstate->iss_ReorderQueue = pairingheap_allocate(reorderqueue_cmp, diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c index 36015ea6c4b4dcf8163f615c86219e01236492a2..b47ef466dc1029bf496e66be71b5c87570474c3b 100644 --- a/src/backend/optimizer/plan/createplan.c +++ b/src/backend/optimizer/plan/createplan.c @@ -22,7 +22,6 @@ #include "access/stratnum.h" #include "access/sysattr.h" #include "catalog/pg_class.h" -#include "catalog/pg_operator.h" #include "foreign/fdwapi.h" #include "miscadmin.h" #include "nodes/makefuncs.h" @@ -172,8 +171,8 @@ static Plan *prepare_sort_from_pathkeys(PlannerInfo *root, Oid **p_sortOperators, Oid **p_collations, bool **p_nullsFirst); -static EquivalenceMember *find_ec_member_for_expr(EquivalenceClass *ec, - Expr *tlexpr, +static EquivalenceMember *find_ec_member_for_tle(EquivalenceClass *ec, + TargetEntry *tle, Relids relids); static Material *make_material(Plan *lefttree); @@ -1325,36 +1324,35 @@ create_indexscan_plan(PlannerInfo *root, } /* - * If there are ORDER BY expressions, look up the sort operators for - * their datatypes. + * If there are ORDER BY expressions, look up the sort operators for their + * result datatypes. */ - if (best_path->path.pathkeys && indexorderbys) + if (indexorderbys) { ListCell *pathkeyCell, *exprCell; /* - * PathKey contains pointer to the equivalence class, but that's not - * enough because we need the expression's datatype to look up the - * sort operator in the operator family. We have to dig out the - * equivalence member for the datatype. + * PathKey contains OID of the btree opfamily we're sorting by, but + * that's not quite enough because we need the expression's datatype + * to look up the sort operator in the operator family. */ + Assert(list_length(best_path->path.pathkeys) == list_length(indexorderbys)); forboth(pathkeyCell, best_path->path.pathkeys, exprCell, indexorderbys) { PathKey *pathkey = (PathKey *) lfirst(pathkeyCell); - Expr *expr = (Expr *) lfirst(exprCell); - EquivalenceMember *em; - - /* Find equivalence member for the order by expression */ - em = find_ec_member_for_expr(pathkey->pk_eclass, expr, NULL); + Node *expr = (Node *) lfirst(exprCell); + Oid exprtype = exprType(expr); + Oid sortop; /* Get sort operator from opfamily */ - indexorderbyops = - lappend_oid(indexorderbyops, - get_opfamily_member(pathkey->pk_opfamily, - em->em_datatype, - em->em_datatype, - pathkey->pk_strategy)); + sortop = get_opfamily_member(pathkey->pk_opfamily, + exprtype, + exprtype, + pathkey->pk_strategy); + if (!OidIsValid(sortop)) + elog(ERROR, "failed to find sort operator for ORDER BY expression"); + indexorderbyops = lappend_oid(indexorderbyops, sortop); } } @@ -4100,7 +4098,7 @@ prepare_sort_from_pathkeys(PlannerInfo *root, Plan *lefttree, List *pathkeys, tle = get_tle_by_resno(tlist, reqColIdx[numsortkeys]); if (tle) { - em = find_ec_member_for_expr(ec, tle->expr, relids); + em = find_ec_member_for_tle(ec, tle, relids); if (em) { /* found expr at right place in tlist */ @@ -4131,7 +4129,7 @@ prepare_sort_from_pathkeys(PlannerInfo *root, Plan *lefttree, List *pathkeys, foreach(j, tlist) { tle = (TargetEntry *) lfirst(j); - em = find_ec_member_for_expr(ec, tle->expr, relids); + em = find_ec_member_for_tle(ec, tle, relids); if (em) { /* found expr already in tlist */ @@ -4252,21 +4250,23 @@ prepare_sort_from_pathkeys(PlannerInfo *root, Plan *lefttree, List *pathkeys, } /* - * find_ec_member_for_expr - * Locate an EquivalenceClass member matching the given expression, if any + * find_ec_member_for_tle + * Locate an EquivalenceClass member matching the given TLE, if any * * Child EC members are ignored unless they match 'relids'. */ static EquivalenceMember * -find_ec_member_for_expr(EquivalenceClass *ec, - Expr *expr, - Relids relids) +find_ec_member_for_tle(EquivalenceClass *ec, + TargetEntry *tle, + Relids relids) { + Expr *tlexpr; ListCell *lc; /* We ignore binary-compatible relabeling on both ends */ - while (expr && IsA(expr, RelabelType)) - expr = ((RelabelType *) expr)->arg; + tlexpr = tle->expr; + while (tlexpr && IsA(tlexpr, RelabelType)) + tlexpr = ((RelabelType *) tlexpr)->arg; foreach(lc, ec->ec_members) { @@ -4292,7 +4292,7 @@ find_ec_member_for_expr(EquivalenceClass *ec, while (emexpr && IsA(emexpr, RelabelType)) emexpr = ((RelabelType *) emexpr)->arg; - if (equal(emexpr, expr)) + if (equal(emexpr, tlexpr)) return em; } diff --git a/src/include/access/genam.h b/src/include/access/genam.h index f129c4b58f9f9bf4cd0731a4f0dc3b14c6cb8ac1..d86590ac111e6064c06a85b4dbc7b9979b9192b2 100644 --- a/src/include/access/genam.h +++ b/src/include/access/genam.h @@ -147,10 +147,7 @@ extern void index_restrpos(IndexScanDesc scan); extern ItemPointer index_getnext_tid(IndexScanDesc scan, ScanDirection direction); extern HeapTuple index_fetch_heap(IndexScanDesc scan); -extern bool index_get_heap_values(IndexScanDesc scan, ItemPointer heapPtr, - Datum values[INDEX_MAX_KEYS], bool isnull[INDEX_MAX_KEYS]); extern HeapTuple index_getnext(IndexScanDesc scan, ScanDirection direction); - extern int64 index_getbitmap(IndexScanDesc scan, TIDBitmap *bitmap); extern IndexBulkDeleteResult *index_bulk_delete(IndexVacuumInfo *info, diff --git a/src/test/regress/expected/gist.out b/src/test/regress/expected/gist.out index 42f6891ffeec00b73c0a379b446218958d414619..c2ab9c92c385c1e6557787acb0c26efe333ab086 100644 --- a/src/test/regress/expected/gist.out +++ b/src/test/regress/expected/gist.out @@ -86,6 +86,34 @@ order by p <-> point(0.2, 0.2); (0.5,0.5) (11 rows) +-- Check commuted case as well +explain (costs off) +select p from gist_tbl where p <@ box(point(0,0), point(0.5, 0.5)) +order by point(0.1, 0.1) <-> p; + QUERY PLAN +-------------------------------------------------------- + Index Only Scan using gist_tbl_point_index on gist_tbl + Index Cond: (p <@ '(0.5,0.5),(0,0)'::box) + Order By: (p <-> '(0.1,0.1)'::point) +(3 rows) + +select p from gist_tbl where p <@ box(point(0,0), point(0.5, 0.5)) +order by point(0.1, 0.1) <-> p; + p +------------- + (0.1,0.1) + (0.15,0.15) + (0.05,0.05) + (0,0) + (0.2,0.2) + (0.25,0.25) + (0.3,0.3) + (0.35,0.35) + (0.4,0.4) + (0.45,0.45) + (0.5,0.5) +(11 rows) + drop index gist_tbl_point_index; -- Test index-only scan with box opclass create index gist_tbl_box_index on gist_tbl using gist (b); diff --git a/src/test/regress/sql/gist.sql b/src/test/regress/sql/gist.sql index d6cbc21717f62a1f3be4dd8fa6e60522f7d4783e..8b87972b4b8f01a7bfb53a232b9f31efd7bbcd2d 100644 --- a/src/test/regress/sql/gist.sql +++ b/src/test/regress/sql/gist.sql @@ -61,6 +61,14 @@ order by p <-> point(0.2, 0.2); select p from gist_tbl where p <@ box(point(0,0), point(0.5, 0.5)) order by p <-> point(0.2, 0.2); +-- Check commuted case as well +explain (costs off) +select p from gist_tbl where p <@ box(point(0,0), point(0.5, 0.5)) +order by point(0.1, 0.1) <-> p; + +select p from gist_tbl where p <@ box(point(0,0), point(0.5, 0.5)) +order by point(0.1, 0.1) <-> p; + drop index gist_tbl_point_index; -- Test index-only scan with box opclass