Re: [PATCH][Documination] Add optional USING keyword before opclass name in INSERT statemet

On Thu, May 26, 2016 at 8:09 AM, Nikolay Shaplov <n(dot)shaplov(at)postgrespro(dot)ru>
wrote:

> В письме от 24 мая 2016 12:47:20 пользователь Tom Lane написал:
> > Nikolay Shaplov <n(dot)shaplov(at)postgrespro(dot)ru> writes:
> > > If I read gram.y code for insert statement, I see that there is an
> > > optional
> > > USING keyword before opclass name
> > >
> > > opt_class: any_name { $$ = $1; }
> > >
> > > | USING any_name { $$ = $2; }
> > > | /*EMPTY*/ { $$ = NIL; }
> > >
> > > ;
> > >
> > > but it the documentation this keyword is omitted.
> >
> > I think we should seriously consider fixing this code/docs discrepancy
> > by making the code match the docs, not vice versa. That is, let's just
> > remove the USING alternative here entirely.
> >
> > If we wanted to make the docs match the code, it would not only be
> > CREATE INDEX that would have to be patched, because that's not the
> > only place that index_elem can appear. As far as I can find in a
> > quick search, none of the relevant statements have ever documented
> > that USING is allowed here; nor does it appear that any client-side
> > code of ours makes use of the keyword.
> >
> > Also, because USING is already used elsewhere in CREATE INDEX (to
> > introduce the optional index AM name), I think that documenting its
> > use in this clause would add confusion not subtract it. References
> > to "the USING clause in CREATE INDEX" would become ambiguous.
> >
> > This wouldn't be something to back-patch, of course, but I think it's
> > an entirely reasonable change to make in HEAD.
> >
> > Comments?
>
> I have two arguments for not removing USING there.
>
> 1. Backward compatibility. Are you sure, that nobody ever wrote a code
> using
> this "USING" keyword? I would say it is unlikely, but will not be sure
> 100%.
> Dropping this backward compatibility (even with small chance that it was
> ever
> used) for no practical reason is not wise at all. If it might bring some
> pain
> to somebody, then why do it after all...
>

IIUC, since pg_dump/pg_restore never includes this there is not hazard on
upgrading or restoration. Furthermore, CREATE INDEX is an administrative
function and thus not likely to cause applications to become inoperative.​

The surface area here is small enough that the decision to disallow should
not be taken off the table.

> 2. I think expression with USING in it is more human readable:
>
> CREATE INDEX (xxx op_yyy);
>
> is less sensible then
>
> CREATE INDEX (xxx USING op_yyy);
>
> in my opinion. In second example person that does not know SQL at all, will
> understand that xxx is main object or action, and op_yyy is about how this
> xxx
> will be done or used.
>
> If somebody would like to write more readable code, why we should forbid
> it to
> him?
>

​I agree.​

​The argument that having a second portion of the query utilizing the USING
keyword would make explanation and comprehension more difficult is also one
I agree with.​

That said we apparently used to interject the word WITH in between but that
ended up generating a potential ambiguity so WITH was changed to USING.
This was circa 1997...

The authors of the docs for the past nearly 20 years have assumed that
there is no USING clause in that location.

If someone was willing to write a doc patch that passed muster before 10.0
goes live its possible that we'd revert the change and commit the doc
patch. The cost/benefit of that effort is not particularly appealing and
the project seems content to take the more expedient (and now without its
own merits) path forward.

> 2.1. As far as I can get the general idea of SQL, there is a tendency to
> put
> keywords (optional or not) between to object names. Like this
>
> SELECT a _AS_ b from ....
>
> I like this tendency
>

Not germane to this discussion.​

> 2.2. I am not familiar with SQL ISO standart, and I suppose there is no
> USING
> at all in that case, but I think it would be good to look there to check
> for
> it or for something similar
>

​Indexes are outside the purview of the ISO SQL Standard.​

> 2.3. And the last, when I found out about this keyword, I started to use
> it in
> my SQL statements that I use while development, and I just liked it. I will
> miss it if you remove it ;-)
>

​Thank you for your patronage and your sacrifice.​

Is there an address where we can send your purple heart :)

While not a great policy to adhere to, a reversion in a 10.x patch release
wouldn't be particularly difficult should people choose to complain rather
than adapt.

​David J.