Re: FOR UPDATE lock problem ?

[Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>]

REYNAUD Jean-Samuel <reynaud(at)elma(dot)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 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.
  	 *
  	 * While at it, we strip off the RestrictInfos to 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 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.
  	 *
  	 * While at it, we strip off the RestrictInfos 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() check to 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 predicates in 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() check to 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 consider partial 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 the right way to proceed because adding more quals to
  		 * the stuff we got from the original query would just make it 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 the right way to proceed because adding more quals to
  		 * the stuff we got from the original query would just make it 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;