aboutsummaryrefslogtreecommitdiff
path: root/src/backend
Commit message (Collapse)AuthorAge
...
* Fix objectaddress.c code for publication relations.Tom Lane2018-05-24
| | | | | | | | | | | | | | | | | | | | getObjectDescription and getObjectIdentity failed to schema-qualify the name of the published table, which is bad in getObjectDescription and unforgivable in getObjectIdentity. Actually, getObjectIdentity failed to emit the table's name at all unless "objname" output is requested, which accidentally works for some (all?) extant callers but is clearly not the intended API. Somebody had also not gotten the memo that the output of getObjectIdentity is not to be translated. To fix getObjectDescription, I made it call getRelationDescription, which required refactoring the translatable string for the case, but is more future-proof in case we ever publish relations that aren't plain tables. While at it, I made the English output look like "publication of table X in publication Y"; the added "of" seems to me to make it read much better. Back-patch to v10 where publications were introduced. Discussion: https://postgr.es/m/20180522.182020.114074746.horiguchi.kyotaro@lab.ntt.co.jp
* Properly schema-qualify additional object types in getObjectDescription().Tom Lane2018-05-24
| | | | | | | | | | | | | | | | | Collations, conversions, extended statistics objects (in >= v10), and all four types of text search objects have schema-qualified names. getObjectDescription() ignored that and would emit just the base name of the object, potentially producing wrong or at least highly misleading output. Fix it to add the schema name whenever the object is not "visible" in the current search path, as is the rule for other schema-qualifiable object types. Although in common situations the output won't change, this seems to me (tgl) to be a bug worthy of back-patching, hence do so. Kyotaro Horiguchi, per a complaint from me Discussion: https://postgr.es/m/20180522.182020.114074746.horiguchi.kyotaro@lab.ntt.co.jp
* Fix incorrect ordering of operations in pg_resetwal and pg_rewind.Tom Lane2018-05-23
| | | | | | | | | | | | | | | | | | | | | Commit c37b3d08c dropped its added GetDataDirectoryCreatePerm call into the wrong place in pg_resetwal.c, namely after the chdir to DataDir. That broke invocations using a relative path, as reported by Tushar Ahuja. We could have left it where it was and changed the argument to be ".", but that'd result in a rather confusing error message in event of a failure, so re-ordering seems like a better solution. Similarly reorder operations in pg_rewind.c. The issue there is that it doesn't seem like a good idea to do any actual operations before the not-root check (on Unix) or the restricted token acquisition (on Windows). I don't know that this is an actual bug, but I'm definitely not convinced that it isn't, either. Assorted other code review for c37b3d08c and da9b580d8: fix some misspelled or otherwise badly worded comments, put the #include for <sys/stat.h> where it actually belongs, etc. Discussion: https://postgr.es/m/aeb9c3a7-3c3f-a57f-1a18-c8d4fcdc2a1f@enterprisedb.com
* Accept "B" in all memory-unit GUCs, and improve error messages.Heikki Linnakangas2018-05-23
| | | | | | | | | | | | | | | | | | | | Commit 6e7baa3227 added support for "B" unit, for specifying config options in bytes. However, it was only accepted in GUC_UNIT_BYTE settings, wal_segment_size and track_activity_query_size, and not e.g. in work_mem. This patch makes it consistent, so that "B" accepted in all the same contexts where "kB", "MB", and so forth are accepted. Add "B" to the list of accepted units in the error hint, along with "kB", "MB", etc. Add an entry in the conversion table for "TB" to "B" conversion. A terabyte is out of range for any GUC_UNIT_BYTE option, so you always get an "out of range" error with that, but without it, you get a confusing error message that claims that "TB" is not an accepted unit, with a hint that nevertheless lists "TB" as an accepted unit. Reviewed-by: Alexander Korotkov, Andres Freund Discussion: https://www.postgresql.org/message-id/1bfe7f4a-7e22-aa6e-7b37-f4d222ed2d67@iki.fi
* Widen COPY FROM's current-line-number counter from 32 to 64 bits.Tom Lane2018-05-22
| | | | | | | | | | | | | | | Because the code for the HEADER option skips a line when this counter is zero, a very long COPY FROM WITH HEADER operation would drop a line every 2^32 lines. A lesser but still unfortunate problem is that errors would show a wrong input line number for errors occurring beyond the 2^31'st input line. While such large input streams seemed impractical when this code was first written, they're not any more. Widening the counter (and some associated variables) to uint64 should be enough to prevent problems for the foreseeable future. David Rowley Discussion: https://postgr.es/m/CAKJS1f88yh-6wwEfO6QLEEvH3BEugOq2QX1TOja0vCauoynmOQ@mail.gmail.com
* Add missing files to src/backend/lib/README.Heikki Linnakangas2018-05-22
| | | | | | | | | The README lists all the files available in the directory, along with short descriptions of each, but a few newly added ones were missing. While we're at it, reorder the list into alphabetical order. Author: Takeshi Ideriha Discussion: https://www.postgresql.org/message-id/4E72940DA2BF16479384A86D54D0988A56793487@G01JPEXMBKW04
* Fix typo in comment.Heikki Linnakangas2018-05-22
|
* Update SQL features listPeter Eisentraut2018-05-21
|
* Translation updatesPeter Eisentraut2018-05-21
| | | | | Source-Git-URL: git://git.postgresql.org/git/pgtranslation/messages.git Source-Git-Hash: 3a5a71cccad5c68e01008e9e3a4f06930197a05e
* Fix SQL:2008 FETCH FIRST syntax to allow parameters.Andrew Gierth2018-05-21
| | | | | | | | | | | | | | | | | | | OFFSET <x> ROWS FETCH FIRST <y> ROWS ONLY syntax is supposed to accept <simple value specification>, which includes parameters as well as literals. When this syntax was added all those years ago, it was done inconsistently, with <x> and <y> being different subsets of the standard syntax. Rectify that by making <x> and <y> accept the same thing, and allowing either a (signed) numeric literal or a c_expr there, which allows for parameters, variables, and parenthesized arbitrary expressions. Per bug #15200 from Lukas Eder. Backpatch all the way, since this has been broken from the start. Discussion: https://postgr.es/m/877enz476l.fsf@news-spur.riddles.org.uk Discussion: http://postgr.es/m/152647780335.27204.16895288237122418685@wrigleys.postgresql.org
* Improve spelling of new FINALFUNC_MODIFY aggregate attribute.Tom Lane2018-05-21
| | | | | | | | I'd used SHARABLE as a value originally, but Peter Eisentraut points out that dictionaries agree that SHAREABLE is the preferred spelling. Run around and change that before it's too late. Discussion: https://postgr.es/m/d2e1afd4-659c-50d6-1b20-7cfd3675e909@2ndquadrant.com
* Fix unsafe usage of strerror(errno) within ereport().Tom Lane2018-05-21
| | | | | | | | This is the converse of the unsafe-usage-of-%m problem: the reason ereport/elog provide that format code is mainly to dodge the hazard of errno getting changed before control reaches functions within the arguments of the macro. I only found one instance of this hazard, but it's been there since 9.4 :-(.
* printf("%lf") is not portable, so omit the "l".Tom Lane2018-05-20
| | | | | | | | | | | | The "l" (ell) width spec means something in the corresponding scanf usage, but not here. While modern POSIX says that applying "l" to "f" and other floating format specs is a no-op, SUSv2 says it's undefined. Buildfarm experience says that some old compilers emit warnings about it, and at least one old stdio implementation (mingw's "ANSI" option) actually produces wrong answers and/or crashes. Discussion: https://postgr.es/m/21670.1526769114@sss.pgh.pa.us Discussion: https://postgr.es/m/c085e1da-0d64-1c15-242d-c921f32e0d5c@dunslane.net
* Assorted minor cleanups for bootstrap-data Perl scripts.Tom Lane2018-05-19
| | | | | | | | | | | | | | | FindDefinedSymbol was intended to take an array of possible include paths, but it never actually worked correctly for any but the first array element. Since there's no use-case for more than one path anyway, let's just simplify this code and its callers by redefining it as taking only one include path. Minor other code-beautification without functional effects, except that in one place we format the output as pgindent would do. John Naylor Discussion: https://postgr.es/m/CAJVSVGXM_n32hTTkircW4_K1LQFsJNb6xjs0pAP4QC0ZpyJfPQ@mail.gmail.com
* Fix for globals.c- c.h must come firstStephen Frost2018-05-18
| | | | | | | | | | Commit da9b580 mistakenly put a system header before postgres.h (which includes c.h). That can cause portability issues and broke (at least) builds with older Windows compilers. Discovered by Mark Dilger. Discussion: https://postgr.es/m/BF04A27A-D132-4927-A80A-BAD18695E954@gmail.com
* Further adjust comment in get_partition_dispatch_recurse.Robert Haas2018-05-18
| | | | | | | | | In editing 09b12d52db1cf1a4c72d876f3fb6c9d06919e51a I made it wrong; fix that and try to more clearly explain the situation. Patch by me, reviewed by David Rowley and Amit Langote Discussion: http://postgr.es/m/CA+TgmobAq+mA5hzm0a5OS38qQY5758DDDGqa3sBJN4hvir-H9w@mail.gmail.com
* Fix error message on short read of pg_controlMagnus Hagander2018-05-18
| | | | | Instead of saying "error: success", indicate that we got a working read but it was too short.
* Message wording and pluralization improvementsPeter Eisentraut2018-05-17
|
* Make numeric power() handle NaNs according to the modern POSIX spec.Tom Lane2018-05-17
| | | | | | | | | | | | | | | In commit 6bdf1303b, we ensured that power()/^ for float8 would honor the NaN behaviors specified by POSIX standards released in this century, ie NaN ^ 0 = 1 and 1 ^ NaN = 1. However, numeric_power() was not touched and continued to follow the once-common behavior that every case involving NaN input produces NaN. For consistency, let's switch the numeric behavior to the modern spec in the same release that ensures that behavior for float8. (Note that while 6bdf1303b was initially back-patched, we later undid that, concluding that any behavioral change should appear only in v11.) Discussion: https://postgr.es/m/10898.1526421338@sss.pgh.pa.us
* Detoast plpgsql variables if they might live across a transaction boundary.Tom Lane2018-05-16
| | | | | | | | | | | | | | | | | | | | | | | | | | | Up to now, it's been safe for plpgsql to store TOAST pointers in its variables because the ActiveSnapshot for whatever query called the plpgsql function will surely protect such TOAST values from being vacuumed away, even if the owning table rows are committed dead. With the introduction of procedures, that assumption is no longer good in "non atomic" executions of plpgsql code. We adopt the slightly brute-force solution of detoasting all TOAST pointers at the time they are stored into variables, if we're in a non-atomic context, just in case the owning row goes away. Some care is needed to avoid long-term memory leaks, since plpgsql tends to run with CurrentMemoryContext pointing to its call-lifespan context, but we shouldn't assume that no memory is leaked by heap_tuple_fetch_attr. In plpgsql proper, we can do the detoasting work in the "eval_mcontext". Most of the code thrashing here is due to the need to add this capability to expandedrecord.c as well as plpgsql proper. In expandedrecord.c, we can't assume that the caller's context is short-lived, so make use of the short-term sub-context that was already invented for checking domain constraints. In view of this repurposing, it seems good to rename that variable and associated code from "domain_check_cxt" to "short_term_cxt". Peter Eisentraut and Tom Lane Discussion: https://postgr.es/m/5AC06865.9050005@anastigmatix.net
* Fix misprocessing of equivalence classes involving record_eq().Tom Lane2018-05-16
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | canonicalize_ec_expression() is supposed to agree with coerce_type() as to whether a RelabelType should be inserted to make a subexpression be valid input for the operators of a given opclass. However, it did the wrong thing with named-composite-type inputs to record_eq(): it put in a RelabelType to RECORDOID, which the parser doesn't. In some cases this was harmless because all code paths involving a particular equivalence class did the same thing, but in other cases this would result in failing to recognize a composite-type expression as being a member of an equivalence class that it actually is a member of. The most obvious bad effect was to fail to recognize that an index on a composite column could provide the sort order needed for a mergejoin on that column, as reported by Teodor Sigaev. I think there might be other, subtler, cases that result in misoptimization. It also seems possible that an unwanted RelabelType would sometimes get into an emitted plan --- but because record_eq and friends don't examine the declared type of their input expressions, that would not create any visible problems. To fix, just treat RECORDOID as if it were a polymorphic type, which in some sense it is. We might want to consider formalizing that a bit more someday, but for the moment this seems to be the only place where an IsPolymorphicType() test ought to include RECORDOID as well. This has been broken for a long time, so back-patch to all supported branches. Discussion: https://postgr.es/m/a6b22369-e3bf-4d49-f59d-0c41d3551e81@sigaev.ru
* Pass the correct PlannerInfo to PlanForeignModify/PlanDirectModify.Robert Haas2018-05-16
| | | | | | | | | | | | | | | | | | Previously, we passed the toplevel PlannerInfo, but we actually want to pass the relevant subroot. One problem with passing the toplevel PlannerInfo is that the FDW which wants to push down an UPDATE or DELETE against a join won't find the relevant joinrel there. As of commit 1bc0100d270e5bcc980a0629b8726a32a497e788, postgres_fdw tries to do exactly this and can be made to fail an assertion as a result. It's possible that this should be regarded as a bug fix and back-patched to earlier releases, but for lack of a test case that fails in earlier releases, no back-patch for now. Etsuro Fujita, reviewed by Amit Langote. Discussion: http://postgr.es/m/5AF43E02.30000@lab.ntt.co.jp
* Improve comment in get_partition_dispatch_recurse.Robert Haas2018-05-16
| | | | | | David Rowley, reviewed by Amit Langote, and revised a bit by me. Discussion: http://postgr.es/m/CAKJS1f9yyimYyFzbHM4EwE+tkj4jvrHqSH0H4S4Kbas=UFpc9Q@mail.gmail.com
* Fix type checking for support functions of parallel VARIADIC aggregates.Tom Lane2018-05-15
| | | | | | | | | | | | | | | | | | | | | The impact of VARIADIC on the combine/serialize/deserialize support functions of an aggregate wasn't thought through carefully. There is actually no impact, because variadicity isn't passed through to these functions (and it doesn't seem like it would need to be). However, lookup_agg_function was mistakenly told to check things as though it were passed through. The net result was that it was impossible to declare an aggregate that had both VARIADIC input and parallelism support functions. In passing, fix a runtime check in nodeAgg.c for the combine function's strictness to make its error message agree with the creation-time check. The previous message was actually backwards, and it doesn't seem like there's a good reason to have two versions of this message text anyway. Back-patch to 9.6 where parallel aggregation was introduced. Alexey Bashtanov; message fix by me Discussion: https://postgr.es/m/f86dde87-fef4-71eb-0480-62754aaca01b@imap.cc
* Don't allow partitioned index on foreign-table partitionsAlvaro Herrera2018-05-14
| | | | | | | | | | | | | | | | Creating indexes on foreign tables is already forbidden, but local partitioned indexes (commit 8b08f7d4820f) forgot to check for them. Add a preliminary check to prevent wasting time. Another school of thought says to allow the index to be created if it's not a unique index; but it's possible to do better in the future (enable indexing of foreign tables, somehow), so we avoid painting ourselves in a corner by rejecting all cases, to avoid future grief (a.k.a. backward incompatible changes). Reported-by: Arseny Sher Author: Amit Langote, Álvaro Herrera Discussion: https://postgr.es/m/87sh71cakz.fsf@ars-thinkpad
* Fix file paths in commentsMagnus Hagander2018-05-14
| | | | Author: Daniel Gustafsson <daniel@yesql.se>
* Various improvements of skipping index scan during vacuum technicsTeodor Sigaev2018-05-10
| | | | | | | | | | | | | | | | | - Change vacuum_cleanup_index_scale_factor GUC to PGC_USERSET. vacuum_cleanup_index_scale_factor GUC was defined as PGC_SIGHUP. But this GUC affects not only autovacuum. So it might be useful to change it from user session in order to influence manually runned VACUUM. - Add missing tab-complete support for vacuum_cleanup_index_scale_factor reloption. - Fix condition for B-tree index cleanup. Zero value of vacuum_cleanup_index_scale_factor means that user wants B-tree index cleanup to be never skipped. - Documentation and comment improvements Authors: Justin Pryzby, Alexander Korotkov, Liudmila Mantrova Reviewed by: all authors and Robert Haas Discussion: https://www.postgresql.org/message-id/flat/20180502023025.GD7631%40telsasoft.com
* Update time zone data files to tzdata release 2018e.Tom Lane2018-05-09
| | | | | | | | | | | | | | | | | | | | | | | | | | DST law changes in North Korea. Redefinition of "daylight savings" in Ireland, as well as for some past years in Namibia and Czechoslovakia. Additional historical corrections for Czechoslovakia. With this change, the IANA database models Irish timekeeping as following "standard time" in summer, and "daylight savings" in winter, so that the daylight savings offset is one hour behind standard time not one hour ahead. This does not change their UTC offset (+1:00 in summer, 0:00 in winter) nor their timezone abbreviations (IST in summer, GMT in winter), though now "IST" is more correctly read as "Irish Standard Time" not "Irish Summer Time". However, the "is_dst" column in the pg_timezone_names view will now be true in winter and false in summer for the Europe/Dublin zone. Similar changes were made for Namibia between 1994 and 2017, and for Czechoslovakia between 1946 and 1947. So far as I can find, no Postgres internal logic cares about which way tm_isdst is reported; in particular, since commit b2cbced9e we do not rely on it to decide how to interpret ambiguous timestamps during DST transitions. So I don't think this change will affect any Postgres behavior other than the timezone-view outputs. Discussion: https://postgr.es/m/30996.1525445902@sss.pgh.pa.us
* Fix assorted partition pruning bugsAlvaro Herrera2018-05-09
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | match_clause_to_partition_key failed to consider COERCION_PATH_ARRAYCOERCE cases in scalar-op-array expressions, so it was possible to crash the server easily. To handle this case properly (ie. prune partitions) we would need to run a bit of executor code during planning. Maybe it can be improved, but for now let's just not crash. Add a test case that used to trigger the crash. Author: Michaël Paquier match_clause_to_partition_key failed to indicate that operators that don't have a commutator in a btree opclass are unsupported. It is possible for this to cause a crash later if such an operator is used in a scalar-op-array expression. Add a test case that used to the crash. Author: Amit Langote One caller of gen_partprune_steps_internal in match_clause_to_partition_key was too optimistic about the former never returning an empty step list. Rid it of its innocence. (Having fixed the bug above, I no longer know how to exploit this, so no test case for it, but it remained a bug.) Revise code flow a little bit, for succintness. Author: Álvaro Herrera Reported-by: Marina Polyakova Reviewed-by: Michaël Paquier Reviewed-by: Amit Langote Reviewed-by: Álvaro Herrera Discussion: https://postgr.es/m/ff8f9bfa485ff961d6bb43e54120485b@postgrespro.ru
* Restrict vertical tightness to parentheses in Perl codeAndrew Dunstan2018-05-09
| | | | | | | | | | | | | | | | | The vertical tightness settings collapse vertical whitespace between opening and closing brackets (parentheses, square brakets and braces). This can make data structures in particular harder to read, and is not very consistent with our style in non-Perl code. This patch restricts that setting to parentheses only, and reformats all the perl code accordingly. Not applying this to parentheses has some unfortunate effects, so the consensus is to keep the setting for parentheses and not for the others. The diff for this patch does highlight some places where structures should have trailing commas. They can be added manually, as there is no automatic tool to do so. Discussion: https://postgr.es/m/a2f2b87c-56be-c070-bfc0-36288b4b41c1@2ndQuadrant.com
* perltidy some recent code changes before changing perltidy settingsAndrew Dunstan2018-05-09
|
* Make gen_partprune_steps staticAlvaro Herrera2018-05-09
| | | | | | | | | | There's no need to export this function, so don't. Michaël didn't actually write the patch, but we list him as first author because with a trivial one like this, intellectual authorship is as important (if not more) as bit shovelling. Author: Michaël Paquier, Amit Langote Discussion: https://postgr.es/m/c91299c4-199b-0f16-339b-a29d6d2a39ee@lab.ntt.co.jp
* Remove useless 'default' clauseAlvaro Herrera2018-05-09
| | | | | | | | Author: Michael Paquier Reviewed-by: Amit Langote Reviewed-by: Álvaro Herrera Discussion: https://postgr.es/m/20180424012042.GD1570@paquier.xyz Discussion: https://postgr.es/m/20180509061039.GC11897@paquier.xyz
* Improve jsonb cast error messageTeodor Sigaev2018-05-09
| | | | | | | | Initial variant of error message didn't follow style of another casting error messages and wasn't informative. Per gripe from Robert Haas. Reviewer: Tom Lane Discussion: https://www.postgresql.org/message-id/flat/CA%2BTgmob08StTV9yu04D0idRFNMh%2BUoyKax5Otvrix7rEZC8rMw%40mail.gmail.com#CA+Tgmob08StTV9yu04D0idRFNMh+UoyKax5Otvrix7rEZC8rMw@mail.gmail.com
* Refine error messagesPeter Eisentraut2018-05-08
| | | | "JSON" when not referring to a data type should be upper case.
* Count heap tuples in non-SnapshotAny path in IndexBuildHeapRangeScan().Tom Lane2018-05-08
| | | | | | | | | | | | | | | | | | | | | | Brown-paper-bag bug in commit 7c91a0364: when we rearranged the placement of "reltuples += 1" statements, we missed including one in this code path. The net effect of that was that CREATE INDEX CONCURRENTLY would set the table's pg_class.reltuples to zero, as would index builds done during bootstrap mode. (It seems like parallel index builds ought to fail similarly, but they don't, perhaps because reltuples is computed in some other way. You certainly couldn't figure that out from the abysmally underdocumented parallelism code in this area.) I was led to this by wondering why initdb seemed to have slowed down as a result of 7c91a0364, as is evident in the buildfarm's timing history. The reason is that every system catalog with indexes had pg_class.reltuples = 0 after bootstrap, causing the planner to make some terrible choices for queries in the post-bootstrap steps. On my workstation, this fix causes the runtime of "initdb -N" to drop from ~2.0 sec to ~1.4 sec, which is almost though not quite back to where it was in v10. That's not much of a deal for production use perhaps, but it makes a noticeable difference for buildfarm and "make check-world" runs, which do a lot of initdbs.
* Clean up some perlcritic warningsAndrew Dunstan2018-05-07
| | | | | | | | | In Catalog.pm, mark eval of a string instead of a block as allowed. Disallow perlcritic completely in Gen_dummy_probes.pl, as it's generated code. Protect a couple of lines in plperl code from perltidy, so that the annotation for perlcritic stays on the same line as the construct it would otherwise object to.
* Suppress compiler warnings when building with --enable-dtrace.Tom Lane2018-05-07
| | | | | | | | | | | | Most versions of "dtrace -h" drop const qualifiers from the declarations of probe functions (though macOS gets it right). This causes compiler warnings when we pass in pointers to const. Repair by extending our existing post-processing of the probes.h file. To do so, assume that all "char *" arguments should be "const char *"; that seems reasonably safe. Thomas Munro Discussion: https://postgr.es/m/CAEepm=2j1pWSruQJqJ91ZDzD8w9ZZDsM4j2C6x75C-VryWg-_w@mail.gmail.com
* Fix bootstrap parser so that its keywords are unreserved words.Tom Lane2018-05-05
| | | | | | | | | | | | | | | | | | | | | | | | | | | Mark Dilger pointed out that the bootstrap parser does not allow any of its keywords to appear as column values unless they're quoted, and proposed dealing with that by quoting such values in genbki.pl. Looking closer, though, we also have that problem with respect to table, column, and type names appearing in the .bki file: the parser would fail if any of those matched any of its keywords. While so far there have been no conflicts (that I've heard of), this seems like a booby trap waiting to catch somebody. Rather than clutter genbki.pl with enough quoting logic to handle all that, let's make the bootstrap parser grow up a little bit and treat its keywords as unreserved. Experimentation shows that it's fairly easy to do so with the exception of _null_, which I don't have a big problem with keeping as a reserved word. The only change needed is that we can't have the "close" command take an optional table name: it has to either require or forbid the table name to avoid shift/reduce conflicts. genbki.pl has historically always included the table name, so I took that option. The implementation has bootscanner.l passing forward the string value of each keyword, in case bootparse.y needs that. This avoids needing to know the precise spelling of each keyword in bootparse.y, which is good because that's not always obvious from the token name. Discussion: https://postgr.es/m/3024FC91-DB6D-4732-B31C-DF772DF039A0@gmail.com
* Put in_range_float4_float8's work in-line.Tom Lane2018-05-05
| | | | | | | | | | | | | | | | | | | In commit 8b29e88cd, I'd dithered about whether to make in_range_float4_float8 be a standalone copy of the float in-range logic or have it punt to in_range_float8_float8. I went with the latter, which saves code space though at the cost of performance and readability. However, it emerges that this tickles a compiler or hardware bug on buildfarm member opossum. Test results from commit 55e0e4581 show conclusively that widening a float4 NaN to float8 produces Inf, not NaN, on that machine; which accounts perfectly for the window RANGE test failures it's been showing. We can dodge this problem by making in_range_float4_float8 be an independent function, so that it checks for NaN inputs before widening them. Ordinarily I'd not be very excited about working around such obviously broken functionality; but given that this was a judgment call to begin with, I don't mind reversing it.
* Fix scenario where streaming standby gets stuck at a continuation record.Heikki Linnakangas2018-05-05
| | | | | | | | | | | | | | | | | | | | | If a continuation record is split so that its first half has already been removed from the master, and is only present in pg_wal, and there is a recycled WAL segment in the standby server that looks like it would contain the second half, recovery would get stuck. The code in XLogPageRead() incorrectly started streaming at the beginning of the WAL record, even if we had already read the first page. Backpatch to 9.4. In principle, older versions have the same problem, but without replication slots, there was no straightforward mechanism to prevent the master from recycling old WAL that was still needed by standby. Without such a mechanism, I think it's reasonable to assume that there's enough slack in how many old segments are kept around to not run into this, or you have a WAL archive. Reported by Jonathon Nelson. Analysis and patch by Kyotaro HORIGUCHI, with some extra comments by me. Discussion: https://www.postgresql.org/message-id/CACJqAM3xVz0JY1XFDKPP%2BJoJAjoGx%3DGNuOAshEDWCext7BFvCQ%40mail.gmail.com
* Don't mark pages all-visible spuriouslyAlvaro Herrera2018-05-04
| | | | | | | | | | | | | | | | | | | | | | | | | Dan Wood diagnosed a long-standing problem that pages containing tuples that are locked by multixacts containing live lockers may spuriously end up as candidates for getting their all-visible flag set. This has the long-term effect that multixacts remain unfrozen; this may previously pass undetected, but since commit XYZ it would be reported as "ERROR: found multixact 134100944 from before relminmxid 192042633" because when a later vacuum tries to freeze the page it detects that a multixact that should have gotten frozen, wasn't. Dan proposed a (correct) patch that simply sets a variable to its correct value, after a bogus initialization. But, per discussion, it seems better coding to avoid the bogus initializations altogether, since they could give rise to more bugs later. Therefore this fix rewrites the logic a little bit to avoid depending on the bogus initializations. This bug was part of a family introduced in 9.6 by commit a892234f830e; later, commit 38e9f90a227d fixed most of them, but this one was unnoticed. Authors: Dan Wood, Pavan Deolasee, Álvaro Herrera Reviewed-by: Masahiko Sawada, Pavan Deolasee, Álvaro Herrera Discussion: https://postgr.es/m/84EBAC55-F06D-4FBE-A3F3-8BDA093CE3E3@amazon.com
* Fix precedence problem in new Perl code.Tom Lane2018-05-04
| | | | I think this bit of commit 1f1cd9b5d didn't do quite what I meant :-(
* Don't truncate away non-key attributes for leftmost downlinks.Teodor Sigaev2018-05-04
| | | | | | | | | | | | nbtsort.c does not need to truncate away non-key attributes for the minimum key of the leftmost page on a level, since this is only used to build a minus infinity downlink for the level's leftmost page. Truncating away non-key attributes in advance of truncating away all attributes in _bt_sortaddtup() does not affect the correctness of CREATE INDEX, but it is misleading. Author: Peter Geoghegan Discussion: https://www.postgresql.org/message-id/CAH2-WzkAS2M3ussHG-s_Av=Zo6dPjOxyu5fNRkYnxQV+YzGQ4w@mail.gmail.com
* Re-think predicate locking on GIN indexes.Teodor Sigaev2018-05-04
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The principle behind the locking was not very well thought-out, and not documented. Add a section in the README to explain how it's supposed to work, and change the code so that it actually works that way. This fixes two bugs: 1. If fast update was turned on concurrently, subsequent inserts to the pending list would not conflict with predicate locks that were acquired earlier, on entry pages. The included 'predicate-gin-fastupdate' test demonstrates that. To fix, make all scans acquire a predicate lock on the metapage. That lock represents a scan of the pending list, whether or not there is a pending list at the moment. Forget about the optimization to skip locking/checking for locks, when fastupdate=off. 2. If a scan finds no match, it still needs to lock the entry page. The point of predicate locks is to lock the gabs between values, whether or not there is a match. The included 'predicate-gin-nomatch' test tests that case. In addition to those two bug fixes, this removes some unnecessary locking, following the principle laid out in the README. Because all items in a posting tree have the same key value, a lock on the posting tree root is enough to cover all the items. (With a very large posting tree, it would possibly be better to lock the posting tree leaf pages instead, so that a "skip scan" with a query like "A & B", you could avoid unnecessary conflict if a new tuple is inserted with A but !B. But let's keep this simple.) Also, some spelling fixes. Author: Heikki Linnakangas with some editorization by me Review: Andrey Borodin, Alexander Korotkov Discussion: https://www.postgresql.org/message-id/0b3ad2c2-2692-62a9-3a04-5724f2af9114@iki.fi
* Avoid overwriting unchanged output files in genbki.pl and Gen_fmgrtab.pl.Tom Lane2018-05-03
| | | | | | | | | | | | | | | | If a particular output file already exists with the contents it should have, leave it alone, so that its mod timestamp is not advanced. In builds using --enable-depend, this can avoid the need to recompile .c files whose included files didn't actually change. It's not clear whether it saves much of anything for users of ccache; but the cost of doing the file comparisons seems to be negligible, so we might as well do it. For developers using the MSVC toolchain, this will create a regression: msvc/Solution.pm will sometimes run genbki.pl or Gen_fmgrtab.pl unnecessarily. I'll look into fixing that separately. Discussion: https://postgr.es/m/16925.1525376229@sss.pgh.pa.us
* Rearrange makefile rules for running Gen_fmgrtab.pl.Tom Lane2018-05-03
| | | | | | | | | | | | | | | | | | | | | | Make these rules look more like the ones associated with genbki.pl, to wit: * Use a stamp file to record when we last ran the script, instead of relying on the timestamps of the individual output files. * Take the knowledge out of backend/Makefile and put it in utils/Makefile where it belongs. I moved down the handling of errcodes.h and probes.h too, although those continue to be built by separate processes. In itself, this is just much-needed cleanup with little practical effect. However, by decoupling these makefile rules from the timestamps of the generated header files, we open the door to not advancing those timestamps unnecessarily, which will be taken advantage of by the next commit. msvc/Solution.pm should be taught to do things similarly, but I'll leave that for another commit. Discussion: https://postgr.es/m/16925.1525376229@sss.pgh.pa.us
* Add HOLD_INTERRUPTS section into FinishPreparedTransaction.Teodor Sigaev2018-05-03
| | | | | | | | | | | | | If an interrupt arrives in the middle of FinishPreparedTransaction and any callback decide to call CHECK_FOR_INTERRUPTS (e.g. RemoveTwoPhaseFile can write a warning with ereport, which checks for interrupts) then it's possible to leave current GXact undeleted. Backpatch to all supported branches Stas Kelvich Discussion: ihttps://www.postgresql.org/message-id/3AD85097-A3F3-4EBA-99BD-C38EDF8D2949@postgrespro.ru
* Further improve code for probing the availability of ARM CRC instructions.Tom Lane2018-05-03
| | | | | | | | | | | | | | | | | | | | Andrew Gierth pointed out that commit 1c72ec6f4 would yield the wrong answer on big-endian ARM systems, because the data being CRC'd would be different. To fix that, and avoid the rather unsightly hard-wired constant, simply compare the hardware and software implementations' results. While we're at it, also log the resulting decision at DEBUG1, and error out if the hw and sw results unexpectedly differ. Also, since this file must compile for both frontend and backend, avoid incorrect dependencies on backend-only headers. In passing, add a comment to postmaster.c about when the CRC function pointer will get initialized. Thomas Munro, based on complaints from Andrew Gierth and Tom Lane Discussion: https://postgr.es/m/HE1PR0801MB1323D171938EABC04FFE7FA9E3110@HE1PR0801MB1323.eurprd08.prod.outlook.com
* Fix SPI error cleanup and memory leakPeter Eisentraut2018-05-03
| | | | | | | | | | | | Since the SPI stack has been moved from TopTransactionContext to TopMemoryContext, setting _SPI_stack to NULL in AtEOXact_SPI() leaks memory. In fact, we don't need to do that anymore: We just leave the allocated stack around for the next SPI use. Also, refactor the SPI cleanup so that it is run both at transaction end and when returning to the main loop on an exception. The latter is necessary when a procedure calls a COMMIT or ROLLBACK command that itself causes an error.