Re: Inadequate executor locking of indexes

Hi David,

On 2019/03/13 10:38, David Rowley wrote:
> I had another go at this patch and fixed the problem by just setting
> the idxlockmode inside the planner just before the call to
> expand_inherited_tables().  This allows the lockmode to be copied into
> child RTEs.

I have one question about the relation between idxlockmode and
rellockmode?  From skimming the patch, it appears that they're almost
always set to the same value.  If so, why not use rellockmode for index
locking too?

> #2 would not address Tom's mention of there possibly being some way to
> have the index scan node initialise before the modify table node
> (currently we pass NoLock for indexes belonging to result rels in the
> index scan nodes).  I can't quite imagine at the moment how that's
> possible, but maybe my imagination is just not good enough.  We could
> fix that by passing RowExclusiveLock instead of NoLock. It just means
> we'd discover the lock already exists in the local lock table...
> unless of course there is a case where the index scan gets initialised
> before modify table is.

Tom gave an example upthread that looked like this:

explain (costs off) with x1 as materialized (select * from foo where a =
1), x2 as (update foo set a = a where a = 1 returning *) select * from x1,
x2 where x1.a = x2.a;
                     QUERY PLAN
────────────────────────────────────────────────────
 Hash Join
   Hash Cond: (x1.a = x2.a)
   CTE x1
     ->  Bitmap Heap Scan on foo
           Recheck Cond: (a = 1)
           ->  Bitmap Index Scan on foo_a_idx
                 Index Cond: (a = 1)
   CTE x2
     ->  Update on foo foo_1
           ->  Bitmap Heap Scan on foo foo_1
                 Recheck Cond: (a = 1)
                 ->  Bitmap Index Scan on foo_a_idx
                       Index Cond: (a = 1)
   ->  CTE Scan on x1
   ->  Hash
         ->  CTE Scan on x2
(16 rows)

When InitPlan() invokes ExecInitNode on the subplans of x1 and x2, it does
so in that order.  So, ExecInitBitmapIndexScan for x1 is called before
ExecInitModifyTable for x2.

But if finalize_lockmodes() in your patch set lockmodes correctly,
ExecInitBitmapIndexScan() called for x1 ought to lock the index with
RowExclusiveLock, no?

Thanks,
Amit