summaryrefslogtreecommitdiff
path: root/src/backend/optimizer
AgeCommit message (Collapse)Author
2011-02-20Implement an API to let foreign-data wrappers actually be functional.Tom Lane
This commit provides the core code and documentation needed. A contrib module test case will follow shortly. Shigeru Hanada, Jan Urbanski, Heikki Linnakangas
2011-02-16Fix bogus test for hypothetical indexes in get_actual_variable_range().Tom Lane
That function was supposing that indexoid == 0 for a hypothetical index, but that is not likely to be true in any non-toy implementation of an index adviser, since assigning a fake OID is the only way to know at EXPLAIN time which hypothetical index got selected. Fix by adding a flag to IndexOptInfo to mark hypothetical indexes. Back-patch to 9.0 where get_actual_variable_range() was added. Gurjeet Singh
2011-02-09Fix improper matching of resjunk column names for FOR UPDATE in subselect.Tom Lane
Flattening of subquery range tables during setrefs.c could lead to the rangetable indexes in PlanRowMark nodes not matching up with the column names previously assigned to the corresponding resjunk ctid (resp. tableoid or wholerow) columns. Typical symptom would be either a "cannot extract system attribute from virtual tuple" error or an Assert failure. This wasn't a problem before 9.0 because we didn't support FOR UPDATE below the top query level, and so the final flattening could never renumber an RTE that was relevant to FOR UPDATE. Fix by using a plan-tree-wide unique number for each PlanRowMark to label the associated resjunk columns, so that the number need not change during flattening. Per report from David Johnston (though I'm darned if I can see how this got past initial testing of the relevant code). Back-patch to 9.0.
2011-02-08Per-column collation supportPeter Eisentraut
This adds collation support for columns and domains, a COLLATE clause to override it per expression, and B-tree index support. Peter Eisentraut reviewed by Pavel Stehule, Itagaki Takahiro, Robert Haas, Noah Misch
2011-01-30Make reduce_outer_joins() smarter about semijoins.Tom Lane
reduce_outer_joins() mistakenly treated a semijoin like a left join for purposes of deciding whether not-null constraints created by the join's quals could be passed down into the join's left-hand side (possibly resulting in outer-join simplification there). Actually, semijoin works like inner join for this purpose, ie, we do not need to see any rows that can't possibly satisfy the quals. Hence, two-line fix to treat semi and inner joins alike. Per observation by Andres Freund about a performance gripe from Yazan Suleiman. Back-patch to 8.4, since this oversight has been there since the current handling of semijoins was implemented.
2011-01-13Revert incorrect memory-conservation hack in inheritance_planner().Tom Lane
This reverts commit d1001a78ce612a16ea622b558f5fc2b68c45ab4c of 2010-12-05, which was broken as reported by Jeff Davis. The problem is that the individual planning steps may have side-effects on substructures of PlannerGlobal, not only the current PlannerInfo root. Arranging to keep all such side effects in the main planning context is probably possible, but it would change this from a quick local hack into a wide-ranging and rather fragile endeavor. Which it's not worth.
2011-01-12Fix PlanRowMark/ExecRowMark structures to handle inheritance correctly.Tom Lane
In an inherited UPDATE/DELETE, each target table has its own subplan, because it might have a column set different from other targets. This means that the resjunk columns we add to support EvalPlanQual might be at different physical column numbers in each subplan. The EvalPlanQual rewrite I did for 9.0 failed to account for this, resulting in possible misbehavior or even crashes during concurrent updates to the same row, as seen in a recent report from Gordon Shannon. Revise the data structure so that we track resjunk column numbers separately for each subplan. I also chose to move responsibility for identifying the physical column numbers back to executor startup, instead of assuming that numbers derived during preprocess_targetlist would stay valid throughout subsequent massaging of the plan. That's a bit slower, so we might want to consider undoing it someday; but it would complicate the patch considerably and didn't seem justifiable in a bug fix that has to be back-patched to 9.0.
2011-01-11Tweak create_index_paths()'s test for whether to consider a bitmap scan.Tom Lane
Per my note of a couple days ago, create_index_paths would refuse to consider any path at all for GIN indexes if the selectivity estimate came out as 1.0; not even if you tried to force it with enable_seqscan. While this isn't really a bad outcome in practice, it could be annoying for testing purposes. Adjust the test for "is this path only useful for sorting" so that it doesn't fire on paths with nil pathkeys, which will include all GIN paths.
2011-01-01Basic foreign table support.Robert Haas
Foreign tables are a core component of SQL/MED. This commit does not provide a working SQL/MED infrastructure, because foreign tables cannot yet be queried. Support for foreign table scans will need to be added in a future patch. However, this patch creates the necessary system catalog structure, syntax support, and support for ancillary operations such as COMMENT and SECURITY LABEL. Shigeru Hanada, heavily revised by Robert Haas
2011-01-01Stamp copyrights for year 2011.Bruce Momjian
2010-12-31Invert and rename flag variable to improve code readability.Tom Lane
No change in functionality. Per discussion with Robert.
2010-12-30Support RIGHT and FULL OUTER JOIN in hash joins.Tom Lane
This is advantageous first because it allows us to hash the smaller table regardless of the outer-join type, and second because hash join can be more flexible than merge join in dealing with arbitrary join quals in a FULL join. For merge join all the join quals have to be mergejoinable, but hash join will work so long as there's at least one hashjoinable qual --- the others can be any condition. (This is true essentially because we don't keep per-inner-tuple match flags in merge join, while hash join can do so.) To do this, we need a has-it-been-matched flag for each tuple in the hashtable, not just one for the current outer tuple. The key idea that makes this practical is that we can store the match flag in the tuple's infomask, since there are lots of bits there that are of no interest for a MinimalTuple. So we aren't increasing the size of the hashtable at all for the feature. To write this without turning the hash code into even more of a pile of spaghetti than it already was, I rewrote ExecHashJoin in a state-machine style, similar to ExecMergeJoin. Other than that decision, it was pretty straightforward.
2010-12-19Fix up handling of simple-form CASE with constant test expression.Tom Lane
eval_const_expressions() can replace CaseTestExprs with constants when the surrounding CASE's test expression is a constant. This confuses ruleutils.c's heuristic for deparsing simple-form CASEs, leading to Assert failures or "unexpected CASE WHEN clause" errors. I had put in a hack solution for that years ago (see commit 514ce7a331c5bea8e55b106d624e55732a002295 of 2006-10-01), but bug #5794 from Peter Speck shows that that solution failed to cover all cases. Fortunately, there's a much better way, which came to me upon reflecting that Peter's "CASE TRUE WHEN" seemed pretty redundant: we can "simplify" the simple-form CASE to the general form of CASE, by simply omitting the constant test expression from the rebuilt CASE construct. This is intuitively valid because there is no need for the executor to evaluate the test expression at runtime; it will never be referenced, because any CaseTestExprs that would have referenced it are now replaced by constants. This won't save a whole lot of cycles, since evaluating a Const is pretty cheap, but a cycle saved is a cycle earned. In any case it beats kluging ruleutils.c still further. So this patch improves const-simplification and reverts the previous change in ruleutils.c. Back-patch to all supported branches. The bug exists in 8.1 too, but it's out of warranty.
2010-12-13Allow plugins to suppress inlining and hook function entry/exit/abort.Robert Haas
This is intended as infrastructure to allow an eventual SE-Linux plugin to support trusted procedures. KaiGai Kohei
2010-12-05Reduce memory consumption inside inheritance_planner().Tom Lane
Avoid eating quite so much memory for large inheritance trees, by reclaiming the space used by temporary copies of the original parsetree and range table, as well as the workspace needed during planning. The cost is needing to copy the finished plan trees out of the child memory context. Although this looks like it ought to slow things down, my testing shows it actually is faster, apparently because fewer interactions with malloc() are needed and/or we can do the work within a more readily cacheable amount of memory. That result might be platform-dependent, but I'll take it. Per a gripe from John Papandriopoulos, in which it was pointed out that the memory consumption actually grew as O(N^2) for sufficiently many child tables, since we were creating N copies of the N-element range table.
2010-12-02Create core infrastructure for KNNGIST.Tom Lane
This is a heavily revised version of builtin_knngist_core-0.9. The ordering operators are no longer mixed in with actual quals, which would have confused not only humans but significant parts of the planner. Instead, ordering operators are carried separately throughout planning and execution. Since the API for ambeginscan and amrescan functions had to be changed anyway, this commit takes the opportunity to rationalize that a bit. RelationGetIndexScan no longer forces a premature index_rescan call; instead, callers of index_beginscan must call index_rescan too. Aside from making the AM-side initialization logic a bit less peculiar, this has the advantage that we do not make a useless extra am_rescan call when there are runtime key values. AMs formerly could not assume that the key values passed to amrescan were actually valid; now they can. Teodor Sigaev and Tom Lane
2010-12-01Prevent inlining a SQL function with multiple OUT parameters.Tom Lane
There were corner cases in which the planner would attempt to inline such a function, which would result in a failure at runtime due to loss of information about exactly what the result record type is. Fix by disabling inlining when the function's recorded result type is RECORD. There might be some sub-cases where inlining could still be allowed, but this is a simple and backpatchable fix, so leave refinements for another day. Per bug #5777 from Nate Carson. Back-patch to all supported branches. 8.1 happens to avoid a core-dump here, but it still does the wrong thing.
2010-11-29Simplify and speed up mapping of index opfamilies to pathkeys.Tom Lane
Formerly we looked up the operators associated with each index (caching them in relcache) and then the planner looked up the btree opfamily containing such operators in order to build the btree-centric pathkey representation that describes the index's sort order. This is quite pointless for btree indexes: we might as well just use the index's opfamily information directly. That saves syscache lookup cycles during planning, and furthermore allows us to eliminate the relcache's caching of operators altogether, which may help in reducing backend startup time. I added code to plancat.c to perform the same type of double lookup on-the-fly if it's ever faced with a non-btree amcanorder index AM. If such a thing actually becomes interesting for production, we should replace that logic with some more-direct method for identifying the corresponding btree opfamily; but it's not worth spending effort on now. There is considerably more to do pursuant to my recent proposal to get rid of sort-operator-based representations of sort orderings, but this patch grabs some of the low-hanging fruit. I'll look at the remainder of that work after the current commitfest.
2010-11-24Create the system catalog infrastructure needed for KNNGIST.Tom Lane
This commit adds columns amoppurpose and amopsortfamily to pg_amop, and column amcanorderbyop to pg_am. For the moment all the entries in amcanorderbyop are "false", since the underlying support isn't there yet. Also, extend the CREATE OPERATOR CLASS/ALTER OPERATOR FAMILY commands with [ FOR SEARCH | FOR ORDER BY sort_operator_family ] clauses to allow the new columns of pg_amop to be populated, and create pg_dump support for dumping that information. I also added some documentation, although it's perhaps a bit premature given that the feature doesn't do anything useful yet. Teodor Sigaev, Robert Haas, Tom Lane
2010-11-23Remove useless whitespace at end of linesPeter Eisentraut
2010-11-20Further cleanup of indxpath logic related to IndexOptInfo.opfamily array.Tom Lane
We no longer need the terminating zero entry in opfamily[], so get rid of it. Also replace assorted ad-hoc looping logic with simple for and foreach constructs. This code is now noticeably more readable than it was an hour ago; credit to Robert for seeing that it could be simplified.
2010-11-20Minor cleanup of indxpath.c.Robert Haas
Eliminate some superfluous notational complexity around match_clause_to_indexcol(), and rip out the DoneMatchingIndexKeys crock.
2010-11-19Improve relation width estimation for subqueries.Tom Lane
As per the ancient comment for set_rel_width, it really wasn't much good for relations that aren't plain tables: it would never find any stats and would always fall back on datatype-based estimates, which are often pretty silly. Fix that by copying up width estimates from the subquery planning process. At some point we might want to do this for CTEs too, but that would be a significantly more invasive patch because the sub-PlannerInfo is no longer accessible by the time it's needed. I refrained from doing anything about that, partly for fear of breaking the unmerged CTE-related patches. In passing, also generate less bogus width estimates for whole-row Vars. Per a gripe from Jon Nelson.
2010-11-18Further fallout from the MergeAppend patch.Tom Lane
Fix things so that top-N sorting can be used in child Sort nodes of a MergeAppend node, when there is a LIMIT and no intervening joins or grouping. Actually doing this on the executor side isn't too bad, but it's a bit messier to get the planner to cost it properly. Per gripe from Robert Haas. In passing, fix an oversight in the original top-N-sorting patch: query_planner should not assume that a LIMIT can be used to make an explicit sort cheaper when there will be grouping or aggregation in between. Possibly this should be back-patched, but I'm not sure the mistake is serious enough to be a real problem in practice.
2010-11-12Fix old oversight in const-simplification of COALESCE() expressions.Tom Lane
Once we have found a non-null constant argument, there is no need to examine additional arguments of the COALESCE. The previous coding got it right only if the constant was in the first argument position; otherwise it tried to simplify following arguments too, leading to unexpected behavior like this: regression=# select coalesce(f1, 42, 1/0) from int4_tbl; ERROR: division by zero It's a minor corner case, but a bug is a bug, so back-patch all the way.
2010-11-08Use appendrel planning logic for top-level UNION ALL structures.Tom Lane
Formerly, we could convert a UNION ALL structure inside a subquery-in-FROM into an appendrel, as a side effect of pulling up the subquery into its parent; but top-level UNION ALL always caused use of plan_set_operations(). That didn't matter too much because you got an Append-based plan either way. However, now that the appendrel code can do things with MergeAppend, it's worthwhile to hack up the top-level case so it also uses appendrels. This is a bit of a stopgap; but going much further than this will require a major rewrite of the planner's set-operations support, which I'm not prepared to undertake now. For the moment let's grab the low-hanging fruit.
2010-11-04Reimplement planner's handling of MIN/MAX aggregate optimization.Tom Lane
Per my recent proposal, get rid of all the direct inspection of indexes and manual generation of paths in planagg.c. Instead, set up EquivalenceClasses for the aggregate argument expressions, and let the regular path generation logic deal with creating paths that can satisfy those sort orders. This makes planagg.c a bit more visible to the rest of the planner than it was originally, but the approach is basically a lot cleaner than before. A major advantage of doing it this way is that we get MIN/MAX optimization on inheritance trees (using MergeAppend of indexscans) practically for free, whereas in the old way we'd have had to add a whole lot more duplicative logic. One small disadvantage of this approach is that MIN/MAX aggregates can no longer exploit partial indexes having an "x IS NOT NULL" predicate, unless that restriction or something that implies it is specified in the query. The previous implementation was able to use the added "x IS NOT NULL" condition as an extra predicate proof condition, but in this version we rely entirely on indexes that are considered usable by the main planning process. That seems a fair tradeoff for the simplicity and functionality gained.
2010-11-02Fix adjust_semi_join to be more cautious about clauseless joins.Tom Lane
It was reporting that these were fully indexed (hence cheap), when of course they're the exact opposite of that. I'm not certain if the case would arise in practice, since a clauseless semijoin is hard to produce in SQL, but if it did happen we'd make some dumb decisions.
2010-10-30Provide hashing support for arrays.Tom Lane
The core of this patch is hash_array() and associated typcache infrastructure, which works just about exactly like the existing support for array comparison. In addition I did some work to ensure that the planner won't think that an array type is hashable unless its element type is hashable, and similarly for sorting. This includes adding a datatype parameter to op_hashjoinable and op_mergejoinable, and adding an explicit "hashable" flag to SortGroupClause. The lack of a cross-check on the element type was a pre-existing bug in mergejoin support --- but it didn't matter so much before, because if you couldn't sort the element type there wasn't any good alternative to failing anyhow. Now that we have the alternative of hashing the array type, there are cases where we can avoid a failure by being picky at the planner stage, so it's time to be picky. The issue of exactly how to combine the per-element hash values to produce an array hash is still open for discussion, but the rest of this is pretty solid, so I'll commit it as-is.
2010-10-29Oops, missed one fix for EquivalenceClass rearrangement.Tom Lane
Now that we're expecting a mergeclause's left_ec/right_ec to persist from the initial assignments, we can't just blithely zero these out when transforming such a clause in adjust_appendrel_attrs. But really it should be okay to keep the parent's values, since a child table's derived Var ought to be equivalent to the parent Var for all EquivalenceClass purposes. (Indeed, I'm wondering whether we couldn't find a way to dispense with add_child_rel_equivalences altogether. But this is wrong in any case.)
2010-10-29Avoid creation of useless EquivalenceClasses during planning.Tom Lane
Zoltan Boszormenyi exhibited a test case in which planning time was dominated by construction of EquivalenceClasses and PathKeys that had no actual relevance to the query (and in fact got discarded immediately). This happened because we generated PathKeys describing the sort ordering of every index on every table in the query, and only after that checked to see if the sort ordering was relevant. The EC/PK construction code is O(N^2) in the number of ECs, which is all right for the intended number of such objects, but it gets out of hand if there are ECs for lots of irrelevant indexes. To fix, twiddle the handling of mergeclauses a little bit to ensure that every interesting EC is created before we begin path generation. (This doesn't cost anything --- in fact I think it's a bit cheaper than before --- since we always eventually created those ECs anyway.) Then, if an index column can't be found in any pre-existing EC, we know that that sort ordering is irrelevant for the query. Instead of creating a useless EC, we can just not build a pathkey for the index column in the first place. The index will still be considered if it's useful for non-order-related reasons, but we will think of its output as unsorted.
2010-10-25Fix inline_set_returning_function() to preserve the invalItems list properly.Tom Lane
This avoids a possible crash when inlining a SRF whose argument list contains a reference to an inline-able user function. The crash is quite reproducible with CLOBBER_FREED_MEMORY enabled, but would be less certain in a production build. Problem introduced in 9.0 by the named-arguments patch, which requires invoking eval_const_expressions() before we can try to inline a SRF. Per report from Brendan Jurd.
2010-10-19Fix incorrect generation of whole-row variables in planner.Tom Lane
A couple of places in the planner need to generate whole-row Vars, and were cutting corners by setting vartype = RECORDOID in the Vars, even in cases where there's an identifiable named composite type for the RTE being referenced. While we mostly got away with this, it failed when there was also a parser-generated whole-row reference to the same RTE, because the two Vars weren't equal() due to the difference in vartype. Fix by providing a subroutine the planner can call to generate whole-row Vars the same way the parser does. Per bug #5716 from Andrew Tipton. Back-patch to 9.0 where one of the bogus calls was introduced (the other one is new in HEAD).
2010-10-14Support MergeAppend plans, to allow sorted output from append relations.Tom Lane
This patch eliminates the former need to sort the output of an Append scan when an ordered scan of an inheritance tree is wanted. This should be particularly useful for fast-start cases such as queries with LIMIT. Original patch by Greg Stark, with further hacking by Hans-Jurgen Schonig, Robert Haas, and Tom Lane.
2010-10-10Improve the planner's simplification of NOT constructs.Tom Lane
This patch merges the responsibility for NOT-flattening into eval_const_expressions' processing. It wasn't done that way originally because prepqual.c is far older than eval_const_expressions. But putting this work into eval_const_expressions saves one pass over the qual trees, and in fact saves even more than that because we can exploit the knowledge that the subexpressions have already been recursively simplified. Doing it this way also lets us do it uniformly over all expressions, whereas prepqual.c formerly just did it at top level to save cycles. That should improve the planner's ability to recognize logically-equivalent constructs. While at it, also add the ability to fold a NOT into BooleanTest and NullTest constructs (the latter only for the scalar-datatype case). Per discussion of bug #5702.
2010-10-10Support triggers on views.Tom Lane
This patch adds the SQL-standard concept of an INSTEAD OF trigger, which is fired instead of performing a physical insert/update/delete. The trigger function is passed the entire old and/or new rows of the view, and must figure out what to do to the underlying tables to implement the update. So this feature can be used to implement updatable views using trigger programming style rather than rule hacking. In passing, this patch corrects the names of some columns in the information_schema.triggers view. It seems the SQL committee renamed them somewhere between SQL:99 and SQL:2003. Dean Rasheed, reviewed by Bernd Helmle; some additional hacking by me.
2010-10-07Teach CLUSTER to use seqscan-and-sort when it's faster than indexscan.Tom Lane
... or at least, when the planner's cost estimates say it will be faster. Leonardo Francalanci, reviewed by Itagaki Takahiro and Tom Lane
2010-09-28Fix PlaceHolderVar mechanism's interaction with outer joins.Tom Lane
The point of a PlaceHolderVar is to allow a non-strict expression to be evaluated below an outer join, after which its value bubbles up like a Var and can be forced to NULL when the outer join's semantics require that. However, there was a serious design oversight in that, namely that we didn't ensure that there was actually a correct place in the plan tree to evaluate the placeholder :-(. It may be necessary to delay evaluation of an outer join to ensure that a placeholder that should be evaluated below the join can be evaluated there. Per recent bug report from Kirill Simonov. Back-patch to 8.4 where the PlaceHolderVar mechanism was introduced.
2010-09-25Fix another join removal bug: the check on PlaceHolderVars was wrong.Tom Lane
The previous coding would decide that join removal was unsafe upon finding a PlaceHolderVar that needed to be evaluated at the inner rel and then used above the join. However, this fails to cover the case of PlaceHolderVars that refer to both the inner rel and some other rels. Per bug report from Andrus.
2010-09-23Avoid sharing subpath list structure when flattening nested AppendRels.Tom Lane
In some situations the original coding led to corrupting the child AppendRel's subpaths list, effectively adding other members of the parent's list to it. This was usually masked because we never made any further use of the child's list, but given the right combination of circumstances, we could do so. The visible symptom would be a relation getting scanned twice, as in bug #5673 from David Schmitt. Backpatch to 8.2, which is as far back as the risky coding appears. The example submitted by David only fails in 8.4 and later, but I'm not convinced that there aren't any even-more-obscure cases where 8.2 and 8.3 would fail.
2010-09-21Some more cleanup of CVS keyword noise.Tom Lane
Poking around for remaining occurrences of CVS keyword strings, I came across one that apparently reflects the use of a $Revision: ...$ string in the original input data. Dunno why anybody would be using that in an MTA's Received: lines, but there it is. Put it back to the way that it was originally, according to inspection of the CVS repo.
2010-09-20Remove cvs keywords from all files.Magnus Hagander
2010-09-14Fix join-removal logic for pseudoconstant and outerjoin-delayed quals.Tom Lane
In these cases a qual can get marked with the removable rel in its required_relids, but this is just to schedule its evaluation correctly, not because it really depends on the rel. We were assuming that, in effect, we could throw away *all* quals so marked, which is nonsense. Tighten up the logic to be a little more paranoid about which quals belong to the outer join being considered for removal, and arrange for all quals that don't belong to be updated so they will still get evaluated correctly. Also fix another problem that happened to be exposed by this test case, which was that make_join_rel() was failing to notice some cases where a constant-false qual could be used to prove a join relation empty. If it's a pushed-down constant false, then the relation is empty even if it's an outer join, because the qual applies after the outer join expansion. Per report from Nathan Grange. Back-patch into 9.0.
2010-08-27Small refactoring of makeVar() from a TargetEntryPeter Eisentraut
2010-08-19Remove extra newlines at end and beginning of files, add missing newlinesPeter Eisentraut
at end of files.
2010-08-14Fix planner to make a reasonable assumption about the amount of memory spaceTom Lane
used by array_agg(), string_agg(), and similar aggregate functions that use "internal" as their transition datatype. The previous coding thought this took *no* extra space, since "internal" is pass-by-value; but actually these aggregates typically consume a great deal of space. Per bug #5608 from Itagaki Takahiro, and fix suggestion from Hitoshi Harada. Back-patch to 8.4, where array_agg was introduced.
2010-07-12Make NestLoop plan nodes pass outer-relation variables into their innerTom Lane
relation using the general PARAM_EXEC executor parameter mechanism, rather than the ad-hoc kluge of passing the outer tuple down through ExecReScan. The previous method was hard to understand and could never be extended to handle parameters coming from multiple join levels. This patch doesn't change the set of possible plans nor have any significant performance effect, but it's necessary infrastructure for future generalization of the concept of an inner indexscan plan. ExecReScan's second parameter is now unused, so it's removed.
2010-07-08Fix "cannot handle unplanned sub-select" error that can occur when aTom Lane
sub-select contains a join alias reference that expands into an expression containing another sub-select. Per yesterday's report from Merlin Moncure and subsequent off-list investigation. Back-patch to 7.4. Older versions didn't attempt to flatten sub-selects in ways that would trigger this problem.
2010-07-06pgindent run for 9.0, second runBruce Momjian
2010-06-21Fix mishandling of whole-row Vars referencing a view or sub-select.Tom Lane
If such a Var appeared within a nested sub-select, we failed to translate it correctly during pullup of the view, because the recursive call to replace_rte_variables_mutator was looking for the wrong sublevels_up value. Bug was introduced during the addition of the PlaceHolderVar mechanism. Per bug #5514 from Marcos Castedo.