Re: Rename withCheckOptions to insertedCheckClauses

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Stephen Frost <sfrost(at)snowman(dot)net>
Cc: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Rename withCheckOptions to insertedCheckClauses
Date: 2015-09-24 20:25:03
Message-ID: 25061.1443126303@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers pgsql-hackers

I wrote:
> - List *ri_WithCheckOptions;
> - List *ri_WithCheckOptionExprs;
> + List *ri_InsertedCheckClauses;
> + List *ri_InsertedCheckClauseExprs;

> The distinction between a "clause" and an "expr" is not very obvious,
> and certainly most other places in the code use those terms pretty
> interchangeably, so I find both the old and new names unclear here.
> How about ri_InsertedCheckClauseStates instead for the second list?
> And similarly if you're using "Expr" to mean ExprState anywhere else.

Actually ... does struct ResultRelInfo need to carry the original WCO
clauses at all, rather than just the exprstate list? In most places
we do not store expr and exprstate lists in the same node in the first
place, so we can get away with using the same field name for corresponding
lists in plan and planstate nodes. That's why we don't already have a
convention like "fooStates" for such lists.

Another thought is that as long as these are lists specifically of
WithCheckOption nodes, and not arbitrary expressions, "clause" isn't an
especially good term for them; it implies generality that isn't there.
And CheckClauses invites confusion with, for example, CHECK clauses of
domain types. So maybe better names would be "ri_InsertedCheckOptions"
(and "ri_InsertedCheckOptionStates" if you still need that). Or maybe
"ri_InsertedWCOClauses" and "ri_InsertedWCOClauseStates". I'm less sure
about whether this is an improvement, though.

regards, tom lane

In response to

Responses

Browse pgsql-committers by date

  From Date Subject
Next Message Tom Lane 2015-09-24 22:36:03 pgsql: Allow planner to use expression-index stats for function calls i
Previous Message Tom Lane 2015-09-24 20:08:00 Re: Rename withCheckOptions to insertedCheckClauses

Browse pgsql-hackers by date

  From Date Subject
Next Message Feng Tian 2015-09-24 20:29:52 Decimal64 and Decimal128
Previous Message Noah Misch 2015-09-24 20:21:59 Re: PGXS "check" target forcing an install ?