Re: BUG #9227: Error on SELECT ROW OVERLAPS ROW with single ROW argument

pythonesque@gmail.com writes:
> # SELECT ROW (1) OVERLAPS ROW (2);
> ERROR:  XX000: unrecognized node type: 656
> LOCATION:  transformExprRecurse, parse_expr.c:359

> I'm not sure why this even parses, as it's not documented to do so.  I do
> notice, however, that in makeOverlaps in gram.y creates a recursive list
> when largs or args has only one argument, which seems wrong to me.

Huh, yeah:

    if (list_length(largs) == 1)
        largs = lappend(largs, largs);
    else if (list_length(largs) != 2)
        ereport(ERROR,
                (errcode(ERRCODE_SYNTAX_ERROR),
                 errmsg("wrong number of parameters on left side of OVERLAPS expression"),
                 parser_errposition(location)));

(and similarly for rargs).  What this is evidently trying to do is
convert

    (foo) overlaps ...

to

    (foo, foo) overlaps ...

which is perhaps sensible on the grounds that a point in time can be
considered as a zero-width interval.  However, as you say, this behavior is
undocumented, and what's more I can find no support for it in any version
of the SQL standard.

My guess is that this code might've originally worked as intended, but it
got broken when Neil Conway rewrote our List implementation, which was a
darn long time ago now.

A quick browse in our git history shows that OVERLAPS support
was introduced in commit 64568100787a5d03d036e70b32147385a35245e2 of
2000-03-14, and the self-append-the-same-list behavior was in that
version though it looked rather different.  Neil's work dates to 2004.

[ thinks some more ... ]  Actually, even back when Lists didn't have
headers, it's not immediately clear to me that this code could've worked.
In the current code we end up with a List that includes itself, which
surely ain't gonna work.

While it would be a reasonably simple change to make this work as
originally intended, I'm strongly tempted to just rip it out instead,
and only support the SQL-mandated syntax.  If anyone was using this
undocumented feature, you'd think they'd have complained sometime in
the last ten years.  And making it work would really entail adding
documentation and a regression test case, so it'd be substantially
more effort than just killing the "list_length == 1" case.

            regards, tom lane