Skip to content

Commit 7208fae

Browse files
tglsfdctglsfdc
committed
Clean up cruft around collation initialization for tupdescs and scankeys.
I found actual bugs in GiST and plpgsql; the rest of this is cosmetic but meant to decrease the odds of future bugs of omission.
1 parent 0c9d9e8 commit 7208fae

File tree

16 files changed

+119
-81
lines changed

16 files changed

+119
-81
lines changed

‎src/backend/access/common/scankey.c

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
#include "postgres.h"
1616

1717
#include "access/skey.h"
18+
#include "catalog/pg_collation.h"
1819

1920

2021
/*
@@ -33,6 +34,7 @@ ScanKeyEntryInitialize(ScanKey entry,
3334
AttrNumber attributeNumber,
3435
StrategyNumber strategy,
3536
Oid subtype,
37+
Oid collation,
3638
RegProcedure procedure,
3739
Datum argument)
3840
{
@@ -42,7 +44,10 @@ ScanKeyEntryInitialize(ScanKey entry,
4244
entry->sk_subtype = subtype;
4345
entry->sk_argument = argument;
4446
if (RegProcedureIsValid(procedure))
47+
{
4548
fmgr_info(procedure, &entry->sk_func);
49+
entry->sk_func.fn_collation = collation;
50+
}
4651
else
4752
{
4853
Assert(flags & (SK_SEARCHNULL | SK_SEARCHNOTNULL));
@@ -53,12 +58,16 @@ ScanKeyEntryInitialize(ScanKey entry,
5358
/*
5459
* ScanKeyInit
5560
* Shorthand version of ScanKeyEntryInitialize: flags and subtype
56-
* are assumed to be zero (the usual value).
61+
* are assumed to be zero (the usual value), and collation is defaulted.
5762
*
5863
* This is the recommended version for hardwired lookups in system catalogs.
5964
* It cannot handle NULL arguments, unary operators, or nondefault operators,
6065
* but we need none of those features for most hardwired lookups.
6166
*
67+
* We set collation to DEFAULT_COLLATION_OID always. This is appropriate
68+
* for textual columns in system catalogs, and it will be ignored for
69+
* non-textual columns, so it's not worth trying to be more finicky.
70+
*
6271
* Note: CurrentMemoryContext at call should be as long-lived as the ScanKey
6372
* itself, because that's what will be used for any subsidiary info attached
6473
* to the ScanKey's FmgrInfo record.
@@ -76,6 +85,7 @@ ScanKeyInit(ScanKey entry,
7685
entry->sk_subtype = InvalidOid;
7786
entry->sk_argument = argument;
7887
fmgr_info(procedure, &entry->sk_func);
88+
entry->sk_func.fn_collation = DEFAULT_COLLATION_OID;
7989
}
8090

8191
/*
@@ -93,6 +103,7 @@ ScanKeyEntryInitializeWithInfo(ScanKey entry,
93103
AttrNumber attributeNumber,
94104
StrategyNumber strategy,
95105
Oid subtype,
106+
Oid collation,
96107
FmgrInfo *finfo,
97108
Datum argument)
98109
{
@@ -102,17 +113,5 @@ ScanKeyEntryInitializeWithInfo(ScanKey entry,
102113
entry->sk_subtype = subtype;
103114
entry->sk_argument = argument;
104115
fmgr_info_copy(&entry->sk_func, finfo, CurrentMemoryContext);
105-
}
106-
107-
/*
108-
* ScanKeyEntryInitializeCollation
109-
*
110-
* Initialize the collation of a scan key. This is just a notational
111-
* convenience and small abstraction.
112-
*/
113-
void
114-
ScanKeyEntryInitializeCollation(ScanKey entry,
115-
Oid collation)
116-
{
117116
entry->sk_func.fn_collation = collation;
118117
}

‎src/backend/access/common/tupdesc.c

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -427,6 +427,10 @@ equalTupleDescs(TupleDesc tupdesc1, TupleDesc tupdesc2)
427427
* TupleDescInitEntry
428428
* This function initializes a single attribute structure in
429429
* a previously allocated tuple descriptor.
430+
*
431+
* Note that attcollation is set to the default for the specified datatype.
432+
* If a nondefault collation is needed, insert it afterwards using
433+
* TupleDescInitEntryCollation.
430434
*/
431435
void
432436
TupleDescInitEntry(TupleDesc desc,
@@ -496,8 +500,8 @@ TupleDescInitEntry(TupleDesc desc,
496500
/*
497501
* TupleDescInitEntryCollation
498502
*
499-
* Fill in the collation for an attribute in a previously initialized
500-
* tuple descriptor.
503+
* Assign a nondefault collation to a previously initialized tuple descriptor
504+
* entry.
501505
*/
502506
void
503507
TupleDescInitEntryCollation(TupleDesc desc,
@@ -571,9 +575,9 @@ BuildDescForRelation(List *schema)
571575

572576
TupleDescInitEntry(desc, attnum, attname,
573577
atttypid, atttypmod, attdim);
574-
TupleDescInitEntryCollation(desc, attnum, attcollation);
575578

576579
/* Override TupleDescInitEntry's settings as requested */
580+
TupleDescInitEntryCollation(desc, attnum, attcollation);
577581
if (entry->storage)
578582
desc->attrs[attnum - 1]->attstorage = entry->storage;
579583

‎src/backend/access/gin/ginutil.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,8 @@ initGinState(GinState *state, Relation index)
5656
origTupdesc->attrs[i]->atttypid,
5757
origTupdesc->attrs[i]->atttypmod,
5858
origTupdesc->attrs[i]->attndims);
59+
TupleDescInitEntryCollation(state->tupdesc[i], (AttrNumber) 2,
60+
origTupdesc->attrs[i]->attcollation);
5961
}
6062

6163
fmgr_info_copy(&(state->compareFn[i]),

‎src/backend/access/gist/gistscan.c

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,8 @@ gistrescan(PG_FUNCTION_ARGS)
168168
* all comparisons. The original operator is passed to the Consistent
169169
* function in the form of its strategy number, which is available
170170
* from the sk_strategy field, and its subtype from the sk_subtype
171-
* field.
171+
* field. Also, preserve sk_func.fn_collation which is the input
172+
* collation for the operator.
172173
*
173174
* Next, if any of keys is a NULL and that key is not marked with
174175
* SK_SEARCHNULL/SK_SEARCHNOTNULL then nothing can be found (ie, we
@@ -179,8 +180,10 @@ gistrescan(PG_FUNCTION_ARGS)
179180
for (i = 0; i < scan->numberOfKeys; i++)
180181
{
181182
ScanKey skey = scan->keyData + i;
183+
Oid collation = skey->sk_func.fn_collation;
182184

183185
skey->sk_func = so->giststate->consistentFn[skey->sk_attno - 1];
186+
skey->sk_func.fn_collation = collation;
184187

185188
if (skey->sk_flags & SK_ISNULL)
186189
{
@@ -201,13 +204,16 @@ gistrescan(PG_FUNCTION_ARGS)
201204
* all comparisons. The original operator is passed to the Distance
202205
* function in the form of its strategy number, which is available
203206
* from the sk_strategy field, and its subtype from the sk_subtype
204-
* field.
207+
* field. Also, preserve sk_func.fn_collation which is the input
208+
* collation for the operator.
205209
*/
206210
for (i = 0; i < scan->numberOfOrderBys; i++)
207211
{
208212
ScanKey skey = scan->orderByData + i;
213+
Oid collation = skey->sk_func.fn_collation;
209214

210215
skey->sk_func = so->giststate->distanceFn[skey->sk_attno - 1];
216+
skey->sk_func.fn_collation = collation;
211217

212218
/* Check we actually have a distance function ... */
213219
if (!OidIsValid(skey->sk_func.fn_oid))

‎src/backend/access/nbtree/nbtsearch.c

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -721,10 +721,9 @@ _bt_first(IndexScanDesc scan, ScanDirection dir)
721721
cur->sk_attno,
722722
InvalidStrategy,
723723
cur->sk_subtype,
724+
cur->sk_func.fn_collation,
724725
procinfo,
725726
cur->sk_argument);
726-
ScanKeyEntryInitializeCollation(scankeys + i,
727-
cur->sk_func.fn_collation);
728727
}
729728
else
730729
{
@@ -743,10 +742,9 @@ _bt_first(IndexScanDesc scan, ScanDirection dir)
743742
cur->sk_attno,
744743
InvalidStrategy,
745744
cur->sk_subtype,
745+
cur->sk_func.fn_collation,
746746
cmp_proc,
747747
cur->sk_argument);
748-
ScanKeyEntryInitializeCollation(scankeys + i,
749-
cur->sk_func.fn_collation);
750748
}
751749
}
752750
}

‎src/backend/access/nbtree/nbtutils.c

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,8 @@ _bt_mkscankey(Relation rel, IndexTuple itup)
7070

7171
/*
7272
* We can use the cached (default) support procs since no cross-type
73-
* comparison can be needed.
73+
* comparison can be needed. The cached support proc entries have
74+
* the right collation for the index, too.
7475
*/
7576
procinfo = index_getprocinfo(rel, i + 1, BTORDER_PROC);
7677
arg = index_getattr(itup, i + 1, itupdesc, &null);
@@ -80,6 +81,7 @@ _bt_mkscankey(Relation rel, IndexTuple itup)
8081
(AttrNumber) (i + 1),
8182
InvalidStrategy,
8283
InvalidOid,
84+
procinfo->fn_collation,
8385
procinfo,
8486
arg);
8587
}
@@ -118,7 +120,8 @@ _bt_mkscankey_nodata(Relation rel)
118120

119121
/*
120122
* We can use the cached (default) support procs since no cross-type
121-
* comparison can be needed.
123+
* comparison can be needed. The cached support proc entries have
124+
* the right collation for the index, too.
122125
*/
123126
procinfo = index_getprocinfo(rel, i + 1, BTORDER_PROC);
124127
flags = SK_ISNULL | (indoption[i] << SK_BT_INDOPTION_SHIFT);
@@ -127,6 +130,7 @@ _bt_mkscankey_nodata(Relation rel)
127130
(AttrNumber) (i + 1),
128131
InvalidStrategy,
129132
InvalidOid,
133+
procinfo->fn_collation,
130134
procinfo,
131135
(Datum) 0);
132136
}

‎src/backend/commands/seclabel.c

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
#include "catalog/catalog.h"
1616
#include "catalog/indexing.h"
1717
#include "catalog/namespace.h"
18-
#include "catalog/pg_collation.h"
1918
#include "catalog/pg_seclabel.h"
2019
#include "commands/seclabel.h"
2120
#include "miscadmin.h"
@@ -166,7 +165,6 @@ GetSecurityLabel(const ObjectAddress *object, const char *provider)
166165
Anum_pg_seclabel_provider,
167166
BTEqualStrategyNumber, F_TEXTEQ,
168167
CStringGetTextDatum(provider));
169-
ScanKeyEntryInitializeCollation(&keys[3], DEFAULT_COLLATION_OID);
170168

171169
pg_seclabel = heap_open(SecLabelRelationId, AccessShareLock);
172170

@@ -236,7 +234,6 @@ SetSecurityLabel(const ObjectAddress *object,
236234
Anum_pg_seclabel_provider,
237235
BTEqualStrategyNumber, F_TEXTEQ,
238236
CStringGetTextDatum(provider));
239-
ScanKeyEntryInitializeCollation(&keys[3], DEFAULT_COLLATION_OID);
240237

241238
pg_seclabel = heap_open(SecLabelRelationId, RowExclusiveLock);
242239

‎src/backend/commands/sequence.c

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1455,11 +1455,16 @@ pg_sequence_parameters(PG_FUNCTION_ARGS)
14551455
RelationGetRelationName(seqrel))));
14561456

14571457
tupdesc = CreateTemplateTupleDesc(5, false);
1458-
TupleDescInitEntry(tupdesc, (AttrNumber) 1, "start_value", INT8OID, -1, 0);
1459-
TupleDescInitEntry(tupdesc, (AttrNumber) 2, "minimum_value", INT8OID, -1, 0);
1460-
TupleDescInitEntry(tupdesc, (AttrNumber) 3, "maximum_value", INT8OID, -1, 0);
1461-
TupleDescInitEntry(tupdesc, (AttrNumber) 4, "increment", INT8OID, -1, 0);
1462-
TupleDescInitEntry(tupdesc, (AttrNumber) 5, "cycle_option", BOOLOID, -1, 0);
1458+
TupleDescInitEntry(tupdesc, (AttrNumber) 1, "start_value",
1459+
INT8OID, -1, 0);
1460+
TupleDescInitEntry(tupdesc, (AttrNumber) 2, "minimum_value",
1461+
INT8OID, -1, 0);
1462+
TupleDescInitEntry(tupdesc, (AttrNumber) 3, "maximum_value",
1463+
INT8OID, -1, 0);
1464+
TupleDescInitEntry(tupdesc, (AttrNumber) 4, "increment",
1465+
INT8OID, -1, 0);
1466+
TupleDescInitEntry(tupdesc, (AttrNumber) 5, "cycle_option",
1467+
BOOLOID, -1, 0);
14631468

14641469
BlessTupleDesc(tupdesc);
14651470

‎src/backend/executor/nodeIndexscan.c

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -829,10 +829,9 @@ ExecIndexBuildScanKeys(PlanState *planstate, Relation index, Index scanrelid,
829829
varattno, /* attribute number to scan */
830830
op_strategy, /* op's strategy */
831831
op_righttype, /* strategy subtype */
832+
((OpExpr *) clause)->inputcollid, /* collation */
832833
opfuncid, /* reg proc to use */
833834
scanvalue); /* constant */
834-
ScanKeyEntryInitializeCollation(this_scan_key,
835-
((OpExpr *) clause)->inputcollid);
836835
}
837836
else if (IsA(clause, RowCompareExpr))
838837
{
@@ -957,10 +956,9 @@ ExecIndexBuildScanKeys(PlanState *planstate, Relation index, Index scanrelid,
957956
varattno, /* attribute number */
958957
op_strategy, /* op's strategy */
959958
op_righttype, /* strategy subtype */
959+
inputcollation, /* collation */
960960
opfuncid, /* reg proc to use */
961961
scanvalue); /* constant */
962-
ScanKeyEntryInitializeCollation(this_sub_key,
963-
inputcollation);
964962
n_sub_key++;
965963
}
966964

@@ -1042,10 +1040,9 @@ ExecIndexBuildScanKeys(PlanState *planstate, Relation index, Index scanrelid,
10421040
varattno, /* attribute number to scan */
10431041
op_strategy, /* op's strategy */
10441042
op_righttype, /* strategy subtype */
1043+
saop->inputcollid, /* collation */
10451044
opfuncid, /* reg proc to use */
10461045
(Datum) 0); /* constant */
1047-
ScanKeyEntryInitializeCollation(this_scan_key,
1048-
saop->inputcollid);
10491046
}
10501047
else if (IsA(clause, NullTest))
10511048
{
@@ -1094,6 +1091,7 @@ ExecIndexBuildScanKeys(PlanState *planstate, Relation index, Index scanrelid,
10941091
varattno, /* attribute number to scan */
10951092
InvalidStrategy, /* no strategy */
10961093
InvalidOid, /* no strategy subtype */
1094+
InvalidOid, /* no collation */
10971095
InvalidOid, /* no reg proc for this */
10981096
(Datum) 0); /* constant */
10991097
}

‎src/backend/executor/nodeMergeAppend.c

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -134,30 +134,32 @@ ExecInitMergeAppend(MergeAppend *node, EState *estate, int eflags)
134134
{
135135
Oid sortFunction;
136136
bool reverse;
137+
int flags;
137138

138139
if (!get_compare_function_for_ordering_op(node->sortOperators[i],
139140
&sortFunction, &reverse))
140141
elog(ERROR, "operator %u is not a valid ordering operator",
141142
node->sortOperators[i]);
142143

144+
/* We use btree's conventions for encoding directionality */
145+
flags = 0;
146+
if (reverse)
147+
flags |= SK_BT_DESC;
148+
if (node->nullsFirst[i])
149+
flags |= SK_BT_NULLS_FIRST;
150+
143151
/*
144152
* We needn't fill in sk_strategy or sk_subtype since these scankeys
145153
* will never be passed to an index.
146154
*/
147-
ScanKeyInit(&mergestate->ms_scankeys[i],
148-
node->sortColIdx[i],
149-
InvalidStrategy,
150-
sortFunction,
151-
(Datum) 0);
152-
153-
ScanKeyEntryInitializeCollation(&mergestate->ms_scankeys[i],
154-
node->collations[i]);
155-
156-
/* However, we use btree's conventions for encoding directionality */
157-
if (reverse)
158-
mergestate->ms_scankeys[i].sk_flags |= SK_BT_DESC;
159-
if (node->nullsFirst[i])
160-
mergestate->ms_scankeys[i].sk_flags |= SK_BT_NULLS_FIRST;
155+
ScanKeyEntryInitialize(&mergestate->ms_scankeys[i],
156+
flags,
157+
node->sortColIdx[i],
158+
InvalidStrategy,
159+
InvalidOid,
160+
node->collations[i],
161+
sortFunction,
162+
(Datum) 0);
161163
}
162164

163165
/*

‎src/backend/utils/adt/pgstatfuncs.c

Lines changed: 24 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -507,18 +507,30 @@ pg_stat_get_activity(PG_FUNCTION_ARGS)
507507
oldcontext = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx);
508508

509509
tupdesc = CreateTemplateTupleDesc(12, false);
510-
TupleDescInitEntry(tupdesc, (AttrNumber) 1, "datid", OIDOID, -1, 0);
511-
TupleDescInitEntry(tupdesc, (AttrNumber) 2, "procpid", INT4OID, -1, 0);
512-
TupleDescInitEntry(tupdesc, (AttrNumber) 3, "usesysid", OIDOID, -1, 0);
513-
TupleDescInitEntry(tupdesc, (AttrNumber) 4, "application_name", TEXTOID, -1, 0);
514-
TupleDescInitEntry(tupdesc, (AttrNumber) 5, "current_query", TEXTOID, -1, 0);
515-
TupleDescInitEntry(tupdesc, (AttrNumber) 6, "waiting", BOOLOID, -1, 0);
516-
TupleDescInitEntry(tupdesc, (AttrNumber) 7, "act_start", TIMESTAMPTZOID, -1, 0);
517-
TupleDescInitEntry(tupdesc, (AttrNumber) 8, "query_start", TIMESTAMPTZOID, -1, 0);
518-
TupleDescInitEntry(tupdesc, (AttrNumber) 9, "backend_start", TIMESTAMPTZOID, -1, 0);
519-
TupleDescInitEntry(tupdesc, (AttrNumber) 10, "client_addr", INETOID, -1, 0);
520-
TupleDescInitEntry(tupdesc, (AttrNumber) 11, "client_hostname", TEXTOID, -1, 0);
521-
TupleDescInitEntry(tupdesc, (AttrNumber) 12, "client_port", INT4OID, -1, 0);
510+
TupleDescInitEntry(tupdesc, (AttrNumber) 1, "datid",
511+
OIDOID, -1, 0);
512+
TupleDescInitEntry(tupdesc, (AttrNumber) 2, "procpid",
513+
INT4OID, -1, 0);
514+
TupleDescInitEntry(tupdesc, (AttrNumber) 3, "usesysid",
515+
OIDOID, -1, 0);
516+
TupleDescInitEntry(tupdesc, (AttrNumber) 4, "application_name",
517+
TEXTOID, -1, 0);
518+
TupleDescInitEntry(tupdesc, (AttrNumber) 5, "current_query",
519+
TEXTOID, -1, 0);
520+
TupleDescInitEntry(tupdesc, (AttrNumber) 6, "waiting",
521+
BOOLOID, -1, 0);
522+
TupleDescInitEntry(tupdesc, (AttrNumber) 7, "act_start",
523+
TIMESTAMPTZOID, -1, 0);
524+
TupleDescInitEntry(tupdesc, (AttrNumber) 8, "query_start",
525+
TIMESTAMPTZOID, -1, 0);
526+
TupleDescInitEntry(tupdesc, (AttrNumber) 9, "backend_start",
527+
TIMESTAMPTZOID, -1, 0);
528+
TupleDescInitEntry(tupdesc, (AttrNumber) 10, "client_addr",
529+
INETOID, -1, 0);
530+
TupleDescInitEntry(tupdesc, (AttrNumber) 11, "client_hostname",
531+
TEXTOID, -1, 0);
532+
TupleDescInitEntry(tupdesc, (AttrNumber) 12, "client_port",
533+
INT4OID, -1, 0);
522534

523535
funcctx->tuple_desc = BlessTupleDesc(tupdesc);
524536

0 commit comments

Comments
 (0)