Re: [PATCH] Opclass parameters

On Wed, Dec 5, 2018 at 6:58 PM Nikita Glukhov <n(dot)gluhov(at)postgrespro(dot)ru> wrote:
> I agree that we should distinguish per-index and per-column options, but they
> can also be AM-specific and opclass-specific.

True, but an index is bound to a single AM, and a column is bound to a
single opclass which is bound to a single AM. So I'm not very worried
about name collisions. Can't we just tell opclass authors to pick
names that are unlikely to collide with names chose by the AM or names
that are globally reserved? It's hard to imagine that we're ever
going to have more than a dozen or so options that could possibly
apply to a column, so splitting things up into different namespaces
seems like an unnecessary burden on the user.

> 'fastupdate' option for GIN is an example of AM-specific per-index option.
>
> ASC/DESC and NULLS LAST/FIRST are examples of AM-class-specific per-column
> options having special SQL syntax. "AM-class-specific" here means "specific
> only for the class of AMs that support ordering". Now they are stored as flags
> in pg_index.indoption[] but later can be moved to pg_attribute.attoptions.

Or left where they are.

> And another problem is the options with default values. They may be not
> explicitly specified by the user, and in this case in current implementation
> nothing will be stored in the catalog because default values can be obtained
> from the code. But this will not allow changing of default values without
> compatibility issues. So I think it would be better to store both default and
> explicitly specified values of all opclass options, but this will require a
> major refactoring of current API.

Hmm. I think if the default ever changes, it will require a new major
release, and we can compensate in pg_dump. That means that a dump
taken with an old version will not preserve the options. However,
using the pg_dump from the newest release is the recommended
procedure, and if you don't do that, you might get outright failures.
Failing to preserve an option value in the rare case that a default
was changed seems less bad than that.

> Also I have idea to define list of opclass parameters declaratively when opclass
> is created using syntax like the following:
>
> CREATE OPERATOR CLASS name [ (param_name param_type [= default_value] [,...]) ]
> FOR TYPE ... AS (
> { OPTIONS function_name ( arg_type [,...] ) /* reloptions => binary */
> | OPERATOR ...
> } [,...]
> )

I'm not sure exposing SQL syntax for this is a very good idea.

> "[opclass] WITH OPTIONS (options)" looks too verbose, of course.

It's not that bad.

> "[opclass] WITH (options)" looks acceptable, but it seems to conflict with
> exclusion constraints syntax ("index_elem WITH operator").

Are you sure? The operator can't be (

But it might be confusing anyhow.

> "opclass (options)" looks the most natural to me. But we still need some
> keyword before the parentheses when the opclass is not specified since we
> can't distinguish "func_name (func_params)" and "col_name (opclass_options)"
> in grammar.

Are you sure? What's the SQL syntax where there is actually a problem
here? CREATE INDEX requires parentheses around a non-trivial
expression.

How about just OPTIONS (options) ?

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company