Thread: pgsql: Make the behavior of HAVING without GROUP BY conform to the SQL

Log Message:
-----------
Make the behavior of HAVING without GROUP BY conform to the SQL spec.
Formerly, if such a clause contained no aggregate functions we mistakenly
treated it as equivalent to WHERE.  Per spec it must cause the query to
be treated as a grouped query of a single group, the same as appearance
of aggregate functions would do.  Also, the HAVING filter must execute
after aggregate function computation even if it itself contains no
aggregate functions.

Modified Files:
--------------
    pgsql/doc/src/sgml:
        query.sgml (r1.43 -> r1.44)
        (http://developer.postgresql.org/cvsweb.cgi/pgsql/doc/src/sgml/query.sgml.diff?r1=1.43&r2=1.44)
    pgsql/doc/src/sgml/ref:
        select.sgml (r1.81 -> r1.82)
        (http://developer.postgresql.org/cvsweb.cgi/pgsql/doc/src/sgml/ref/select.sgml.diff?r1=1.81&r2=1.82)
    pgsql/src/backend/executor:
        nodeGroup.c (r1.59 -> r1.60)
        (http://developer.postgresql.org/cvsweb.cgi/pgsql/src/backend/executor/nodeGroup.c.diff?r1=1.59&r2=1.60)
    pgsql/src/backend/nodes:
        copyfuncs.c (r1.296 -> r1.297)
        (http://developer.postgresql.org/cvsweb.cgi/pgsql/src/backend/nodes/copyfuncs.c.diff?r1=1.296&r2=1.297)
        equalfuncs.c (r1.235 -> r1.236)
        (http://developer.postgresql.org/cvsweb.cgi/pgsql/src/backend/nodes/equalfuncs.c.diff?r1=1.235&r2=1.236)
    pgsql/src/backend/optimizer/path:
        allpaths.c (r1.123 -> r1.124)
        (http://developer.postgresql.org/cvsweb.cgi/pgsql/src/backend/optimizer/path/allpaths.c.diff?r1=1.123&r2=1.124)
    pgsql/src/backend/optimizer/plan:
        createplan.c (r1.175 -> r1.176)

(http://developer.postgresql.org/cvsweb.cgi/pgsql/src/backend/optimizer/plan/createplan.c.diff?r1=1.175&r2=1.176)
        planner.c (r1.178 -> r1.179)
        (http://developer.postgresql.org/cvsweb.cgi/pgsql/src/backend/optimizer/plan/planner.c.diff?r1=1.178&r2=1.179)
    pgsql/src/backend/optimizer/util:
        pathnode.c (r1.111 -> r1.112)
        (http://developer.postgresql.org/cvsweb.cgi/pgsql/src/backend/optimizer/util/pathnode.c.diff?r1=1.111&r2=1.112)
    pgsql/src/backend/parser:
        analyze.c (r1.315 -> r1.316)
        (http://developer.postgresql.org/cvsweb.cgi/pgsql/src/backend/parser/analyze.c.diff?r1=1.315&r2=1.316)
        parse_agg.c (r1.66 -> r1.67)
        (http://developer.postgresql.org/cvsweb.cgi/pgsql/src/backend/parser/parse_agg.c.diff?r1=1.66&r2=1.67)
    pgsql/src/backend/rewrite:
        rewriteHandler.c (r1.147 -> r1.148)
        (http://developer.postgresql.org/cvsweb.cgi/pgsql/src/backend/rewrite/rewriteHandler.c.diff?r1=1.147&r2=1.148)
        rewriteManip.c (r1.89 -> r1.90)
        (http://developer.postgresql.org/cvsweb.cgi/pgsql/src/backend/rewrite/rewriteManip.c.diff?r1=1.89&r2=1.90)
    pgsql/src/include/nodes:
        parsenodes.h (r1.272 -> r1.273)
        (http://developer.postgresql.org/cvsweb.cgi/pgsql/src/include/nodes/parsenodes.h.diff?r1=1.272&r2=1.273)
    pgsql/src/include/optimizer:
        planmain.h (r1.79 -> r1.80)
        (http://developer.postgresql.org/cvsweb.cgi/pgsql/src/include/optimizer/planmain.h.diff?r1=1.79&r2=1.80)
    pgsql/src/include/rewrite:
        rewriteManip.h (r1.39 -> r1.40)
        (http://developer.postgresql.org/cvsweb.cgi/pgsql/src/include/rewrite/rewriteManip.h.diff?r1=1.39&r2=1.40)
    pgsql/src/test/regress/expected:
        select_having.out (r1.9 -> r1.10)

(http://developer.postgresql.org/cvsweb.cgi/pgsql/src/test/regress/expected/select_having.out.diff?r1=1.9&r2=1.10)
        select_having_1.out (r1.3 -> r1.4)

(http://developer.postgresql.org/cvsweb.cgi/pgsql/src/test/regress/expected/select_having_1.out.diff?r1=1.3&r2=1.4)
        select_having_2.out (r1.1 -> r1.2)

(http://developer.postgresql.org/cvsweb.cgi/pgsql/src/test/regress/expected/select_having_2.out.diff?r1=1.1&r2=1.2)
    pgsql/src/test/regress/sql:
        select_having.sql (r1.8 -> r1.9)
        (http://developer.postgresql.org/cvsweb.cgi/pgsql/src/test/regress/sql/select_having.sql.diff?r1=1.8&r2=1.9)

I wonder how much back-compatibility and dump reloading problems this
might cause? :(

Tom Lane wrote:
> Log Message:
> -----------
> Make the behavior of HAVING without GROUP BY conform to the SQL spec.
> Formerly, if such a clause contained no aggregate functions we mistakenly
> treated it as equivalent to WHERE.  Per spec it must cause the query to
> be treated as a grouped query of a single group, the same as appearance
> of aggregate functions would do.  Also, the HAVING filter must execute
> after aggregate function computation even if it itself contains no
> aggregate functions.

Christopher Kings-Lynne <chriskl@familyhealth.com.au> writes:
> Tom Lane wrote:
>> Make the behavior of HAVING without GROUP BY conform to the SQL spec.

> I wonder how much back-compatibility and dump reloading problems this
> might cause? :(

Could happen, but considering that this has been wrong for circa five
years and no one noticed, I think it's unlikely to be a widespread
issue.

For the record, it looks like I introduced the faulty
HAVING-equals-WHERE meme in this patch, which was released in 7.0:

1999-10-07 00:23  tgl

    * src/: backend/commands/view.c, backend/executor/execMain.c,
    backend/nodes/copyfuncs.c, backend/nodes/equalfuncs.c,
    backend/nodes/outfuncs.c, backend/nodes/readfuncs.c,
    backend/optimizer/plan/initsplan.c,
    backend/optimizer/plan/planmain.c,
    backend/optimizer/plan/planner.c, backend/optimizer/util/clauses.c,
    backend/parser/analyze.c, backend/parser/gram.y,
    backend/parser/parse_agg.c, backend/parser/parse_clause.c,
    backend/parser/parse_func.c, backend/parser/parse_relation.c,
    backend/rewrite/rewriteHandler.c, include/nodes/parsenodes.h,
    include/optimizer/planmain.h, include/parser/parse_relation.h,
    test/regress/expected/rules.out: Fix planner and rewriter to follow
    SQL semantics for tables that are mentioned in FROM but not
    elsewhere in the query: such tables should be joined over anyway.
    Aside from being more standards-compliant, this allows removal of
    some very ugly hacks for COUNT(*) processing.  Also, allow HAVING
    clause without aggregate functions, since SQL does.  Clean up
    CREATE RULE statement-list syntax the same way Bruce just fixed the
    main stmtmulti production.  CAUTION: addition of a field to
    RangeTblEntry nodes breaks stored rules; you will have to initdb if
    you have any rules.

Mind you, our processing of HAVING was not *right* before that; it
was even more broken ...

            regards, tom lane