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
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 |
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 ? |