Re: Transforming IN (...) to ORs, volatility

On 02.04.2011 20:48, Tom Lane wrote:
> Heikki Linnakangas<heikki.linnakangas@enterprisedb.com>  writes:
>> We sometimes transform IN-clauses to a list of ORs:
>> postgres=# explain SELECT * FROM foo WHERE a IN (b, c);
>>                         QUERY PLAN
>> ------------------------------------------------------
>>    Seq Scan on foo  (cost=0.00..39.10 rows=19 width=12)
>>      Filter: ((a = b) OR (a = c))
>> (2 rows)
>
>> But what if you replace "a" with a volatile function? It doesn't seem
>> legal to do that transformation in that case, but we do it:
>
> This is the fault of transformAExprIn().  But please let's *not* fix
> this by adding volatility to the set of heuristics used there.  Looking
> at this again, it seems to me that most of the problem with this code
> is that we're trying to make optimization decisions in the parser.

Agreed. The history of this is that before 8.2 all IN clauses were 
transformed to OR clauses straight in the grammar. 8.2 added the code to 
represent IN clause as a ScalarArrayOpExpr, but it was changed in 8.2.10 
to use the OR-form again for Vars 
(http://archives.postgresql.org/pgsql-hackers/2008-10/msg01269.php)

> I think what we ought to do is have the parser emit a full-fledged
> InExpr node type (with semantics rather like CaseExpr) and then teach
> the planner to optimize that to something else when it seems
> safe/prudent to do so.  One nontrivial advantage of that is that
> rules/views containing IN constructs would start to reverse-parse
> in the same fashion, instead of introducing weird substitute
> expressions.

Here's my first cut at that. The lefthand expression is now evaluated 
only once, and stored in econtext->caseValue. Parse analysis turns the 
righthand expressions into a list of comparison expressions like 
"CaseTestExpr = value1". It's perhaps time that we rename CaseTestExpr 
into something more generic, now that it's used not only in CASE 
expressions, but also in IN and in UPDATE targets, but I didn't do that 
in this patch.

eval_const_expressions checks the lefthand expression for volatile 
functions. If there aren't any, it transform the InExprs to a list of ORs.

This isn't finished, because it doesn't yet do the transformation to 
ScalarArrayOpExpr. The OR form is much slower to plan, which is why the 
ScalarArrayOpExpr transformation was introduced in 8.2. I'll continue 
hacking on that, but please let me know if you have a better idea on how 
to handle that. One alternative is to teach the machinery that matches 
restrictinfos to usable indexes to handle InExpr like it does 
ScalarArrayOpExprs, but I don't know that code very well.

-- 
   Heikki Linnakangas
   EnterpriseDB   http://www.enterprisedb.com