Track sort direction in SortGroupClause

Functions make_pathkey_from_sortop() and transformWindowDefinitions(),
which receive a SortGroupClause, were determining the sort order
(ascending vs. descending) by comparing that structure's operator
strategy to BTLessStrategyNumber, but could just as easily have gotten
it from the SortGroupClause object, if it had such a field, so add
one.  This reduces the number of places that hardcode the assumption
that the strategy refers specifically to a btree strategy, rather than
some other index AM's operators.

Author: Mark Dilger <[email protected]>
Discussion: https://www.postgresql.org/message-id/flat/E72EAA49-354D-4C2E-8EB9-255197F55330@enterprisedb.com

diff --git a/src/backend/optimizer/path/pathkeys.c b/src/backend/optimizer/path/pathkeys.c
index 035bbaa3856eb689682fdfbe8898f8c6d5c76b0a..7cf15e89b634735c5e1fc8d74658ea0303fa215f 100644 (file)
--- a/src/backend/optimizer/path/pathkeys.c
+++ b/src/backend/optimizer/path/pathkeys.c
@@ -256,6 +256,7 @@ static PathKey *
 make_pathkey_from_sortop(PlannerInfo *root,
                                                 Expr *expr,
                                                 Oid ordering_op,
+                                                bool reverse_sort,
                                                 bool nulls_first,
                                                 Index sortref,
                                                 bool create_it)
@@ -279,7 +280,7 @@ make_pathkey_from_sortop(PlannerInfo *root,
                                                                          opfamily,
                                                                          opcintype,
                                                                          collation,
-                                                                         (strategy == BTGreaterStrategyNumber),
+                                                                         reverse_sort,
                                                                          nulls_first,
                                                                          sortref,
                                                                          NULL,
@@ -1411,6 +1412,7 @@ make_pathkeys_for_sortclauses_extended(PlannerInfo *root,
                pathkey = make_pathkey_from_sortop(root,
                                                                                   sortkey,
                                                                                   sortcl->sortop,
+                                                                                  sortcl->reverse_sort,
                                                                                   sortcl->nulls_first,
                                                                                   sortcl->tleSortGroupRef,
                                                                                   true);

diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c
index c13586c537e0170fcebb87ca004ca8ce979892ad..bd54a0d032d374e1cd4415401a8eb23e130a8b34 100644 (file)
--- a/src/backend/optimizer/plan/createplan.c
+++ b/src/backend/optimizer/plan/createplan.c
@@ -1896,6 +1896,7 @@ create_unique_plan(PlannerInfo *root, UniquePath *best_path, int flags)
                                                                                                                 subplan->targetlist);
                        sortcl->eqop = eqop;
                        sortcl->sortop = sortop;
+                       sortcl->reverse_sort = false;
                        sortcl->nulls_first = false;
                        sortcl->hashable = false;       /* no need to make this accurate */
                        sortList = lappend(sortList, sortcl);

diff --git a/src/backend/optimizer/plan/planagg.c b/src/backend/optimizer/plan/planagg.c
index afb5445b77b9499e458874a3d875c3cc3a59c674..dbcd3b9a0a02d56930fdb0456a473d57dcd2b179 100644 (file)
--- a/src/backend/optimizer/plan/planagg.c
+++ b/src/backend/optimizer/plan/planagg.c
@@ -48,7 +48,8 @@
 
 static bool can_minmax_aggs(PlannerInfo *root, List **context);
 static bool build_minmax_path(PlannerInfo *root, MinMaxAggInfo *mminfo,
-                                                         Oid eqop, Oid sortop, bool nulls_first);
+                                                         Oid eqop, Oid sortop, bool reverse_sort,
+                                                         bool nulls_first);
 static void minmax_qp_callback(PlannerInfo *root, void *extra);
 static Oid     fetch_agg_sort_op(Oid aggfnoid);
 
@@ -172,9 +173,9 @@ preprocess_minmax_aggregates(PlannerInfo *root)
                 * FIRST is more likely to be available if the operator is a
                 * reverse-sort operator, so try that first if reverse.
                 */
-               if (build_minmax_path(root, mminfo, eqop, mminfo->aggsortop, reverse))
+               if (build_minmax_path(root, mminfo, eqop, mminfo->aggsortop, reverse, reverse))
                        continue;
-               if (build_minmax_path(root, mminfo, eqop, mminfo->aggsortop, !reverse))
+               if (build_minmax_path(root, mminfo, eqop, mminfo->aggsortop, reverse, !reverse))
                        continue;
 
                /* No indexable path for this aggregate, so fail */
@@ -314,7 +315,7 @@ can_minmax_aggs(PlannerInfo *root, List **context)
  */
 static bool
 build_minmax_path(PlannerInfo *root, MinMaxAggInfo *mminfo,
-                                 Oid eqop, Oid sortop, bool nulls_first)
+                                 Oid eqop, Oid sortop, bool reverse_sort, bool nulls_first)
 {
        PlannerInfo *subroot;
        Query      *parse;
@@ -399,6 +400,7 @@ build_minmax_path(PlannerInfo *root, MinMaxAggInfo *mminfo,
        sortcl->tleSortGroupRef = assignSortGroupRef(tle, subroot->processed_tlist);
        sortcl->eqop = eqop;
        sortcl->sortop = sortop;
+       sortcl->reverse_sort = reverse_sort;
        sortcl->nulls_first = nulls_first;
        sortcl->hashable = false;       /* no need to make this accurate */
        parse->sortClause = list_make1(sortcl);

diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c
index e901203424d1ebc687022cef948dbed4513ef2ec..2d3d8fcf7699824fcadca11de4abed5160b64826 100644 (file)
--- a/src/backend/parser/analyze.c
+++ b/src/backend/parser/analyze.c
@@ -1985,6 +1985,7 @@ makeSortGroupClauseForSetOp(Oid rescoltype, bool require_hash)
        grpcl->tleSortGroupRef = 0;
        grpcl->eqop = eqop;
        grpcl->sortop = sortop;
+       grpcl->reverse_sort = false;    /* Sort-op is "less than", or InvalidOid */
        grpcl->nulls_first = false; /* OK with or without sortop */
        grpcl->hashable = hashable;
 

diff --git a/src/backend/parser/parse_clause.c b/src/backend/parser/parse_clause.c
index 8118036495b2b4851178e3a3b52aff319903a786..4c9769090884daeb43745304b5b42176bd1c6143 100644 (file)
--- a/src/backend/parser/parse_clause.c
+++ b/src/backend/parser/parse_clause.c
@@ -2933,7 +2933,7 @@ transformWindowDefinitions(ParseState *pstate,
                                         sortcl->sortop);
                        /* Record properties of sort ordering */
                        wc->inRangeColl = exprCollation(sortkey);
-                       wc->inRangeAsc = (rangestrategy == BTLessStrategyNumber);
+                       wc->inRangeAsc = !sortcl->reverse_sort;
                        wc->inRangeNullsFirst = sortcl->nulls_first;
                }
 
@@ -3489,6 +3489,7 @@ addTargetToSortList(ParseState *pstate, TargetEntry *tle,
                sortcl->eqop = eqop;
                sortcl->sortop = sortop;
                sortcl->hashable = hashable;
+               sortcl->reverse_sort = reverse;
 
                switch (sortby->sortby_nulls)
                {
@@ -3571,6 +3572,8 @@ addTargetToGroupList(ParseState *pstate, TargetEntry *tle,
                grpcl->tleSortGroupRef = assignSortGroupRef(tle, targetlist);
                grpcl->eqop = eqop;
                grpcl->sortop = sortop;
+               grpcl->reverse_sort = false;    /* sortop is "less than", or
+                                                                                * InvalidOid */
                grpcl->nulls_first = false; /* OK with or without sortop */
                grpcl->hashable = hashable;
 

diff --git a/src/backend/utils/cache/lsyscache.c b/src/backend/utils/cache/lsyscache.c
index 48a280d089b70d1606240dd3357ea9afeba5c918..a85dc0d891f2c3da6de94c4843936314f19de9f0 100644 (file)
--- a/src/backend/utils/cache/lsyscache.c
+++ b/src/backend/utils/cache/lsyscache.c
@@ -289,7 +289,7 @@ get_equality_op_for_ordering_op(Oid opno, bool *reverse)
 
 /*
  * get_ordering_op_for_equality_op
- *             Get the OID of a datatype-specific btree ordering operator
+ *             Get the OID of a datatype-specific btree "less than" ordering operator
  *             associated with an equality operator.  (If there are multiple
  *             possibilities, assume any one will do.)
  *

diff --git a/src/include/catalog/catversion.h b/src/include/catalog/catversion.h
index 38f0f9c1588f31f8c44419140b802fa0e8b285c3..842c43c0b684b49fe0b40b83f480457033efaddb 100644 (file)
--- a/src/include/catalog/catversion.h
+++ b/src/include/catalog/catversion.h
@@ -57,6 +57,6 @@
  */
 
 /*                                                     yyyymmddN */
-#define CATALOG_VERSION_NO     202410114
+#define CATALOG_VERSION_NO     202410141
 
 #endif

diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 5b62df32733f4184055351590035ff0201c1a76e..c92cef3d16de45fc2c925c9b6a832b687a8a5ec4 100644 (file)
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -1438,6 +1438,7 @@ typedef struct SortGroupClause
        Index           tleSortGroupRef;        /* reference into targetlist */
        Oid                     eqop;                   /* the equality operator ('=' op) */
        Oid                     sortop;                 /* the ordering operator ('<' op), or 0 */
+       bool            reverse_sort;   /* is sortop a "greater than" operator? */
        bool            nulls_first;    /* do NULLs come before normal values? */
        /* can eqop be implemented by hashing? */
        bool            hashable pg_node_attr(query_jumble_ignore);