Re: Re: [CORE] Re: MY PATCH - Mailing list pgsql-patches

From Tom Lane
Subject Re: Re: [CORE] Re: MY PATCH
Date
Msg-id 21424.960602565@sss.pgh.pa.us
Whole thread Raw
In response to Re: Re: [CORE] Re: MY PATCH  (Chris Bitmead <chris@bitmead.com>)
List pgsql-patches
OK, I see at least one of the problems here.

UPDATE/DELETE with an Append plan have never worked --- not in recent
memory anyway --- because execMain's InitPlan() creates a result
RelationInfo that EndPlan() is expecting to close.  But if the top plan
node is an Append then nodeAppend.c scribbles on
estate->es_result_relation_info.  This leaves the originally created
RelationInfo unreachable ... which is bad enough, since that means a
leaked rel refcount, but there's worse to come.  nodeAppend.c creates
its own set of RelationInfo nodes for the relation(s) it will scan.
At query end, it's left estate->es_result_relation_info pointing at a
random one of these.  nodeAppend.c closes *and deletes* all its
RelationInfos.  Now estate->es_result_relation_info is pointing at a
closed-and-deleted RelationInfo that EndPlan() will then try to close
again.  Oops.

I believe the correct fix is
  (a) ExecInitAppend() must incorporate the RelationInfo already
      created by InitPlan() into its list of RelationInfos, rather
      than creating a second RelationInfo (= extra relcache refcount)
      for that relation.
  (b) ExecEndAppend() must reset estate->es_result_relation_info to
      NULL, so it won't be left pointing to a deleted RelationInfo.
  (c) EndPlan() must not examine estate->es_result_relation_info until
      after it has called the node-type-specific end routine; otherwise
      change (b) will not be able to prevent EndPlan() from doing the
      wrong thing.

On it now...

            regards, tom lane

pgsql-patches by date:

Previous
From: Bruce Momjian
Date:
Subject: Re: Re: [CORE] Re: MY PATCH
Next
From: Denis Perchine
Date:
Subject: Fwd: Patch for 'Not to stuff everything as files in a single directory, hash dirs'