Hi,

On 02/25/2016 11:56 AM, Kyotaro HORIGUCHI wrote:
Hello, Tomas. my cerebral cortext gets squeezed by jumping from
parser to planner.

LOL


At Wed, 24 Feb 2016 01:13:22 +0100, Tomas Vondra <tomas.von...@2ndquadrant.com> wrote 
in <56ccf5a2.5040...@2ndquadrant.com>
Hi,

On 12/06/2015 11:48 PM, Tomas Vondra wrote:
    /*
     * Frequently, there will be no partial indexes, so first check to
     * make sure there's something useful to do here.
     */
    have_partial = false;
    foreach(lc, rel->indexlist)
    {
      IndexOptInfo *index = (IndexOptInfo *) lfirst(lc);

      /*
       * index rinfos are the same to baseristrict infos for non-partial
       * indexes
       */
      index->indrinfos = rel->baserestrictinfo;

      if (index->indpred == NIL)
        continue;      /* ignore non-partial indexes */

      if (index->predOK)
        continue;      /* don't repeat work if already proven OK */

      have_partial = true;
      break;
    }

Attached is a v6 of the patch, which is actually the version
submitted by Kyotaro-san on 2015/10/8 rebased to current master and
with two additional changes.

This relies on the fact I believe that no indexrelinfos are
shared among any two baserels. Are you OK with that?

I'm not sure what this refers to? You mean the fact that an index will have one IndexOptInfo instance for each baserel? Yes, that seems fine to me.


Firstly, I've removed the "break" from the initial foreach loop in
check_partial_indexes(). As explained in the previous message, I
believe this was a bug in the patch.

I agreed. The break is surely a bug. (the regression didn't find
it though..)

Secondly, I've tried to improve the comments to explain a bit better
what the code is doing.

In check_partial_indexes, the following commnet.

   /*
    * Update restrictinfo for this index by excluding clauses that
    * are implied by the index predicates. We first quickly check if
    * there are any implied clauses, and if we found at least one
    * we do the actual work. This way we don't need the construct
    * a copy of the list unless actually needed.
    */

Is "need the construct" a mistake of "need to construct" ?


match_restriction_clauses_to_index is called only from
create_index_paths, and now the former doesn't use its first
parameter rel. We can safely remove the useless parameter.

-  match_restriction_clauses_to_index(RelOptInfo *rel, IndexOptInfo *index,
-                                     IndexClauseSet *clauseset)
+  match_restriction_clauses_to_index(IndexOptInfo *index,
+                                     IndexClauseSet *clauseset)

I find no other problem and have no objection to this. May I mark
this "Ready for committer" after fixing them?

OK. Do you want me to do the fixes, or will you do that?

regards

--
Tomas Vondra                  http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to