Re: Assertion failure on UNLOGGED VIEW and SEQUENCE

On Fri, Feb 18, 2011 at 2:17 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Robert Haas <robertmhaas(at)gmail(dot)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