Tom Lane wrote:
"Rushabh Lathia" <[EMAIL PROTECTED]> writes:
Simple select give wrong result when it uses the Bitmap Heap Scan path.

It's generally appropriate to mention which PG version you're working
with when you report a bug.

postgres=# explain select proname from pg_proc where proname like 'my_pro1';
                                         QUERY
PLAN

--------------------------------------------------------------------------------
-------------
 Bitmap Heap Scan on pg_proc  (cost=4.26..8.27 rows=1 width=64)
   Recheck Cond: (proname ~~ 'my_pro1'::text)
   ->  Bitmap Index Scan on pg_proc_proname_args_nsp_index  (cost=0.00..4.26
row
s=1 width=0)
         Index Cond: ((proname >= 'my'::name) AND (proname < 'mz'::name))
(4 rows)

Hmm, the ~~ condition should get treated as a "filter" not a "recheck".
I suppose I broke this somewhere ...

I started to look at this last night. The culprit seems to be this patch:

Author: Tom Lane <[EMAIL PROTECTED]>
Date:   Sun Apr 13 19:18:14 2008 +0000

    Phase 2 of project to make index operator lossiness be determined at runtime
    instead of plan time.  Extend the amgettuple API so that the index AM 
returns
    a boolean indicating whether the indexquals need to be rechecked, and make
    that rechecking happen in nodeIndexscan.c (currently the only place where
    it's expected to be needed; other callers of index_getnext are just erroring
    out for now).  For the moment, GIN and GIST have stub logic that just always
    sets the recheck flag to TRUE --- I'm hoping to get Teodor to handle pushing
    that control down to the opclass consistent() functions.  The planner no
    longer pays any attention to amopreqcheck, and that catalog column will go
    away in due course.

and the changes around create_bitmap_scan_plan in particular.

create_bitmap_subplan puts the original ~~ qual into the recheck condition, even though the indexqual is only ((proname >= 'my'::name) AND (proname < 'mz'::name)). So, the condition put into the recheck condition is stronger than the checked by the index.

create_bitmap_scan_plan puts all index clauses that are not in the the Recheck condition into the Filter. If the condition in the recheck condition is stronger than the condition normally checked by the index, that's wrong.

Attached is a patch that changes create_bitmap_subplan so that the condition put into Recheck condition is never stronger than the condition automatically handled by the index. Does that look right to you?

--
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com
*** src/backend/optimizer/plan/createplan.c
--- src/backend/optimizer/plan/createplan.c
***************
*** 1197,1202 **** create_bitmap_subplan(PlannerInfo *root, Path *bitmapqual,
--- 1197,1203 ----
  		IndexPath  *ipath = (IndexPath *) bitmapqual;
  		IndexScan  *iscan;
  		ListCell   *l;
+ 		List	   *scan_clauses;
  
  		/* Use the regular indexscan plan build machinery... */
  		iscan = create_indexscan_plan(root, ipath, NIL, NIL);
***************
*** 1210,1216 **** create_bitmap_subplan(PlannerInfo *root, Path *bitmapqual,
  		plan->plan_rows =
  			clamp_row_est(ipath->indexselectivity * ipath->path.parent->tuples);
  		plan->plan_width = 0;	/* meaningless */
! 		*qual = get_actual_clauses(ipath->indexclauses);
  		foreach(l, ipath->indexinfo->indpred)
  		{
  			Expr	   *pred = (Expr *) lfirst(l);
--- 1211,1233 ----
  		plan->plan_rows =
  			clamp_row_est(ipath->indexselectivity * ipath->path.parent->tuples);
  		plan->plan_width = 0;	/* meaningless */
! 
! 		/*
! 		 * Put those indexquals that are automatically handled by the index to
! 		 * the Recheck condition. Don't include clauses that are derived from,
! 		 * but not directly included in the original scan quals. The original
! 		 * clause they're derived from need to be checked anyway in the Filter,
! 		 * even for non-lossy bitmaps.
! 		 */
! 		scan_clauses = get_actual_clauses(ipath->indexclauses);
! 		*qual = NIL;
! 		foreach(l, iscan->indexqualorig)
! 		{
! 			Expr	   *q = (Expr *) lfirst(l);
! 			if (list_member(scan_clauses, q))
! 				*qual = lappend(*qual, q);
! 		}
! 
  		foreach(l, ipath->indexinfo->indpred)
  		{
  			Expr	   *pred = (Expr *) lfirst(l);
-- 
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