- 26 Oct, 2017 5 commits
-
-
Tom Lane authored
Previously, we skipped using search_indexed_tlist_for_sortgroupref() if the tlist expression being sought in the child plan node was merely a Var. This is purely an optimization, based on the theory that search_indexed_tlist_for_var() is faster, and one copy of a Var should be as good as another. However, the GROUPING SETS patch broke the latter assumption: grouping columns containing the "same" Var can sometimes have different outputs, as shown in the test case added here. So do it the hard way whenever a ressortgroupref marking exists. (If this seems like a bottleneck, we could imagine building a tlist index data structure for ressortgroupref values, as we do for Vars. But I'll let that idea go until there's some evidence it's worthwhile.) Back-patch to 9.6. The problem also exists in 9.5 where GROUPING SETS came in, but this patch is insufficient to resolve the problem in 9.5: there is some obscure dependency on the upper-planner-pathification work that happened in 9.6. Given that this is such a weird corner case, and no end users have complained about it, it doesn't seem worth the work to develop a fix for 9.5. Patch by me, per a report from Heikki Linnakangas. (This does not fix Heikki's original complaint, just the follow-on one.) Discussion: https://postgr.es/m/aefc657e-edb2-64d5-6df1-a0828f6e9104@iki.fi
-
Andrew Dunstan authored
There have been numerous buildfarm failures but the diagnostic is currently silent about the reason for failure to open the file. Let's see if we can get to the bottom of it. Backpatch to all live branches.
-
Andrew Dunstan authored
-
Robert Haas authored
Kyotaro Horiguchi, reviewed Masahiko Sawada, David G. Johnston, Álvaro Herrera, and me. Grammar correction to the final posted patch by me. Discussion: http://postgr.es/m/20170329.124649.193656100.horiguchi.kyotaro@lab.ntt.co.jp
-
Michael Meskes authored
Patch by Tsunakawa Takayuki <tsunakawa.takay@jp.fujitsu.com>
-
- 25 Oct, 2017 3 commits
-
-
Tom Lane authored
Some people like to run libpq-using applications in environments where there's no home directory. We've broken that scenario before (cf commits 5b406779 and bd58d9d8), and commit ba005f19 broke it again, by making it a hard error if we fail to get the home directory name while looking for ~/.pgpass. The previous precedent is that if we can't get the home directory name, we should just silently act as though the file we hoped to find there doesn't exist. Rearrange the new code to honor that. Looking around, the service-file code added by commit 41a4e459 had the same disease. Apparently, that escaped notice because it only runs when a service name has been specified, which I guess the people who use this scenario don't do. Nonetheless, it's wrong too, so fix that case as well. Add a comment about this policy to pqGetHomeDirectory, in the probably vain hope of forestalling the same error in future. And upgrade the rather miserable commenting in parseServiceInfo, too. In passing, also back off parseServiceInfo's assumption that only ENOENT is an ignorable error from stat() when checking a service file. We would need to ignore at least ENOTDIR as well (cf 5b406779), and seeing that the far-better-tested code for ~/.pgpass treats all stat() failures alike, I think this code ought to as well. Per bug #14872 from Dan Watson. Back-patch the .pgpass change to v10 where ba005f19 came in. The service-file bugs are far older, so back-patch the other changes to all supported branches. Discussion: https://postgr.es/m/20171025200457.1471.34504@wrigleys.postgresql.org
-
Andrew Dunstan authored
json_build_object and json_build_array and the jsonb equivalents did not correctly process explicit VARIADIC arguments. They are modified to use the new extract_variadic_args() utility function which abstracts away the details of the call method. Michael Paquier, reviewed by Tom Lane and Dmitry Dolgov. Backpatch to 9.5 for the jsonb fixes and 9.4 for the json fixes, as that's where they originated.
-
Andrew Dunstan authored
This is epecially useful in the case or "VARIADIC ANY" functions. The caller can get the artguments and types regardless of whether or not and explicit VARIADIC array argument has been used. The function also provides an option to convert arguments on type "unknown" to to "text". Michael Paquier and me, reviewed by Tom Lane. Backpatch to 9.4 in order to support the following json bug fix.
-
- 24 Oct, 2017 2 commits
-
-
Tom Lane authored
Although joinaliasvars lists coming out of the parser are quite simple, those lists can contain arbitrarily complex expressions after subquery pullup. We do not perform expression preprocessing on them, meaning that expressions in those lists will not meet the expectations of later phases of the planner (for example, that they do not contain SubLinks). This had been thought pretty harmless, since we don't intentionally touch those lists in later phases --- but Andreas Seltenreich found a case in which adjust_appendrel_attrs() could recurse into a joinaliasvars list and then die on its assertion that it never sees a SubLink. We considered a couple of localized fixes to prevent that specific case from looking at the joinaliasvars lists, but really this seems like a generic hazard for all expression processing in the planner. Therefore, probably the best answer is to delete the joinaliasvars lists from the parsetree at the end of expression preprocessing, so that there are no reachable expressions that haven't been through preprocessing. The case Andreas found seems to be harmless in non-Assert builds, and so far there are no field reports suggesting that there are user-visible effects in other cases. I considered back-patching this anyway, but it turns out that Andreas' test doesn't fail at all in 9.4-9.6, because in those versions adjust_appendrel_attrs contains code (added in commit 842faa71 and removed again in commit 215b43cd) to process SubLinks rather than complain about them. Barring discovery of another path by which unprocessed joinaliasvars lists can cause trouble, the most prudent compromise seems to be to patch this into v10 but not further. Patch by me, with thanks to Amit Langote for initial investigation and review. Discussion: https://postgr.es/m/87r2tvt9f1.fsf@ansel.ydns.eu
-
Tom Lane authored
I was a bit surprised to find that domains were almost completely unmentioned in the main SGML documentation, outside of the reference pages for CREATE/ALTER/DROP DOMAIN. In particular, noplace was it mentioned that we don't support domains over composite, making it hard to document the planned fix for that. Hence, add a section about domains to chapter 8 (Data Types). Also, modernize the type system overview in section 37.2; it had never heard of range types, and insisted on calling arrays base types, which seems a bit odd from a user's perspective; furthermore it didn't fit well with the fact that we now support arrays over types other than base types. It seems appropriate to use the term "container types" to describe all of arrays, composites, and ranges, so let's do that. Also a few other minor improvements, notably improve an example query in rowtypes.sgml by using a LATERAL function instead of an ad-hoc OFFSET 0 clause. In part this is mop-up for commit c12d570f, which missed updating 37.2 to reflect the fact that it added arrays of domains. We could possibly back-patch this without that claim, but I don't feel a strong need to.
-
- 23 Oct, 2017 3 commits
-
-
Tom Lane authored
DST law changes in Fiji, Namibia, Northern Cyprus, Sudan, Tonga, and Turks & Caicos Islands. Historical corrections for Alaska, Apia, Burma, Calcutta, Detroit, Ireland, Namibia, and Pago Pago.
-
Tom Lane authored
find_expr_references() neglected to record a dependency on the result type of a FieldSelect node, allowing a DROP TYPE to break a view or rule that contains such an expression. I think we'd omitted this case intentionally, reasoning that there would always be a related dependency ensuring that the DROP would cascade to the view. But at least with nested field selection expressions, that's not true, as shown in bug #14867 from Mansur Galiev. Add the dependency, and for good measure a dependency on the node's exposed collation. Likewise add a dependency on the result type of a FieldStore. I think here the reasoning was that it'd only appear within an assignment to a field, and the dependency on the field's column would be enough ... but having seen this example, I think that's wrong for nested-composites cases. Looking at nearby code, I notice we're not recording a dependency on the exposed collation of CoerceViaIO, which seems inconsistent with our choices for related node types. Maybe that's OK but I'm feeling suspicious of this code today, so let's add that; it certainly can't hurt. This patch does not do anything to protect already-existing views, only views created after it's installed. But seeing that the issue has been there a very long time and nobody noticed till now, that's probably good enough. Back-patch to all supported branches. Discussion: https://postgr.es/m/20171023150118.1477.19174@wrigleys.postgresql.org
-
- 22 Oct, 2017 1 commit
-
-
Tom Lane authored
It seems that the parray_gin extension has seen fit to introduce a "text[] @> text[]" operator, which conflicts with the core "anyarray @> anyarray" operator, causing ambiguous-operator failures if the input arguments are coercible to text[] without being exactly that type. This strikes me as a bad idea, but it's out there and people use it. As of v10, that breaks psql's query that tries to test "pg_statistic_ext.stxkind @> '{d}'", since stxkind is char[]. The best workaround seems to be to avoid use of that operator. We can use a scalar-vs-array test "'d' = any(stxkind)" instead; that's arguably more readable anyway. Per report from Justin Pryzby. Backpatch to v10 where this query was added. Discussion: https://postgr.es/m/20171022181525.GA21884@telsasoft.com
-
- 21 Oct, 2017 1 commit
-
-
Peter Eisentraut authored
The mostly automated conversion in 1ff01b39 missed this one because of the unusual whitespace.
-
- 20 Oct, 2017 4 commits
-
-
Peter Eisentraut authored
IDs in SGML are case insensitive, and we have accumulated a mix of upper and lower case IDs, including different variants of the same ID. In XML, these will be case sensitive, so we need to fix up those differences. Going to all lower case seems most straightforward, and the current build process already makes all anchors and lower case anyway during the SGML->XML conversion, so this doesn't create any difference in the output right now. A future XML-only build process would, however, maintain any mixed case ID spellings in the output, so that is another reason to clean this up beforehand. Author: Alexander Lakhin <exclusion@gmail.com>
-
Tom Lane authored
Like the similar logic for arrays and records, it's necessary to examine the range's subtype to decide whether the range type can support hashing. We can omit checking the subtype for btree-defined operations, though, since range subtypes are required to have those operations. (Possibly that simplification for btree cases led us to overlook that it does not apply for hash cases.) This is only an issue if the subtype lacks hash support, which is not true of any built-in range type, but it's easy to demonstrate a problem with a range type over, eg, money: you can get a "could not identify a hash function" failure when the planner is misled into thinking that hash join or aggregation would work. This was born broken, so back-patch to all supported branches.
-
Tom Lane authored
The previous coding would report that an array type supports extended hashing if its element type supports regular hashing. This bug is only latent at the moment, since AFAICS there is not yet any code that depends on checking presence of extended-hashing support to make any decisions. (And in any case it wouldn't matter unless the element type has only regular hashing, which isn't true of any core data type.) But that doesn't make it less broken. Extend the cache_array_element_properties infrastructure to check this properly.
-
Robert Haas authored
Inspired by a patch from Julien Rouhaud, but I reworded it. Discussion: http://postgr.es/m/CAOBaU_a8AH8=ypfqgHnDYu06ts+jWTUgh=VgCxA3yNV-K10j9w@mail.gmail.com
-
- 19 Oct, 2017 6 commits
-
-
Tom Lane authored
As noted by M. Justin. Also, to keep the HEAD and REL_10 versions of release-10.sgml in sync, back-patch the effects of c29c5789 on that file. We have a bigger problem there though :-( Discussion: https://postgr.es/m/CALtA7pmsQyTTD3fC2rmfUWgfivv5sCJJ84PHY0F_5t_SRc07Qg@mail.gmail.com Discussion: https://postgr.es/m/6d137bd0-eef6-1d91-d9b8-1a5e9195a899@2ndquadrant.com
-
Alvaro Herrera authored
This is preparation for a future patch to extensively change how reloptions work. Author: Nikolay Shaplov Reviewed-by: Michael Paquier Discussion: https://postgr.es/m/2615372.orqtEn8VGB@x200m
-
Magnus Hagander authored
David Rowley
-
Magnus Hagander authored
Masahiko Sawada
-
Magnus Hagander authored
Spotted by Piotr Stefaniak
-
Peter Eisentraut authored
Otherwise the order depends on the Perl hash implementation, making it cumbersome to scan the output when debugging.
-
- 18 Oct, 2017 2 commits
-
-
Alvaro Herrera authored
We say 'OWNER TO' in the synopsis; let's use that form elsewhere. There is a paragraph in the <note> section that refers to various subcommands very loosely (including OWNER); I didn't think it was an improvement to change that one. This is a fairly inconsequential change, so no backpatch. Author: Amit Langote Discussion: https://postgr.es/m/69ec7b51-03e5-f523-95ce-c070ee790e70@lab.ntt.co.jp
-
Alvaro Herrera authored
Author: David G. Johnston Discussion: https:/postgr.es/m/CAKFQuwaCsb-OKOjQXGeN0R7byxiRWvr7OtyKDbJoYgiF2vBG4Q@mail.gmail.com
-
- 17 Oct, 2017 2 commits
-
-
Peter Eisentraut authored
For DocBook XML compatibility, don't use SGML empty tags (</>) anymore, replace by the full tag name. Add a warning option to catch future occurrences. Alexander Lakhin, Jürgen Purtz
-
Alvaro Herrera authored
Reported by: David G. Johnston Discussion: https://postgr.es/m/CAKFQuwajWqjqEL9xc1xnnmTyBg32EdAZKJXijzigbosGSs_vag@mail.gmail.com
-
- 16 Oct, 2017 6 commits
-
-
Tom Lane authored
setTargetTable threw an error if the proposed target RangeVar's relname matched any visible CTE or ENR. This breaks backwards compatibility in the CTE case, since pre-v10 we never looked for a CTE here at all, so that CTE names did not mask regular tables. It does seem like a good idea to throw an error for the ENR case, though, thus causing ENRs to mask tables for this purpose; ENRs are new in v10 so we're not breaking existing code, and we may someday want to allow them to be the targets of DML. To fix that, replace use of getRTEForSpecialRelationTypes, which was overkill anyway, with use of scanNameSpaceForENR. A second problem was that the check neglected to verify null schemaname, so that a CTE or ENR could incorrectly be thought to match a qualified RangeVar. That happened because getRTEForSpecialRelationTypes relied on its caller to have checked for null schemaname. Even though the one remaining caller got it right, this is obviously bug-prone, so move the check inside getRTEForSpecialRelationTypes. Also, revert commit 18ce3a4a's extremely poorly thought out decision to add a NULL return case to parserOpenTable --- without either documenting that or adjusting any of the callers to check for it. The current bug seems to have arisen in part due to working around that bad idea. In passing, remove the one-line shim functions transformCTEReference and transformENRReference --- they don't seem to be adding any clarity or functionality. Per report from Hugo Mercier (via Julien Rouhaud). Back-patch to v10 where the bug was introduced. Thomas Munro, with minor editing by me Discussion: https://postgr.es/m/CAOBaU_YdPVH+PTtiKSSLOiiW3mVDYsnNUekK+XPbHXiP=wrFLA@mail.gmail.com
-
Peter Eisentraut authored
Flex generates a lot of functions that are not actually used. In order to avoid coverage figures being ruined by that, mark up the part of the .l files where the generated code appears by lcov exclusion markers. That way, lcov will typically only reported on coverage for the .l file, which is under our control, but not for the .c file. Reviewed-by: Michael Paquier <michael.paquier@gmail.com>
-
Tom Lane authored
There is no reason to insist that direct arguments must match before we can merge transition states of two aggregate calls. They're only used during the finalfn call, so we can treat them as like the finalfn itself. This allows, eg, merging of select percentile_cont(0.25) within group (order by a), percentile_disc(0.5) within group (order by a) from ... This didn't matter (and could not have been tested) before we allowed state merging of OSAs otherwise. Discussion: https://postgr.es/m/CAB4ELO5RZhOamuT9Xsf72ozbenDLLXZKSk07FiSVsuJNZB861A@mail.gmail.com
-
Tom Lane authored
The built-in OSAs all share the same transition function, so they can share transition state as long as the final functions cooperate to not do the sort step more than once. To avoid running the tuplesort object in randomAccess mode unnecessarily, add a bit of infrastructure to nodeAgg.c to let the aggregate functions find out whether the transition state is actually being shared or not. This doesn't work for the hypothetical aggregates, since those inject a hypothetical row that isn't traceable to the shared input state. So they remain marked aggfinalmodify = 'w'. Discussion: https://postgr.es/m/CAB4ELO5RZhOamuT9Xsf72ozbenDLLXZKSk07FiSVsuJNZB861A@mail.gmail.com
-
Tom Lane authored
An aggregate's input expression(s) are not supposed to be evaluated at all for a row where its FILTER test fails ... but commit 8ed3f11b overlooked that requirement. Reshuffle so that aggregates having a filter clause evaluate their arguments separately from those without. This still gets the benefit of doing only one ExecProject in the common case of multiple Aggrefs, none of which have filters. While at it, arrange for filter clauses to be included in the common ExecProject evaluation, thus perhaps buying a little bit even when there are filters. Back-patch to v10 where the bug was introduced. Discussion: https://postgr.es/m/30065.1508161354@sss.pgh.pa.us
-
Alvaro Herrera authored
Simplify coding using a switch rather than nested if tests. Author: Álvaro Reviewed-by: Robert Haas, Amit Langote, Michaël Paquier Discussion: https://postgr.es/m/20171013163820.pai7djcaxrntaxtn@alvherre.pgsql
-
- 15 Oct, 2017 2 commits
-
-
Tom Lane authored
While poking around in the aggregate logic, I noticed that commit 8ed3f11b broke the logic in nodeAgg.c that purports to detect nested aggregates, by moving initialization of regular aggregate argument expressions out of the code segment that checks for that. You could argue that this check is unnecessary, but it's not much code so I'm inclined to keep it as a backstop against parser and planner bugs. However, there's certainly zero value in checking only some of the subexpressions. We can make the check complete again, and as a bonus make it a good deal more bulletproof against future mistakes of the same ilk, by moving it out to the outermost level of ExecInitAgg. This means we need to check only once per Agg node not once per aggregate, which also seems like a good thing --- if the check does find something wrong, it's not urgent that we report it before the plan node initialization finishes. Since this requires remembering the original length of the aggs list, I deleted a long-obsolete stanza that changed numaggs from 0 to 1. That's so old it predates our decision that palloc(0) is a valid operation, in (digs...) 2004, see commit 24a1e20f. In passing improve a few comments. Back-patch to v10, just in case.
-
Peter Eisentraut authored
-
- 14 Oct, 2017 3 commits
-
-
Tom Lane authored
Buildfarm member gaur says it wasn't there in 2.95.3. Guess that 3.0 and later have it.
-
Tom Lane authored
Up to now, there's been hard-wired assumptions that normal aggregates' final functions never modify their transition states, while ordered-set aggregates' final functions always do. This has always been a bit limiting, and in particular it's getting in the way of improving the built-in ordered-set aggregates to allow merging of transition states. Therefore, let's introduce catalog and CREATE AGGREGATE infrastructure that lets the finalfn's behavior be declared explicitly. There are now three possibilities for the finalfn behavior: it's purely read-only, it trashes the transition state irrecoverably, or it changes the state in such a way that no more transfn calls are possible but the state can still be passed to other, compatible finalfns. There are no examples of this third case today, but we'll shortly make the built-in OSAs act like that. This change allows user-defined aggregates to explicitly disclaim support for use as window functions, and/or to prevent transition state merging, if their implementations cannot handle that. While it was previously possible to handle the window case with a run-time error check, there was not any way to prevent transition state merging, which in retrospect is something commit 804163bc should have provided for. But better late than never. In passing, split out pg_aggregate.c's extern function declarations into a new header file pg_aggregate_fn.h, similarly to what we've done for some other catalog headers, so that pg_aggregate.h itself can be safe for frontend files to include. This lets pg_dump use the symbolic names for relevant constants. Discussion: https://postgr.es/m/4834.1507849699@sss.pgh.pa.us
-
Peter Eisentraut authored
In c3d9a660, the genhtml --prefix option was removed to get slightly better behavior for vpath builds. genhtml would then automatically pick a suitable prefix. However, for non-vpath builds, this makes the coverage output dependent on the length of the path where the source code happens to be, leading to confusingly arbitrary results. So put the --prefix option back for non-vpath builds.
-