Re: [HACKERS] Misleading CREATE TABLE error
On Fri, Feb 24, 2012 at 4:03 AM, Peter Eisentraut pete...@gmx.net wrote: On tis, 2011-11-29 at 06:33 +0200, Peter Eisentraut wrote: I'm not trying to inherit a relation, I'm trying to base a table on it. As it happens, cows is a foreign table, which *is* a table, just not a regular table. It might be useful to add support to clone foreign tables into regular tables, the use-case being that you may wish to import all the data locally into a table of the same structure. But the gripe here is the suggestion that the relation would have been inherited, which would actually be achieved using INHERITS. Interesting. I agree that there's no obvious reason why that shouldn't be allowed to work. Could be useful with views, too. I recently came across a situation where LIKE with a composite type might have been useful. This was the last piece of the puzzle that was missing in this area, for which I have now developed a fix. The problem was that parserOpenTable() rejected composite types. But the only thing that was really adding over using relation_open() directly was nicer error pointers. So I removed a few levels of indirection there, and integrated the error pointer support directly into transformTableLikeClause(). This also has the advantage that the ... is not a table, view, or ... message now has error pointer support. Looks reasonable. The only thing you didn't copy from parserOpenTable() is the special error-handling for CTEs, but AFAICS that's irrelevant here. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Misleading CREATE TABLE error
On tis, 2011-11-29 at 06:33 +0200, Peter Eisentraut wrote:
I'm not trying to inherit a relation, I'm trying to base a table on
it. As it happens, cows is a foreign table, which *is* a table,
just not a regular table. It might be useful to add support to clone
foreign tables into regular tables, the use-case being that you may
wish to import all the data locally into a table of the same
structure. But the gripe here is the suggestion that the relation
would have been inherited, which would actually be achieved using
INHERITS.
Interesting. I agree that there's no obvious reason why that
shouldn't be allowed to work. Could be useful with views, too.
I recently came across a situation where LIKE with a composite type
might have been useful.
This was the last piece of the puzzle that was missing in this area, for
which I have now developed a fix. The problem was that
parserOpenTable() rejected composite types. But the only thing that was
really adding over using relation_open() directly was nicer error
pointers. So I removed a few levels of indirection there, and
integrated the error pointer support directly into
transformTableLikeClause(). This also has the advantage that the ...
is not a table, view, or ... message now has error pointer support.
diff --git i/doc/src/sgml/ref/create_table.sgml w/doc/src/sgml/ref/create_table.sgml
index f55a001..bb93214 100644
--- i/doc/src/sgml/ref/create_table.sgml
+++ w/doc/src/sgml/ref/create_table.sgml
@@ -370,7 +370,7 @@ CREATE [ [ GLOBAL | LOCAL ] { TEMPORARY | TEMP } | UNLOGGED ] TABLE [ IF NOT EXI
/para
para
The literalLIKE/literal clause can also be used to copy columns from
- views or foreign tables. Inapplicable options (e.g., literalINCLUDING
+ views, foreign tables, or composite types. Inapplicable options (e.g., literalINCLUDING
INDEXES/literal from a view) are ignored.
/para
/listitem
diff --git i/src/backend/parser/parse_utilcmd.c w/src/backend/parser/parse_utilcmd.c
index f1a108a..43f5634 100644
--- i/src/backend/parser/parse_utilcmd.c
+++ w/src/backend/parser/parse_utilcmd.c
@@ -636,26 +636,42 @@ transformTableLikeClause(CreateStmtContext *cxt, TableLikeClause *table_like_cla
TupleConstr *constr;
AclResult aclresult;
char *comment;
+ ParseCallbackState pcbstate;
- relation = parserOpenTable(cxt-pstate, table_like_clause-relation,
- AccessShareLock);
+ setup_parser_errposition_callback(pcbstate, cxt-pstate, table_like_clause-relation-location);
+
+ relation = relation_openrv(table_like_clause-relation, AccessShareLock);
if (relation-rd_rel-relkind != RELKIND_RELATION
relation-rd_rel-relkind != RELKIND_VIEW
- relation-rd_rel-relkind != RELKIND_FOREIGN_TABLE)
+ relation-rd_rel-relkind != RELKIND_FOREIGN_TABLE
+ relation-rd_rel-relkind != RELKIND_COMPOSITE_TYPE)
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
- errmsg(LIKE source relation \%s\ is not a table, view, or foreign table,
+ errmsg(\%s\ is not a table, view, composite type, or foreign table,
table_like_clause-relation-relname)));
+ cancel_parser_errposition_callback(pcbstate);
+
/*
- * Check for SELECT privileges
+ * Check for privileges
*/
- aclresult = pg_class_aclcheck(RelationGetRelid(relation), GetUserId(),
+ if (relation-rd_rel-relkind == RELKIND_COMPOSITE_TYPE)
+ {
+ aclresult = pg_type_aclcheck(relation-rd_rel-reltype, GetUserId(),
+ ACL_USAGE);
+ if (aclresult != ACLCHECK_OK)
+ aclcheck_error(aclresult, ACL_KIND_TYPE,
+ RelationGetRelationName(relation));
+ }
+ else
+ {
+ aclresult = pg_class_aclcheck(RelationGetRelid(relation), GetUserId(),
ACL_SELECT);
- if (aclresult != ACLCHECK_OK)
- aclcheck_error(aclresult, ACL_KIND_CLASS,
- RelationGetRelationName(relation));
+ if (aclresult != ACLCHECK_OK)
+ aclcheck_error(aclresult, ACL_KIND_CLASS,
+ RelationGetRelationName(relation));
+ }
tupleDesc = RelationGetDescr(relation);
constr = tupleDesc-constr;
diff --git i/src/test/regress/expected/create_table_like.out w/src/test/regress/expected/create_table_like.out
index 40b6766..8bec55c 100644
--- i/src/test/regress/expected/create_table_like.out
+++ w/src/test/regress/expected/create_table_like.out
@@ -8,6 +8,10 @@ CREATE TABLE inhx (xx text DEFAULT 'text');
*/
CREATE TABLE ctla (aa TEXT);
CREATE TABLE ctlb (bb TEXT) INHERITS (ctla);
+CREATE TABLE foo (LIKE nonexistent);
+ERROR: relation nonexistent does not exist
+LINE 1: CREATE TABLE foo (LIKE nonexistent);
+ ^
CREATE TABLE inhe (ee text, LIKE inhx) inherits (ctlb);
INSERT INTO inhe VALUES ('ee-col1', 'ee-col2', DEFAULT, 'ee-col4');
SELECT * FROM inhe; /* Columns aa, bb, xx value NULL, ee */
@@ -224,18 +228,16 @@ NOTICE: drop cascades to table inhe
CREATE TABLE ctlt4 (a int, b text);
CREATE SEQUENCE ctlseq1;
CREATE TABLE ctlt10 (LIKE ctlseq1); -- fail
-ERROR: LIKE source relation
Re: [HACKERS] Misleading CREATE TABLE error
On tis, 2011-11-08 at 21:49 +, Thom Brown wrote:
I found the following error message misleading:
test=# create table cows2 (LIKE cows);
ERROR: inherited relation cows is not a table
STATEMENT: create table cows2 (LIKE cows);
I'm not trying to inherit a relation, I'm trying to base a table on
it.
It's not only the error message that's misleading, but the whole code,
because the entire code for CREATE TABLE ... (LIKE ...) claims to do
inheritance based on an ancient understanding of the SQL standard. I
know this has confused me many times already, so I decided to clean this
up and rename all the internal parser structures, split up the
regression tests for real inheritance and CREATE TABLE LIKE, and adjust
the error messages. Patch attached.
As it happens, cows is a foreign table, which *is* a table,
just not a regular table. It might be useful to add support to clone
foreign tables into regular tables, the use-case being that you may
wish to import all the data locally into a table of the same
structure.
This is easy to fix, and I mangled it into my big renaming patch, which
I shouldn't have. Anyway, one question that's perhaps worth discussing
is whether we should allow and disallow the various INCLUDING options
depending on the relation type. For example, views don't have indexes,
so should we disallow INCLUDING INDEXES or just assume they don't have
any?
diff --git i/doc/src/sgml/ref/create_table.sgml w/doc/src/sgml/ref/create_table.sgml
index 30e4154..97968bb 100644
--- i/doc/src/sgml/ref/create_table.sgml
+++ w/doc/src/sgml/ref/create_table.sgml
@@ -24,7 +24,7 @@ PostgreSQL documentation
CREATE [ [ GLOBAL | LOCAL ] { TEMPORARY | TEMP } | UNLOGGED ] TABLE [ IF NOT EXISTS ] replaceable class=PARAMETERtable_name/replaceable ( [
{ replaceable class=PARAMETERcolumn_name/replaceable replaceable class=PARAMETERdata_type/replaceable [ COLLATE replaceablecollation/replaceable ] [ replaceable class=PARAMETERcolumn_constraint/replaceable [ ... ] ]
| replaceabletable_constraint/replaceable
-| LIKE replaceableparent_table/replaceable [ replaceablelike_option/replaceable ... ] }
+| LIKE replaceablesource_table/replaceable [ replaceablelike_option/replaceable ... ] }
[, ... ]
] )
[ INHERITS ( replaceableparent_table/replaceable [, ... ] ) ]
@@ -312,7 +312,7 @@ CREATE [ [ GLOBAL | LOCAL ] { TEMPORARY | TEMP } | UNLOGGED ] TABLE [ IF NOT EXI
/varlistentry
varlistentry
-termliteralLIKE replaceableparent_table/replaceable [ replaceablelike_option/replaceable ... ]/literal/term
+termliteralLIKE replaceablesource_table/replaceable [ replaceablelike_option/replaceable ... ]/literal/term
listitem
para
The literalLIKE/literal clause specifies a table from which
diff --git i/src/backend/nodes/copyfuncs.c w/src/backend/nodes/copyfuncs.c
index dd2dd25..a4d3912 100644
--- i/src/backend/nodes/copyfuncs.c
+++ w/src/backend/nodes/copyfuncs.c
@@ -2726,10 +2726,10 @@ _copyCreateStmt(const CreateStmt *from)
return newnode;
}
-static InhRelation *
-_copyInhRelation(const InhRelation *from)
+static TableLikeClause *
+_copyTableLikeClause(const TableLikeClause *from)
{
- InhRelation *newnode = makeNode(InhRelation);
+ TableLikeClause *newnode = makeNode(TableLikeClause);
COPY_NODE_FIELD(relation);
COPY_SCALAR_FIELD(options);
@@ -4133,8 +4133,8 @@ copyObject(const void *from)
case T_CreateStmt:
retval = _copyCreateStmt(from);
break;
- case T_InhRelation:
- retval = _copyInhRelation(from);
+ case T_TableLikeClause:
+ retval = _copyTableLikeClause(from);
break;
case T_DefineStmt:
retval = _copyDefineStmt(from);
diff --git i/src/backend/nodes/equalfuncs.c w/src/backend/nodes/equalfuncs.c
index c2fdb2b..c1bf94f 100644
--- i/src/backend/nodes/equalfuncs.c
+++ w/src/backend/nodes/equalfuncs.c
@@ -1159,7 +1159,7 @@ _equalCreateStmt(const CreateStmt *a, const CreateStmt *b)
}
static bool
-_equalInhRelation(const InhRelation *a, const InhRelation *b)
+_equalTableLikeClause(const TableLikeClause *a, const TableLikeClause *b)
{
COMPARE_NODE_FIELD(relation);
COMPARE_SCALAR_FIELD(options);
@@ -2676,8 +2676,8 @@ equal(const void *a, const void *b)
case T_CreateStmt:
retval = _equalCreateStmt(a, b);
break;
- case T_InhRelation:
- retval = _equalInhRelation(a, b);
+ case T_TableLikeClause:
+ retval = _equalTableLikeClause(a, b);
break;
case T_DefineStmt:
retval = _equalDefineStmt(a, b);
diff --git i/src/backend/nodes/outfuncs.c w/src/backend/nodes/outfuncs.c
index cdc2cab..9d14141 100644
--- i/src/backend/nodes/outfuncs.c
+++ w/src/backend/nodes/outfuncs.c
@@ -2064,9 +2064,9 @@ _outDefElem(StringInfo str, const DefElem *node)
}
static void
-_outInhRelation(StringInfo str, const InhRelation *node)
+_outTableLikeClause(StringInfo str, const TableLikeClause *node)
{
- WRITE_NODE_TYPE(INHRELATION);
+ WRITE_NODE_TYPE(TABLELIKECLAUSE);
WRITE_NODE_FIELD(relation);
Re: [HACKERS] Misleading CREATE TABLE error
On 27 December 2011 20:16, Peter Eisentraut pete...@gmx.net wrote: It's not only the error message that's misleading, but the whole code, because the entire code for CREATE TABLE ... (LIKE ...) claims to do inheritance based on an ancient understanding of the SQL standard. I know this has confused me many times already, so I decided to clean this up and rename all the internal parser structures, split up the regression tests for real inheritance and CREATE TABLE LIKE, and adjust the error messages. Patch attached. Thanks for the patch. +1 for changing parent to source in the docs. The patch doesn't apply cleanly for me for some reason though. Anyway, one question that's perhaps worth discussing is whether we should allow and disallow the various INCLUDING options depending on the relation type. For example, views don't have indexes, so should we disallow INCLUDING INDEXES or just assume they don't have any? I'd personally prefer the latter, primarily because it won't create another syntax variation with no discernable benefit. -- Thom -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Misleading CREATE TABLE error
On ons, 2011-11-09 at 12:00 -0500, Robert Haas wrote: On Tue, Nov 8, 2011 at 4:49 PM, Thom Brown t...@linux.com wrote: I found the following error message misleading: test=# create table cows2 (LIKE cows); ERROR: inherited relation cows is not a table STATEMENT: create table cows2 (LIKE cows); I'm not trying to inherit a relation, I'm trying to base a table on it. As it happens, cows is a foreign table, which *is* a table, just not a regular table. It might be useful to add support to clone foreign tables into regular tables, the use-case being that you may wish to import all the data locally into a table of the same structure. But the gripe here is the suggestion that the relation would have been inherited, which would actually be achieved using INHERITS. Interesting. I agree that there's no obvious reason why that shouldn't be allowed to work. Could be useful with views, too. I recently came across a situation where LIKE with a composite type might have been useful. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Misleading CREATE TABLE error
On Tue, Nov 8, 2011 at 4:49 PM, Thom Brown t...@linux.com wrote: I found the following error message misleading: test=# create table cows2 (LIKE cows); ERROR: inherited relation cows is not a table STATEMENT: create table cows2 (LIKE cows); I'm not trying to inherit a relation, I'm trying to base a table on it. As it happens, cows is a foreign table, which *is* a table, just not a regular table. It might be useful to add support to clone foreign tables into regular tables, the use-case being that you may wish to import all the data locally into a table of the same structure. But the gripe here is the suggestion that the relation would have been inherited, which would actually be achieved using INHERITS. Interesting. I agree that there's no obvious reason why that shouldn't be allowed to work. Could be useful with views, too. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
