Horton

Re: executor relation handling

Jesper Pedersen <jesper.pedersen@redhat.com>

View thread View on postgr.es
From: Jesper Pedersen <jesper.pedersen@redhat.com>
To: David Rowley <david.rowley@2ndquadrant.com>, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
Cc: Amit Langote <amitlangote09@gmail.com>, Pg Hackers <pgsql-hackers@postgresql.org>, Tom Lane <tgl@sss.pgh.pa.us>
Date: 2018-09-27T14:24:46Z
Lists: pgsql-hackers 
Hi,

On 9/27/18 5:15 AM, David Rowley wrote:
> I've just completed a review of the v5 patch set. I ended up just
> making the changes myself since Amit mentioned he was on leave for a
> few weeks.
> 
> Summary of changes:
> 
> 1. Changed the way we verify the lock already exists with debug
> builds. I reverted some incorrect code added to LockRelationOid that
> seems to have gotten broken after being rebased on f868a8143a9.  I've
> just added some functions that verify the lock is in the
> LockMethodLocalHash hashtable.
> 2. Fixed some incorrect lock types being passed into
> addRangeTableEntryForRelation()
> 3. Added code in addRangeTableEntryForRelation to verify we actually
> hold the lock that the parameter claims we do. (This found all the
> errors I fixed in #2)
> 4. Updated various comments outdated by the patches
> 5. Updated executor README's mention that we close relations when
> calling the end node function.  This is now handled at the end of
> execution.
> 6. Renamed nominalRelation to targetRelation.  I think this fits
> better since we're overloading the variable.
> 7. Use LOCKMODE instead of int in some places.
> 8. Changed warning about relation not locked to WARNING instead of NOTICE.
> 9. Renamed get_unpruned_rowmarks() to get_nondummy_rowmarks(). Pruning
> makes me think of partition pruning but the function checks for dummy
> rels. These could be dummy for reasons other than partition pruning.
> 
> I've attached a diff showing the changes I made along with the full
> patches which I tagged as v6.
> 

Thanks David and Amit -- this version passes check-world. I'll also take 
a deeper look.

Best regards,
  Jesper

Commits

  1. Avoid O(N^2) cost in ExecFindRowMark().

  2. Remove some unnecessary fields from Plan trees.

  3. Restore sane locking behavior during parallel query.

  4. Remove more redundant relation locking during executor startup.

  5. In the executor, use an array of pointers to access the rangetable.

  6. Centralize executor's opening/closing of Relations for rangetable entries.

  7. Change executor to just Assert that table locks were already obtained.

  8. Change rewriter/planner/executor/plancache to depend on RTE rellockmode.

  9. Add assertions that we hold some relevant lock during relation open.

  10. Create an RTE field to record the query's lock mode for each relation.