Re: FOR UPDATE lock problem ? - Mailing list pgsql-hackers

REYNAUD Jean-Samuel <reynaud@elma.fr> writes:
> When I use a "select for update" request whitch uses an index, the
> locking system is inconsistant. 

I've applied the attached patch to HEAD and 8.1 to fix this.
        regards, tom lane


Index: createplan.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/optimizer/plan/createplan.c,v
retrieving revision 1.202.2.2
diff -c -r1.202.2.2 createplan.c
*** createplan.c    29 Jan 2006 18:55:55 -0000    1.202.2.2
--- createplan.c    25 Apr 2006 16:46:12 -0000
***************
*** 816,823 ****      * are not equal to, but are logically implied by, the index quals; so we      * also try a
predicate_implied_by()check to see if we can discard quals      * that way.  (predicate_implied_by assumes its first
inputcontains only
 
!      * immutable functions, so we have to check that.)    We can also discard
!      * quals that are implied by a partial index's predicate.      *      * While at it, we strip off the
RestrictInfosto produce a list of plain      * expressions.
 
--- 816,827 ----      * are not equal to, but are logically implied by, the index quals; so we      * also try a
predicate_implied_by()check to see if we can discard quals      * that way.  (predicate_implied_by assumes its first
inputcontains only
 
!      * immutable functions, so we have to check that.)
!      *
!      * We can also discard quals that are implied by a partial index's
!      * predicate, but only in a plain SELECT; when scanning a target relation
!      * of UPDATE/DELETE/SELECT FOR UPDATE, we must leave such quals in the
!      * plan so that they'll be properly rechecked by EvalPlanQual testing.      *      * While at it, we strip off
theRestrictInfos to produce a list of plain      * expressions.
 
***************
*** 836,843 ****              if (predicate_implied_by(clausel, nonlossy_indexquals))                 continue;
!             if (predicate_implied_by(clausel, best_path->indexinfo->indpred))
!                 continue;         }         qpqual = lappend(qpqual, rinfo->clause);     }
--- 840,853 ----              if (predicate_implied_by(clausel, nonlossy_indexquals))                 continue;
!             if (best_path->indexinfo->indpred)
!             {
!                 if (baserelid != root->parse->resultRelation &&
!                     !list_member_int(root->parse->rowMarks, baserelid))
!                     if (predicate_implied_by(clausel,
!                                              best_path->indexinfo->indpred))
!                         continue;
!             }         }         qpqual = lappend(qpqual, rinfo->clause);     }
***************
*** 920,927 ****      * but are logically implied by, the index quals; so we also try a      * predicate_implied_by()
checkto see if we can discard quals that way.      * (predicate_implied_by assumes its first input contains only
immutable
!      * functions, so we have to check that.)  We can also discard quals that
!      * are implied by a partial index's predicate.      *      * XXX For the moment, we only consider partial index
predicatesin the      * simple single-index-scan case.  Is it worth trying to be smart about
 
--- 930,941 ----      * but are logically implied by, the index quals; so we also try a      * predicate_implied_by()
checkto see if we can discard quals that way.      * (predicate_implied_by assumes its first input contains only
immutable
!      * functions, so we have to check that.)
!      *
!      * We can also discard quals that are implied by a partial index's
!      * predicate, but only in a plain SELECT; when scanning a target relation
!      * of UPDATE/DELETE/SELECT FOR UPDATE, we must leave such quals in the
!      * plan so that they'll be properly rechecked by EvalPlanQual testing.      *      * XXX For the moment, we only
considerpartial index predicates in the      * simple single-index-scan case.  Is it worth trying to be smart about
 
***************
*** 945,952 ****             {                 IndexPath  *ipath = (IndexPath *) best_path->bitmapqual; 
!                 if (predicate_implied_by(clausel, ipath->indexinfo->indpred))
!                     continue;             }         }         qpqual = lappend(qpqual, clause);
--- 959,972 ----             {                 IndexPath  *ipath = (IndexPath *) best_path->bitmapqual; 
!                 if (ipath->indexinfo->indpred)
!                 {
!                     if (baserelid != root->parse->resultRelation &&
!                         !list_member_int(root->parse->rowMarks, baserelid))
!                         if (predicate_implied_by(clausel,
!                                                  ipath->indexinfo->indpred))
!                             continue;
!                 }             }         }         qpqual = lappend(qpqual, clause);
***************
*** 1282,1288 ****          * join quals; failing to prove that doesn't result in an incorrect          * plan.  It is
theright way to proceed because adding more quals to          * the stuff we got from the original query would just
makeit harder
 
!          * to detect duplication.          */         BitmapHeapPath *innerpath = (BitmapHeapPath *)
best_path->innerjoinpath;
 
--- 1302,1310 ----          * join quals; failing to prove that doesn't result in an incorrect          * plan.  It is
theright way to proceed because adding more quals to          * the stuff we got from the original query would just
makeit harder
 
!          * to detect duplication.  (Also, to change this we'd have to be
!          * wary of UPDATE/DELETE/SELECT FOR UPDATE target relations; see
!          * notes above about EvalPlanQual.)          */         BitmapHeapPath *innerpath = (BitmapHeapPath *)
best_path->innerjoinpath;