Re: WITH RECURSIVE ... CYCLE in vanilla SQL: issues with arrays of rows

On Wed, Oct 8, 2008 at 4:27 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> "Merlin Moncure" <mmoncure@gmail.com> writes:
>> On Tue, Oct 7, 2008 at 9:58 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> * Instead of the above, we could try to make
>>> ROW(some columns) = ANY (array variable)
>>> work.  This is shorter than the above syntax and would presumably have
>>> a lot less overhead too.  But it doesn't work right now, not even for
>>> named rowtypes much less anonymous ones.
>
>> By extension, would this also mean things like
>> select row(1,2,3)::foo = foo from foo;
>> Would work (presumably as row-wise comparison does)?
>
> Well, it turned out to be easier than I thought to get the base case
> working --- all that's necessary is to define an array type for RECORD
> and add generic comparison functions, and the cases that are needed for
> recursive cycle detection Just Work!  See attached WIP patch, and
> particularly note the new test case in with.sql.
>
> The part that actually seems to need some thought is the relationship
> between this and operations on named rowtypes.  In the patch I tweaked
> parse_coerce.c to treat coercion from a named rowtype's array type to
> record[] as an allowed binary-compatible case, but I didn't do the
> other direction yet (I'm not fully convinced that it's necessary).
>
> What I'm really not clear about is the extent to which record[] ought
> to be treated like a polymorphic type --- should we consider that
> it acts like an "anyrecordarray" type, or is that a distinct notion?
> Do we even want that?  record itself is not considered a polymorphic
> type, though it has some similar qualities.
>
> Another point worth making is that I made the comparisons work like
> IS NOT DISTINCT, ie, they don't return NULL just because some field of
> the row is NULL.  This is *not* like SQL-spec row comparison, but we
> can't build a btree opclass from these functions if they insist on
> returning null for null fields.  (Our array comparisons work like this,
> too.)  I'm not sure how big a deal that is either way, but I am pretty
> sure that IS NOT DISTINCT is the semantics you need to have if you want
> cycle detection to work reliably.  (Hm, is that a bug in the spec?
> They say to use = rather than DISTINCT in cycle checking ...)
>
> Comments?  Do we want to go forward with this?

The record ops work as promised.  IMO this patch stands in its own
merits with or without the CTE changes.  I played around with it and
noticed a couple of oddities:

select foo = foo from foo;  --works
but

select distinct foo from foo; --yields
ERROR:  could not identify an equality operator for type foo
also,

select foo from foo order by foo;
ERROR:  could not identify an ordering operator for type foo

postgres=# create index foo_idx on foo((foo));
ERROR:  cache lookup failed for type 0

The above are probably not required to fulfill the CTE
requirements...but would be nice.

merlin