On Sun, Feb 9, 2014 at 2:22 PM, Fabrízio de Royes Mello <
fabriziome...@gmail.com> wrote:
>
>
> On Sat, Jan 11, 2014 at 2:47 AM, Peter Eisentraut <pete...@gmx.net> wrote:
> >
> > On Sat, 2014-01-11 at 00:48 -0200, Fabrízio de Royes Mello wrote:
> > > > Now, if bdr is installed but the validation doesn't happen unless
> > > bdr
> > > > is "loaded" in some sense, then that is an implementation deficiency
> > > > that I think we can insist be rectified before this feature is
> > > accepted.
> > > >
> >
> > > Check if extension is already installed is not enough for the first
> > > version of this feature?
> >
> > Elsewhere it was argued that tying this to extensions is not
> > appropriate. I agree.
> >
> > It depends on how this feature is supposed to be used exactly. A
> > replication plugin might very well be loaded via
> > session_preload_libraries and not appear in SQL at all. In that case
> > you need some C-level hook. In another case, an extension might want to
> > inspect relation options from user-space triggers. So you'd need to
> > register some SQL-level function for option validation.
> >
> > This could end up being two separate but overlapping features.
> >
>
> Hi all,
>
>
> I taken this weekend to work on this patch and on monday or tuesday I'll
send it.
>
> But I have some doubts:
>
> 1) I'm not convinced to tying this to extensions. I think this feature
must enable us to just store a custom GUC. We can set custom GUCs in a
backend session using "SET class.variable = value", and this feature could
just enable us to store it for relations/attributes. Without the complexity
and overhead to register a function to validate them. That way we can use
this feature to extensions and other needs too.
>
> 2) If we're implement the Robert's idea to have a function to validate
the extension options then we must think about how a extension developer
will register this function. Beacuse when we install a extension must have
one way to get de pg_proc OID and store it in the pg_extension (or a
different catalog). Or we'll implement some way to register this function
at the SQL level, like "ALTER EXTENSION bdr SET VALIDATE FUNCTION
bdr_options_validate();" or another sintax of course.
>
> I don't know if you guys understood my concerns!! :-)
>
> Comments?
>
Hi all,
The attached patch implements the first option that I suggested before.
Regards,
--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Timbira: http://www.timbira.com.br
>> Blog sobre TI: http://fabriziomello.blogspot.com
>> Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello
diff --git a/doc/src/sgml/ref/alter_index.sgml b/doc/src/sgml/ref/alter_index.sgml
index d210077..5e9ee9d 100644
--- a/doc/src/sgml/ref/alter_index.sgml
+++ b/doc/src/sgml/ref/alter_index.sgml
@@ -82,6 +82,14 @@ ALTER INDEX [ IF EXISTS ] <replaceable class="PARAMETER">name</replaceable> RESE
<xref linkend="SQL-REINDEX">
to get the desired effects.
</para>
+ <note>
+ <para>
+ A custom name can be used as namespace to define a storage parameter.
+ Storage option pattern: namespace.option=value
+ (namespace=custom name, option=option name and value=option value).
+ See example bellow.
+ </para>
+ </note>
</listitem>
</varlistentry>
@@ -202,6 +210,17 @@ ALTER INDEX distributors SET (fillfactor = 75);
REINDEX INDEX distributors;
</programlisting></para>
+ <para>
+ To set a custom storage parameter:
+<programlisting>
+ALTER INDEX distributors
+ SET (bdr.do_replicate=true);
+</programlisting>
+ (bdr=custom name, do_replicate=option name and
+ true=option value)
+</para>
+
+
</refsect1>
<refsect1>
diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml
index 89649a2..6fd9d67 100644
--- a/doc/src/sgml/ref/alter_table.sgml
+++ b/doc/src/sgml/ref/alter_table.sgml
@@ -213,6 +213,16 @@ ALTER TABLE [ IF EXISTS ] <replaceable class="PARAMETER">name</replaceable>
of statistics by the <productname>PostgreSQL</productname> query
planner, refer to <xref linkend="planner-stats">.
</para>
+
+ <note>
+ <para>
+ A custom name can be used as namespace to define a storage parameter.
+ Storage option pattern: namespace.option=value
+ (namespace=custom name, option=option name and value=option value).
+ See example bellow.
+ </para>
+ </note>
+
</listitem>
</varlistentry>
@@ -476,6 +486,10 @@ ALTER TABLE [ IF EXISTS ] <replaceable class="PARAMETER">name</replaceable>
<command>ALTER TABLE</> does not treat <literal>OIDS</> as a
storage parameter. Instead use the <literal>SET WITH OIDS</>
and <literal>SET WITHOUT OIDS</> forms to change OID status.
+ A custom name can be used as namespace to define a storage parameter.
+ Storage option pattern: namespace.option=value
+ (namespace=custom name, option=option name and value=option value).
+ See example bellow.
</para>
</note>
</listitem>
@@ -1112,6 +1126,26 @@ ALTER TABLE distributors DROP CONSTRAINT distributors_pkey,
ADD CONSTRAINT distributors_pkey PRIMARY KEY USING INDEX dist_id_temp_idx;
</programlisting></para>
+ <para>
+ To set a custom per-attribute option:
+<programlisting>
+ALTER TABLE distributors
+ ALTER COLUMN dist_id SET (bdr.do_replicate=true);
+</programlisting>
+ (bdr=custom name, do_replicate=option name and
+ true=option value)
+</para>
+
+ <para>
+ To set a custom storage parameter:
+<programlisting>
+ALTER TABLE distributors
+ SET (bdr.do_replicate=true);
+</programlisting>
+ (bdr=custom name, do_replicate=option name and
+ true=option value)
+</para>
+
</refsect1>
<refsect1>
diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c
index fa08c45..7de2003 100644
--- a/src/backend/access/common/reloptions.c
+++ b/src/backend/access/common/reloptions.c
@@ -283,6 +283,9 @@ static void initialize_reloptions(void);
static void parse_one_reloption(relopt_value *option, char *text_str,
int text_len, bool validate);
+static bool is_custom_namespace(char *namespace);
+static bool is_reserved_namespace(char *namespace);
+
/*
* initialize_reloptions
* initialization routine, must be called before parsing
@@ -610,13 +613,15 @@ transformRelOptions(Datum oldOptions, List *defList, char *namspace,
/* Search for a match in defList */
foreach(cell, defList)
{
- DefElem *def = (DefElem *) lfirst(cell);
+ DefElem *def = (DefElem *) lfirst(cell);
int kw_len;
+ char *text_compare;
/* ignore if not in the same namespace */
if (namspace == NULL)
{
- if (def->defnamespace != NULL)
+ if (def->defnamespace != NULL &&
+ !is_custom_namespace(def->defnamespace))
continue;
}
else if (def->defnamespace == NULL)
@@ -625,8 +630,17 @@ transformRelOptions(Datum oldOptions, List *defList, char *namspace,
continue;
kw_len = strlen(def->defname);
+ if (is_custom_namespace(def->defnamespace))
+ kw_len += strlen(def->defnamespace) + 1;
+
+ text_compare = (char *) palloc(kw_len + 1);
+ if (is_custom_namespace(def->defnamespace))
+ sprintf(text_compare, "%s.%s", def->defnamespace, def->defname);
+ else
+ sprintf(text_compare, "%s", def->defname);
+
if (text_len > kw_len && text_str[kw_len] == '=' &&
- pg_strncasecmp(text_str, def->defname, kw_len) == 0)
+ pg_strncasecmp(text_str, text_compare, kw_len) == 0)
break;
}
if (!cell)
@@ -668,22 +682,11 @@ transformRelOptions(Datum oldOptions, List *defList, char *namspace,
if (def->defnamespace != NULL)
{
bool valid = false;
- int i;
if (validnsps)
- {
- for (i = 0; validnsps[i]; i++)
- {
- if (pg_strcasecmp(def->defnamespace,
- validnsps[i]) == 0)
- {
- valid = true;
- break;
- }
- }
- }
+ valid = is_reserved_namespace(def->defnamespace);
- if (!valid)
+ if (!valid && !is_custom_namespace(def->defnamespace))
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("unrecognized parameter namespace \"%s\"",
@@ -696,7 +699,8 @@ transformRelOptions(Datum oldOptions, List *defList, char *namspace,
/* ignore if not in the same namespace */
if (namspace == NULL)
{
- if (def->defnamespace != NULL)
+ if (def->defnamespace != NULL &&
+ !is_custom_namespace(def->defnamespace))
continue;
}
else if (def->defnamespace == NULL)
@@ -714,10 +718,18 @@ transformRelOptions(Datum oldOptions, List *defList, char *namspace,
else
value = "true";
len = VARHDRSZ + strlen(def->defname) + 1 + strlen(value);
+
+ if (is_custom_namespace(def->defnamespace))
+ len += strlen(def->defnamespace) + 1;
+
/* +1 leaves room for sprintf's trailing null */
t = (text *) palloc(len + 1);
SET_VARSIZE(t, len);
- sprintf(VARDATA(t), "%s=%s", def->defname, value);
+
+ if (is_custom_namespace(def->defnamespace))
+ sprintf(VARDATA(t), "%s.%s=%s", def->defnamespace, def->defname, value);
+ else
+ sprintf(VARDATA(t), "%s=%s", def->defname, value);
astate = accumArrayResult(astate, PointerGetDatum(t),
false, TEXTOID,
@@ -921,13 +933,24 @@ parseRelOptions(Datum options, bool validate, relopt_kind kind,
if (j >= numoptions && validate)
{
- char *s;
- char *p;
+ char *s;
+ char *n;
+ char *p;
s = TextDatumGetCString(optiondatums[i]);
p = strchr(s, '=');
if (p)
+ {
*p = '\0';
+ n = strchr(s, '.');
+ if (n)
+ {
+ *n = '\0';
+ if (is_custom_namespace(s))
+ continue;
+ }
+ }
+
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("unrecognized parameter \"%s\"", s)));
@@ -1326,3 +1349,36 @@ tablespace_reloptions(Datum reloptions, bool validate)
return (bytea *) tsopts;
}
+
+bool
+is_reserved_namespace(char *namespace)
+{
+ static char *validnsps[] = HEAP_RELOPT_NAMESPACES;
+ bool is_reserved = false;
+ int i;
+
+ for (i = 0; validnsps[i]; i++)
+ {
+ if (pg_strcasecmp(namespace,
+ validnsps[i]) == 0)
+ {
+ is_reserved = true;
+ break;
+ }
+ }
+
+ return is_reserved;
+}
+
+bool
+is_custom_namespace(char *namespace)
+{
+
+ if (namespace == NULL)
+ return false;
+
+ if (is_reserved_namespace(namespace))
+ return false;
+
+ return true;
+}
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index 0f0c638..0d15403 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -2370,3 +2370,101 @@ TRUNCATE old_system_table;
ALTER TABLE old_system_table DROP CONSTRAINT new_system_table_pkey;
ALTER TABLE old_system_table DROP COLUMN othercol;
DROP TABLE old_system_table;
+--
+-- Custom Option Test
+--
+CREATE TABLE custom_reloption_test(id SERIAL PRIMARY KEY);
+SELECT relname, reloptions FROM pg_class WHERE oid IN ('custom_reloption_test'::regclass, 'custom_reloption_test_pkey'::regclass)
+UNION ALL
+SELECT attrelid::regclass||'.'||attname, attoptions FROM pg_attribute WHERE attrelid = 'custom_reloption_test'::regclass AND attname = 'id'
+ORDER BY 1;
+ relname | reloptions
+----------------------------+------------
+ custom_reloption_test |
+ custom_reloption_test.id |
+ custom_reloption_test_pkey |
+(3 rows)
+
+ALTER TABLE custom_reloption_test SET (fillfactor=70);
+ALTER INDEX custom_reloption_test_pkey SET (fillfactor=80);
+ALTER TABLE custom_reloption_test ALTER COLUMN id SET (n_distinct=100);
+SELECT relname, reloptions FROM pg_class WHERE oid IN ('custom_reloption_test'::regclass, 'custom_reloption_test_pkey'::regclass)
+UNION ALL
+SELECT attrelid::regclass||'.'||attname, attoptions FROM pg_attribute WHERE attrelid = 'custom_reloption_test'::regclass AND attname = 'id'
+ORDER BY 1;
+ relname | reloptions
+----------------------------+------------------
+ custom_reloption_test | {fillfactor=70}
+ custom_reloption_test.id | {n_distinct=100}
+ custom_reloption_test_pkey | {fillfactor=80}
+(3 rows)
+
+ALTER TABLE custom_reloption_test SET (xx.bdr.do_replicate=true);
+ERROR: syntax error at or near "."
+LINE 1: ALTER TABLE custom_reloption_test SET (xx.bdr.do_replicate=t...
+ ^
+ALTER INDEX custom_reloption_test_pkey SET (xx.bdr.do_replicate=true);
+ERROR: syntax error at or near "."
+LINE 1: ALTER INDEX custom_reloption_test_pkey SET (xx.bdr.do_replic...
+ ^
+ALTER TABLE custom_reloption_test ALTER COLUMN id SET (xx.bdr.do_replicate=true);
+ERROR: syntax error at or near "."
+LINE 1: ... custom_reloption_test ALTER COLUMN id SET (xx.bdr.do_replic...
+ ^
+ALTER TABLE custom_reloption_test SET (xx.bdr=true);
+ALTER INDEX custom_reloption_test_pkey SET (xx.bdr=true);
+ALTER TABLE custom_reloption_test ALTER COLUMN id SET (xx.bdr=true);
+SELECT relname, reloptions FROM pg_class WHERE oid IN ('custom_reloption_test'::regclass, 'custom_reloption_test_pkey'::regclass)
+UNION ALL
+SELECT attrelid::regclass||'.'||attname, attoptions FROM pg_attribute WHERE attrelid = 'custom_reloption_test'::regclass AND attname = 'id'
+ORDER BY 1;
+ relname | reloptions
+----------------------------+------------------------------
+ custom_reloption_test | {fillfactor=70,xx.bdr=true}
+ custom_reloption_test.id | {n_distinct=100,xx.bdr=true}
+ custom_reloption_test_pkey | {fillfactor=80,xx.bdr=true}
+(3 rows)
+
+-- test extension reloptions
+ALTER TABLE custom_reloption_test SET (plpgsql.option=true);
+ALTER INDEX custom_reloption_test_pkey SET (plpgsql.option=true);
+ALTER TABLE custom_reloption_test ALTER COLUMN id SET (plpgsql.option=true);
+SELECT relname, reloptions FROM pg_class WHERE oid IN ('custom_reloption_test'::regclass, 'custom_reloption_test_pkey'::regclass)
+UNION ALL
+SELECT attrelid::regclass||'.'||attname, attoptions FROM pg_attribute WHERE attrelid = 'custom_reloption_test'::regclass AND attname = 'id'
+ORDER BY 1;
+ relname | reloptions
+----------------------------+--------------------------------------------------
+ custom_reloption_test | {fillfactor=70,xx.bdr=true,plpgsql.option=true}
+ custom_reloption_test.id | {n_distinct=100,xx.bdr=true,plpgsql.option=true}
+ custom_reloption_test_pkey | {fillfactor=80,xx.bdr=true,plpgsql.option=true}
+(3 rows)
+
+ALTER TABLE custom_reloption_test SET (plpgsql.foo=1234);
+ALTER INDEX custom_reloption_test_pkey SET (plpgsql.foo=1234);
+ALTER TABLE custom_reloption_test ALTER COLUMN id SET (plpgsql.foo=1234);
+SELECT relname, reloptions FROM pg_class WHERE oid IN ('custom_reloption_test'::regclass, 'custom_reloption_test_pkey'::regclass)
+UNION ALL
+SELECT attrelid::regclass||'.'||attname, attoptions FROM pg_attribute WHERE attrelid = 'custom_reloption_test'::regclass AND attname = 'id'
+ORDER BY 1;
+ relname | reloptions
+----------------------------+-------------------------------------------------------------------
+ custom_reloption_test | {fillfactor=70,xx.bdr=true,plpgsql.option=true,plpgsql.foo=1234}
+ custom_reloption_test.id | {n_distinct=100,xx.bdr=true,plpgsql.option=true,plpgsql.foo=1234}
+ custom_reloption_test_pkey | {fillfactor=80,xx.bdr=true,plpgsql.option=true,plpgsql.foo=1234}
+(3 rows)
+
+ALTER TABLE custom_reloption_test RESET (plpgsql.foo);
+ALTER INDEX custom_reloption_test_pkey RESET (plpgsql.foo);
+ALTER TABLE custom_reloption_test ALTER COLUMN id RESET (plpgsql.foo);
+SELECT relname, reloptions FROM pg_class WHERE oid IN ('custom_reloption_test'::regclass, 'custom_reloption_test_pkey'::regclass)
+UNION ALL
+SELECT attrelid::regclass||'.'||attname, attoptions FROM pg_attribute WHERE attrelid = 'custom_reloption_test'::regclass AND attname = 'id'
+ORDER BY 1;
+ relname | reloptions
+----------------------------+--------------------------------------------------
+ custom_reloption_test | {fillfactor=70,xx.bdr=true,plpgsql.option=true}
+ custom_reloption_test.id | {n_distinct=100,xx.bdr=true,plpgsql.option=true}
+ custom_reloption_test_pkey | {fillfactor=80,xx.bdr=true,plpgsql.option=true}
+(3 rows)
+
diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
index 87973c1..9b04c08 100644
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -1594,3 +1594,51 @@ TRUNCATE old_system_table;
ALTER TABLE old_system_table DROP CONSTRAINT new_system_table_pkey;
ALTER TABLE old_system_table DROP COLUMN othercol;
DROP TABLE old_system_table;
+
+--
+-- Custom Option Test
+--
+CREATE TABLE custom_reloption_test(id SERIAL PRIMARY KEY);
+SELECT relname, reloptions FROM pg_class WHERE oid IN ('custom_reloption_test'::regclass, 'custom_reloption_test_pkey'::regclass)
+UNION ALL
+SELECT attrelid::regclass||'.'||attname, attoptions FROM pg_attribute WHERE attrelid = 'custom_reloption_test'::regclass AND attname = 'id'
+ORDER BY 1;
+ALTER TABLE custom_reloption_test SET (fillfactor=70);
+ALTER INDEX custom_reloption_test_pkey SET (fillfactor=80);
+ALTER TABLE custom_reloption_test ALTER COLUMN id SET (n_distinct=100);
+SELECT relname, reloptions FROM pg_class WHERE oid IN ('custom_reloption_test'::regclass, 'custom_reloption_test_pkey'::regclass)
+UNION ALL
+SELECT attrelid::regclass||'.'||attname, attoptions FROM pg_attribute WHERE attrelid = 'custom_reloption_test'::regclass AND attname = 'id'
+ORDER BY 1;
+ALTER TABLE custom_reloption_test SET (xx.bdr.do_replicate=true);
+ALTER INDEX custom_reloption_test_pkey SET (xx.bdr.do_replicate=true);
+ALTER TABLE custom_reloption_test ALTER COLUMN id SET (xx.bdr.do_replicate=true);
+ALTER TABLE custom_reloption_test SET (xx.bdr=true);
+ALTER INDEX custom_reloption_test_pkey SET (xx.bdr=true);
+ALTER TABLE custom_reloption_test ALTER COLUMN id SET (xx.bdr=true);
+SELECT relname, reloptions FROM pg_class WHERE oid IN ('custom_reloption_test'::regclass, 'custom_reloption_test_pkey'::regclass)
+UNION ALL
+SELECT attrelid::regclass||'.'||attname, attoptions FROM pg_attribute WHERE attrelid = 'custom_reloption_test'::regclass AND attname = 'id'
+ORDER BY 1;
+-- test extension reloptions
+ALTER TABLE custom_reloption_test SET (plpgsql.option=true);
+ALTER INDEX custom_reloption_test_pkey SET (plpgsql.option=true);
+ALTER TABLE custom_reloption_test ALTER COLUMN id SET (plpgsql.option=true);
+SELECT relname, reloptions FROM pg_class WHERE oid IN ('custom_reloption_test'::regclass, 'custom_reloption_test_pkey'::regclass)
+UNION ALL
+SELECT attrelid::regclass||'.'||attname, attoptions FROM pg_attribute WHERE attrelid = 'custom_reloption_test'::regclass AND attname = 'id'
+ORDER BY 1;
+ALTER TABLE custom_reloption_test SET (plpgsql.foo=1234);
+ALTER INDEX custom_reloption_test_pkey SET (plpgsql.foo=1234);
+ALTER TABLE custom_reloption_test ALTER COLUMN id SET (plpgsql.foo=1234);
+SELECT relname, reloptions FROM pg_class WHERE oid IN ('custom_reloption_test'::regclass, 'custom_reloption_test_pkey'::regclass)
+UNION ALL
+SELECT attrelid::regclass||'.'||attname, attoptions FROM pg_attribute WHERE attrelid = 'custom_reloption_test'::regclass AND attname = 'id'
+ORDER BY 1;
+ALTER TABLE custom_reloption_test RESET (plpgsql.foo);
+ALTER INDEX custom_reloption_test_pkey RESET (plpgsql.foo);
+ALTER TABLE custom_reloption_test ALTER COLUMN id RESET (plpgsql.foo);
+SELECT relname, reloptions FROM pg_class WHERE oid IN ('custom_reloption_test'::regclass, 'custom_reloption_test_pkey'::regclass)
+UNION ALL
+SELECT attrelid::regclass||'.'||attname, attoptions FROM pg_attribute WHERE attrelid = 'custom_reloption_test'::regclass AND attname = 'id'
+ORDER BY 1;
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
