On Mon, Jun 12, 2017 at 04:04:23PM +1200, Thomas Munro wrote: > On Sun, Jun 11, 2017 at 11:11 PM, Thomas Munro > <thomas.mu...@enterprisedb.com> wrote: > > On Sun, Jun 11, 2017 at 6:25 PM, Noah Misch <n...@leadboat.com> wrote: > >> This fourth point is not necessarily a defect: I wonder if RangeTblEntry is > >> the right place for enrtuples. It's a concept regularly seen in planner > >> data > >> structures but not otherwise seen at parse tree level. > > > > I agree that this is strange. Perhaps > > set_namedtuplestore_size_estimates should instead look up the > > EphemeralNamedRelation by rte->enrname to find its way to > > enr->md.enrtuples, but I'm not sure off the top of my head how it > > should get its hands on the QueryEnvironment required to do that. I > > will look into this on Monday, but other ideas/clues welcome... > > Here's some background: If you look at the interface changes > introduced by 18ce3a4 you will see that it is now possible for a > QueryEnvironment object to be injected into the parser and executor. > Currently the only use for it is to inject named tuplestores into the > system via SPI_register_relation or SPI_register_trigger_data. That's > to support SQL standard transition tables, but anyone can now use SPI > to expose data to SQL via an ephemeral named relation in this way. > (In future we could imagine other kinds of objects like table > variables, anonymous functions or streams). > > The QueryEnvironment is used by the parser to resolve names and build > RangeTblEntry objects. The planner doesn't currently need it, because > all the information it needs is in the RangeTblEntry, including the > offending row estimate. Then the executor needs it to get its hands > on the tuplestores. So the question is: how can we get it into > costsize.c, so that it can look up the EphermalNamedRelationMetaData > object by name, instead of trafficking statistics through parser data > structures? > > Here are a couple of ways forward that I can see: > > 1. Figure out how to get the QueryEnvironment through more of these > stack frames (possibly inside other objects), so that > set_namedtuplestore_size_estimates can look up enrtuples by enrname: > > set_namedtuplestore_size_estimates <-- would need QueryEnvironment > set_namedtuplestore_pathlist > set_rel_size > set_base_rel_sizes > make_one_rel > query_planner > grouping_planner > subquery_planner > standard_planner > planner > pg_plan_query > pg_plan_queries <-- doesn't receive QueryEnvironment > BuildCachedPlan <-- receives QueryEnvironment > GetCachedPlan > _SPI_execute_plan > SPI_execute_plan_with_paramlist > > 2. Rip the row estimation out for now, use a bogus hard coded > estimate like we do in some other cases, and revisit later. See > attached (including changes from my previous message). > Unsurprisingly, a query plan changes. > > Thoughts?
I'm not sufficiently familiar with the relevant code to judge this one. Let's see if planner experts voice an opinion. Absent more opinions, the current design stands. -- Sent via pgsql-hackers mailing list (email@example.com) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers