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:08:00
Message-ID: 24625.1443125280@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers pgsql-hackers

Stephen Frost <sfrost(at)snowman(dot)net> writes:
> Alright, attached is an attempt at doing these renames. I went a bit
> farther since it seemed to make sense to me (at least at the time, I'm
> wondering a bit about it now), so, please provide any thoughts or
> feedback you have regarding these changes.

> Further, rename 'withCheckOptions' in Query and ModifyTable to
> 'insertedCheckClauses', ExecWithCheckOption to ExecCheckClauses,
> ri_WithCheckOptions and ri_WithCheckOptionExprs to
> ri_InsertedCheckClauses and ri_InsertedCheckExprs, respectively, all to
> make it clear that these are the check clauses which were added by the
> rewriter, not the actual WITH CHECK OPTION indication for a view (which
> is stored in reloptions for the view) nor the WITH CHECK expressions for

That commitlog entry doesn't seem to quite square with the patch,
wherein I see

- * WithCheckOptions list of WithCheckOption's to be checked
- * WithCheckOptionExprs list of WithCheckOption expr states
+ * InsertedCheckClauses list of WithCheckOption's to be checked
+ * InsertedCheckClauseExprs list of WithCheckOption expr states

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

regards, tom lane

In response to

Responses

Browse pgsql-committers by date

  From Date Subject
Next Message Tom Lane 2015-09-24 20:25:03 Re: Rename withCheckOptions to insertedCheckClauses
Previous Message Stephen Frost 2015-09-24 19:45:29 Rename withCheckOptions to insertedCheckClauses

Browse pgsql-hackers by date

  From Date Subject
Next Message Noah Misch 2015-09-24 20:21:59 Re: PGXS "check" target forcing an install ?
Previous Message Stephen Frost 2015-09-24 20:07:03 Re: No Issue Tracker - Say it Ain't So!