Re: Add CREATE support to event triggers - Mailing list pgsql-hackers

Robert Haas escribió:
> On Tue, Feb 4, 2014 at 12:11 AM, Alvaro Herrera
> <alvherre@2ndquadrant.com> wrote:
> > I have run into some issues, though:
> >
> > 1. certain types, particularly timestamp/timestamptz but really this
> > could happen for any type, have unusual typmod output behavior.  For
> > those one cannot just use the schema-qualified catalog names and then
> > append the typmod at the end; because what you end up is something like
> >    pg_catalog.timestamptz(4) with time zone
> > because, for whatever reason, the "with time zone" is part of typmod
> > output.  But this doesn't work at all for input.  I'm not sure how to
> > solve this.
> 
> How about doing whatever pg_dump does?

We use format_type() for that as far as I know.  What it does
differently is use undecorated names defined by the standard for some
types, which are never schema qualified and are never ambiguous because
they are reserved words that would require quoting if used by
user-defined type names.  We can't use that here: somewhere upthread we
noticed issues when using those which is why we're now trying to use
catalog names instead of those special names.  (I don't think it's
impossible to use such names: we just need to ensure we handle quoting
correctly for the funny cases such as "char" and "bit.)

One idea is to chop the typmod output string at the closing parens.
That seems to work well for the types that come with core code ... and
the rule with the funny string at the end after the parenthised part of
the typmod works only by type names with hardcoded productions in
gram.y, so there is no danger that outside code will have a problem with
that strategy.

(I verified PostGIS types with typmods just to see an example of
out-of-core code at work, and unsurprisingly it only emits stuff inside
parens.)

> > 2. I have been having object definitions be emitted complete; in
> > particular, sequences have OWNED BY clauses when they have an owning
> > column.  But this doesn't work with a SERIAL column, because we get
> > output like this:

> Well, the sequence can't depend on a table column that doesn't exist
> yet, so if it's in effect doing what you've shown there, it's
> "cheating" by virtue of knowing that nobody can observe the
> intermediate state.  Strictly speaking, there's nothing "wrong" with
> emitting those commands just as you have them there; they won't run,
> but if what you want to do is log what's happened rather than replay
> it, that's OK.  Producing output that is actually executable is a
> strictly harder problem than producing output that accurately
> describes what happened.  As you say, pg_dump already splits things
> and getting executable output out of this facility will require the
> same kinds of tricks here.

Yeah.  Moreover this will require that this new event trigger code is
able to handle (at least certain kinds of) ALTER, which expands this
patch in scope by a wide margin.

Producing executable commands is an important goal.

> This gets back to my worry about maintaining two or three copies of
> the code that solve many of the same problems in quite different
> ways...

Agreed.  It would be real good to be able to use this code for pg_dump
too, but it seems dubious.  The two environments seem just too different
to be able to reuse anything.  But if you see a way, by all means shout.

> > 3. It is possible to coerce ruleutils.c to emit always-qualified names
> > by using PushOverrideSearchPath() facility; but actually this doesn't
> > always work, because some places in namespace.c believe that
> > PG_CATALOG_NAMESPACE is always visible and so certain objects are not
> > qualified.  In particular, text columns using default collation will be
> > emitted as having collation "default" and not pg_catalog.default as I
> > would have initially expected.  Right now it doesn't seem like this is a
> > problem, but it is unusual.
> 
> We have a quote_all_identifiers flag.  We could have a
> schema_qualify_all_identifiers flag, too.

Actually the bit about collations was just a garden variety bug: turns
out I was using a %{}I specifier (and correspondingly only the collation
name as a string) instead of %{}D and get_catalog_object_by_oid() to
match.  I'm not yet sure if this new flag you suggest will really be
needed, but thanks for the idea.

> Then again, why is the behavior of schema-qualifying absolutely
> everything even desirable?

Well, someone could create a collation in another schema with the same
name as a system collation and the command would become ambiguous.  For
example, this command create table f (a text collate "POSIX");
creates a column whose collation is either public."POSIX" or the system
POSIX collation, depending on whether public appears before pg_catalog
in search_path.  Having someone create a POSIX collation in a schema
that appears earlier than pg_catalog is pretty far-fetched, but ISTM
that we really need to cover that scenario.

-- 
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services