Kevin Grittner <kgri...@ymail.com> wrote:
> Tom Lane <t...@sss.pgh.pa.us> wrote:
>> Kevin Grittner <kgri...@ymail.com> writes:
>>> I failed to touch everything necessary to prevent MVs from
>>> having OIDs. This patch fixes the reported problem, and
>>> doesn't leave any gaps as far as I know; but I will do
>>> additional review to try to catch any other omissions. I
>>> figured I should address the reported problem now, though.
>>
>>> Will push later today if there are no objections.
>>
>> I object --- that's not a fix, that's a crude hack. It should
>> not be necessary to introduce relkind tests there.
>> Determination of whether OIDs exist in the target table should
>> happen well upstream, ie in whatever is constructing the
>> intoClause. Otherwise we'll be fixing code that examines the
>> intoClause until doomsday.
>
> OK. I started doing it that way, but saw how much more code was
> changed than this way and gave in to an impulse to do a minimal
> change. I really need to resist that impulse more....
The presence of default_with_oids and the special-handling of the
oids option via interpretOidsOption() makes it hard to come up with
a solution which would qualify as "elegant". Here's a rough cut at
an approach which seems best to me. If this sits well with others
I'll add comments and think about that error message some more.
I'm not entirely sure I like accepting WITH (oids = false) but
throwing an error on WITH (oids = true), but it seems marginally
better than rejecting both.
Comments?
--
Kevin Grittner
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
*** a/src/backend/commands/createas.c
--- b/src/backend/commands/createas.c
***************
*** 223,228 **** GetIntoRelEFlags(IntoClause *intoClause)
--- 223,231 ----
else
flags = EXEC_FLAG_WITHOUT_OIDS;
+ Assert(intoClause->relkind != RELKIND_MATVIEW ||
+ flags == EXEC_FLAG_WITHOUT_OIDS);
+
if (intoClause->skipData)
flags |= EXEC_FLAG_WITH_NO_DATA;
*** a/src/backend/nodes/makefuncs.c
--- b/src/backend/nodes/makefuncs.c
***************
*** 17,22 ****
--- 17,23 ----
#include "catalog/pg_class.h"
#include "catalog/pg_type.h"
+ #include "commands/defrem.h"
#include "nodes/makefuncs.h"
#include "nodes/nodeFuncs.h"
#include "utils/lsyscache.h"
***************
*** 508,510 **** makeDefElemExtended(char *nameSpace, char *name, Node *arg,
--- 509,539 ----
return res;
}
+
+ List *
+ makeOptsWithoutOids(List *defList)
+ {
+ ListCell *cell;
+
+ /* Scan list to see if OIDS was included */
+ foreach(cell, defList)
+ {
+ DefElem *def = (DefElem *) lfirst(cell);
+
+ if (def->defnamespace == NULL &&
+ pg_strcasecmp(def->defname, "oids") == 0)
+ {
+ bool oids = defGetBoolean(def);
+
+ if (oids)
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("WITH OIDS is not an allowed option")));
+
+ return defList;
+ }
+ }
+
+ /* OIDS option was not specified, add it as false. */
+ return lappend(defList, defWithOids(false));
+ }
*** a/src/backend/parser/gram.y
--- b/src/backend/parser/gram.y
***************
*** 3271,3277 **** create_mv_target:
$$ = makeNode(IntoClause);
$$->rel = $1;
$$->colNames = $2;
! $$->options = $3;
$$->onCommit = ONCOMMIT_NOOP;
$$->tableSpaceName = $4;
$$->skipData = false; /* might get changed later */
--- 3271,3277 ----
$$ = makeNode(IntoClause);
$$->rel = $1;
$$->colNames = $2;
! $$->options = makeOptsWithoutOids($3);
$$->onCommit = ONCOMMIT_NOOP;
$$->tableSpaceName = $4;
$$->skipData = false; /* might get changed later */
*** a/src/include/nodes/makefuncs.h
--- b/src/include/nodes/makefuncs.h
***************
*** 79,82 **** extern DefElem *makeDefElem(char *name, Node *arg);
--- 79,84 ----
extern DefElem *makeDefElemExtended(char *nameSpace, char *name, Node *arg,
DefElemAction defaction);
+ extern List *makeOptsWithoutOids(List *options);
+
#endif /* MAKEFUNC_H */
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
