Thread: Window function bug

In branch postgresql/master:

SELECT SUM(SUM(a)) OVER ()
 FROM (SELECT NULL::int4 AS a WHERE FALSE) R;

ERROR:  XX000: cannot extract attribute from empty tuple slot

Honestly, I'm not sure what the semantics of that are supposed to be. Is
it even allowed by the standard?

Regards,
    Jeff Davis

Jeff Davis <pgsql@j-davis.com> writes:
> In branch postgresql/master:
> SELECT SUM(SUM(a)) OVER ()
>  FROM (SELECT NULL::int4 AS a WHERE FALSE) R;
> ERROR:  XX000: cannot extract attribute from empty tuple slot

Huh, interesting.

> Honestly, I'm not sure what the semantics of that are supposed to be. Is
> it even allowed by the standard?

Yeah, I believe so.  Aggregate calls within window function calls are
supposed to be legal.  They're not terribly useful unless there's a
GROUP BY clause --- when there is, you get a row per group out of the
aggregates, and then it's sensible to apply windowing functions on that
rowset.  This is a pretty degenerate case ... but it ought not fail.

After tracing through it, it seems the bug is that the planner generates
a targetlist for the Agg node containing "a, SUM(a)", and then when that
is evaluated for a case where no row was ever produced by the subquery,
the executor quite properly fails, since there's noplace to get a value
of "a" from.  The targetlist is built by these statements in planner.c:

            window_tlist = flatten_tlist(tlist);
            if (parse->hasAggs)
                window_tlist = add_to_flat_tlist(window_tlist,
                                            pull_agg_clause((Node *) tlist));
            window_tlist = add_volatile_sort_exprs(window_tlist, tlist,
                                                   activeWindows);

so I guess the answer is that this code ought to avoid adding Vars that
are only mentioned within aggregates.  Perhaps also omit those only used
within volatile sort expressions, though I think that would just be an
efficiency issue not a correctness issue, and it may be unreasonably
expensive to determine that.

            regards, tom lane

I wrote:
> ... so I guess the answer is that this code ought to avoid adding Vars that
> are only mentioned within aggregates.

The cleanest way to fix this would involve adding another flag parameter
to flatten_tlist and pull_var_clause.  This is no problem to do in HEAD
or even 9.1, but I'm a bit worried about breaking third-party code if we
backpatch further than that.  So far as I can see, the failure only
occurs if we have a plain (non-grouping) Agg node, which implies that
the user is trying to use windowing functions on a result set that's
guaranteed to contain exactly one aggregated row.  That seems pretty
useless, so I'm thinking it's not worth back-patching a fix for.
Comments?

            regards, tom lane

Excerpts from Tom Lane's message of mar jul 12 11:20:39 -0400 2011:
> I wrote:
> > ... so I guess the answer is that this code ought to avoid adding Vars that
> > are only mentioned within aggregates.
>
> The cleanest way to fix this would involve adding another flag parameter
> to flatten_tlist and pull_var_clause.  This is no problem to do in HEAD
> or even 9.1, but I'm a bit worried about breaking third-party code if we
> backpatch further than that.  So far as I can see, the failure only
> occurs if we have a plain (non-grouping) Agg node, which implies that
> the user is trying to use windowing functions on a result set that's
> guaranteed to contain exactly one aggregated row.  That seems pretty
> useless, so I'm thinking it's not worth back-patching a fix for.
> Comments?

Since there have been no previous bug reports on this, it seems clear
that a backpatch is not strictly necessary either.

--
Álvaro Herrera <alvherre@commandprompt.com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

On Tue, 2011-07-12 at 11:20 -0400, Tom Lane wrote:
> So far as I can see, the failure only
> occurs if we have a plain (non-grouping) Agg node, which implies that
> the user is trying to use windowing functions on a result set that's
> guaranteed to contain exactly one aggregated row.  That seems pretty
> useless, so I'm thinking it's not worth back-patching a fix for.
> Comments?

Agreed. I'm not worried about backpatching it.

Regards,
    Jeff Davis

Jeff Davis <pgsql@j-davis.com> writes:
> On Tue, 2011-07-12 at 11:20 -0400, Tom Lane wrote:
>> So far as I can see, the failure only
>> occurs if we have a plain (non-grouping) Agg node, which implies that
>> the user is trying to use windowing functions on a result set that's
>> guaranteed to contain exactly one aggregated row.  That seems pretty
>> useless, so I'm thinking it's not worth back-patching a fix for.
>> Comments?

> Agreed. I'm not worried about backpatching it.

I've fixed this in HEAD and 9.1 only.

            regards, tom lane