Kevin Grittner <kgri...@ymail.com> wrote:
> Robert Haas <robertmh...@gmail.com> wrote:
>> It seems to me that the right place to fix this is in
>> interpretOidsOption(), by returning false rather than
>> default_with_oids whenever the relation is a materialized view.
> I like it.
In working up a patch for this approach, I see that if CREATE
FOREIGN TABLE is executed with default_with_oids set to true, it
adds an oid column which appears to be always zero in my tests so
far (although maybe other FDWs support it?). Do we want to leave
that alone? If we're going to add code to ignore that setting for
matviews do we also want to ignore it for FDWs?
[ thinks... ]
I suppose I should post a patch which preserves the status quo for
FDWs and treat that as a separate issue. So, rough cut attached.
Obviously some docs should be added around this, and I still need
to do another pass to make sure I didn't miss anything; but it
passes make world-check, make installworld-check, and the
regression database can be dumped and loaded without problem.
Comments?
--
Kevin Grittner
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
*** a/src/backend/commands/createas.c
--- b/src/backend/commands/createas.c
***************
*** 218,228 **** GetIntoRelEFlags(IntoClause *intoClause)
* because it doesn't have enough information to do so itself (since we
* can't build the target relation until after ExecutorStart).
*/
! if (interpretOidsOption(intoClause->options))
flags = EXEC_FLAG_WITH_OIDS;
else
flags = EXEC_FLAG_WITHOUT_OIDS;
if (intoClause->skipData)
flags |= EXEC_FLAG_WITH_NO_DATA;
--- 218,232 ----
* because it doesn't have enough information to do so itself (since we
* can't build the target relation until after ExecutorStart).
*/
! if (interpretOidsOption(intoClause->options, intoClause->relkind))
flags = EXEC_FLAG_WITH_OIDS;
else
flags = EXEC_FLAG_WITHOUT_OIDS;
+ Assert(intoClause->relkind != RELKIND_MATVIEW ||
+ (flags & (EXEC_FLAG_WITH_OIDS | EXEC_FLAG_WITHOUT_OIDS)) ==
+ EXEC_FLAG_WITHOUT_OIDS);
+
if (intoClause->skipData)
flags |= EXEC_FLAG_WITH_NO_DATA;
*** a/src/backend/commands/tablecmds.c
--- b/src/backend/commands/tablecmds.c
***************
*** 559,565 **** DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId)
*/
descriptor = BuildDescForRelation(schema);
! localHasOids = interpretOidsOption(stmt->options);
descriptor->tdhasoid = (localHasOids || parentOidCount > 0);
/*
--- 559,565 ----
*/
descriptor = BuildDescForRelation(schema);
! localHasOids = interpretOidsOption(stmt->options, relkind);
descriptor->tdhasoid = (localHasOids || parentOidCount > 0);
/*
*** a/src/backend/parser/parse_clause.c
--- b/src/backend/parser/parse_clause.c
***************
*** 243,251 **** interpretInhOption(InhOption inhOpt)
* table/result set should be created with OIDs. This needs to be done after
* parsing the query string because the return value can depend upon the
* default_with_oids GUC var.
*/
bool
! interpretOidsOption(List *defList)
{
ListCell *cell;
--- 243,254 ----
* table/result set should be created with OIDs. This needs to be done after
* parsing the query string because the return value can depend upon the
* default_with_oids GUC var.
+ *
+ * Materialized views are handled here rather than reloptions.c because that
+ * code explicitly punts checking for oids to here.
*/
bool
! interpretOidsOption(List *defList, char relkind)
{
ListCell *cell;
***************
*** 256,264 **** interpretOidsOption(List *defList)
--- 259,277 ----
if (def->defnamespace == NULL &&
pg_strcasecmp(def->defname, "oids") == 0)
+ {
+ if (relkind == RELKIND_MATVIEW)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("unrecognized parameter \"oids\"")));
+
return defGetBoolean(def);
+ }
}
+ if (relkind == RELKIND_MATVIEW)
+ return false;
+
/* OIDS option was not specified, so use default. */
return default_with_oids;
}
*** a/src/backend/parser/parse_utilcmd.c
--- b/src/backend/parser/parse_utilcmd.c
***************
*** 199,209 **** transformCreateStmt(CreateStmt *stmt, const char *queryString)
--- 199,212 ----
{
cxt.stmtType = "CREATE FOREIGN TABLE";
cxt.isforeign = true;
+ cxt.hasoids = interpretOidsOption(stmt->options,
+ RELKIND_FOREIGN_TABLE);
}
else
{
cxt.stmtType = "CREATE TABLE";
cxt.isforeign = false;
+ cxt.hasoids = interpretOidsOption(stmt->options, RELKIND_RELATION);
}
cxt.relation = stmt->relation;
cxt.rel = NULL;
***************
*** 217,223 **** transformCreateStmt(CreateStmt *stmt, const char *queryString)
cxt.blist = NIL;
cxt.alist = NIL;
cxt.pkey = NULL;
- cxt.hasoids = interpretOidsOption(stmt->options);
Assert(!stmt->ofTypename || !stmt->inhRelations); /* grammar enforces */
--- 220,225 ----
*** a/src/include/parser/parse_clause.h
--- b/src/include/parser/parse_clause.h
***************
*** 20,26 **** extern void transformFromClause(ParseState *pstate, List *frmList);
extern int setTargetTable(ParseState *pstate, RangeVar *relation,
bool inh, bool alsoSource, AclMode requiredPerms);
extern bool interpretInhOption(InhOption inhOpt);
! extern bool interpretOidsOption(List *defList);
extern Node *transformWhereClause(ParseState *pstate, Node *clause,
ParseExprKind exprKind, const char *constructName);
--- 20,26 ----
extern int setTargetTable(ParseState *pstate, RangeVar *relation,
bool inh, bool alsoSource, AclMode requiredPerms);
extern bool interpretInhOption(InhOption inhOpt);
! extern bool interpretOidsOption(List *defList, char relkind);
extern Node *transformWhereClause(ParseState *pstate, Node *clause,
ParseExprKind exprKind, const char *constructName);
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
