Skip to content

Commit 4246619

Browse files
tglsfdctglsfdc
committed
Fix failure to copy IndexScan.indexorderbyops in copyfuncs.c.
This oversight results in a crash at executor startup if the plan has been copied. outfuncs.c was missed as well. While we could probably have taught both those files to cope with the originally chosen representation of an Oid array, it would have been painful, not least because there'd be no easy way to verify the array length. An Oid List is far easier to work with. And AFAICS, there is no particular notational benefit to using an array rather than a list in the existing parts of the patch either. So just change it to a list. Error in commit 35fcb1b, which is new, so no need for back-patch.
1 parent b14cf22 commit 4246619

File tree

5 files changed

+36
-34
lines changed

5 files changed

+36
-34
lines changed

‎src/backend/executor/nodeIndexscan.c

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -793,7 +793,6 @@ ExecInitIndexScan(IndexScan *node, EState *estate, int eflags)
793793
IndexScanState *indexstate;
794794
Relation currentRelation;
795795
bool relistarget;
796-
int i;
797796

798797
/*
799798
* create state structure
@@ -917,35 +916,40 @@ ExecInitIndexScan(IndexScan *node, EState *estate, int eflags)
917916
if (indexstate->iss_NumOrderByKeys > 0)
918917
{
919918
int numOrderByKeys = indexstate->iss_NumOrderByKeys;
919+
int i;
920+
ListCell *lc;
920921

921922
/*
922923
* Prepare sort support, and look up the distance type for each ORDER
923924
* BY expression.
924925
*/
926+
Assert(numOrderByKeys == list_length(node->indexorderbyops));
925927
indexstate->iss_SortSupport =
926928
palloc0(numOrderByKeys * sizeof(SortSupportData));
927929
indexstate->iss_OrderByTypByVals =
928930
palloc(numOrderByKeys * sizeof(bool));
929931
indexstate->iss_OrderByTypLens =
930932
palloc(numOrderByKeys * sizeof(int16));
931-
for (i = 0; i < indexstate->iss_NumOrderByKeys; i++)
933+
i = 0;
934+
foreach(lc, node->indexorderbyops)
932935
{
936+
Oid orderbyop = lfirst_oid(lc);
933937
Oid orderbyType;
934938
Oid opfamily;
935939
int16 strategy;
936940

937-
PrepareSortSupportFromOrderingOp(node->indexorderbyops[i],
941+
PrepareSortSupportFromOrderingOp(orderbyop,
938942
&indexstate->iss_SortSupport[i]);
939943

940-
if (!get_ordering_op_properties(node->indexorderbyops[i],
944+
if (!get_ordering_op_properties(orderbyop,
941945
&opfamily, &orderbyType, &strategy))
942-
{
943-
elog(LOG, "operator %u is not a valid ordering operator",
944-
node->indexorderbyops[i]);
945-
}
946+
elog(ERROR, "operator %u is not a valid ordering operator",
947+
orderbyop);
948+
946949
get_typlenbyval(orderbyType,
947950
&indexstate->iss_OrderByTypLens[i],
948951
&indexstate->iss_OrderByTypByVals[i]);
952+
i++;
949953
}
950954

951955
/* allocate arrays to hold the re-calculated distances */

‎src/backend/nodes/copyfuncs.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -381,6 +381,7 @@ _copyIndexScan(const IndexScan *from)
381381
COPY_NODE_FIELD(indexqualorig);
382382
COPY_NODE_FIELD(indexorderby);
383383
COPY_NODE_FIELD(indexorderbyorig);
384+
COPY_NODE_FIELD(indexorderbyops);
384385
COPY_SCALAR_FIELD(indexorderdir);
385386

386387
return newnode;

‎src/backend/nodes/outfuncs.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -457,6 +457,7 @@ _outIndexScan(StringInfo str, const IndexScan *node)
457457
WRITE_NODE_FIELD(indexqualorig);
458458
WRITE_NODE_FIELD(indexorderby);
459459
WRITE_NODE_FIELD(indexorderbyorig);
460+
WRITE_NODE_FIELD(indexorderbyops);
460461
WRITE_ENUM_FIELD(indexorderdir, ScanDirection);
461462
}
462463

‎src/backend/optimizer/plan/createplan.c

Lines changed: 17 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,8 @@ static SeqScan *make_seqscan(List *qptlist, List *qpqual, Index scanrelid);
106106
static SampleScan *make_samplescan(List *qptlist, List *qpqual, Index scanrelid);
107107
static IndexScan *make_indexscan(List *qptlist, List *qpqual, Index scanrelid,
108108
Oid indexid, List *indexqual, List *indexqualorig,
109-
List *indexorderby, List *indexorderbyorig, Oid *indexorderbyops,
109+
List *indexorderby, List *indexorderbyorig,
110+
List *indexorderbyops,
110111
ScanDirection indexscandir);
111112
static IndexOnlyScan *make_indexonlyscan(List *qptlist, List *qpqual,
112113
Index scanrelid, Oid indexid,
@@ -1211,7 +1212,7 @@ create_indexscan_plan(PlannerInfo *root,
12111212
List *stripped_indexquals;
12121213
List *fixed_indexquals;
12131214
List *fixed_indexorderbys;
1214-
Oid *indexorderbyops = NULL;
1215+
List *indexorderbyops = NIL;
12151216
ListCell *l;
12161217

12171218
/* it should be a base rel... */
@@ -1329,37 +1330,31 @@ create_indexscan_plan(PlannerInfo *root,
13291330
*/
13301331
if (best_path->path.pathkeys && indexorderbys)
13311332
{
1332-
int numOrderBys = list_length(indexorderbys);
1333-
int i;
13341333
ListCell *pathkeyCell,
13351334
*exprCell;
1336-
PathKey *pathkey;
1337-
Expr *expr;
1338-
EquivalenceMember *em;
1339-
1340-
indexorderbyops = (Oid *) palloc(numOrderBys * sizeof(Oid));
13411335

13421336
/*
13431337
* PathKey contains pointer to the equivalence class, but that's not
13441338
* enough because we need the expression's datatype to look up the
1345-
* sort operator in the operator family. We have to dig the
1339+
* sort operator in the operator family. We have to dig out the
13461340
* equivalence member for the datatype.
13471341
*/
1348-
i = 0;
1349-
forboth (pathkeyCell, best_path->path.pathkeys, exprCell, indexorderbys)
1342+
forboth(pathkeyCell, best_path->path.pathkeys, exprCell, indexorderbys)
13501343
{
1351-
pathkey = (PathKey *) lfirst(pathkeyCell);
1352-
expr = (Expr *) lfirst(exprCell);
1344+
PathKey *pathkey = (PathKey *) lfirst(pathkeyCell);
1345+
Expr *expr = (Expr *) lfirst(exprCell);
1346+
EquivalenceMember *em;
13531347

13541348
/* Find equivalence member for the order by expression */
13551349
em = find_ec_member_for_expr(pathkey->pk_eclass, expr, NULL);
13561350

13571351
/* Get sort operator from opfamily */
1358-
indexorderbyops[i] = get_opfamily_member(pathkey->pk_opfamily,
1359-
em->em_datatype,
1360-
em->em_datatype,
1361-
pathkey->pk_strategy);
1362-
i++;
1352+
indexorderbyops =
1353+
lappend_oid(indexorderbyops,
1354+
get_opfamily_member(pathkey->pk_opfamily,
1355+
em->em_datatype,
1356+
em->em_datatype,
1357+
pathkey->pk_strategy));
13631358
}
13641359
}
13651360

@@ -3457,7 +3452,7 @@ make_indexscan(List *qptlist,
34573452
List *indexqualorig,
34583453
List *indexorderby,
34593454
List *indexorderbyorig,
3460-
Oid *indexorderbyops,
3455+
List *indexorderbyops,
34613456
ScanDirection indexscandir)
34623457
{
34633458
IndexScan *node = makeNode(IndexScan);
@@ -5008,6 +5003,8 @@ make_modifytable(PlannerInfo *root,
50085003
node->onConflictSet = NIL;
50095004
node->onConflictWhere = NULL;
50105005
node->arbiterIndexes = NIL;
5006+
node->exclRelRTI = 0;
5007+
node->exclRelTlist = NIL;
50115008
}
50125009
else
50135010
{

‎src/include/nodes/plannodes.h

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,6 @@ typedef struct PlannedStmt
7373
int nParamExec; /* number of PARAM_EXEC Params used */
7474

7575
bool hasRowSecurity; /* row security applied? */
76-
7776
} PlannedStmt;
7877

7978
/* macro for fetching the Plan associated with a SubPlan node */
@@ -320,10 +319,10 @@ typedef Scan SampleScan;
320319
* indexorderbyorig is used at runtime to recheck the ordering, if the index
321320
* cannot calculate an accurate ordering. It is also needed for EXPLAIN.
322321
*
323-
* indexorderbyops is an array of operators used to sort the ORDER BY
324-
* expressions, used together with indexorderbyorig to recheck ordering at run
325-
* time. (Note these fields are used for amcanorderbyop cases, not amcanorder
326-
* cases.)
322+
* indexorderbyops is a list of the OIDs of the operators used to sort the
323+
* ORDER BY expressions. This is used together with indexorderbyorig to
324+
* recheck ordering at run time. (Note that indexorderby, indexorderbyorig,
325+
* and indexorderbyops are used for amcanorderbyop cases, not amcanorder.)
327326
*
328327
* indexorderdir specifies the scan ordering, for indexscans on amcanorder
329328
* indexes (for other indexes it should be "don't care").
@@ -337,7 +336,7 @@ typedef struct IndexScan
337336
List *indexqualorig; /* the same in original form */
338337
List *indexorderby; /* list of index ORDER BY exprs */
339338
List *indexorderbyorig; /* the same in original form */
340-
Oid *indexorderbyops; /* operators to sort ORDER BY exprs */
339+
List *indexorderbyops; /* OIDs of sort ops for ORDER BY exprs */
341340
ScanDirection indexorderdir; /* forward or backward or don't care */
342341
} IndexScan;
343342

0 commit comments

Comments
 (0)