Re: A bug in mapping attributes in ATExecAttachPartition()

On 2017/08/02 20:35, Robert Haas wrote:
> On Tue, Aug 1, 2017 at 9:44 PM, Amit Langote
> <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp> wrote:
>> I too dislike the shape of attachRel. How about we rename attachRel to
>> attachrel? So, attachrel_children, attachrel_constr, etc. It's still
>> long though... :)
>
> OK, I can live with that, I guess.

Alright, attached updated 0001 does that.

About the other hunk, it seems we will have to go with the following
structure after all;

if (a)
if (b)
c();
d();

Note that we were missing that there is a d(), which executes if a is
true. c() executes *only* if b is true. So I left that hunk unchanged,
viz. the following:

/*
- * Skip if it's a partitioned table. Only RELKIND_RELATION
- * relations (ie, leaf partitions) need to be scanned.
+ * Skip if the partition is itself a partitioned table. We can
+ * only ever scan RELKIND_RELATION relations.
*/
- if (part_rel != attachRel &&
- part_rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
+ if (part_rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
{
- heap_close(part_rel, NoLock);
+ if (part_rel != attachrel)
+ heap_close(part_rel, NoLock);
continue;
}

You might ask why the earlier code worked if there was this kind of
logical bug - accident; even if we failed skipping attachRel, the AT
rewrite phase which is in charge of actually scanning the table knows to
skip the partitioned tables, so no harm would be done.

Thanks,
Amit