- 13 Dec, 2017 4 commits
-
-
Andres Freund authored
Otherwise the detection can spuriously detect symbol as available, because the compiler may just emits reference to non-existant symbol.
-
Andres Freund authored
A previous commit added inline functions that provide fast(er) and correct overflow checks for signed integer math. Use them in a significant portion of backend code. There's more to touch in both backend and frontend code, but these were the easily identifiable cases. The old overflow checks are noticeable in integer heavy workloads. A secondary benefit is that getting rid of overflow checks that rely on signed integer overflow wrapping around, will allow us to get rid of -fwrapv in the future. Which in turn slows down other code. Author: Andres Freund Discussion: https://postgr.es/m/20171024103954.ztmatprlglz3rwke@alap3.anarazel.de
-
Andres Freund authored
It's not easy to get signed integer overflow checks correct and fast. Therefore abstract the necessary infrastructure into a common header providing addition, subtraction and multiplication for 16, 32, 64 bit signed integers. The new macros aren't yet used, but a followup commit will convert several open coded overflow checks. Author: Andres Freund, with some code stolen from Greg Stark Reviewed-By: Robert Haas Discussion: https://postgr.es/m/20171024103954.ztmatprlglz3rwke@alap3.anarazel.de
-
Robert Haas authored
Commit 8b304b8b removed replacement selection, but left behind this comment text. The optimization to which the comment refers is not relevant without replacement selection, because if we had so few tuples as to require only one tape, we would have just completed the sort in memory. Peter Geoghegan Discussion: http://postgr.es/m/CAH2-WznqupLA8CMjp+vqzoe0yXu0DYYbQSNZxmgN76tLnAOZ_w@mail.gmail.com
-
- 12 Dec, 2017 2 commits
-
-
Robert Haas authored
Etsuro Fujita, reviewed by Ashutosh Bapat Discussion: http://postgr.es/m/5A2A60E6.6000008@lab.ntt.co.jp
-
Teodor Sigaev authored
regular expression which contains backslash.
-
- 11 Dec, 2017 3 commits
-
-
Peter Eisentraut authored
Reported-by: Noah Misch <noah@leadboat.com>
-
Tom Lane authored
I noticed that _SPI_execute_plan initially sets spierrcontext.arg = NULL, and only fills it in some time later. If an error were to happen in between, _SPI_error_callback would try to dereference the null pointer. This is unlikely --- there's not much between those points except push-snapshot calls --- but it's clearly not impossible. Tweak the callback to do nothing if the pointer isn't set yet. It's been like this for awhile, so back-patch to all supported branches.
-
Robert Haas authored
Ashutosh Bapat, per discussion with Julien Rouhaund, who also reviewed this patch. Discussion: http://postgr.es/m/CAFjFpReBR3ftK9C23LLCZY_TDXhhjB_dgE-L9+mfTnA=gkvdvQ@mail.gmail.com
-
- 10 Dec, 2017 1 commit
-
-
Tom Lane authored
The test added by commit 390d5813 turns out to have different output in CLOBBER_CACHE_ALWAYS builds: there's an extra CONTEXT line in the error message as a result of detecting the error at a different place. Possibly we should do something to make that more consistent. But as a stopgap measure to make the buildfarm green again, adjust the test to suppress CONTEXT entirely. We can revert this if we do something in the backend to eliminate the inconsistency. Discussion: https://postgr.es/m/31545.1512924904@sss.pgh.pa.us
-
- 09 Dec, 2017 5 commits
-
-
Tom Lane authored
If one exits and re-enters a DECLARE ... BEGIN ... END block within a single execution of a plpgsql function, perhaps due to a surrounding loop, the declared variables are supposed to get re-initialized to null (or whatever their initializer is). But this failed to happen for variables of type "record", because while exec_stmt_block() expected such variables to be included in the block's initvarnos list, plpgsql_add_initdatums() only adds DTYPE_VAR variables to that list. This bug appears to have been there since the aboriginal addition of plpgsql to our tree. Fix by teaching plpgsql_add_initdatums() to include DTYPE_REC variables as well. (We don't need to consider other DTYPEs because they don't represent separately-stored values.) I failed to resist the temptation to make some nearby cosmetic adjustments, too. No back-patch, because there have not been field complaints, and it seems possible that somewhere out there someone has code depending on the incorrect behavior. In any case this change would have no impact on correctly-written code. Discussion: https://postgr.es/m/22994.1512800671@sss.pgh.pa.us
-
Magnus Hagander authored
Missed this in the last commit.
-
Magnus Hagander authored
Reported by Robins Tharakan
-
Noah Misch authored
Notably, this permits linking to the 32-bit Perl binaries advertised on perl.org, namely Strawberry Perl and ActivePerl. This has a side effect of permitting linking to binaries built with obsolete MSVC versions. By default, MSVC 2012 and later require a "safe exception handler table" in each binary. MinGW-built, 32-bit DLLs lack the relevant exception handler metadata, so linking to them failed with error LNK2026. Restore the semantics of MSVC 2010, which omits the table from a given binary if some linker input lacks metadata. This has no effect on 64-bit builds or on MSVC 2010 and earlier. Back-patch to 9.3 (all supported versions). Reported by Victor Wagner. Discussion: https://postgr.es/m/20160326154321.7754ab8f@wagner.wagner.home
-
Noah Misch authored
Commits 5a5c2fec and b5178c5d introduced support for modern MSVC-built, 32-bit Perl, but they broke use of MinGW-built, 32-bit Perl distributions like Strawberry Perl and modern ActivePerl. Perl has no robust means to report whether it expects a -D_USE_32BIT_TIME_T ABI, so test this. Back-patch to 9.3 (all supported versions). The chief alternative was a heuristic of adding -D_USE_32BIT_TIME_T when $Config{gccversion} is nonempty. That banks on every gcc-built Perl using the same ABI. gcc could change its default ABI the way MSVC once did, and one could build Perl with gcc and the non-default ABI. The GNU make build system could benefit from a similar test, without which it does not support MSVC-built Perl. For now, just add a comment. Most users taking the special step of building Perl with MSVC probably build PostgreSQL with MSVC. Discussion: https://postgr.es/m/20171130041441.GA3161526@rfd.leadboat.com
-
- 08 Dec, 2017 4 commits
-
-
Peter Eisentraut authored
Those cases currently crash and supporting them is more work then originally thought, so we'll just prohibit these scenarios for now. Author: Michael Paquier <michael.paquier@gmail.com> Reviewed-by: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> Reported-by: Мансур Галиев <gomer94@yandex.ru> Bug: #14866
-
Peter Eisentraut authored
Reported-by: Masahiko Sawada <sawada.mshk@gmail.com>
-
Tom Lane authored
plpgsql's function exec_move_row() handles assignment of a composite source value to either a PLpgSQL_rec or PLpgSQL_row target variable. Oddly, rather than taking a single target argument which it could do run-time type detection on, it was coded to take two separate arguments (only one of which is allowed to be non-NULL). This choice had then back-propagated into storing two separate target variables in various plpgsql statement nodes, with lots of duplicative coding and awkward interface logic to support that. Simplify matters by folding those pairs down to single variables, distinguishing the two cases only where we must ... which turns out to be only in exec_move_row itself. This is purely refactoring and should not change any behavior. In passing, remove unused field PLpgSQL_stmt_open.returntype. Discussion: https://postgr.es/m/11787.1512713374@sss.pgh.pa.us
-
Peter Eisentraut authored
A COPY into a table should apply identity sequence values just like it does for ordinary defaults. This was previously forgotten, leading to null values being inserted, which in turn would fail because identity columns have not-null constraints. Author: Michael Paquier <michael.paquier@gmail.com> Reported-by: Steven Winfield <steven.winfield@cantabcapital.com> Bug: #14952
-
- 07 Dec, 2017 1 commit
-
-
Robert Haas authored
Per Tom Lane, the old test sometimes times out with CLOBBER_CACHE_ALWAYS. Nathan Bossart Discussion: http://postgr.es/m/28614.1512583046@sss.pgh.pa.us
-
- 06 Dec, 2017 4 commits
-
-
Robert Haas authored
When a worker is flagged as BGW_NEVER_RESTART and we fail to start it, or if it is not marked BGW_NEVER_RESTART but is terminated before startup succeeds, what BgwHandleStatus should be reported? The previous code really hadn't considered this possibility (as indicated by the comments which ignore it completely) and would typically return BGWH_NOT_YET_STARTED, but that's not a good answer, because then there's no way for code using GetBackgroundWorkerPid() to tell the difference between a worker that has not started but will start later and a worker that has not started and will never be started. So, when this case happens, return BGWH_STOPPED instead. Update the comments to reflect this. The preceding fix by itself is insufficient to fix the problem, because the old code also didn't send a notification to the process identified in bgw_notify_pid when startup failed. That might've been technically correct under the theory that the status of the worker was BGWH_NOT_YET_STARTED, because the status would indeed not change when the worker failed to start, but now that we're more usefully reporting BGWH_STOPPED, a notification is needed. Without these fixes, code which starts background workers and then uses the recommended APIs to wait for those background workers to start would hang indefinitely if the postmaster failed to fork a worker. Amit Kapila and Robert Haas Discussion: http://postgr.es/m/CAA4eK1KDfKkvrjxsKJi3WPyceVi3dH1VCkbTJji2fuwKuB=3uw@mail.gmail.com
-
Robert Haas authored
Reported by Tom Lane and the buildfarm. Amul Sul and Amit Khandekar Discussion: http://postgr.es/m/17868.1512519318@sss.pgh.pa.us Discussion: http://postgr.es/m/CAJ3gD9cJQ4d-XhmZ6BqM9rMM2KDBfpkdgOAb4+psz56uBuMQ_A@mail.gmail.com
-
Tom Lane authored
I suppose it is a copy-and-paste error that this test doesn't actually test the "Parallel Append with both partial and non-partial subplans" case (EXPLAIN alone surely doesn't qualify as a test of executor behavior). Fix that. Also, add cosmetic aliases to make it possible to tell apart these otherwise-identical test cases in log_statement output.
-
Peter Eisentraut authored
Remove the designation that Flex is a GNU package. Even though Bison is a GNU package, leave out the designation to not make the sentence unnecessarily complicated. Author: Pavan Maddamsetti <pavan.maddamsetti@gmail.com>
-
- 05 Dec, 2017 11 commits
-
-
Tom Lane authored
-
Robert Haas authored
When we create an Append node, we can spread out the workers over the subplans instead of piling on to each subplan one at a time, which should typically be a bit more efficient, both because the startup cost of any plan executed entirely by one worker is paid only once and also because of reduced contention. We can also construct Append plans using a mix of partial and non-partial subplans, which may allow for parallelism in places that otherwise couldn't support it. Unfortunately, this patch doesn't handle the important case of parallelizing UNION ALL by running each branch in a separate worker; the executor infrastructure is added here, but more planner work is needed. Amit Khandekar, Robert Haas, Amul Sul, reviewed and tested by Ashutosh Bapat, Amit Langote, Rafia Sabih, Amit Kapila, and Rajkumar Raghuwanshi. Discussion: http://postgr.es/m/CAJ3gD9dy0K_E8r727heqXoBmWZ83HwLFwdcaSSmBQ1+S+vRuUQ@mail.gmail.com
-
Peter Eisentraut authored
Reported-by: Dave Page <dpage@pgadmin.org>
-
Robert Haas authored
When a Gather or Gather Merge node is started and stopped multiple times, the old code wouldn't reset the shared state between executions, potentially resulting in dramatically inflated instrumentation data for nodes beneath it. (The per-worker instrumentation ended up OK, I think, but the overall totals were inflated.) Report by hubert depesz lubaczewski. Analysis and fix by Amit Kapila, reviewed and tweaked a bit by me. Discussion: http://postgr.es/m/20171127175631.GA405@depesz.com
-
Andres Freund authored
If a hash join appears in a parallel query, there may be no hash table available for explain.c to inspect even though a hash table may have been built in other processes. This could happen either because parallel_leader_participation was set to off or because the leader happened to hit the end of the outer relation immediately (even though the complete relation is not empty) and decided not to build the hash table. Commit bf11e7ee introduced a way for workers to exchange instrumentation via the DSM segment for Sort nodes even though they are not parallel-aware. This commit does the same for Hash nodes, so that explain.c has a way to find instrumentation data from an arbitrary participant that actually built the hash table. Author: Thomas Munro Reviewed-By: Andres Freund Discussion: https://postgr.es/m/CAEepm%3D3DUQC2-z252N55eOcZBer6DPdM%3DFzrxH9dZc5vYLsjaA%40mail.gmail.com
-
Robert Haas authored
Commit ab3f008a broke this. Report by Stephen Frost. Discussion: http://postgr.es/m/20171205180342.GO4628@tamriel.snowman.net
-
Robert Haas authored
This is a backward incompatibility which should be noted in the release notes for PostgreSQL 11. For security reasons, we require that a postgres_fdw foreign table use password authentication when accessing a remote server, so that an unprivileged user cannot usurp the server's credentials. Superusers are exempt from this requirement, because we assume they are entitled to usurp the server's credentials or, at least, can find some other way to do it. But what should happen when the foreign table is accessed by a view owned by a user different from the session user? Is it the view owner that must be a superuser in order to avoid the requirement of using a password, or the session user? Historically it was the latter, but this requirement makes it the former instead. This allows superusers to delegate to other users the right to select from a foreign table that doesn't use password authentication by creating a view over the foreign table and handing out rights to the view. It is also more consistent with the idea that access to a view should use the view owner's privileges rather than the session user's privileges. The upshot of this change is that a superuser selecting from a view created by a non-superuser may now get an error complaining that no password was used, while a non-superuser selecting from a view created by a superuser will no longer receive such an error. No documentation changes are present in this patch because the wording of the documentation already suggests that it works this way. We should perhaps adjust the documentation in the back-branches, but that's a task for another patch. Originally proposed by Jeff Janes, but with different semantics; adjusted to work like this by me per discussion. Discussion: http://postgr.es/m/CA+TgmoaY4HsVZJv5SqEjCKLDwtCTSwXzKpRftgj50wmMMBwciA@mail.gmail.com
-
Robert Haas authored
This makes life easier for extension authors who wish to support Windows. Brian Cloutier, slightly amended by me. Discussion: http://postgr.es/m/CAJCy68fscdNhmzFPS4kyO00CADkvXvEa-28H-OtENk-pa2OTWw@mail.gmail.com
-
Peter Eisentraut authored
This ensures that automatically generated HTML anchors don't change in every build.
-
Tom Lane authored
Previously, this code just reported such problems at LOG level and kept going. The problem with this approach is that transient failures (e.g., ENFILE) could prevent us from resetting unlogged relations to empty, yet allow recovery to appear to complete successfully. That seems like a data corruption hazard large enough to treat such problems as reasons to fail startup. For the same reason, treat unlink failures for unlogged files as hard errors not just LOG messages. It's a little odd that we did it like that when file-level errors in other steps (copy_file, fsync_fname) are ERRORs. The sole case that I left alone is that ENOENT failure on a tablespace (not database) directory is not an error, though it will now be logged rather than just silently ignored. This is to cover the scenario where a previous DROP TABLESPACE removed the tablespace directory but failed before removing the pg_tblspc symlink. I'm not sure that that's very likely in practice, but that seems like the only real excuse for the old behavior here, so let's allow for it. (As coded, this will also allow ENOENT on $PGDATA/base/. But since we'll fail soon enough if that's gone, I don't think we need to complicate this code by distinguishing that from a true tablespace case.) Discussion: https://postgr.es/m/21040.1512418508@sss.pgh.pa.us
-
Peter Eisentraut authored
Fix warnings about "comparison between signed and unsigned integer expressions" in inline functions in header files by adding some casts.
-
- 04 Dec, 2017 5 commits
-
-
Tom Lane authored
do_pg_start_backup() expects its callers to pass in an open DIR pointer for the pg_tblspc directory, but there's no apparent advantage in that. It complicates the callers without adding any flexibility, and there's no robustness advantage, since we surely have to be prepared for errors during the scan of pg_tblspc anyway. In fact, by holding an extra kernel resource during operations like the preliminary checkpoint, we might be making things a fraction more failure-prone not less. Hence, remove that argument and open the directory just for the duration of the actual scan. Discussion: https://postgr.es/m/28752.1512413887@sss.pgh.pa.us
-
Tom Lane authored
Modify this function and its subsidiaries so that syscall failures are reported via ereport(LOG), rather than silently ignored as before. We don't want to throw a hard ERROR, as that would prevent database startup, and getting rid of leftover temporary files is not important enough for that. On the other hand, not reporting trouble at all seems like an odd choice not in line with current project norms, especially since any failure here is quite unexpected. On the same reasoning, adjust these functions' AllocateDir/ReadDir calls so that failure to scan a directory results in LOG not ERROR. I also removed the previous practice of silently ignoring ENOENT failures during directory opens --- there are some corner cases where that could happen given a previous database crash, but that seems like a bad excuse for ignoring a condition that isn't expected in most cases. A LOG message during postmaster start seems OK in such situations, and better than no output at all. In passing, make RemovePgTempRelationFiles' test for "is the file name all digits" look more like the way it's done elsewhere. Discussion: https://postgr.es/m/19907.1512402254@sss.pgh.pa.us
-
Tom Lane authored
This patch fixes a couple of low-probability bugs that could lead to reporting an irrelevant errno value (and hence possibly a wrong SQLSTATE) concerning directory-open or file-open failures. It also fixes places where we took shortcuts in reporting such errors, either by using elog instead of ereport or by using ereport but forgetting to specify an errcode. And it eliminates a lot of just plain redundant error-handling code. In service of all this, export fd.c's formerly-static function ReadDirExtended, so that external callers can make use of the coding pattern dir = AllocateDir(path); while ((de = ReadDirExtended(dir, path, LOG)) != NULL) if they'd like to treat directory-open failures as mere LOG conditions rather than errors. Also fix FreeDir to be a no-op if we reach it with dir == NULL, as such a coding pattern would cause. Then, remove code at many call sites that was throwing an error or log message for AllocateDir failure, as ReadDir or ReadDirExtended can handle that job just fine. Aside from being a net code savings, this gets rid of a lot of not-quite-up-to-snuff reports, as mentioned above. (In some places these changes result in replacing a custom error message such as "could not open tablespace directory" with more generic wording "could not open directory", but it was agreed that the custom wording buys little as long as we report the directory name.) In some other call sites where we can't just remove code, change the error reports to be fully project-style-compliant. Also reorder code in restoreTwoPhaseData that was acquiring a lock between AllocateDir and ReadDir; in the unlikely but surely not impossible case that LWLockAcquire changes errno, AllocateDir failures would be misreported. There is no great value in opening the directory before acquiring TwoPhaseStateLock, so just do it in the other order. Also fix CheckXLogRemoved to guarantee that it preserves errno, as quite a number of call sites are implicitly assuming. (Again, it's unlikely but I think not impossible that errno could change during a SpinLockAcquire. If so, this function was broken for its own purposes as well as breaking callers.) And change a few places that were using not-per-project-style messages, such as "could not read directory" when "could not open directory" is more correct. Back-patch the exporting of ReadDirExtended, in case we have occasion to back-patch some fix that makes use of it; it's not needed right now but surely making it global is pretty harmless. Also back-patch the restoreTwoPhaseData and CheckXLogRemoved fixes. The rest of this is essentially cosmetic and need not get back-patched. Michael Paquier, with a bit of additional work by me Discussion: https://postgr.es/m/CAB7nPqRpOCxjiirHmebEFhXVTK7V5Jvw4bz82p7Oimtsm3TyZA@mail.gmail.com
-
Robert Haas authored
Hopefully, the additional logging will help avoid confusion that could otherwise result. Nathan Bossart, reviewed by Michael Paquier, Fabrízio Mello, and me
-
Tom Lane authored
There's no good reason that the multicolumn stats stuff shouldn't work on booleans. But it looked only for "Var = pseudoconstant" clauses, and it will seldom find those for boolean Vars, since earlier phases of planning will fold "boolvar = true" or "boolvar = false" to just "boolvar" or "NOT boolvar" respectively. Improve dependencies_clauselist_selectivity() to recognize such clauses as equivalent to equality restrictions. This fixes a failure of the extended stats mechanism to apply in a case reported by Vitaliy Garnashevich. It's not a complete solution to his problem because the bitmap-scan costing code isn't consulting extended stats where it should, but that's surely an independent issue. In passing, improve some comments, get rid of a NumRelids() test that's redundant with the preceding bms_membership() test, and fix dependencies_clauselist_selectivity() so that estimatedclauses actually is a pure output argument as stated by its API contract. Back-patch to v10 where this code was introduced. Discussion: https://postgr.es/m/73a4936d-2814-dc08-ed0c-978f76f435b0@gmail.com
-