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

Поиск
Список
Период
Сортировка
От David G. Johnston
Тема Re: [PATCH][Documination] Add optional USING keyword before opclass name in INSERT statemet
Дата
Msg-id CAKFQuwYA5qMtFy=RJ=9_WyLq-bUqQBeorXM9d09tpT0edd2=dQ@mail.gmail.com
обсуждение исходный текст
Ответ на Re: [PATCH][Documination] Add optional USING keyword before opclass name in INSERT statemet  (Nikolay Shaplov <n.shaplov@postgrespro.ru>)
Список pgsql-hackers
On Thu, May 26, 2016 at 8:09 AM, Nikolay Shaplov <n.shaplov@postgrespro.ru> wrote:
В письме от 24 мая 2016 12:47:20 пользователь Tom Lane написал:
> Nikolay Shaplov <n.shaplov@postgrespro.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.

 

В списке pgsql-hackers по дате отправления:

Предыдущее
От: Christoph Berg
Дата:
Сообщение: Re: Login into PostgreSQL without password
Следующее
От: Tom Lane
Дата:
Сообщение: Re: [PATCH][Documination] Add optional USING keyword before opclass name in INSERT statemet