On Thu, Oct 6, 2016 at 12:44 PM, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:
> On 2016/10/05 2:12, Robert Haas wrote:
> Attached revised patches.
Few assorted review comments for 0001-Catalog*:
1.
@@ -1775,6 +1775,12 @@ BeginCopyTo(ParseState *pstate,
{
..
+ else if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("cannot copy from partitioned table \"%s\"",
+ RelationGetRelationName(rel)),
+ errhint("Try the COPY (SELECT ...) TO variant.")));
..
}
Why is this restriction? Won't it be useful to allow it for the cases
when user wants to copy the data of all the partitions?
2.
+ if (!pg_strcasecmp(stmt->partspec->strategy, "list") &&
+ partnatts > 1)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+ errmsg("cannot list partition using more than one column")));
/cannot list/cannot use list
3.
@@ -77,7 +77,7 @@ typedef enum DependencyType DEPENDENCY_INTERNAL = 'i', DEPENDENCY_EXTENSION = 'e',
DEPENDENCY_AUTO_EXTENSION= 'x',
- DEPENDENCY_PIN = 'p'
+ DEPENDENCY_PIN = 'p',} DependencyType;
Why is this change required?
4.
@@ -0,0 +1,69 @@
+/*-------------------------------------------------------------------------
+ *
+ * pg_partitioned_table.h
+ * definition of the system "partitioned table" relation
+ * along with the relation's initial contents.
+ *
+ *
+ * Portions Copyright (c) 1996-2015, PostgreSQL Global Development Group
Copyright year should be 2016.
5.
+/*
+ * PartitionSpec - partition key definition including the strategy
+ *
+ * 'strategy' partition strategy name ('list', 'range', etc.)
etc. in above comment seems to be unnecessary.
6.
+ {PartitionedRelationId, /* PARTEDRELID */
Here PARTEDRELID sounds inconvenient, how about PARTRELID?
--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
