Update upstream #8
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The new functions return a list of files in the corresponding directory, not the name of the directory itself. Pointed out by Gianni Ciolli.
The grammar will only accept something syntactically similar to a function call in a function-in-FROM expression. However, there are various ways to input something that ruleutils.c won't deparse that way, potentially leading to a view or rule that fails dump/reload. Fix by inserting a dummy CAST around anything that isn't going to deparse as a function (which is one of the ways to get something like that in there in the first place). In HEAD, also make use of the infrastructure added by this to avoid emitting unnecessary parentheses in CREATE INDEX deparsing. I did not change that in back branches, thinking that people might find it to be unexpected/unnecessary behavioral change. In HEAD, also fix incorrect logic for when to add extra parens to partition key expressions. Somebody apparently thought they could get away with simpler logic than pg_get_indexdef_worker has, but they were wrong --- a counterexample is PARTITION BY LIST ((a[1])). Ignoring the prettyprint flag for partition expressions isn't exactly a nice solution anyway. This has been broken all along, so back-patch to all supported branches. Discussion: https://postgr.es/m/[email protected]
When writing a backup to stdout with pg_basebackup on Windows, put stdout to binary mode. Any CR bytes in the output will otherwise be output incorrectly as CR+LF. In the passing, standardize on using "_setmode" instead of "setmode", for the sake of consistency. They both do the same thing, but according to MSDN documentation, setmode is deprecated. Fixes bug #14634, reported by Henry Boehlert. Patch by Haribabu Kommi. Backpatch to all supported versions. Discussion: https://www.postgresql.org/message-id/[email protected]
In the frontend Makefiles that pull in libpgfeutils, we'd generally done it like this: LDFLAGS += -L$(top_builddir)/src/fe_utils -lpgfeutils $(libpq_pgport) That method is badly broken, as seen in bug #14742 from Chris Ruprecht. The -L flag for src/fe_utils ends up being placed after whatever random -L flags are in LDFLAGS already. That puts us at risk of pulling in libpgfeutils.a from some previous installation rather than the freshly built one in src/fe_utils. Also, the lack of an "override" is hazardous if someone tries to specify some LDFLAGS on the make command line. The correct way to do it is like this: override LDFLAGS := -L$(top_builddir)/src/fe_utils -lpgfeutils $(libpq_pgport) $(LDFLAGS) so that libpgfeutils, along with libpq, libpgport, and libpgcommon, are guaranteed to be pulled in from the build tree and not from any referenced system directory, because their -L flags will appear first. In some places we'd been even lazier and done it like this: LDFLAGS += -L$(top_builddir)/src/fe_utils -lpgfeutils -lpq which is subtly wrong in an additional way: on platforms where we can't restrict the symbols exported by libpq.so, it allows libpgfeutils to latch onto libpgport and libpgcommon symbols from libpq.so, rather than directly from those static libraries as intended. This carries hazards like those explained in the comments for the libpq_pgport macro. In addition to fixing the broken libpgfeutils usages, I tried to standardize on using $(libpq_pgport) like so: override LDFLAGS := $(libpq_pgport) $(LDFLAGS) even where libpgfeutils is not in the picture. This makes no difference right now but will hopefully discourage future mistakes of the same ilk. And it's more like the way we handle CPPFLAGS in libpq-using Makefiles. In passing, just for consistency, make pgbench include PTHREAD_LIBS the same way everyplace else does, ie just after LIBS rather than in some random place in the command line. This might have practical effect if there are -L switches in that macro on some platform. It looks to me like the MSVC build scripts are not affected by this error, but someone more familiar with them than I might want to double check. Back-patch to 9.6 where libpgfeutils was introduced. In 9.6, the hazard this error creates is that a reinstallation might link to the prior installation's copy of libpgfeutils.a and thereby fail to absorb a minor-version bug fix. Discussion: https://postgr.es/m/[email protected]
Add missing infrastructure for this node type, notably in ruleutils.c where its lack could demonstrably cause EXPLAIN to fail. Add outfuncs/readfuncs support. (outfuncs support is useful today for debugging purposes. The readfuncs support may never be needed, since at present it would only matter for parallel query and NextValueExpr should never appear in a parallelizable query; but it seems like a bad idea to have a primnode type that isn't fully supported here.) Teach planner infrastructure that NextValueExpr is a volatile, parallel-unsafe, non-leaky expression node with cost cpu_operator_cost. Given its limited scope of usage, there *might* be no live bug today from the lack of that knowledge, but it's certainly going to bite us on the rear someday. Teach pg_stat_statements about the new node type, too. While at it, also teach cost_qual_eval() that MinMaxExpr, SQLValueFunction, XmlExpr, and CoerceToDomain should be charged as cpu_operator_cost. Failing to do this for SQLValueFunction was an oversight in my commit 0bb51aa. The others are longer-standing oversights, but no time like the present to fix them. (In principle, CoerceToDomain could have cost much higher than this, but it doesn't presently seem worth trying to examine the domain's constraints here.) Modify execExprInterp.c to execute NextValueExpr as an out-of-line function; it seems quite unlikely to me that it's worth insisting that it be inlined in all expression eval methods. Besides, providing the out-of-line function doesn't stop anyone from inlining if they want to. Adjust some places where NextValueExpr support had been inserted with the aid of a dartboard rather than keeping it in the same order as elsewhere. Discussion: https://postgr.es/m/[email protected]
The original wording was impossible to translate correctly. Discussion: https://postgr.es/m/[email protected]
I got confused about why this function doesn't need to recursively search the expression tree for a CaseTestExpr node. After figuring that out, add a comment to save the next person some time.
Given this code's general eagerness to use subexpressions' output variables as temporary workspace, it's not exactly clear that it is safe for FieldStore to tell a newval subexpression that it can write into the same variable that is being supplied as a potential input. Document the chain of assumptions needed for that to be safe.
This change didn't adjust the publicly visible taptest function, causing buildfarm failures on bowerbird. Backpatch to 9.4 like previous change.
One, logging for CREATE INDEX was oblivious to the fact that when an unlogged table is created, *only* operations on the init fork should be logged. Two, init fork buffers need to be flushed after they are written; otherwise, a filesystem-level copy following recovery may do the wrong thing. (There may be a better fix for this issue than the one used here, but this is transposed from the similar logic already present in XLogReadBufferForRedoExtended, and a broader refactoring after beta2 seems inadvisable.) Amit Kapila, reviewed by Ashutosh Sharma, Kyotaro Horiguchi, and Michael Paquier Discussion: http://postgr.es/m/CAA4eK1JpcMsEtOL_J7WODumeEfyrPi7FPYHeVdS7fyyrCrgp4w@mail.gmail.com
select() for pure timeouts is not portable, and in particular doesn't work on Windows. Discussion: https://postgr.es/m/[email protected]
It seems pretty confusing to have tests named both largeobject and large_object. The latter is of very recent vintage (commit ff992c0), so get rid of it in favor of merging into the former. Also, enable the LO comment test that was added by commit 70ad7ed, since the later commit added the then-missing pg_upgrade functionality. The large_object.sql test case is almost completely redundant with that, but not quite: it seems like creating a user-defined LO with an OID in the system range might be an interesting case for pg_upgrade, so let's keep it. Like the earlier patch, back-patch to all supported branches. Discussion: https://postgr.es/m/[email protected]
We're throwing people into the guts of the syntax with not much context; let's back up one step and point out that this goes inside a literal in a CREATE FUNCTION command. Per suggestion from Kurt Kartaltepe. Discussion: https://postgr.es/m/CACawnnyWAmH+au8nfZhLiFfWKjXy4d0kY+eZWfcxPRnjVfaa_Q@mail.gmail.com
Before, we always used a dummy value of 1, but that's not right when the partitioned table being modified is inside of a WITH clause rather than part of the main query. Amit Langote, reported and reviewd by Etsuro Fujita, with a comment change by me. Discussion: http://postgr.es/m/[email protected]
Just as we already do in ExecConstraints, and for the same reason: to improve the quality of error messages. Etsuro Fujita, reviewed by Amit Langote Discussion: http://postgr.es/m/[email protected]
s/log_destination/log_directory/, per Jov in bug #14749. Report: https://postgr.es/m/[email protected]
In an off-list followup to bug #14745, Bob Jones complained that to_tsvector() on a 2MB jsonb value took an unreasonable amount of time and space --- enough to draw the wrath of the OOM killer on his machine. On my machine, his example proved to require upwards of 18 seconds and 4GB, which seemed pretty bogus considering that to_tsvector() on the same data treated as text took just a couple hundred msec and 10 or so MB. On investigation, the problem is that the implementation scans each string element of the json(b) and converts it to tsvector separately, then applies tsvector_concat() to join those separate tsvectors. The unreasonable memory usage came from leaking every single one of the transient tsvectors --- but even without that mistake, this is an O(N^2) or worse algorithm, because tsvector_concat() has to repeatedly process the words coming from earlier elements. We can fix it by accumulating all the lexeme data and applying make_tsvector() just once. As a side benefit, that also makes the desired adjustment of lexeme positions far cheaper, because we can just tweak the running "pos" counter between JSON elements. In passing, try to make the explanation of that tweak more intelligible. (I didn't think that a barely-readable comment far removed from the actual code was helpful.) And do some minor other code beautification.
It seemed a bit silly that each caller of make_tsvector() was laboriously special-casing the situation where no lexemes were found, when it would be easy and much more bullet-proof to make make_tsvector() handle that.
log_lock_waits is PGC_SUSET, but config.sgml lacked the standard boilerplate sentence noting that. Report: https://postgr.es/m/[email protected]
When pg_control was first designed, sizeof(ControlFileData) was small enough that a comment seemed like plenty to document the assumption that it'd fit into one disk sector. Now it's nearly 300 bytes, raising the possibility that somebody would carelessly add enough stuff to create a problem. Let's add a StaticAssertStmt() to ensure that the situation doesn't pass unnoticed if it ever occurs. While at it, rename PG_CONTROL_SIZE to PG_CONTROL_FILE_SIZE to make it clearer what that symbol means, and convert the existing runtime comparisons of sizeof(ControlFileData) vs. PG_CONTROL_FILE_SIZE to be static asserts --- we didn't have that technology when this code was first written. Discussion: https://postgr.es/m/[email protected]
Normally, a JoinExpr would have empty "quals" only if it came from CROSS JOIN syntax. However, it's possible to get to this state by specifying NATURAL JOIN between two tables with no common column names, and there might be other ways too. The code previously printed no ON clause if "quals" was empty; that's right for CROSS JOIN but syntactically invalid if it's some type of outer join. Fix by printing ON TRUE in that case. This got broken by commit 2ffa740, which stopped using NATURAL JOIN syntax in ruleutils output due to its brittleness in the face of column renamings. Back-patch to 9.3 where that commit appeared. Per report from Tushar Ahuja. Discussion: https://postgr.es/m/[email protected]
Claiming that NATURAL JOIN is equivalent to CROSS JOIN when there are no common column names is only strictly correct if it's an inner join; you can't say e.g. CROSS LEFT JOIN. Better to explain it as meaning JOIN ON TRUE, instead. Per a suggestion from David Johnston. Discussion: https://postgr.es/m/CAKFQuwb+mYszQhDS9f_dqRrk1=Pe-S6D=XMkAXcDf4ykKPmgKQ@mail.gmail.com
The previous description didn't make it clear that this change potentially breaks applications, partly because the entry wasn't even in the compatibility-hazard section. Move and clarify. Discussion: https://postgr.es/m/[email protected]
Previously, UNBOUNDED meant no lower bound when used in the FROM list, and no upper bound when used in the TO list, which was OK for single-column range partitioning, but problematic with multiple columns. For example, an upper bound of (10.0, UNBOUNDED) would not be collocated with a lower bound of (10.0, UNBOUNDED), thus making it difficult or impossible to define contiguous multi-column range partitions in some cases. Fix this by using MINVALUE and MAXVALUE instead of UNBOUNDED to represent a partition column that is unbounded below or above respectively. This syntax removes any ambiguity, and ensures that if one partition's lower bound equals another partition's upper bound, then the partitions are contiguous. Also drop the constraint prohibiting finite values after an unbounded column, and just document the fact that any values after MINVALUE or MAXVALUE are ignored. Previously it was necessary to repeat UNBOUNDED multiple times, which was needlessly verbose. Note: Forces a post-PG 10 beta2 initdb. Report by Amul Sul, original patch by Amit Langote with some additional hacking by me. Discussion: https://postgr.es/m/CAAJ_b947mowpLdxL3jo3YLKngRjrq9+Ej4ymduQTfYR+8=YAYQ@mail.gmail.com
The order of partitions listed by \d+ is in general locale-dependent. Rename the partitions in the test added by d363d42 to force them to be listed in a consistent order.
SLRU buffer lwlocks are allocated twice by oversight in commit fe702a7 where that locks were moved to separate tranche. The bug doesn't have user-visible effects except small overspending of shared memory. Backpatch to 9.6 where it was introduced. Alexander Korotkov with small editorization by me.
…anges. Previously, postgres_fdw would keep on using an existing connection even if the user did ALTER SERVER or ALTER USER MAPPING commands that should affect connection parameters. Teach it to watch for catcache invals on these catalogs and re-establish connections when the relevant catalog entries change. Per bug #14738 from Michal Lis. In passing, clean up some rather crufty decisions in commit ae9bfc5 about where fields of ConnCacheEntry should be reset. We now reset all the fields whenever we open a new connection. Kyotaro Horiguchi, reviewed by Ashutosh Bapat and myself. Back-patch to 9.3 where postgres_fdw appeared. Discussion: https://postgr.es/m/[email protected]
Add general purpose chaining hash tables for DSA memory. Unlike DynaHash in shared memory mode, these hash tables can grow as required, and cope with being mapped into different addresses in different backends. There is a wide range of potential users for such a hash table, though it's very likely the interface will need to evolve as we come to understand the needs of different kinds of users. E.g support for iterators and incremental resizing is planned for later commits and the details of the callback signatures are likely to change. Author: Thomas Munro Reviewed-By: John Gorman, Andres Freund, Dilip Kumar, Robert Haas Discussion: https://postgr.es/m/CAEepm=3d8o8XdVwYT6O=bHKsKAM2pu2D6sV1S_=4d+jStVCE7w@mail.gmail.com https://postgr.es/m/CAEepm=0ZtQ-SpsgCyzzYpsXS6e=kZWqk3g5Ygn3MDV7A8dabUA@mail.gmail.com
This was erroneously removed in 55a70a0.
Clarify/correct some error messages, fix up some code comments that confused SASL and SCRAM, and other minor fixes. No changes in functionality.
This does not use the normal plural handling, because no numbers appear in the actual message.
Author: Thomas Munro <[email protected]>
Reported-by: Alvaro Herrera <[email protected]>
for commit 237a0b8
The test case added by commit 1f6d515 fails on buildfarm members that have force_parallel_mode turned on, because we currently don't report sort performance details from worker processes back to the master. To fix that, just make the test table be temp rather than regular; that's a good idea anyway to forestall any possible interference from auto-analyze. (The restriction that workers can't access temp tables might go away someday, but almost certainly not before the other thing gets fixed.) Also, improve the test so that we retain as much as possible of the EXPLAIN ANALYZE output. This aids debugging failures, and might also expose problems that the preceding version masked. Discussion: http://postgr.es/m/CADE5jYLuugnEEUsyW6Q_4mZFYTxHxaVCQmGAsF0yiY8ZDggi-w@mail.gmail.com
The original value 12 was set based on RFC 5802 for SCRAM-SHA-1, but RFC 7677 for SCRAM-SHA-256 uses 16, so use that. (This does not affect the validity of already stored verifiers.) Discussion: https://www.postgresql.org/message-id/flat/12cc9297-7e05-932f-d863-765e5626ead4%402ndquadrant.com
Remove code meant for upgrading to a particular version of PostgreSQL 9.0. Since pg_upgrade only supports upgrading to the current major version, this code is no longer useful.
Set expanded output when requested through \gx in ExecQueryUsingCursor() (used when FETCH_COUNT is set). Discussion: https://www.postgresql.org/message-id/CB7A53AA-5645-4BDD-AB07-4D22CD9D8FF1%40gmx.net Author: Tobias Bussmann
Commit 16be2fd added DSA_ALLOC_HUGE, DSA_ALLOC_ZERO and DSA_ALLOC_NO_OOM which have the same numerical values and meanings as the similarly named MCXT_... macros. In one place we accidentally used MCXT_ALLOC_NO_OOM when DSA_ALLOC_NO_OOM is wanted, so tidy that up. Author: Thomas Munro Discussion: http://postgr.es/m/CAEepm=2AimHxVkkxnMfQvbZMkXy0uKbVa0-D38c5-qwrCm4CMQ@mail.gmail.com Backpatch: 10, where dsa was introduced.
Tidy-up for commit 8c0d7ba, based on a complaint from Andres Freund. Author: Thomas Munro Reviewed-By: Andres Freund Discussion: https://postgr.es/m/20170823054644.efuzftxjpfi6wwqs%40alap3.anarazel.de
Commit 8c0d7ba introduced dshash with hash and compare functions like DynaHash's, and also variants that take a user data pointer instead of size. Simplify the interface by merging them into a single pair of function pointer types that take both size and a user data pointer. Since it is anticipated that memcmp and tag_hash behavior will be a common requirement, provide wrapper functions dshash_memcmp and dshash_memhash that conform to the new function types. Author: Thomas Munro Reviewed-By: Andres Freund Discussion: https://postgr.es/m/20170823054644.efuzftxjpfi6wwqs%40alap3.anarazel.de
Test that blessed records can be transferred through a TupleQueue and correctly decoded by another backend. While touching the file, make sure that force_parallel_mode settings only cover relevant tests. Author: Thomas Munro, editorialized by Andres Freund Reviewed-By: Andres Freund Discussion: https://postgr.es/m/20170823054644.efuzftxjpfi6wwqs%40alap3.anarazel.de
Minor improvements for commit 1f6d515. We do not need the (rather expensive) test for SRFs in the targetlist, because since v10 any such SRFs would appear in separate ProjectSet nodes. Also, make the code look more like the existing cases by turning it into a simple recursion --- the argument that there might be some performance benefit to contorting the code seems unfounded to me, especially since any good compiler should turn the tail-recursion into iteration anyway. Discussion: http://postgr.es/m/CADE5jYLuugnEEUsyW6Q_4mZFYTxHxaVCQmGAsF0yiY8ZDggi-w@mail.gmail.com
Author: Vinayak Pokale Reviewed-By: Masahiko Sawada
related to 6ce6a61 Reported-by: Christoph Berg <[email protected]>
Force sorting in "C" locale so that the output ordering doesn't vary, per buildfarm. In passing, add missing .gitignore entries. Discussion: https://postgr.es/m/[email protected]
Our documentation hasn't really caught up with the fact that non-exclusive backups can now be taken using pg_start_backup and pg_stop_backup even on standbys. Update, also correcting some errors introduced by 52f8a59. Updates to the 9.6 documentation are needed as well, but that will need a separate patch as some things are different on that version. David Steele, reviewed by Robert Haas and Michael Paquier Discussion: http://postgr.es/m/[email protected]
Fix threaded test cases on Windows not to crash in setlocale() which can be global or local to a thread on Windows. Author: Christian Ullrich
The string "% of total" was marked by xgettext to be a c-format, but it is actually not, so mark up the source to prevent that. Compute the column widths of the final display dynamically based on the translated strings, so that translations don't mess up the display accidentally.
…make ecpg thread test cases work on Windows.
As usual, the release notes for other branches will be made by cutting these down, but put them up for community review first. Note the first entry is only for 9.4.
Fix thinko in commit 8be8510: it's okay to have dbid == 0 in normal (non-pin) entries in pg_shdepend, because global objects such as databases are entered that way. The test would pass so long as it was run in a cluster containing no databases/tablespaces owned by, or granted to, roles other than the bootstrap superuser. That's the expected situation for "make check", but for "make installcheck", not so much. Reported by Ryan Murphy. Discussion: https://postgr.es/m/CAHeEsBc6EQe0mxGBKDXAwJbntgfvoAd5MQC-5362SmC3Tng_6g@mail.gmail.com
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.