On Fri, Feb 18, 2011 at 2:17 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> OK. Proposed patch attached. It looks to me like an unlogged view is
>> inherently nonsensical, whereas an unlogged sequence is sensible but
>> we don't implement it (and may never implement it, absent some
>> evidence that the overhead of WAL logging sequence changes is worth
>> getting excited about), so I wrote the error messages to reflect that
>> distinction. I also added a couple of matching regression tests, and
>> documented that UNLOGGED works with SELECT INTO. I put the check for
>> views in DefineView adjacent to the other check that already cares
>> about relpersistence, and I put the one in DefineSequence to match, at
>> the top for lack of any compelling theory of where it ought to go. I
>> looked at stuffing it all the way down into DefineRelation but that
>> looks like it would require some other rejiggering of existing logic
>> and assertions, which seems pointless and potentially prone to
>> breaking things.
>
> Regression tests for this seem pretty pointless (ie, a waste of cycles
> forevermore). +1 for where you put the tests, but I don't think
> ERRCODE_SYNTAX_ERROR is an appropriate errcode. I'd go with
> FEATURE_NOT_SUPPORTED for both, I think.
I hesitate to use FEATURE_NOT_SUPPORTED for something that's
nonsensical anyway. I picked SYNTAX_ERROR after some scrutiny of what
I believe to be parallel cases, such as EXPLAIN (FOO) SELECT 1 and
CREATE TABLE t AS SELECT 1 INTO me.
> Also, it might be worth
> putting some of the above justification into the comments, eg
>
> /* Unlogged sequences are not implemented --- not clear if useful */
>
> versus
>
> /* Unlogged views are pretty nonsensical */
>
> rather than duplicate comments describing non-duplicate cases.
Good idea.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company