Обсуждение: A Typo in regress/sql/privileges.sql
Hi, This is my first post to -hackers. I found typos in privileges.sql and privileges.out Please find attached a patch. Best regards, Tatsuro Yamada
Вложения
On Wed, Dec 16, 2015 at 11:51 PM, Tatsuro Yamada <yamada.tatsuro@lab.ntt.co.jp> wrote: > I found typos in privileges.sql and privileges.out > Please find attached a patch. Thanks, good catch. But even aside from this particular issue, isn't that comment in need of a little more love? An inference means a deduction, or something you can figure out from something else. ON CONFLICT (four) is not an inference. Why don't we say: -- Check that the columns in the ON CONFLICT clause require select privileges -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On 2015-12-18 13:50:34 -0500, Robert Haas wrote: > On Wed, Dec 16, 2015 at 11:51 PM, Tatsuro Yamada > <yamada.tatsuro@lab.ntt.co.jp> wrote: > > I found typos in privileges.sql and privileges.out > > Please find attached a patch. > > Thanks, good catch. But even aside from this particular issue, isn't > that comment in need of a little more love? An inference means a > deduction, or something you can figure out from something else. ON > CONFLICT (four) is not an inference. It's the index(es) that are inferred, from the ON(columns) and the ON CONFLICT's WHERE clause. If we want to get rid of that terminology we'd need to start elsewhere, and it'd be a bigger patch. Andres
On Fri, Dec 18, 2015 at 1:57 PM, Andres Freund <andres@anarazel.de> wrote: > On 2015-12-18 13:50:34 -0500, Robert Haas wrote: >> On Wed, Dec 16, 2015 at 11:51 PM, Tatsuro Yamada >> <yamada.tatsuro@lab.ntt.co.jp> wrote: >> > I found typos in privileges.sql and privileges.out >> > Please find attached a patch. >> >> Thanks, good catch. But even aside from this particular issue, isn't >> that comment in need of a little more love? An inference means a >> deduction, or something you can figure out from something else. ON >> CONFLICT (four) is not an inference. > > It's the index(es) that are inferred, from the ON(columns) and the ON > CONFLICT's WHERE clause. If we want to get rid of that terminology we'd > need to start elsewhere, and it'd be a bigger patch. It might be an inference specification, but there is no way that it is an inference. If we use that terminology in other places, it's wrong there, too. Mind you, I don't think "inference specification" is very good terminology, but what's there right now is just wrong. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Fri, Dec 18, 2015 at 12:52 PM, Robert Haas <robertmhaas@gmail.com> wrote: > Mind you, I don't think "inference specification" is very good > terminology, but what's there right now is just wrong. It doesn't appear in the documentation. The term "inference specification" only appears where it's necessary to precisely describe the input to unique index inference. -- Peter Geoghegan
On Tue, Dec 22, 2015 at 4:36 AM, Peter Geoghegan <pg@heroku.com> wrote: > On Fri, Dec 18, 2015 at 12:52 PM, Robert Haas <robertmhaas@gmail.com> wrote: >> Mind you, I don't think "inference specification" is very good >> terminology, but what's there right now is just wrong. > > It doesn't appear in the documentation. The term "inference > specification" only appears where it's necessary to precisely describe > the input to unique index inference. Well, we can change this to say "inference specification", but I still think calling it the "ON CONFLICT" clause would be clearer in this context. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas wrote: > On Tue, Dec 22, 2015 at 4:36 AM, Peter Geoghegan <pg@heroku.com> wrote: > > On Fri, Dec 18, 2015 at 12:52 PM, Robert Haas <robertmhaas@gmail.com> wrote: > >> Mind you, I don't think "inference specification" is very good > >> terminology, but what's there right now is just wrong. > > > > It doesn't appear in the documentation. The term "inference > > specification" only appears where it's necessary to precisely describe > > the input to unique index inference. > > Well, we can change this to say "inference specification", but I still > think calling it the "ON CONFLICT" clause would be clearer in this > context. TBH I'm kinda inclined to sort this out by removing all usage of the word "inference" everywhere --- error messages and code comments and documentation wording, and replace it with some other wording as appropriate for each context. -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Tue, Dec 22, 2015 at 1:39 PM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote: > Robert Haas wrote: >> On Tue, Dec 22, 2015 at 4:36 AM, Peter Geoghegan <pg@heroku.com> wrote: >> > On Fri, Dec 18, 2015 at 12:52 PM, Robert Haas <robertmhaas@gmail.com> wrote: >> >> Mind you, I don't think "inference specification" is very good >> >> terminology, but what's there right now is just wrong. >> > >> > It doesn't appear in the documentation. The term "inference >> > specification" only appears where it's necessary to precisely describe >> > the input to unique index inference. >> >> Well, we can change this to say "inference specification", but I still >> think calling it the "ON CONFLICT" clause would be clearer in this >> context. > > TBH I'm kinda inclined to sort this out by removing all usage of the > word "inference" everywhere --- error messages and code comments and > documentation wording, and replace it with some other wording as > appropriate for each context. I would not object to that. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Tue, Dec 22, 2015 at 11:01 AM, Robert Haas <robertmhaas@gmail.com> wrote: >> TBH I'm kinda inclined to sort this out by removing all usage of the >> word "inference" everywhere --- error messages and code comments and >> documentation wording, and replace it with some other wording as >> appropriate for each context. > > I would not object to that. Why? There is nothing wrong with the term inference. It's perfectly descriptive. It's also a term I've used as part of many talks about ON CONFLICT. No one has said a word about it before now. -- Peter Geoghegan
On Tue, Dec 22, 2015 at 2:23 PM, Peter Geoghegan <pg@heroku.com> wrote: > On Tue, Dec 22, 2015 at 11:01 AM, Robert Haas <robertmhaas@gmail.com> wrote: >>> TBH I'm kinda inclined to sort this out by removing all usage of the >>> word "inference" everywhere --- error messages and code comments and >>> documentation wording, and replace it with some other wording as >>> appropriate for each context. >> >> I would not object to that. > > Why? There is nothing wrong with the term inference. It's perfectly > descriptive. It's also a term I've used as part of many talks about ON > CONFLICT. No one has said a word about it before now. If it's an axiom that there is nothing wrong with the term inference, then obviously we should not change anything. But that seems to me to be putting the cart before the horse. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Tue, Dec 22, 2015 at 2:58 PM, Robert Haas <robertmhaas@gmail.com> wrote: > If it's an axiom that there is nothing wrong with the term inference, > then obviously we should not change anything. But that seems to me to > be putting the cart before the horse. OK, then. What's wrong with the term inference? -- Peter Geoghegan
On Tue, Dec 22, 2015 at 6:10 PM, Peter Geoghegan <pg@heroku.com> wrote: > On Tue, Dec 22, 2015 at 2:58 PM, Robert Haas <robertmhaas@gmail.com> wrote: >> If it's an axiom that there is nothing wrong with the term inference, >> then obviously we should not change anything. But that seems to me to >> be putting the cart before the horse. > > OK, then. What's wrong with the term inference? In my opinion a term more closely coupled to the concrete syntax would be easier to understand. I have no objection to referring to the *process* of trying to deduce a suitable index from the ON CONFLICT clause as "inference". But calling the ON CONFLICT clause an "inference specification" is, in my opinion, an unnecessary oblique way of referring to it. If you renamed InferenceElem to InsertOnConflictElem, I think that would be strictly more clear. I also kind of dislike the way that this feature has commandeered the word "inference" for its own use. The server infers a lot of things, and it might infer more in the future, but if somebody else ever wants to use the word for something, they're going to have a hard time not getting false hits when they grep. Like there are comments like this in the regression tests: -- inference succeeds: -- inference fails: That's pretty darn generic. Whether the command succeeds or fails will be clear enough from the output. Mind you, I don't think the problems with inference are a super-serious issue, or I would have raised a stink sooner. But I don't find it particularly appealing, either. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Tue, Dec 22, 2015 at 3:38 PM, Robert Haas <robertmhaas@gmail.com> wrote:
> In my opinion a term more closely coupled to the concrete syntax would
> be easier to understand. I have no objection to referring to the
> *process* of trying to deduce a suitable index from the ON CONFLICT
> clause as "inference". But calling the ON CONFLICT clause an
> "inference specification" is, in my opinion, an unnecessary oblique
> way of referring to it. If you renamed InferenceElem to
> InsertOnConflictElem, I think that would be strictly more clear.
The documentation uses the term "unique index inference" to introduce
the concept. It then uses "inference" as a shorthand a couple of times
when the context is very well established. So I don't see that I've
done that at all.
As for the one user-visible error messages where the term "inference
specification" is used, that message also has a hint that draws
particular attention to what is meant:
if (onConflictClause->action == ONCONFLICT_UPDATE && !infer) ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR), errmsg("ON CONFLICT DO UPDATE requires inference
specification or constraint name"), errhint("For example, ON CONFLICT (column_name)."),
parser_errposition(pstate, exprLocation((Node *) onConflictClause))));
(There is one appearance of "inference specification" in a defensive
elog() call).
So I still don't understand why anyone takes issue with this. It's a
total mystery to me.
--
Peter Geoghegan
On 2015/12/23 8:45, Peter Geoghegan wrote:
> On Tue, Dec 22, 2015 at 3:38 PM, Robert Haas <robertmhaas@gmail.com> wrote:
>> In my opinion a term more closely coupled to the concrete syntax would
>> be easier to understand. I have no objection to referring to the
>> *process* of trying to deduce a suitable index from the ON CONFLICT
>> clause as "inference". But calling the ON CONFLICT clause an
>> "inference specification" is, in my opinion, an unnecessary oblique
>> way of referring to it. If you renamed InferenceElem to
>> InsertOnConflictElem, I think that would be strictly more clear.
>
> The documentation uses the term "unique index inference" to introduce
> the concept. It then uses "inference" as a shorthand a couple of times
> when the context is very well established. So I don't see that I've
> done that at all.
>
> As for the one user-visible error messages where the term "inference
> specification" is used, that message also has a hint that draws
> particular attention to what is meant:
>
> if (onConflictClause->action == ONCONFLICT_UPDATE && !infer)
> ereport(ERROR,
> (errcode(ERRCODE_SYNTAX_ERROR),
> errmsg("ON CONFLICT DO UPDATE requires inference
> specification or constraint name"),
> errhint("For example, ON CONFLICT (column_name)."),
> parser_errposition(pstate,
> exprLocation((Node *) onConflictClause))));
>
> (There is one appearance of "inference specification" in a defensive
> elog() call).
>
> So I still don't understand why anyone takes issue with this. It's a
> total mystery to me.
IMHO, a term like "arbiter (index) specification" would be clear as well.
I don't deny though that there is the process of inference ("choosing" as
INSERT documentation calls it).
Thanks,
Amit