On 2015-09-02 17:31, Andres Freund wrote:
On 2015-09-02 17:27:38 +0200, Andres Freund wrote:
1) Passing the list of parents through the cascade DefElem strikes me as
incredibly ugly.
For one the cascade option really should take a true/false type option
on the C level (so you can do defGetBoolean()), for another passing
through the list of parents via DefElem->arg seems wrong. You're
supposed to be able to copy parsenodes and at the very least that's
broken by the approach.
I think the fix here is to split off the bulk of CreateExtension() into
an internal function that takes additional parameters.
Yes that sounds cleaner. Just as a side note, List is a Node and does
have copy support (and we pass List as DefElem->arg from gram.y in
several places).
> 2) I don't like the control flow around the schema selection.
>
> It seems to be getting a bit arcane. How about instead moving the
> "extension \"%s\" must be installed in schema \"%s\" check into the if
> (control->schema != NULL) block and check for d_schema after it? That
> should look cleaner.
>
I did something like that in one of the revisions, the complaint there
was that it changes order of errors you get in situation when the schema
is not the same as the one in control file and it also does not exist.
--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
