Hi,
On 03/03/2018 11:20 AM, David Rowley wrote:
> I've attached a rebased patch which fixes up the conflicts caused by 8b08f7d.
>
The patch seems fine to me. A couple of minor points
1) I wonder if we should make the list in create_table.sgml to also be
alphabetically sorted.
2) I think the code in parse_utilcmd.c was missing a bunch of comments.
Nothing particularly serious, but the surrounding code has them ...
3) The transformExtendedStatistics call in transformCreateStmt was a bit
too high, interleaving with the transforms of various constraints. I
think we should move it a couple of lines down, to keep those calls
together (transformAlterTableStmt also does the call after handling all
the constraint-related stuff).
4) I see you've added generateClonedExtStatsStmt to parse_utilcmd.h, but
it was only really called in parse_utilcmd.c, so I've made it static. I
don't think we need to expose stuff unnecessarily.
The attached patch does those changes - feel free to pick only those you
like / agree with.
BTW the last point made me thinking, because parse_utilcmd.h also
exposes generateClonedIndexStmt. That is however necessary, because it's
called from DefineRelation when copying indexes from partitioned table
to partitions. I'm wondering - shouldn't we do the same thing for
extended statistics?
regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
