On Wed, Oct 22, 2014 at 5:29 PM, Tom Lane <t...@sss.pgh.pa.us> wrote: >> I was thinking that the hook would return a RelationParam. When parse >> analysis sees the returned RelationParam, it adds an entry for that to >> the range table, and creates the RangeTblRef for it. The way you >> describe it works too, but manipulating the range table directly in the >> hook seems a bit too low-level. > > The problem with that idea is that then the API for the hook has to cover > every possible sort of RTE that hooks might wish to create; I see no > reason to restrict them to creating just one kind. I agree that the hook > should avoid *physically* manipulating the rangetable, but it seems > reasonable to expect that it can call one of the addRangeTableEntryXXX > functions provided by parse_relation.c, and then return a RangeTblEntry* > gotten that way. So hooks would have an API more or less equivalent > to, eg, transformRangeFunction().
Right, that reasoning makes sense to me. Unlike regular parameters, where the existence of the parameter is known at parse time but the value isn't available until bind time, we would be creating a RelationParam node and then, literally immediately, turning it into a range-table entry. That seems like unnecessary complexity, and it's also something we can invent later if a more compelling use case emerges. So what I'm imagining now is: 1. During parse analysis, p_tableref_hook gets control and calls addRangeTableEntryForTuplestore(), creating an RTE of type RTE_TUPLESTORE. The RTE stores an integer parameter-index. 2. Path generation doesn't need to do anything very exciting; it just generates a Path node of type T_TuplestoreScan. The RTE is still available, so the path itself doesn't need to know which tuplestore we're referencing, because that information is present in the RTE. 3. At plan generation time, we look up the RTE for the path and extract the parameter index, which is what gets stored in the TuplestoreScan node. 4. At executor initialization time, we use the parameter index in the TuplestoreScan to index into the EState's es_param_list_info and retrieve the tuplestore. This means that Kevin's notion of a Tsrcache goes away completely, which means a lot of the function-signature changes in his last version of the patch can be reverted. The EState doesn't need a es_tsrcache either. The mapping from name (OLD/NEW) to parameter index happens inside the p_paramref_hook and after that we use integer indices throughout. All that sees good. One thing that's not too clear to me is how we're imagining that the TuplestoreScan will get it's tupledesc. Right now the Tsrcache stores essentially (tupledesc, tuplestore), but I understood the suggestions above to imply that the ParamListInfo should point only to the tuplestore, not to the tupledesc. I *think* the information we need to reconstruct the TupleDesc is mostly present in the RTE; Kevin reused the ctecoltypes, ctecoltypmods, and ctecolcollations fields to store that information, which (a) probably requires some thought about renaming those fields but (b) seems like it ought to be enough to construct a viable TupleDesc. It seems that for CTEs, we somehow engineer things so that the RecursiveUnion's target-list is such that we can apply ExecAssignResultTypeFromTL() to it and get the tupledesc that matches its Tuplestorestate, but I'm kinda unclear about what makes that work and whether we can use a similar trick here. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers