Mutating EquivalenceClasses --- just a note for the archives

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: pgsql-patches(at)postgreSQL(dot)org
Subject: Mutating EquivalenceClasses --- just a note for the archives
Date: 2008-03-27 19:25:39
Message-ID: 29327.1206645939@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-patches

I looked into Taiki Yamaguchi's recent bug report
http://archives.postgresql.org/pgsql-bugs/2008-03/msg00275.php
and soon figured out that the problem is that when planagg.c is able to
optimize a MIN/MAX aggregate, it replaces the Aggref node(s) by Param
nodes throughout the query targetlist and HAVING qual --- but not in the
planner's auxiliary data structures. In particular, the
EquivalenceClass lists weren't touched, and so make_sort_from_pathkeys()
failed to find a targetlist entry matching the clause it's supposed
to sort by. So any attempt to ORDER BY or DISTINCT on an expression
including the optimized aggregate failed. This didn't happen before 8.3
because the sort item matching was mostly driven by ressortgroupref
labels.

I considered several possible fixes and ultimately settled on a trivial
one: in the cases of interest, ORDER BY or DISTINCT is useless anyway
since there can be only one output row. The simplest and most efficient
fix is therefore to disregard those clauses when planagg.c has
succeeded. That might not work forever, however --- someday we might
want to do this optimization even in the presence of GROUP BY.

A relatively straightforward approach to fixing it is to apply the
Aggref-to-Param substitution to the EquivalenceClass lists too.
I actually coded that up, and want to attach the patch here so it
gets archived, just in case we want to resurrect it someday.
There were a couple of reasons I didn't like it, however:

* It's not real clear whether we should change the stored properties
of eclass members (eg, the relation membership em_relids) to match
the updated expressions.

* There might be other places besides the EquivalenceClass lists
that'd have to be fixed too. The patch seemed to work as-is, but
I wonder in particular about the query's sortClause and distinctClause.
It's just an implementation happenstance that the contents of those
aren't currently examined after planagg.c runs.

So on the whole this is just a failed patch, but we might need it
or something like it someday.

regards, tom lane

Attachment Content-Type Size
unknown_filename text/plain 3.3 KB

Browse pgsql-patches by date

  From Date Subject
Next Message Merlin Moncure 2008-03-27 20:19:07 Re: libpq type system 0.9a
Previous Message Simon Riggs 2008-03-27 19:22:36 Re: Auto-explain patch