On Wed, 6 Jul 2022 at 19:35, Matthias van de Meent
<boekewurm+postgres@gmail.com> wrote:
>
> On Sun, 15 May 2022 at 14:20, Simon Riggs <simon.riggs@enterprisedb.com> wrote:
> >
> > Currently, CREATE STATS requires you to think of a name for each stats
> > object, which is fairly painful, so users would prefer an
> > automatically assigned name.
> >
> > Attached patch allows this, which turns out to be very simple, since a
> > name assignment function already exists.
> >
> > The generated name is simple, but that's exactly what users do anyway,
> > so it is not too bad.
>
> Cool.
Thanks for your review.
> > Tests, docs included.
>
> Something I noticed is that this grammar change is quite different
> from how create index specifies its optional name. Because we already
> have a seperate statement sections for with and without IF NOT EXISTS,
> adding another branch will add even more duplication. Using a new
> opt_name production (potentially renamed from opt_index_name?) would
> probably reduce the amount of duplication in the grammar.
There are various other ways of doing this and, yes, we could refactor
other parts of the grammar to make this work. There is a specific
guideline about patch submission that says the best way to get a patch
rejected is to include unnecessary changes. With that in mind, let's
keep the patch simple and exactly aimed at the original purpose.
I'll leave it for committers to decide whether other refactoring is wanted.
> We might be able to use opt_if_not_exists to fully remove the
> duplicated grammars, but I don't think we would be able to keep the
> "CREATE STATISTICS IF NOT EXISTS <<no name>> ON col1, col2 FROM table"
> syntax illegal.
>
> Please also update the comment in gram.y above the updated section
> that details the expected grammar for CREATE STATISTICS, as you seem
> to have overlooked that copy of grammar documentation.
I have made the comment show that the name is optional, thank you.
> Apart from these two small issues, this passes tests and seems complete.
Patch v4 attached
--
Simon Riggs http://www.EnterpriseDB.com/