On Mon, Nov 13, 2017 at 4:32 PM, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:
> In early versions of the partitioning patch, there used to be a
> transformPartitionOf() which would do the same job as transformOfType(),
> that is, make new ColumnDef's from the parent table's attributes. But, I
> removed it in favor of making MergeAttributes() create those ColumnDef's,
> because the latter needed to lock and open the parent anyway. However, to
> handle identity columns, it seems we might need to go back to having a
> transformPartitionOf() after all, but it's going to be quite some work.
Another idea that could be done here is to rework as well
transformOfType() so as its column definition logic is pulled down to
MergeAttributes(). There is some logic telling that the typed columns
are first in the list. That's not lovely IMO.
>> for two, for typed tables, this patch adds another code path to
>> complain about ERRCODE_UNDEFINED_COLUMN, which is also a duplicate of
>> something that should fail in tablecmds.c in my opinion.
>
> The duplication is unfortunate, but the code in tablecmds.c you seem to be
> talking about (in MergeAttributes()) won't be able to satisfy the
> requirement that the sequence for the identity column of a typed table be
> created before the table itself is created. By that point, we're already
> in DefineRelation() creating the table. In fact, I think to fix the
> duplication, it might be better to work on removing the relevant code in
> MergeAttributes().
Yeah, possibly. This really requires more thinking as we likely don't
want to make the code more a mess than it is now. There is no urgency
in getting things refactored now anyway. And perhaps there will be no
need to do so.
--
Michael
