On Fri, Oct 3, 2014 at 5:26 AM, Marti Raudsepp <
marti@juffo.org> wrote:
>
> "On Fri, Oct 3, 2014 at 6:25 AM, Fabrízio de Royes Mello
> <
fabriziomello@gmail.com> wrote:
> >> Documentation: I would prefer if the explanation were consistent with
>
> > "Do not throw an error if the index already exists. A notice is issued in
> > this case."
> > Fixed in that way. Ok?
>
> And also "Note that there is no guarantee that the existing index is
> anything like the one that would have been created."
>
Fixed.
> >> I think ERRCODE_SYNTAX_ERROR makes more sense, it's something that we
> >> decided we *don't want* to support.
> > I don't think so. It's the same as CREATE SCHEMA IF NOT EXISTS that not
> > support to include schema elements.
>
> IMO that's wrong too, the CREATE SCHEMA documentation doesn't list it
> as valid syntax.
>
> But now that you split the syntax in two, you can simply replace
> "opt_index_name" with "index_name" and it will naturally cause a
> syntax error without the need for an if(). What do you think?
> Patch attached, which applies on top of your v4 patch.
>
Fine. Thanks.
> On Fri, Oct 3, 2014 at 6:29 AM, Fabrízio de Royes Mello
> <
fabriziomello@gmail.com> wrote:
> >> I would also move this check to after all the attributes have been
> >> assigned, rather than splitting the assignments in half.
> > Why? If you see other places in gram.y it's a common usage...
>
> Looks cleaner to me: first input all the fields, then validate. And
> there are examples like this too, like "COPY select_with_parens". But
> this is moot now, if you agree with my grammar change.
>
I agree with your grammar change.
> On Fri, Oct 3, 2014 at 6:35 AM, Fabrízio de Royes Mello
> <
fabriziomello@gmail.com> wrote:
> > And just to remember please add your review to commitfest
>
> Thanks for reminding, I always forget to update the CommitFest... :(
>
You're welcome.
The version 5 (attached) contains all discussed until now.
Regards.
