Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 121f49a

Browse files
committed
Clean up collation processing in prepunion.c.
This area was a few bricks shy of a load, and badly under-commented too. We have to ensure that the generated targetlist entries for a set-operation node expose the correct collation for each entry, since higher-level processing expects the tlist to reflect the true ordering of the plan's output. This hackery wouldn't be necessary if SortGroupClause carried collation info ... but making it do so would inject more pain in the parser than would be saved here. Still, we might want to rethink that sometime.
1 parent 5809a64 commit 121f49a

File tree

4 files changed

+132
-36
lines changed

4 files changed

+132
-36
lines changed

src/backend/optimizer/prep/prepjointree.c

+1-1
Original file line numberDiff line numberDiff line change
@@ -1136,7 +1136,7 @@ is_simple_union_all_recurse(Node *setOp, Query *setOpQuery, List *colTypes)
11361136
Assert(subquery != NULL);
11371137

11381138
/* Leaf nodes are OK if they match the toplevel column types */
1139-
/* We don't have to compare typmods here */
1139+
/* We don't have to compare typmods or collations here */
11401140
return tlist_same_datatypes(subquery->targetList, colTypes, true);
11411141
}
11421142
else if (IsA(setOp, SetOperationStmt))

src/backend/optimizer/prep/prepunion.c

+96-35
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,8 @@
5454

5555
static Plan *recurse_set_operations(Node *setOp, PlannerInfo *root,
5656
double tuple_fraction,
57-
List *colTypes, bool junkOK,
57+
List *colTypes, List *colCollations,
58+
bool junkOK,
5859
int flag, List *refnames_tlist,
5960
List **sortClauses, double *pNumGroups);
6061
static Plan *generate_recursion_plan(SetOperationStmt *setOp,
@@ -81,12 +82,14 @@ static bool choose_hashed_setop(PlannerInfo *root, List *groupClauses,
8182
double dNumGroups, double dNumOutputRows,
8283
double tuple_fraction,
8384
const char *construct);
84-
static List *generate_setop_tlist(List *colTypes, int flag,
85+
static List *generate_setop_tlist(List *colTypes, List *colCollations,
86+
int flag,
8587
Index varno,
8688
bool hack_constants,
8789
List *input_tlist,
8890
List *refnames_tlist);
89-
static List *generate_append_tlist(List *colTypes, List *colCollations, bool flag,
91+
static List *generate_append_tlist(List *colTypes, List *colCollations,
92+
bool flag,
9093
List *input_plans,
9194
List *refnames_tlist);
9295
static List *generate_setop_grouplist(SetOperationStmt *op, List *targetlist);
@@ -169,7 +172,8 @@ plan_set_operations(PlannerInfo *root, double tuple_fraction,
169172
* on upper-level nodes to deal with that).
170173
*/
171174
return recurse_set_operations((Node *) topop, root, tuple_fraction,
172-
topop->colTypes, true, -1,
175+
topop->colTypes, topop->colCollations,
176+
true, -1,
173177
leftmostQuery->targetList,
174178
sortClauses, NULL);
175179
}
@@ -179,7 +183,8 @@ plan_set_operations(PlannerInfo *root, double tuple_fraction,
179183
* Recursively handle one step in a tree of set operations
180184
*
181185
* tuple_fraction: fraction of tuples we expect to retrieve from node
182-
* colTypes: list of type OIDs of expected output columns
186+
* colTypes: OID list of set-op's result column datatypes
187+
* colCollations: OID list of set-op's result column collations
183188
* junkOK: if true, child resjunk columns may be left in the result
184189
* flag: if >= 0, add a resjunk output column indicating value of flag
185190
* refnames_tlist: targetlist to take column names from
@@ -196,7 +201,8 @@ plan_set_operations(PlannerInfo *root, double tuple_fraction,
196201
static Plan *
197202
recurse_set_operations(Node *setOp, PlannerInfo *root,
198203
double tuple_fraction,
199-
List *colTypes, bool junkOK,
204+
List *colTypes, List *colCollations,
205+
bool junkOK,
200206
int flag, List *refnames_tlist,
201207
List **sortClauses, double *pNumGroups)
202208
{
@@ -239,7 +245,8 @@ recurse_set_operations(Node *setOp, PlannerInfo *root,
239245
* Add a SubqueryScan with the caller-requested targetlist
240246
*/
241247
plan = (Plan *)
242-
make_subqueryscan(generate_setop_tlist(colTypes, flag,
248+
make_subqueryscan(generate_setop_tlist(colTypes, colCollations,
249+
flag,
243250
rtr->rtindex,
244251
true,
245252
subplan->targetlist,
@@ -287,11 +294,13 @@ recurse_set_operations(Node *setOp, PlannerInfo *root,
287294
* generate_setop_tlist() to use varno 0.
288295
*/
289296
if (flag >= 0 ||
290-
!tlist_same_datatypes(plan->targetlist, colTypes, junkOK))
297+
!tlist_same_datatypes(plan->targetlist, colTypes, junkOK) ||
298+
!tlist_same_collations(plan->targetlist, colCollations, junkOK))
291299
{
292300
plan = (Plan *)
293301
make_result(root,
294-
generate_setop_tlist(colTypes, flag,
302+
generate_setop_tlist(colTypes, colCollations,
303+
flag,
295304
0,
296305
false,
297306
plan->targetlist,
@@ -336,12 +345,14 @@ generate_recursion_plan(SetOperationStmt *setOp, PlannerInfo *root,
336345
* separately without any intention of combining them into one Append.
337346
*/
338347
lplan = recurse_set_operations(setOp->larg, root, tuple_fraction,
339-
setOp->colTypes, false, -1,
348+
setOp->colTypes, setOp->colCollations,
349+
false, -1,
340350
refnames_tlist, sortClauses, NULL);
341351
/* The right plan will want to look at the left one ... */
342352
root->non_recursive_plan = lplan;
343353
rplan = recurse_set_operations(setOp->rarg, root, tuple_fraction,
344-
setOp->colTypes, false, -1,
354+
setOp->colTypes, setOp->colCollations,
355+
false, -1,
345356
refnames_tlist, sortClauses, NULL);
346357
root->non_recursive_plan = NULL;
347358

@@ -499,12 +510,14 @@ generate_nonunion_plan(SetOperationStmt *op, PlannerInfo *root,
499510
/* Recurse on children, ensuring their outputs are marked */
500511
lplan = recurse_set_operations(op->larg, root,
501512
0.0 /* all tuples needed */ ,
502-
op->colTypes, false, 0,
513+
op->colTypes, op->colCollations,
514+
false, 0,
503515
refnames_tlist,
504516
&child_sortclauses, &dLeftGroups);
505517
rplan = recurse_set_operations(op->rarg, root,
506518
0.0 /* all tuples needed */ ,
507-
op->colTypes, false, 1,
519+
op->colTypes, op->colCollations,
520+
false, 1,
508521
refnames_tlist,
509522
&child_sortclauses, &dRightGroups);
510523

@@ -620,6 +633,13 @@ generate_nonunion_plan(SetOperationStmt *op, PlannerInfo *root,
620633
*
621634
* NOTE: we can also pull a UNION ALL up into a UNION, since the distinct
622635
* output rows will be lost anyway.
636+
*
637+
* NOTE: currently, we ignore collations while determining if a child has
638+
* the same properties. This is semantically sound only so long as all
639+
* collations have the same notion of equality. It is valid from an
640+
* implementation standpoint because we don't care about the ordering of
641+
* a UNION child's result: UNION ALL results are always unordered, and
642+
* generate_union_plan will force a fresh sort if the top level is a UNION.
623643
*/
624644
static List *
625645
recurse_union_children(Node *setOp, PlannerInfo *root,
@@ -660,8 +680,10 @@ recurse_union_children(Node *setOp, PlannerInfo *root,
660680
*/
661681
return list_make1(recurse_set_operations(setOp, root,
662682
tuple_fraction,
663-
top_union->colTypes, false,
664-
-1, refnames_tlist,
683+
top_union->colTypes,
684+
top_union->colCollations,
685+
false, -1,
686+
refnames_tlist,
665687
&child_sortclauses, NULL));
666688
}
667689

@@ -830,35 +852,41 @@ choose_hashed_setop(PlannerInfo *root, List *groupClauses,
830852
/*
831853
* Generate targetlist for a set-operation plan node
832854
*
833-
* colTypes: column datatypes for non-junk columns
855+
* colTypes: OID list of set-op's result column datatypes
856+
* colCollations: OID list of set-op's result column collations
834857
* flag: -1 if no flag column needed, 0 or 1 to create a const flag column
835858
* varno: varno to use in generated Vars
836859
* hack_constants: true to copy up constants (see comments in code)
837860
* input_tlist: targetlist of this node's input node
838861
* refnames_tlist: targetlist to take column names from
839862
*/
840863
static List *
841-
generate_setop_tlist(List *colTypes, int flag,
864+
generate_setop_tlist(List *colTypes, List *colCollations,
865+
int flag,
842866
Index varno,
843867
bool hack_constants,
844868
List *input_tlist,
845869
List *refnames_tlist)
846870
{
847871
List *tlist = NIL;
848872
int resno = 1;
849-
ListCell *i,
850-
*j,
851-
*k;
873+
ListCell *ctlc,
874+
*cclc,
875+
*itlc,
876+
*rtlc;
852877
TargetEntry *tle;
853878
Node *expr;
854879

855-
j = list_head(input_tlist);
856-
k = list_head(refnames_tlist);
857-
foreach(i, colTypes)
880+
/* there's no forfour() so we must chase one list manually */
881+
rtlc = list_head(refnames_tlist);
882+
forthree(ctlc, colTypes, cclc, colCollations, itlc, input_tlist)
858883
{
859-
Oid colType = lfirst_oid(i);
860-
TargetEntry *inputtle = (TargetEntry *) lfirst(j);
861-
TargetEntry *reftle = (TargetEntry *) lfirst(k);
884+
Oid colType = lfirst_oid(ctlc);
885+
Oid colColl = lfirst_oid(cclc);
886+
TargetEntry *inputtle = (TargetEntry *) lfirst(itlc);
887+
TargetEntry *reftle = (TargetEntry *) lfirst(rtlc);
888+
889+
rtlc = lnext(rtlc);
862890

863891
Assert(inputtle->resno == resno);
864892
Assert(reftle->resno == resno);
@@ -887,21 +915,48 @@ generate_setop_tlist(List *colTypes, int flag,
887915
exprTypmod((Node *) inputtle->expr),
888916
exprCollation((Node *) inputtle->expr),
889917
0);
918+
890919
if (exprType(expr) != colType)
891920
{
921+
/*
922+
* Note: it's not really cool to be applying coerce_to_common_type
923+
* here; one notable point is that assign_expr_collations never
924+
* gets run on any generated nodes. For the moment that's not a
925+
* problem because we force the correct exposed collation below.
926+
* It would likely be best to make the parser generate the correct
927+
* output tlist for every set-op to begin with, though.
928+
*/
892929
expr = coerce_to_common_type(NULL, /* no UNKNOWNs here */
893930
expr,
894931
colType,
895932
"UNION/INTERSECT/EXCEPT");
896933
}
934+
935+
/*
936+
* Ensure the tlist entry's exposed collation matches the set-op.
937+
* This is necessary because plan_set_operations() reports the result
938+
* ordering as a list of SortGroupClauses, which don't carry collation
939+
* themselves but just refer to tlist entries. If we don't show the
940+
* right collation then planner.c might do the wrong thing in
941+
* higher-level queries.
942+
*
943+
* Note we use RelabelType, not CollateExpr, since this expression
944+
* will reach the executor without any further processing.
945+
*/
946+
if (exprCollation(expr) != colColl)
947+
{
948+
expr = (Node *) makeRelabelType((Expr *) expr,
949+
exprType(expr),
950+
exprTypmod(expr),
951+
colColl,
952+
COERCE_DONTCARE);
953+
}
954+
897955
tle = makeTargetEntry((Expr *) expr,
898956
(AttrNumber) resno++,
899957
pstrdup(reftle->resname),
900958
false);
901959
tlist = lappend(tlist, tle);
902-
903-
j = lnext(j);
904-
k = lnext(k);
905960
}
906961

907962
if (flag >= 0)
@@ -928,17 +983,19 @@ generate_setop_tlist(List *colTypes, int flag,
928983
/*
929984
* Generate targetlist for a set-operation Append node
930985
*
931-
* colTypes: column datatypes for non-junk columns
986+
* colTypes: OID list of set-op's result column datatypes
987+
* colCollations: OID list of set-op's result column collations
932988
* flag: true to create a flag column copied up from subplans
933989
* input_plans: list of sub-plans of the Append
934990
* refnames_tlist: targetlist to take column names from
935991
*
936992
* The entries in the Append's targetlist should always be simple Vars;
937-
* we just have to make sure they have the right datatypes and typmods.
993+
* we just have to make sure they have the right datatypes/typmods/collations.
938994
* The Vars are always generated with varno 0.
939995
*/
940996
static List *
941-
generate_append_tlist(List *colTypes, List *colCollations, bool flag,
997+
generate_append_tlist(List *colTypes, List *colCollations,
998+
bool flag,
942999
List *input_plans,
9431000
List *refnames_tlist)
9441001
{
@@ -1000,7 +1057,8 @@ generate_append_tlist(List *colTypes, List *colCollations, bool flag,
10001057
* Now we can build the tlist for the Append.
10011058
*/
10021059
colindex = 0;
1003-
forthree(curColType, colTypes, curColCollation, colCollations, ref_tl_item, refnames_tlist)
1060+
forthree(curColType, colTypes, curColCollation, colCollations,
1061+
ref_tl_item, refnames_tlist)
10041062
{
10051063
Oid colType = lfirst_oid(curColType);
10061064
int32 colTypmod = colTypmods[colindex++];
@@ -1331,7 +1389,7 @@ expand_inherited_rtentry(PlannerInfo *root, RangeTblEntry *rte, Index rti)
13311389
* Build the list of translations from parent Vars to child Vars for
13321390
* an inheritance child.
13331391
*
1334-
* For paranoia's sake, we match type as well as attribute name.
1392+
* For paranoia's sake, we match type/collation as well as attribute name.
13351393
*/
13361394
static void
13371395
make_inh_translation_list(Relation oldrelation, Relation newrelation,
@@ -1410,10 +1468,13 @@ make_inh_translation_list(Relation oldrelation, Relation newrelation,
14101468
attname, RelationGetRelationName(newrelation));
14111469
}
14121470

1413-
/* Found it, check type */
1471+
/* Found it, check type and collation match */
14141472
if (atttypid != att->atttypid || atttypmod != att->atttypmod)
14151473
elog(ERROR, "attribute \"%s\" of relation \"%s\" does not match parent's type",
14161474
attname, RelationGetRelationName(newrelation));
1475+
if (attcollation != att->attcollation)
1476+
elog(ERROR, "attribute \"%s\" of relation \"%s\" does not match parent's collation",
1477+
attname, RelationGetRelationName(newrelation));
14171478

14181479
vars = lappend(vars, makeVar(newvarno,
14191480
(AttrNumber) (new_attno + 1),

src/backend/optimizer/util/tlist.c

+34
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,40 @@ tlist_same_datatypes(List *tlist, List *colTypes, bool junkOK)
195195
return true;
196196
}
197197

198+
/*
199+
* Does tlist have same exposed collations as listed in colCollations?
200+
*
201+
* Identical logic to the above, but for collations.
202+
*/
203+
bool
204+
tlist_same_collations(List *tlist, List *colCollations, bool junkOK)
205+
{
206+
ListCell *l;
207+
ListCell *curColColl = list_head(colCollations);
208+
209+
foreach(l, tlist)
210+
{
211+
TargetEntry *tle = (TargetEntry *) lfirst(l);
212+
213+
if (tle->resjunk)
214+
{
215+
if (!junkOK)
216+
return false;
217+
}
218+
else
219+
{
220+
if (curColColl == NULL)
221+
return false; /* tlist longer than colCollations */
222+
if (exprCollation((Node *) tle->expr) != lfirst_oid(curColColl))
223+
return false;
224+
curColColl = lnext(curColColl);
225+
}
226+
}
227+
if (curColColl != NULL)
228+
return false; /* tlist shorter than colCollations */
229+
return true;
230+
}
231+
198232

199233
/*
200234
* get_sortgroupref_tle

src/include/optimizer/tlist.h

+1
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ extern List *add_to_flat_tlist(List *tlist, List *exprs);
2525

2626
extern List *get_tlist_exprs(List *tlist, bool includeJunk);
2727
extern bool tlist_same_datatypes(List *tlist, List *colTypes, bool junkOK);
28+
extern bool tlist_same_collations(List *tlist, List *colCollations, bool junkOK);
2829

2930
extern TargetEntry *get_sortgroupref_tle(Index sortref,
3031
List *targetList);

0 commit comments

Comments
 (0)