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.
+1 > 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 = tuplestore_tuple_count(trigdata->tg_newtable); 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 http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
