[Postgres-xc-developers] Review comments for commit: 31cf556fdb44d0872998f37ac222e0ac6f1faeb6

[Ashutosh B. <ash...@en...>]

Hi Amit
Not your change but, we need to update the prologue of rewriteTargetListUD
with XC specific changes.

Again not your change, but there's comment in the function
1256     /*
1257      * In Postgres-XC, we need to evaluate quals of the parse tree and
determine
1258      * if they are Coordinator quals. If they are, their attribute
need to be
1259      * added to target list for evaluation. In case some are found,
add them as
1260      * junks in the target list. The junk status will be used by
remote UPDATE
1261      * planning to associate correct element to a clause.
1262      * For DELETE, having such columns in target list helps to
evaluate Quals
1263      * correctly on Coordinator.
1264      * PGXCTODO: This list could be reduced to keep only in target
list the
1265      * vars using Coordinator Quals.
1266      */
1267     if (IS_PGXC_COORDINATOR && parsetree->jointree)
1268         var_list = pull_qual_vars((Node *) parsetree->jointree,
parsetree->resultRelation);

I think we need to do this only when these attributes are not in the
targetlist already. My guess is we don't need this code at all. Can we
check this?

Please use RelationGetLocInfo to get location info given Relation in
1336         if
(!IsLocatorReplicated(GetLocatorType(RelationGetRelid(target_relation))))

Rest of the changes look fine.
-- 
Best Wishes,
Ashutosh Bapat
EntepriseDB Corporation
The Enterprise Postgres Company