Hi, while working on [1] (works, harmless regression check failures aside, needs cleanup), I noticed $subject. Without fixing EXPR_KIND_VALUES to set p_hasTargetSRFs the query has Query->hasTargetSRF wrongly set. Which in turn breaks your planner code, as it's not being triggered anymore.
Is there a reason not to just set p_hasTargetSRFs once towards the end of the function, instead of doing so for all the non-error cases? That fixes INSERT ... VALUES(generate_series()) with your approach for me. I also noticed that we currently don't actually handle Query->hasTargetSRF correct when said SRF is in a VALUES() block. As there the SRFs are't in the targetlist PlannerInfo * subquery_planner(PlannerGlobal *glob, Query *parse, PlannerInfo *parent_root, bool hasRecursion, double tuple_fraction) ... /* Constant-folding might have removed all set-returning functions */ if (parse->hasTargetSRFs) parse->hasTargetSRFs = expression_returns_set((Node *) parse->targetList); unsets that SRFs exist. I'm not really sure whether that's bad or good - it right now doesn't cause active problems because of transformInsertStmt's /* * Process INSERT ... VALUES with a single VALUES sublist. We treat * this case separately for efficiency. The sublist is just computed * directly as the Query's targetlist, with no VALUES RTE. So it * works just like a SELECT without any FROM. */ optimization. As we don't support SRFs in any other kind of values (ValuesNext calls ExecEvalExpr isDone = NULL), that appears to be effectless right now. But we'd get better errormessage if we made check_srf_call_placement() error out. I wonder if there should be a seperate expression type for the INSERT ... VALUES(exactly-one-row); since that behaves quite differently. Greetings, Andres Freund [1] http://archives.postgresql.org/message-id/20170116032952.z2re55hcfhzbkmrm%40alap3.anarazel.de -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers