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
insteadmoving the> "extension \"%s\" must be installed in schema \"%s\" check into the if> (control->schema != NULL)
blockand 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