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
diff --git a/doc/src/sgml/ref/create_table.sgml b/doc/src/sgml/ref/create_table.sgml
index 51804d7..3851546 100644
--- a/doc/src/sgml/ref/create_table.sgml
+++ b/doc/src/sgml/ref/create_table.sgml
@@ -82,7 +82,7 @@ CREATE [ [ GLOBAL | LOCAL ] { TEMPORARY | TEMP } | UNLOGGED ] TABLE [ IF NOT EXI
<phrase>and <replaceable class="parameter">like_option</replaceable> is:</phrase>
-{ INCLUDING | EXCLUDING } { DEFAULTS | CONSTRAINTS | IDENTITY | INDEXES | STORAGE | STATISTICS | COMMENTS | ALL }
+{ INCLUDING | EXCLUDING } { COMMENTS | CONSTRAINTS | DEFAULTS | IDENTITY | INDEXES | STATISTICS | STORAGE | ALL }
<phrase>and <replaceable class="parameter">partition_bound_spec</replaceable> is:</phrase>
diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c
index 21fb6db..9b109d7 100644
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -139,6 +139,8 @@ static void transformPartitionCmd(CreateStmtContext *cxt, PartitionCmd *cmd);
static void validateInfiniteBounds(ParseState *pstate, List *blist);
static Const *transformPartitionBoundValue(ParseState *pstate, A_Const *con,
const char *colName, Oid colType, int32 colTypmod);
+static CreateStatsStmt *generateClonedExtStatsStmt(RangeVar *heapRel, Oid heapRelid,
+ Oid source_statsid);
/*
@@ -328,8 +330,6 @@ transformCreateStmt(CreateStmt *stmt, const char *queryString)
*/
transformIndexConstraints(&cxt);
- transformExtendedStatistics(&cxt);
-
/*
* Postprocess foreign-key constraints.
*/
@@ -341,6 +341,11 @@ transformCreateStmt(CreateStmt *stmt, const char *queryString)
transformCheckConstraints(&cxt, !is_foreign_table ? true : false);
/*
+ * Postprocess extended statistics.
+ */
+ transformExtendedStatistics(&cxt);
+
+ /*
* Output results.
*/
stmt->tableElts = cxt.columns;
@@ -1195,6 +1200,9 @@ transformTableLikeClause(CreateStmtContext *cxt, TableLikeClause *table_like_cla
}
}
+ /*
+ * Likewise, copy extended statistics if requested
+ */
if (table_like_clause->options & CREATE_TABLE_LIKE_STATISTICS)
{
List *parent_extstats;
@@ -1608,7 +1616,7 @@ generateClonedIndexStmt(RangeVar *heapRel, Oid heapRelid, Relation source_idx,
* extended statistic "source_statsid", for the rel identified by heapRel and
* heapRelid.
*/
-CreateStatsStmt *
+static CreateStatsStmt *
generateClonedExtStatsStmt(RangeVar *heapRel, Oid heapRelid,
Oid source_statsid)
{
@@ -2246,13 +2254,16 @@ transformIndexConstraint(Constraint *constraint, CreateStmtContext *cxt)
return index;
}
+/*
+ * transformExtendedStatistics
+ * handle extended statistics
+ *
+ * Right now, there's nothing to do here, so we just copy the list.
+ */
static void
transformExtendedStatistics(CreateStmtContext *cxt)
{
- ListCell *lc;
-
- foreach(lc, cxt->extstats)
- cxt->alist = lappend(cxt->alist, lfirst(lc));
+ cxt->alist = list_copy(cxt->extstats);
}
/*
@@ -3094,6 +3105,7 @@ transformAlterTableStmt(Oid relid, AlterTableStmt *stmt,
newcmds = lappend(newcmds, newcmd);
}
+ /* Postprocess extended statistics */
transformExtendedStatistics(&cxt);
/* Close rel */
diff --git a/src/include/parser/parse_utilcmd.h b/src/include/parser/parse_utilcmd.h
index cdc98d8..35ac979 100644
--- a/src/include/parser/parse_utilcmd.h
+++ b/src/include/parser/parse_utilcmd.h
@@ -31,8 +31,5 @@ extern IndexStmt *generateClonedIndexStmt(RangeVar *heapRel, Oid heapOid,
Relation source_idx,
const AttrNumber *attmap, int attmap_length,
Oid *constraintOid);
-extern CreateStatsStmt *generateClonedExtStatsStmt(RangeVar *heapRel,
- Oid heapRelid,
- Oid source_statsid);
#endif /* PARSE_UTILCMD_H */
