On Wed, Jun 11, 2014 at 2:46 PM, Alvaro Herrera
<alvhe...@2ndquadrant.com> wrote:
> I just noticed by chance that view relations are using StdRdOptions to
> allocate their reloptions. I can't find any reason for this, other than
> someone failed to realize that they should instead have a struct defined
> of its own, just like (say) GIN indexes do. Views using StdRdOptions is
> quite pointless, given that it's used for stuff like fillfactor and
> autovacuum, neither of which apply to views.
>
> 9.2 added security_barrier to StdRdOptions, and 9.4 is now adding
> check_option_offset, which is a string reloption with some funny rules.
>
[...]
> 2) it would mean that security_barrier would change for external code
> that expects StdRdOptions rather than, say, ViewOptions
> 3) I don't have time to do the legwork before CF1 anyway
>
> If we don't change it now, I'm afraid we'll be stuck with using
> StdRdOptions for views for all eternity.
>
> (It's a pity I didn't become aware of this earlier in 9.4 cycle when I
> added the multixact freeze reloptions ... I could have promoted a patch
> back then.)
>
Attached is a patch moving the reloptions of views into its own structure.
i also created a view_reloptions() function in order to not use
heap_reloptions() for views, but maybe that was too much?
i haven't seen the check_option_offset thingy yet, but i hope to take
a look at that tomorrow.
--
Jaime Casanova www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación
Phone: +593 4 5107566 Cell: +593 987171157
From 87ed78a4f276484a37917c417286a16082030f13 Mon Sep 17 00:00:00 2001
From: Jaime Casanova <ja...@2ndquadrant.com>
Date: Fri, 13 Jun 2014 01:10:24 -0500
Subject: [PATCH] Move reloptions from views into its own structure.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Per gripe from Álvaro Herrera.
---
src/backend/access/common/reloptions.c | 42 ++++++++++++++++++++++++++-----
src/backend/commands/tablecmds.c | 9 +++++-
src/include/access/reloptions.h | 1 +
src/include/utils/rel.h | 43 ++++++++++++++++++++------------
src/tools/pgindent/typedefs.list | 1 +
5 files changed, 71 insertions(+), 25 deletions(-)
diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c
index 522b671..c7ad6f9 100644
--- a/src/backend/access/common/reloptions.c
+++ b/src/backend/access/common/reloptions.c
@@ -834,10 +834,12 @@ extractRelOptions(HeapTuple tuple, TupleDesc tupdesc, Oid amoptions)
{
case RELKIND_RELATION:
case RELKIND_TOASTVALUE:
- case RELKIND_VIEW:
case RELKIND_MATVIEW:
options = heap_reloptions(classForm->relkind, datum, false);
break;
+ case RELKIND_VIEW:
+ options = view_reloptions(datum, false);
+ break;
case RELKIND_INDEX:
options = index_reloptions(amoptions, datum, false);
break;
@@ -1200,10 +1202,6 @@ default_reloptions(Datum reloptions, bool validate, relopt_kind kind)
offsetof(StdRdOptions, autovacuum) +offsetof(AutoVacOpts, vacuum_scale_factor)},
{"autovacuum_analyze_scale_factor", RELOPT_TYPE_REAL,
offsetof(StdRdOptions, autovacuum) +offsetof(AutoVacOpts, analyze_scale_factor)},
- {"security_barrier", RELOPT_TYPE_BOOL,
- offsetof(StdRdOptions, security_barrier)},
- {"check_option", RELOPT_TYPE_STRING,
- offsetof(StdRdOptions, check_option_offset)},
{"user_catalog_table", RELOPT_TYPE_BOOL,
offsetof(StdRdOptions, user_catalog_table)}
};
@@ -1225,6 +1223,38 @@ default_reloptions(Datum reloptions, bool validate, relopt_kind kind)
}
/*
+ * Option parser for views
+ */
+bytea *
+view_reloptions(Datum reloptions, bool validate)
+{
+ relopt_value *options;
+ ViewOptions *vopts;
+ int numoptions;
+ static const relopt_parse_elt tab[] = {
+ {"security_barrier", RELOPT_TYPE_BOOL,
+ offsetof(ViewOptions, security_barrier)},
+ {"check_option", RELOPT_TYPE_STRING,
+ offsetof(ViewOptions, check_option_offset)}
+ };
+
+ options = parseRelOptions(reloptions, validate, RELOPT_KIND_VIEW, &numoptions);
+
+ /* if none set, we're done */
+ if (numoptions == 0)
+ return NULL;
+
+ vopts = allocateReloptStruct(sizeof(ViewOptions), options, numoptions);
+
+ fillRelOptions((void *) vopts, sizeof(ViewOptions), options, numoptions,
+ validate, tab, lengthof(tab));
+
+ pfree(options);
+
+ return (bytea *) vopts;
+}
+
+/*
* Parse options for heaps, views and toast tables.
*/
bytea *
@@ -1248,8 +1278,6 @@ heap_reloptions(char relkind, Datum reloptions, bool validate)
case RELKIND_RELATION:
case RELKIND_MATVIEW:
return default_reloptions(reloptions, validate, RELOPT_KIND_HEAP);
- case RELKIND_VIEW:
- return default_reloptions(reloptions, validate, RELOPT_KIND_VIEW);
default:
/* other relkinds are not supported */
return NULL;
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 341262b..fd27cdb 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -533,7 +533,10 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId)
reloptions = transformRelOptions((Datum) 0, stmt->options, NULL, validnsps,
true, false);
- (void) heap_reloptions(relkind, reloptions, true);
+ if (relkind == RELKIND_VIEW)
+ (void) view_reloptions(reloptions, true);
+ else
+ (void) heap_reloptions(relkind, reloptions, true);
if (stmt->ofTypename)
{
@@ -8890,10 +8893,12 @@ ATExecSetRelOptions(Relation rel, List *defList, AlterTableType operation,
{
case RELKIND_RELATION:
case RELKIND_TOASTVALUE:
- case RELKIND_VIEW:
case RELKIND_MATVIEW:
(void) heap_reloptions(rel->rd_rel->relkind, newOptions, true);
break;
+ case RELKIND_VIEW:
+ (void) view_reloptions(newOptions, true);
+ break;
case RELKIND_INDEX:
(void) index_reloptions(rel->rd_am->amoptions, newOptions, true);
break;
diff --git a/src/include/access/reloptions.h b/src/include/access/reloptions.h
index 81ff328..c226448 100644
--- a/src/include/access/reloptions.h
+++ b/src/include/access/reloptions.h
@@ -268,6 +268,7 @@ extern void fillRelOptions(void *rdopts, Size basesize,
extern bytea *default_reloptions(Datum reloptions, bool validate,
relopt_kind kind);
extern bytea *heap_reloptions(char relkind, Datum reloptions, bool validate);
+extern bytea *view_reloptions(Datum reloptions, bool validate);
extern bytea *index_reloptions(RegProcedure amoptions, Datum reloptions,
bool validate);
extern bytea *attribute_reloptions(Datum reloptions, bool validate);
diff --git a/src/include/utils/rel.h b/src/include/utils/rel.h
index af4f53f..f619011 100644
--- a/src/include/utils/rel.h
+++ b/src/include/utils/rel.h
@@ -216,8 +216,6 @@ typedef struct StdRdOptions
int32 vl_len_; /* varlena header (do not touch directly!) */
int fillfactor; /* page fill factor in percent (0..100) */
AutoVacOpts autovacuum; /* autovacuum-related options */
- bool security_barrier; /* for views */
- int check_option_offset; /* for views */
bool user_catalog_table; /* use as an additional catalog
* relation */
} StdRdOptions;
@@ -248,12 +246,33 @@ typedef struct StdRdOptions
(BLCKSZ * (100 - RelationGetFillFactor(relation, defaultff)) / 100)
/*
+ * RelationIsUsedAsCatalogTable
+ * Returns whether the relation should be treated as a catalog table
+ * from the pov of logical decoding.
+ */
+#define RelationIsUsedAsCatalogTable(relation) \
+ ((relation)->rd_options ? \
+ ((StdRdOptions *) (relation)->rd_options)->user_catalog_table : false)
+
+
+/*
+ * ViewOptions
+ * Contents of rd_options for views
+ */
+typedef struct ViewOptions
+{
+ int32 vl_len_; /* varlena header (do not touch directly!) */
+ bool security_barrier;
+ int check_option_offset;
+} ViewOptions;
+
+/*
* RelationIsSecurityView
* Returns whether the relation is security view, or not
*/
#define RelationIsSecurityView(relation) \
((relation)->rd_options ? \
- ((StdRdOptions *) (relation)->rd_options)->security_barrier : false)
+ ((ViewOptions *) (relation)->rd_options)->security_barrier : false)
/*
* RelationHasCheckOption
@@ -262,7 +281,7 @@ typedef struct StdRdOptions
*/
#define RelationHasCheckOption(relation) \
((relation)->rd_options && \
- ((StdRdOptions *) (relation)->rd_options)->check_option_offset != 0)
+ ((ViewOptions *) (relation)->rd_options)->check_option_offset != 0)
/*
* RelationHasLocalCheckOption
@@ -271,9 +290,9 @@ typedef struct StdRdOptions
*/
#define RelationHasLocalCheckOption(relation) \
((relation)->rd_options && \
- ((StdRdOptions *) (relation)->rd_options)->check_option_offset != 0 ? \
+ ((ViewOptions *) (relation)->rd_options)->check_option_offset != 0 ? \
strcmp((char *) (relation)->rd_options + \
- ((StdRdOptions *) (relation)->rd_options)->check_option_offset, \
+ ((ViewOptions *) (relation)->rd_options)->check_option_offset, \
"local") == 0 : false)
/*
@@ -283,19 +302,11 @@ typedef struct StdRdOptions
*/
#define RelationHasCascadedCheckOption(relation) \
((relation)->rd_options && \
- ((StdRdOptions *) (relation)->rd_options)->check_option_offset != 0 ? \
+ ((ViewOptions *) (relation)->rd_options)->check_option_offset != 0 ? \
strcmp((char *) (relation)->rd_options + \
- ((StdRdOptions *) (relation)->rd_options)->check_option_offset, \
+ ((ViewOptions *) (relation)->rd_options)->check_option_offset, \
"cascaded") == 0 : false)
-/*
- * RelationIsUsedAsCatalogTable
- * Returns whether the relation should be treated as a catalog table
- * from the pov of logical decoding.
- */
-#define RelationIsUsedAsCatalogTable(relation) \
- ((relation)->rd_options ? \
- ((StdRdOptions *) (relation)->rd_options)->user_catalog_table : false)
/*
* RelationIsValid
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index f75fc69..913d6ef 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -1955,6 +1955,7 @@ VariableSpace
VariableStatData
Vfd
ViewCheckOption
+ViewOptions
ViewStmt
VirtualTransactionId
Vsrt
--
1.7.2.5
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
