On Sun, Dec 18, 2016 at 3:15 PM, Kevin Grittner <kgri...@gmail.com> wrote:
> On Sun, Dec 4, 2016 at 11:35 PM, Haribabu Kommi
> <kommi.harib...@gmail.com> wrote:
>> Moved to next CF with "waiting on author" status.
> Patch v8 attempts to address the issues explicitly raised in
> Thomas Munro's review.  An opaque query environment is created
> that, for now, only passes through ephemeral named relations, of
> which the only initial implementation is named tuplestores; but
> the techniques are intended to support both other types of
> ephemeral named relations and environmental properties (things
> affecting parsing, planning, and execution that are not coming from
> the system catalog) besides ENRs.  There is no clue in the access
> to the QE whether something is, for example, stored in a list or a
> hash table.  That's on purpose, so that the addition of other
> properties or changes to their implementation doesn't affect the
> calling code.


> There were a few changes Thomas included in the version he posted,
> without really delving into an explanation for those changes.  Some
> or all of them are likely to be worthwhile, but I would rather
> incorporate them based on explicit discussion, so this version
> doesn't do much other than generalize the interface a little,
> change some names, and add more regression tests for the new
> feature.  (The examples I worked up for the rough proof of concept
> of enforcement of RI through set logic rather than row-at-a-time
> navigation were the basis for the new tests, so the idea won't get
> totally lost.)  Thomas, please discuss each suggested change (e.g.,
> the inclusion of the query environment in the parameter list of a
> few more functions).

I was looking for omissions that would cause some kind of statements
to miss out on ENRs arbitrarily.  It seemed to me that
parse_analyze_varparams should take a QueryEnvironment, mirroring
parse_analyze, so that PrepareQuery could pass it in.  Otherwise,
PREPARE wouldn't see ENRs.  Is there any reason why SPI_prepare should
see them but PREPARE not?

Should we attempt to detect if the tupledesc changes incompatibly
between planning and execution?

> Changed to "Needs review" status.

+                       enr->md.enrtuples =

I was wondering about this.  As I understand it, plans for statements
in plpgsql functions are automatically cached.  Is it OK for the
number of tuples on the first invocation of the trigger in a given
session to determine the estimate used for the plan?  I suppose that
is the case with regular tables, so maybe it is.

+                       register_enr(estate.queryEnv, enr);
+                       SPI_register_relation(enr);

Here plpgsql calls both register_enr and SPI_register_relation.  Yet
SPI_register_relation calls register_enr too, so is this a mistake?
Also, the return code isn't checked.

Thomas Munro

Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:

Reply via email to