From 5d044fc62c9dfd973cde4f96609f16e673a0fbc6 Mon Sep 17 00:00:00 2001 From: Greg Burd Date: Wed, 7 Oct 2026 04:15:45 -0400 Subject: [PATCH] Improve comments and docs for lossy ORDER BY operator index scans The code that rechecks and reorders the output of an index scan using ORDER BY operators, added in commit 35fcb1b3d0, was commented only in terms of "re-checking ORDER BY expressions". That doesn't say when it is needed, or that it has nothing to do with the ordering a btree index provides, and the contract an index AM must meet to use it was not stated anywhere in the index AM documentation. It was spelled out only in the GiST and SP-GiST operator class chapters, in terms of their distance support functions. Document the contract in the amgettuple section of the index AM chapter: an amgettuple implementation with ordering operators must fill xs_orderbyvals/xs_orderbynulls and set xs_recheckorderby, return tuples in nondecreasing order of the values it reports, and, when xs_recheckorderby is set, report a value no greater than the one the ordering operator yields for the heap tuple. Expand the comments at IndexNextWithReorder to explain why that contract makes the reorder queue correct, when a queued tuple can be returned, what "index returned tuples in wrong order" means, which half of the contract is not checked, and what the reordering costs. Also clarify the comments on ReorderTuple, the reorder setup in ExecInitIndexScan, the related IndexScanState fields, the xs_orderbyvals/xs_recheckorderby fields in IndexScanDescData, and IndexPath.indexorderbys, which holds ORDER BY operator expressions and is NIL for ordinary amcanorder ordering. No code changes. Discussion: https://postgr.es/m/20190419003020.6u5uob4yhltrp6t2@alap3.anarazel.de --- doc/src/sgml/indexam.sgml | 32 ++++++++++++ src/backend/executor/nodeIndexscan.c | 74 +++++++++++++++++++++++++--- src/include/access/relscan.h | 13 +++++ src/include/nodes/execnodes.h | 12 ++++- src/include/nodes/pathnodes.h | 6 +++ 5 files changed, 130 insertions(+), 7 deletions(-) diff --git a/doc/src/sgml/indexam.sgml b/doc/src/sgml/indexam.sgml index f48da31..ee1255e 100644 --- a/doc/src/sgml/indexam.sgml +++ b/doc/src/sgml/indexam.sgml @@ -722,6 +722,38 @@ amgettuple (IndexScanDesc scan, callers. + + If the scan has ordering operators (see + amcanorderbyop in + ), then on success + amgettuple must also set + scan->xs_recheckorderby to true or false, and store + the value of each ORDER BY expression for the returned + tuple, as computed by the index, in + scan->xs_orderbyvals and + scan->xs_orderbynulls. (The access method must + allocate these arrays, with norderbys entries; this is + typically done in ambeginscan.) Tuples must be + returned in nondecreasing order of these values, with nulls sorting after + all non-null values. If xs_recheckorderby is set to + true, the values are only lower bounds: each must be less than or equal to + the value the ordering operator yields for the heap tuple. The executor + then computes the exact values from the heap tuple and reorders tuples as + needed; it reports an error if a computed value is less than the value the + index returned for that tuple. If xs_recheckorderby is + false, the executor takes the values to be exact. In a scan that may + return tuples with xs_recheckorderby true, the index's + values are compared with values the executor computes, so they must be of + the ordering operator's result type, and those returned with + xs_recheckorderby false must equal the ordering + operator's results. If no tuple in the scan has + xs_recheckorderby set, the executor does not examine + these values. This provision supports lossy + distance functions, such as a distance computed from a bounding box. It + is not supported in index-only scans; an error is raised if + xs_recheckorderby is set during one. + + If the index supports index-only scans (i.e., amcanreturn returns true for any diff --git a/src/backend/executor/nodeIndexscan.c b/src/backend/executor/nodeIndexscan.c index 129d005..0ffd117 100644 --- a/src/backend/executor/nodeIndexscan.c +++ b/src/backend/executor/nodeIndexscan.c @@ -46,8 +46,12 @@ #include "utils/sortsupport.h" /* - * When an ordering operator is used, tuples fetched from the index that - * need to be reordered are queued in a pairing heap, as ReorderTuples. + * When the index reports inexact ORDER BY values (xs_recheckorderby), + * IndexNextWithReorder holds back tuples that cannot be returned yet in a + * pairing heap, as ReorderTuples. htup is a copy of the heap tuple. + * orderbyvals/orderbynulls are its ORDER BY values: recomputed from the heap + * tuple if the index flagged them inexact (xs_recheckorderby), otherwise as + * the index reported them. */ typedef struct { @@ -166,8 +170,49 @@ IndexNext(IndexScanState *node) /* ---------------------------------------------------------------- * IndexNextWithReorder * - * Like IndexNext, but this version can also re-check ORDER BY - * expressions, and reorder the tuples as necessary. + * Like IndexNext, but used when the scan has ORDER BY operators + * (amcanorderbyop), as in "ORDER BY col <-> constant" on a GiST + * index. This is unrelated to the ordering an amcanorder index such + * as btree provides; IndexNext handles that. + * + * The index returns tuples in order of the ORDER BY values it reports + * in xs_orderbyvals. If it also sets xs_recheckorderby, those values + * are only lower bounds on the true values; a lossy distance function + * might, for example, return the distance to a bounding box rather + * than to the indexed value. The order of the bounds then need not be + * the order of the true values, so the tuples must be reordered. + * + * The index promises that each value it reports is <= the true value, + * and that it returns tuples in nondecreasing order of reported value. + * Together these mean that once the index has reported value X, no + * tuple it returns later has a true value less than X. So for each + * tuple flagged inexact, we evaluate the original ORDER BY expressions + * (indexorderbyorig) on the heap tuple; if the result is larger than + * the reported value, we hold the tuple in a pairing heap keyed by the + * recomputed values. A queued tuple can be returned once the index + * reports a value at least as large as the tuple's, or reaches the end + * of the scan. A tuple whose value is exact (including one whose + * recomputed value equals the reported one) is returned at once, unless + * the queue holds a tuple that sorts before it, in which case it is + * queued too. Values reported without xs_recheckorderby are taken as + * exact and compared with recomputed ones, so in a scan that mixes the + * two they must equal the true values. + * + * Only the first promise is checked, and only for tuples flagged + * inexact: if a recomputed value is less than the value the index + * reported for that tuple, the index has violated its contract, and we + * raise "index returned tuples in wrong order". A violation of the + * second is not detected, and can make the output silently misordered. + * + * If the index never sets xs_recheckorderby, the queue stays empty and + * each tuple is returned as soon as it is fetched. Otherwise, every + * tuple flagged inexact costs an evaluation of the ORDER BY + * expressions, and every queued tuple is copied into query memory. A + * tuple whose reported value is below its true value cannot be returned + * until the index has reported a value at least as large as the true + * value, or the scan has ended, so returning even the first row can + * require fetching many tuples from the index. How many depends on how + * close the index's bounds are to the true values. * ---------------------------------------------------------------- */ static TupleTableSlot * @@ -236,7 +281,10 @@ IndexNextWithReorder(IndexScanState *node) /* * Check the reorder queue first. If the topmost tuple in the queue * has an ORDER BY value smaller than (or equal to) the value last - * returned by the index, we can return it now. + * returned by the index, we can return it now. We must compare with + * the value the index reported, not with the value recomputed from + * the last fetched tuple: only the former is a bound on the tuples + * the index has yet to return. */ if (!pairingheap_is_empty(node->iss_ReorderQueue)) { @@ -309,6 +357,10 @@ next_indextuple: * the index with the recalculated value. (If the value returned * by the index happened to be exact right, we can often avoid * pushing the tuple to the queue, just to pop it back out again.) + * The index's value must be a lower bound on the recalculated + * one, so a smaller recalculated value means the index (or its + * operator class's distance function) is broken; see the comments + * at the top of this function. */ cmp = cmp_orderbyvals(node->iss_OrderByValues, node->iss_OrderByNulls, @@ -1024,7 +1076,17 @@ ExecInitIndexScan(IndexScan *node, EState *estate, int eflags) NULL, /* no ArrayKeys */ NULL); - /* Initialize sort support, if we need to re-check ORDER BY exprs */ + /* + * If the scan uses ORDER BY operators (which only amcanorderbyop indexes + * support), set up what IndexNextWithReorder needs to recheck and reorder + * tuples whose ORDER BY values the index reports as inexact: sort support + * for comparing ORDER BY values, using the sort operators the planner + * chose for their result types (indexorderbyops); type information for + * copying the values; arrays to hold the values recomputed from the heap + * tuple; and the reorder queue. Whether the index returns any inexact + * values is known only at run time, from xs_recheckorderby on each tuple, + * so this is done for every scan with ORDER BY operators. + */ if (indexstate->iss_NumOrderByKeys > 0) { int numOrderByKeys = indexstate->iss_NumOrderByKeys; diff --git a/src/include/access/relscan.h b/src/include/access/relscan.h index d761410..27bcdf2 100644 --- a/src/include/access/relscan.h +++ b/src/include/access/relscan.h @@ -188,6 +188,19 @@ typedef struct IndexScanDescData * xs_recheckorderby is true, these need to be rechecked just like the * scan keys, and the values returned here are a lower-bound on the actual * values. + * + * "Ordering operator" here means an amcanorderbyop operator such as a + * distance ("ORDER BY col <-> constant"); these fields are not used for + * amcanorder scans. The AM must return tuples in nondecreasing order of + * the values it reports here, and when xs_recheckorderby is set, each + * reported value must be <= the value the ORDER BY expression yields for + * the heap tuple. The executor recomputes that value and reorders tuples + * as needed (see IndexNextWithReorder), and raises an error if a + * recomputed value is smaller than the reported one. Values reported + * with xs_recheckorderby false are used as is; if the scan may also + * return tuples with it set, they must equal the ORDER BY expression's + * value. The AM sets xs_recheckorderby separately for each tuple it + * returns. */ Datum *xs_orderbyvals; bool *xs_orderbynulls; diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h index 0998c9b..0925dbb 100644 --- a/src/include/nodes/execnodes.h +++ b/src/include/nodes/execnodes.h @@ -1722,6 +1722,16 @@ typedef struct * OrderByTypByVals is the datatype of order by expression pass-by-value? * OrderByTypLens typlens of the datatypes of order by expressions * PscanLen size of parallel index scan descriptor + * + * ReorderQueue, OrderByValues, OrderByNulls, SortSupport, + * OrderByTypByVals and OrderByTypLens are allocated only when the scan + * uses ORDER BY operators (amcanorderbyop, NumOrderByKeys > 0), not for + * the ordering an amcanorder index such as btree provides. They are + * used to recheck and reorder tuples for which the index reports only a + * lower bound on the ORDER BY values (xs_recheckorderby). OrderByValues + * holds the values recomputed from the heap tuple, as opposed to the + * index's values in xs_orderbyvals. See IndexNextWithReorder in + * nodeIndexscan.c. * ---------------- */ typedef struct IndexScanState @@ -1742,7 +1752,7 @@ typedef struct IndexScanState IndexScanInstrumentation *iss_Instrument; SharedIndexScanInstrumentation *iss_SharedInfo; - /* These are needed for re-checking ORDER BY expr ordering */ + /* These are needed for re-checking ORDER BY operator ordering */ pairingheap *iss_ReorderQueue; bool iss_ReachedEnd; Datum *iss_OrderByValues; diff --git a/src/include/nodes/pathnodes.h b/src/include/nodes/pathnodes.h index 1c6d1fe..9ce334f 100644 --- a/src/include/nodes/pathnodes.h +++ b/src/include/nodes/pathnodes.h @@ -2033,6 +2033,12 @@ typedef struct Path * in the same order. These are not RestrictInfos, just bare expressions, * since they generally won't yield booleans. It's guaranteed that each * expression has the index key on the left side of the operator. + * Typically these are distance operators, as in "ORDER BY col <-> constant". + * Ordinary column ordering from an amcanorder index (btree) is not + * represented here; such a path has indexorderbys = NIL, and its ordering + * is shown only by its pathkeys. An amcanorderbyop index may compute the + * ORDER BY values only approximately, in which case the executor rechecks + * and reorders its output; see IndexNextWithReorder. * * 'indexorderbycols' is an integer list of index column numbers (zero-based) * of the same length as 'indexorderbys', showing which index column each -- 2.54.0