summaryrefslogtreecommitdiff
path: root/src/backend/optimizer
AgeCommit message (Collapse)Author
2009-11-15Improve planning of Materialize nodes inserted atop the inner input of aTom Lane
mergejoin to shield it from doing mark/restore and refetches. Put an explicit flag in MergePath so we can centralize the logic that knows about this, and add costing logic that considers using Materialize even when it's not forced by the previously-existing considerations. This is in response to a discussion back in August that suggested that materializing an inner indexscan can be helpful when the refetch percentage is high enough.
2009-10-28When FOR UPDATE/SHARE is used with LIMIT, put the LockRows plan nodeTom Lane
underneath the Limit node, not atop it. This fixes the old problem that such a query might unexpectedly return fewer rows than the LIMIT says, due to LockRows discarding updated rows. There is a related problem that LockRows might destroy the sort ordering produced by earlier steps; but fixing that by pushing LockRows below Sort would create serious performance problems that are unjustified in many real-world applications, as well as potential deadlock problems from locking many more rows than expected. Instead, keep the present semantics of applying FOR UPDATE after ORDER BY within a single query level; but allow the user to specify the other way by writing FOR UPDATE in a sub-select. To make that work, track whether FOR UPDATE appeared explicitly in sub-selects or got pushed down from the parent, and don't flatten a sub-select that contained an explicit FOR UPDATE.
2009-10-26Re-implement EvalPlanQual processing to improve its performance and eliminateTom Lane
a lot of strange behaviors that occurred in join cases. We now identify the "current" row for every joined relation in UPDATE, DELETE, and SELECT FOR UPDATE/SHARE queries. If an EvalPlanQual recheck is necessary, we jam the appropriate row into each scan node in the rechecking plan, forcing it to emit only that one row. The former behavior could rescan the whole of each joined relation for each recheck, which was terrible for performance, and what's much worse could result in duplicated output tuples. Also, the original implementation of EvalPlanQual could not re-use the recheck execution tree --- it had to go through a full executor init and shutdown for every row to be tested. To avoid this overhead, I've associated a special runtime Param with each LockRows or ModifyTable plan node, and arranged to make every scan node below such a node depend on that Param. Thus, by signaling a change in that Param, the EPQ machinery can just rescan the already-built test plan. This patch also adds a prohibition on set-returning functions in the targetlist of SELECT FOR UPDATE/SHARE. This is needed to avoid the duplicate-output-tuple problem. It seems fairly reasonable since the other restrictions on SELECT FOR UPDATE are meant to ensure that there is a unique correspondence between source tuples and result tuples, which an output SRF destroys as much as anything else does.
2009-10-23When querying a table with child tables, do not check permissions on thePeter Eisentraut
child tables. This was found to be useless and confusing in virtually all cases, and also contrary to the SQL standard.
2009-10-14Support SQL-compliant triggers on columns, ie fire only if certain columnsTom Lane
are named in the UPDATE's SET list. Note: the schema of pg_trigger has not actually changed; we've just started to use a column that was there all along. catversion bumped anyway so that this commit is included in the history of potentially interesting changes to system catalog contents. Itagaki Takahiro
2009-10-12Move the handling of SELECT FOR UPDATE locking and rechecking out ofTom Lane
execMain.c and into a new plan node type LockRows. Like the recent change to put table updating into a ModifyTable plan node, this increases planning flexibility by allowing the operations to occur below the top level of the plan tree. It's necessary in any case to restore the previous behavior of having FOR UPDATE locking occur before ModifyTable does. This partially refactors EvalPlanQual to allow multiple rows-under-test to be inserted into the EPQ machinery before starting an EPQ test query. That isn't sufficient to fix EPQ's general bogosity in the face of plans that return multiple rows per test row, though. Since this patch is mostly about getting some plan node infrastructure in place and not about fixing ten-year-old bugs, I will leave EPQ improvements for another day. Another behavioral change that we could now think about is doing FOR UPDATE before LIMIT, but that too seems like it should be treated as a followon patch.
2009-10-10Split the processing of INSERT/UPDATE/DELETE operations out of execMain.c.Tom Lane
They are now handled by a new plan node type called ModifyTable, which is placed at the top of the plan tree. In itself this change doesn't do much, except perhaps make the handling of RETURNING lists and inherited UPDATEs a tad less klugy. But it is necessary preparation for the intended extension of allowing RETURNING queries inside WITH. Marko Tiikkaja
2009-10-08Support use of function argument names to identify which actual argumentsTom Lane
match which function parameters. The syntax uses AS, for example funcname(value AS arg1, anothervalue AS arg2) Pavel Stehule
2009-09-29Fix equivclass.c's not-quite-right strategy for handling X=X clauses.Tom Lane
The original coding correctly noted that these aren't just redundancies (they're effectively X IS NOT NULL, assuming = is strict). However, they got treated that way if X happened to be in a single-member EquivalenceClass already, which could happen if there was an ORDER BY X clause, for instance. The simplest and most reliable solution seems to be to not try to process such clauses through the EquivalenceClass machinery; just throw them back for traditional processing. The amount of work that'd be needed to be smarter than that seems out of proportion to the benefit. Per bug #5084 from Bernt Marius Johnsen, and analysis by Andrew Gierth.
2009-09-19Rename new subroutine, per discussion with Robert Haas.Tom Lane
2009-09-18Marginal code cleanup in joinpath.c: factor out clause variable-membershipTom Lane
tests into a small common subroutine, and eliminate an unnecessary difference in the order in which conditions are tested. Per a comment from Robert Haas.
2009-09-17Implement "join removal" for cases where the inner side of a left joinTom Lane
is unique and is not referenced above the join. In this case the inner side doesn't affect the query result and can be thrown away entirely. Although perhaps nobody would ever write such a thing by hand, it's a reasonably common case in machine-generated SQL. The current implementation only recognizes the case where the inner side is a simple relation with a unique index matching the query conditions. This is enough for the use-cases that have been shown so far, but we might want to try to handle other cases later. Robert Haas, somewhat rewritten by Tom
2009-09-12Rewrite the planner's handling of materialized plan types so that there isTom Lane
an explicit model of rescan costs being different from first-time costs. The costing of Material nodes in particular now has some visible relationship to the actual runtime behavior, where before it was essentially fantasy. This also fixes up a couple of places where different materialized plan types were treated differently for no very good reason (probably just oversights). A couple of the regression tests are affected, because the planner now chooses to put the other relation on the inside of a nestloop-with-materialize. So far as I can see both changes are sane, and the planner is now more consistently following the expectation that it should prefer to materialize the smaller of two relations. Per a recent discussion with Robert Haas.
2009-09-12Fix assertion failure when a SELECT DISTINCT ON expression is volatile.Tom Lane
In this case we generate two PathKey references to the expression (one for DISTINCT and one for ORDER BY) and they really need to refer to the same EquivalenceClass. However get_eclass_for_sort_expr was being overly paranoid and creating two different EC's. Correct behavior is to use the SortGroupRef index to decide whether two references to volatile expressions that are equal() (ie textually equivalent) should be considered the same. Backpatch to 8.4. Possibly this should be changed in 8.3 as well, but I'll refrain in the absence of evidence of a visible failure in that branch. Per bug #5049.
2009-09-02Fix subquery pullup to wrap a PlaceHolderVar around the entire RowExprTom Lane
that's generated for a whole-row Var referencing the subquery, when the subquery is in the nullable side of an outer join. The previous coding instead put PlaceHolderVars around the elements of the RowExpr. The effect was that when the outer join made the subquery outputs go to null, the whole-row Var produced ROW(NULL,NULL,...) rather than just NULL. There are arguments afoot about whether those things ought to be semantically indistinguishable, but for the moment they are not entirely so, and the planner needs to take care that its machinations preserve the difference. Per bug #5025. Making this feasible required refactoring ResolveNew() to allow more caller control over what is substituted for a Var. I chose to make ResolveNew() a wrapper around a new general-purpose function replace_rte_variables(). I also fixed the ancient bogosity that ResolveNew might fail to set a query's hasSubLinks field after inserting a SubLink in it. Although all current callers make sure that happens anyway, we've had bugs of that sort before, and it seemed like a good time to install a proper solution. Back-patch to 8.4. The problem can be demonstrated clear back to 8.0, but the fix would be too invasive in earlier branches; not to mention that people may be depending on the subtly-incorrect behavior. The 8.4 series is new enough that fixing this probably won't cause complaints, but it might in older branches. Also, 8.4 shows the incorrect behavior in more cases than older branches do, because it is able to flatten subqueries in more cases.
2009-08-13Put back adjust_appendrel_attrs()'s code for dealing with RestrictInfo.Tom Lane
I mistakenly removed it last month, thinking it was no longer needed --- but it is still needed for dealing with joininfo lists. Fortunately this bit of brain fade hadn't made it into any released versions yet.
2009-08-04Support hex-string input and output for type BYTEA.Tom Lane
Both hex format and the traditional "escape" format are automatically handled on input. The output format is selected by the new GUC variable bytea_output. As committed, bytea_output defaults to HEX, which is an *incompatible change*. We will keep it this way for awhile for testing purposes, but should consider whether to switch to the more backwards-compatible default of ESCAPE before 8.5 is released. Peter Eisentraut
2009-07-24Add commentary about Cygwin's broken erand48, per report from Andrew Dunstan.Tom Lane
2009-07-23Fix another thinko in join_is_legal's handling of semijoins: we have to testTom Lane
for the case that the semijoin was implemented within either input by unique-ifying its RHS before we test to see if it appears to match the current join situation. The previous coding would select semijoin logic in situations where we'd already unique-ified the RHS and joined it to some unrelated relation(s), and then came to join it to the semijoin's LHS. That still gave the right answer as far as the semijoin itself was concerned, but would lead to incorrectly examining only an arbitrary one of the matchable rows from the unrelated relation(s). The cause of this thinko was incorrect unification of the pre-8.4 logic for IN joins and OUTER joins --- the comparable case for outer joins can be handled after making the match test, but that's because there is nothing like the unique-ification escape hatch for outer joins. Per bug #4934 from Benjamin Reed.
2009-07-21Fix another semijoin-ordering bug. We already knew that we couldn'tTom Lane
reorder a semijoin into or out of the righthand side of another semijoin, but actually it doesn't work to reorder it into or out of the righthand side of a left or antijoin, either. Per bug #4906 from Mathieu Fenniak. This was sloppy thinking on my part. This identity does work: ( A left join B on (Pab) ) semijoin C on (Pac) == ( A semijoin C on (Pac) ) left join B on (Pab) but I failed to see that that doesn't mean this does: ( A left join B on (Pab) ) semijoin C on (Pbc) != A left join ( B semijoin C on (Pbc) ) on (Pab)
2009-07-20Teach simplify_boolean_equality to simplify the forms foo <> true andTom Lane
foo <> false, along with its previous duties of simplifying foo = true and foo = false. (All of these are equivalent to just foo or NOT foo as the case may be.) It's not clear how often this is really useful; but it costs almost nothing to do, and it seems some people think we should be smart about such cases. Per recent bug report.
2009-07-19Rewrite GEQO's gimme_tree function so that it always finds a legal joinTom Lane
sequence, even when the input "tour" doesn't lead directly to such a sequence. The stack logic that was added in 2004 only supported cases where relations that had to be joined to each other (due to join order restrictions) were adjacent in the tour. However, relying on a random search to figure that out is tremendously inefficient in large join problems, and could even fail completely (leading to "failed to make a valid plan" errors) if random_init_pool ran out of patience. It seems better to make the tour-to-plan transformation a little bit fuzzier so that every tour can form a legal plan, even though this means that apparently different tours will sometimes yield the same plan. In the same vein, get rid of the logic that knew that tours (a,b,c,d,...) are the same as tours (b,a,c,d,...), and therefore insisted the latter are invalid. The chance of generating two tours that differ only in this way isn't that high, and throwing out 50% of possible tours to avoid such duplication seems more likely to waste valuable genetic- refinement generations than to do anything useful. This leaves us with no cases in which geqo_eval will deem a tour invalid, so get rid of assorted kluges that tried to deal with such cases, in particular the undocumented assumption that DBL_MAX is an impossible plan cost. This is all per testing of Robert Haas' lets-remove-the-collapse-limits patch. That idea has crashed and burned, at least for now, but we still got something useful out of it. It's possible we should back-patch this change, since the "failed to make a valid plan" error can happen in existing releases; but I'd rather not until it has gotten more testing.
2009-07-19Fix a thinko in join_is_legal: when we decide we can implement a semijoinTom Lane
by unique-ifying the RHS and then inner-joining to some other relation, that is not grounds for violating the RHS of some other outer join. Noticed while regression-testing new GEQO code, which will blindly follow any path that join_is_legal says is legal, and then complain later if that leads to a dead end. I'm not certain that this can result in any visible failure in 8.4: the mistake may always be masked by the fact that subsequent attempts to join the rest of the RHS of the other join will fail. But I'm not certain it can't, either, and it's definitely not operating as intended. So back-patch. The added regression test depends on the new no-failures-allowed logic that I'm about to commit in GEQO, so no point back-patching that.
2009-07-17Repair bug #4926 "too few pathkeys for mergeclauses". This example showsTom Lane
that the sanity checking I added to create_mergejoin_plan() in 8.3 was a few bricks shy of a load: the mergeclauses could reference pathkeys in a noncanonical order such as x,y,x, not only cases like x,x,y which is all that the code had allowed for. The odd cases only turn up when using redundant clauses in an outer join condition, which is why no one had noticed before.
2009-07-16Make GEQO's planning deterministic by having it start from a predictableTom Lane
random number seed each time. This is how it used to work years ago, but we got rid of the seed reset because it was resetting the main random() sequence and thus having undesirable effects on the rest of the system. To fix, establish a private random number state for each execution of geqo(), and initialize the state using the new GUC variable geqo_seed. People who want to experiment with different random searches can do so by changing geqo_seed, but you'll always get the same plan for the same value of geqo_seed (if holding all other planner inputs constant, of course). The new state is kept in PlannerInfo by adding a "void *" field reserved for use by join_search hooks. Most of the rather bulky code changes in this commit are just arranging to pass PlannerInfo around to all the GEQO functions (many of which formerly didn't receive it). Andres Freund, with some editorialization by Tom
2009-07-16Make backend header files C++ safePeter Eisentraut
This alters various incidental uses of C++ key words to use other similar identifiers, so that a C++ compiler won't choke outright. You still (probably) need extern "C" { }; around the inclusion of backend headers. based on a patch by Kurt Harriman <harriman@acm.org> Also add a script cpluspluscheck to check for C++ compatibility in the future. As of right now, this passes without error for me.
2009-07-11Fix set_rel_width() to do something reasonable with non-Var items in aTom Lane
RelOptInfo targetlist. It used to be that the only possibility other than a Var was a RowExpr representing a whole-row child Var, but as of 8.4's expanded ability to flatten appendrel members, we can get arbitrary expressions in there. Use the expression's type info and get_typavgwidth() to produce an at-least-marginally-sane result. Note that get_typavgwidth()'s fallback estimate (32 bytes) is the same as what was here before, so there will be no behavioral change for RowExprs. Noted while looking at recent gripe about constant quals pushed down to FunctionScan appendrel members ... not only were we failing to recognize the constant qual, we were getting the width estimate wrong :-(
2009-07-06Fix set_append_rel_pathlist() to deal intelligently with cases whereTom Lane
substituting a child rel's output expressions into the appendrel's restriction clauses yields a pseudoconstant restriction. We might be able to skip scanning that child rel entirely (if we get constant FALSE), or generate a one-time filter. 8.3 more or less accidentally generated plans that weren't completely stupid in these cases, but that was only because an extra recursive level of subquery_planner() always occurred and allowed const-simplification to happen. 8.4's ability to pull up appendrel members with non-Var outputs exposes the fact that we need to work harder here. Per gripe from Sergey Burladyan.
2009-07-06Fix handling of changed-Param signaling for CteScan plan nodes. We were usingTom Lane
the "cteParam" as a proxy for the possibility that the underlying CTE plan depends on outer-level variables or Params, but that doesn't work very well because it sometimes causes calling subqueries to be treated as SubPlans when they could be InitPlans. This is inefficient and also causes the outright failure exhibited in bug #4902. Instead, leave the cteParam out of it and copy the underlying CTE plan's extParams directly. Per bug #4902 from Marko Tiikkaja.
2009-06-118.4 pgindent run, with new combined Linux/FreeBSD/MinGW typedef listBruce Momjian
provided by Andrew.
2009-05-12Modify find_inheritance_children() and find_all_inheritors() to add theTom Lane
ability to lock relations as they scan pg_inherits, and to ignore any relations that have disappeared by the time we get lock on them. This makes uses of these functions safe against concurrent DROP operations on child tables: we will effectively ignore any just-dropped child, rather than possibly throwing an error as in recent bug report from Thomas Johansson (and similar past complaints). The behavior should not change otherwise, since the code was acquiring those same locks anyway, just a little bit later. An exception is LockTableCommand(), which is still behaving unsafely; but that seems to require some more discussion before we change it.
2009-05-12Do some minor code refactoring in preparation for changing the APIs ofTom Lane
find_inheritance_children() and find_all_inheritors(). I got annoyed that these are buried inside the planner but mostly used elsewhere. So, create a new file catalog/pg_inherits.c and put them there, along with a couple of other functions that search pg_inherits. The code that modifies pg_inherits is (still) in tablecmds.c --- it's kind of entangled with unrelated code that modifies pg_depend and other stuff, so pulling it out seemed like a bigger change than I wanted to make right now. But this file provides a natural home for it if anyone ever gets around to that. This commit just moves code around; it doesn't change anything, except I succumbed to the temptation to make a couple of trivial optimizations in typeInheritsFrom().
2009-05-11Partially revert my patch of 2008-11-12 that installed a limit on the numberTom Lane
of AND/OR clause branches that predtest.c would attempt to deal with. As noted in bug #4721, that change disabled proof attempts for sizes of problems that people are actually expecting it to work for. The original complaint it was trying to solve was O(N^2) behavior for long IN-lists, so let's try applying the limit to just ScalarArrayOpExprs rather than everything. Another case of "foolish consistency" I fear. Back-patch to 8.2, same as the previous patch was.
2009-05-10Make a marginal performance improvement in predicate_implied_by andTom Lane
predicate_refuted_by: if either top-level input is a single-element list, reduce it to its lone member before proceeding. This avoids a useless level of AND-recursion within the recursive proof routines. It's worth doing because, for example, if the clause is a 100-element list and the predicate is a 1-element list then we'd otherwise strip the predicate's list structure 100 times as we iterate through the clause. It's only needed at top level because there won't be any trivial ANDs below that --- this situation is an artifact of the decision to represent even single-item conditions as Lists in the "implicit AND" format, and that format is only used at the top level of any predicate or restriction condition.
2009-05-09Fix cost_nestloop and cost_hashjoin to model the behavior of semi and antiTom Lane
joins a bit better, ie, understand the differing cost functions for matched and unmatched outer tuples. There is more that could be done in cost_hashjoin but this already helps a great deal. Per discussions with Robert Haas.
2009-05-07Ooops ... make_outerjoininfo wasn't actually enforcing the join orderTom Lane
restrictions specified for semijoins in optimizer/README, to wit that you can't reassociate outer joins into or out of the RHS of a semijoin. Per report from Heikki.
2009-05-06Tweak distribute_qual_to_rels so that when we decide a pseudoconstant qualTom Lane
can be pushed to the top of the join tree, we update both the relids and qualscope variables to keep them in sync. This prevents a possible later failure of an Assert clause, and affects nothing else since qualscope isn't used later except for that Assert. At the moment the Assert shouldn't be reachable when we've pushed the qual up; but this is cheap insurance, and it's more sensible anyway in terms of the overall logic of the routine. Per analysis of a bug report from Stefan Huehner. I'm not back-patching this since it's just future-proofing; but if anyone gets tempted to change check_outerjoin_delay again in the back branches, this might be needed.
2009-04-28Improve pull_up_subqueries logic so that it doesn't insert unnecessaryTom Lane
PlaceHolderVar nodes in join quals appearing in or below the lowest outer join that could null the subquery being pulled up. This improves the planner's ability to recognize constant join quals, and probably helps with detection of common sort keys (equivalence classes) as well.
2009-04-25Fix the handling of sub-SELECTs appearing in the arguments of an outer-levelTom Lane
aggregate function. By definition, such a sub-SELECT cannot reference any variables of query levels between itself and the aggregate's semantic level (else the aggregate would've been assigned to that lower level instead). So the correct, most efficient implementation is to treat the sub-SELECT as being a sub-select of that outer query level, not the level the aggregate syntactically appears in. Not doing so also confuses the heck out of our parameter-passing logic, as illustrated in bug report from Daniel Grace. Fortunately, we were already copying the whole Aggref expression up to the outer query level, so all that's needed is to delay SS_process_sublinks processing of the sub-SELECT until control returns to the outer level. This has been broken since we introduced spec-compliant treatment of outer aggregates in 7.4; so patch all the way back.
2009-04-19Fix estimate_num_groups() to not fail on PlaceHolderVars, per report fromTom Lane
Stefan Kaltenbrunner. The most reasonable behavior (at least for the near term) seems to be to ignore the PlaceHolderVar and examine its argument instead. In support of this, change the API of pull_var_clause() to allow callers to request recursion into PlaceHolderVars. Currently estimate_num_groups() is the only customer for that behavior, but where there's one there may be others.
2009-04-17Bump disable_cost up from 1e8 to 1e10, per gripe from Kris Jurka.Tom Lane
2009-04-16Fix planner to restore its previous level of intelligence about pushingTom Lane
constants through full joins, as in select * from tenk1 a full join tenk1 b using (unique1) where unique1 = 42; which should generate a fairly cheap plan where we apply the constraint unique1 = 42 in each relation scan. This had been broken by my patch of 2008-06-27, which is now reverted in favor of a more invasive but hopefully less incorrect approach. That patch was meant to prevent incorrect extraction of OR'd indexclauses from OR conditions above an outer join. To do that correctly we need more information than the outerjoin_delay flag can provide, so add a nullable_relids field to RestrictInfo that records exactly which relations are nulled by outer joins that are underneath a particular qual clause. A side benefit is that we can make the test in create_or_index_quals more specific: it is now smart enough to extract an OR'd indexclause into the outer side of an outer join, even though it must not do so in the inner side. The old coding couldn't distinguish these cases so it could not do either.
2009-04-05Change EXPLAIN output so that subplans and initplans (particularly CTEs)Tom Lane
are individually labeled, rather than just grouped under an "InitPlan" or "SubPlan" heading. This in turn makes it possible for decompilation of a subplan reference to usefully identify which subplan it's referencing. I also made InitPlans identify which parameter symbol(s) they compute, so that references to those parameters elsewhere in the plan tree can be connected to the initplan that will be executed. Per a gripe from Robert Haas about EXPLAIN output of a WITH query being inadequate, plus some longstanding pet peeves of my own.
2009-03-31Modify the relcache to record the temp status of both local and nonlocalTom Lane
temp relations; this is no more expensive than before, now that we have pg_class.relistemp. Insert tests into bufmgr.c to prevent attempting to fetch pages from nonlocal temp relations. This provides a low-level defense against bugs-of-omission allowing temp pages to be loaded into shared buffers, as in the contrib/pgstattuple problem reported by Stuart Bishop. While at it, tweak a bunch of places to use new relcache tests (instead of expensive probes into pg_namespace) to detect local or nonlocal temp tables.
2009-03-30Fix window function plan generation to cope with volatile sort expressions.Tom Lane
(Not clear how useful these really are, but failing is no good...) Per report from David Fetter and Robert Treat.
2009-03-26If we expect a hash join to be performed in multiple batches, suppressTom Lane
"physical tlist" optimization on the outer relation (ie, force a projection step to occur in its scan). This avoids storing useless column values when the outer relation's tuples are written to temporary batch files. Modified version of a patch by Michael Henderson and Ramon Lawrence.
2009-03-24Fix stupid parenthesization mistake. Per bug #4728 from Bruce Toll.Tom Lane
2009-03-21Optimize multi-batch hash joins when the outer relation has a nonuniformTom Lane
distribution, by creating a special fast path for the (first few) most common values of the outer relation. Tuples having hashvalues matching the MCVs are effectively forced to be in the first batch, so that we never write them out to the batch temp files. Bryce Cutt and Ramon Lawrence, with some editorialization by me.
2009-03-11Improve match_special_index_operator() to recognize that LIKE with anTom Lane
exact-match pattern (no wildcard) can be index-optimized in some cases where a prefix-match pattern cannot; specifically, since the required index clause is simple equality, it works for regular text/varchar indexes even when the locale is not C. I'm not sure how often this case really comes up, but since it requires hardly any additional work to handle it, we might as well get it right. Motivated by a discussion on the JDBC list.
2009-03-10Make SubPlan nodes carry the result's typmod as well as datatype OID. This isTom Lane
for consistency with the (relatively) recent addition of typmod to SubLink. An example of why it's a good idea is to be seen in the recent "failed to locate grouping columns" bug, which wouldn't have happened if a SubPlan exposed the same typmod info as the SubLink it was derived from. This could be back-patched, since it doesn't affect any on-disk data format, but for the moment it doesn't seem necessary to do so.