Re: [PATCH] Caching for stable expressions with constant arguments v3 - Mailing list pgsql-hackers

From Heikki Linnakangas
Subject Re: [PATCH] Caching for stable expressions with constant arguments v3
Date
Msg-id 4EDBDDD1.4050003@enterprisedb.com
Whole thread Raw
In response to [PATCH] Caching for stable expressions with constant arguments v3  (Marti Raudsepp <marti@juffo.org>)
Responses Re: Re: [PATCH] Caching for stable expressions with constant arguments v3  (Tom Lane <tgl@sss.pgh.pa.us>)
Re: [PATCH] Caching for stable expressions with constant arguments v3  (Marti Raudsepp <marti@juffo.org>)
Re: [PATCH] Caching for stable expressions with constant arguments v3  (Marti Raudsepp <marti@juffo.org>)
Re: [PATCH] Caching for stable expressions with constant arguments v3  (Marti Raudsepp <marti@juffo.org>)
List pgsql-hackers
On 25.09.2011 05:09, Marti Raudsepp wrote:
> This is the third version of my CacheExpr patch.

This seems to have bitrotted, thanks to the recent refactoring in 
eval_const_expressions().

> For explanation about design decisions, please read these earlier messages:
> http://archives.postgresql.org/pgsql-hackers/2011-09/msg00579.php
> http://archives.postgresql.org/pgsql-hackers/2011-09/msg00812.php
> http://archives.postgresql.org/pgsql-hackers/2011-09/msg00833.php

I wonder if it would be better to add the CacheExpr nodes to the tree as 
a separate pass, instead of shoehorning it into eval_const_expressions? 
I think would be more readable that way, even though a separate pass 
would be more expensive. And there are callers of 
eval_const_expressions() that have no use for the caching, like 
process_implied_equality().

This comment in RelationGetExpressions() also worries me:

>     /*
>      * Run the expressions through eval_const_expressions. This is not just an
>      * optimization, but is necessary, because the planner will be comparing
>      * them to similarly-processed qual clauses, and may fail to detect valid
>      * matches without this.  We don't bother with canonicalize_qual, however.
>      */
>     result = (List *) eval_const_expressions(NULL, (Node *) result);

Do the injected CacheExprs screw up that equality? Or the constraint 
exclusion logic in predtest.c?

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


pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: planner fails on HEAD
Next
From: Tomas Vondra
Date:
Subject: Re: cannot read pg_class without having selected a database / is this a bug?