- 25 Sep, 2015 3 commits
-
-
Tom Lane authored
(Third time's the charm, I hope.) Additional testing disclosed that this code could mangle already-localized output from the "money" datatype. We can't very easily skip applying it to "money" values, because the logic is tied to column right-justification and people expect "money" output to be right-justified. Short of decoupling that, we can fix it in what should be a safe enough way by testing to make sure the string doesn't contain any characters that would not be expected in plain numeric output.
-
Tom Lane authored
On closer inspection, those seemingly redundant atoi() calls were not so much inefficient as just plain wrong: the author of this code either had not read, or had not understood, the POSIX specification for localeconv(). The grouping field is *not* a textual digit string but separate integers encoded as chars. We'll follow the existing code as well as the backend's cash.c in only honoring the first group width, but let's at least honor it correctly. This doesn't actually result in any behavioral change in any of the locales I have installed on my Linux box, which may explain why nobody's complained; grouping width 3 is close enough to universal that it's barely worth considering other cases. Still, wrong is wrong, so back-patch.
-
Tom Lane authored
This code did the wrong thing entirely for numbers with an exponent but no decimal point (e.g., '1e6'), as reported by Jeff Janes in bug #13636. More generally, it made lots of unverified assumptions about what the input string could possibly look like. Rearrange so that it only fools with leading digits that it's directly verified are there, and an immediately adjacent decimal point. While at it, get rid of some useless inefficiencies, like converting the grouping count string to integer over and over (and over). This has been broken for a long time, so back-patch to all supported branches.
-
- 24 Sep, 2015 5 commits
-
-
Tom Lane authored
Previously, a function call appearing at the top level of WHERE had a hard-wired selectivity estimate of 0.3333333, a kludge conveniently dated in the source code itself to July 1992. The expectation at the time was that somebody would soon implement estimator support functions analogous to those for operators; but no such code has appeared, nor does it seem likely to in the near future. We do have an alternative solution though, at least for immutable functions on single relations: creating an expression index on the function call will allow ANALYZE to gather stats about the function's selectivity. But the code in clause_selectivity() failed to make use of such data even if it exists. Refactor so that that will happen. I chose to make it try this technique for any clause type for which clause_selectivity() doesn't have a special case, not just functions. To avoid adding unnecessary overhead in the common case where we don't learn anything new, make selfuncs.c provide an API that hooks directly to examine_variable() and then var_eq_const(), rather than the previous coding which laboriously constructed an OpExpr only so that it could be expensively deconstructed again. I preserved the behavior that the default estimate for a function call is 0.3333333. (For any other expression node type, it's 0.5, as before.) I had originally thought to make the default be 0.5 across the board, but changing a default estimate that's survived for twenty-three years seems like something not to do without a lot more testing than I care to put into it right now. Per a complaint from Jehan-Guillaume de Rorthais. Back-patch into 9.5, but not further, at least for the moment.
-
Tom Lane authored
If we have a local Var of say varchar type with default collation, and we apply a RelabelType to convert that to text with default collation, we don't want to consider that as creating an FDW_COLLATE_UNSAFE situation. It should be okay to compare that to a remote Var, so long as the remote Var determines the comparison collation. (When we actually ship such an expression to the remote side, the local Var would become a Param with default collation, meaning the remote Var would in fact control the comparison collation, because non-default implicit collation overrides default implicit collation in parse_collate.c.) To fix, be more precise about what FDW_COLLATE_NONE means: it applies either to a noncollatable data type or to a collatable type with default collation, if that collation can't be traced to a remote Var. (When it can, FDW_COLLATE_SAFE is appropriate.) We were essentially using that interpretation already at the Var/Const/Param level, but we weren't bubbling it up properly. An alternative fix would be to introduce a separate FDW_COLLATE_DEFAULT value to describe the second situation, but that would add more code without changing the actual behavior, so it didn't seem worthwhile. Also, since we're clarifying the rule to be that we care about whether operator/function input collations match, there seems no need to fail immediately upon seeing a Const/Param/non-foreign-Var with nondefault collation. We only have to reject if it appears in a collation-sensitive context (for example, "var IS NOT NULL" is perfectly safe from a collation standpoint, whatever collation the var has). So just set the state to UNSAFE rather than failing immediately. Per report from Jeevan Chalke. This essentially corrects some sloppy thinking in commit ed3ddf91, so back-patch to 9.3 where that logic appeared.
-
Robert Haas authored
The comments here stated that this was just in case we ever had an ALTER OPERATOR command that could remap an operator to a different function. But those comments have been here for a long time, and no such command has come about. In the absence of such a feature, forcing the pg_proc OID to be looked up again each time we reread a stored rule or similar is just a waste of cycles. Moreover, parallel query needs a way to reread the exact same node tree that was written out, not one that has been slightly stomped on. So just get rid of this for now. Per discussion with Tom Lane.
-
Fujii Masao authored
Previously pg_controldata didn't report newestCommitTs and this was an oversight in commit 73c986ad. Also this patch changes pg_resetxlog so that it uses the same sentences as pg_controldata does, regarding oldestCommitTs and newestCommitTs, for the sake of consistency. Back-patch to 9.5 where track_commit_timestamp was added. Euler Taveira
-
Andres Freund authored
The old minimum values are rather large, making it time consuming to test related behaviour. Additionally the current limits, especially for multixacts, can be problematic in space-constrained systems. 10000000 multixacts can contain a lot of members. Since there's no good reason for the current limits, lower them a good bit. Setting them to 0 would be a bad idea, triggering endless vacuums, so still retain a limit. While at it fix autovacuum_multixact_freeze_max_age to refer to multixact.c instead of varsup.c. Reviewed-By: Robert Haas Discussion: CA+TgmoYmQPHcrc3GSs7vwvrbTkbcGD9Gik=OztbDGGrovkkEzQ@mail.gmail.com Backpatch: back to 9.0 (in parts)
-
- 23 Sep, 2015 5 commits
-
-
Tom Lane authored
Previously, ANALYZE simply ignored columns of datatypes that have neither a btree nor hash opclass (which means they have no recognized equality operator). Without a notion of equality, we can't identify most-common values nor estimate the number of distinct values. But we can still count nulls and compute the average physical column width, and those stats might be of value. Moreover there are some tools out there that don't work so well if rows are missing from pg_statistic. So let's add suitable logic for this case. While this is arguably a bug fix, it also has the potential to change query plans, and the gain seems not worth taking a risk of that in stable branches. So back-patch into 9.5 but not further. Oleksandr Shulgin, rewritten a bit by me.
-
Robert Haas authored
For parallel query, we need to be able to pass a Plan to a worker, so that it knows what it's supposed to do. We could invent our own way of serializing plans for that purpose, but piggybacking on the existing node infrastructure seems like a much better idea. Initially, we'll probably only support a limited number of nodes within parallel workers, but this commit adds support for everything in plannodes.h except CustomScan, because doing it all at once seems easier than doing it piecemeal, and it makes testing this code easier, too. CustomScan is excluded because making that work requires a larger rework of that facility. Amit Kapila, reviewed and slightly revised by me.
-
Robert Haas authored
It's declared as being an array of bool, but it's printed differently from the way bool and arrays of bool are handled elsewhere. Patch by Amit Kapila. Anomaly noted independently by Amit Kapila and KaiGai Kohei.
-
Teodor Sigaev authored
Commit e9568083 introduces adding pages to FSM for ordinary insert, but autoanalyze was able just cleanup pending list without adding to FSM. Also fix double call of IndexFreeSpaceMapVacuum() during ginvacuumcleanup() Report from Fujii Masao Patch by me Review by Jeff Janes
-
Robert Haas authored
This logic was missing from ExplainPreScanNode, from which I derived planstate_tree_walker. But it shouldn't be missing, especially not from a generic walker function, so add it. KaiGai Kohei
-
- 22 Sep, 2015 6 commits
-
-
Tom Lane authored
Per bug #13631 from KOIZUMI Satoru.
-
Andres Freund authored
A bunch of tests missed specifying that empty transactions shouldn't be displayed. That causes problems when e.g. autovacuum runs in an unfortunate moment. The tests in question only run for a very short time, making this quite unlikely. Reported-By: Buildfarm member axolotl Backpatch: 9.4, where logical decoding was introduced
-
Andres Freund authored
The previous wrong value lead to wrong LOCK_DEBUG output, never showing any shared lock holders. Reported-By: Alexander Korotkov Discussion: CAPpHfdsPmWqz9FB0AnxJrwp1=KLF0n=-iB+QvR0Q8GSmpFVdUQ@mail.gmail.com Backpatch: 9.5, where the bug was introduced.
-
Andres Freund authored
This deserves to be greatly expanded and improved, but it's a start. Discussion: 20150827145219.GI2435@awork2.anarazel.de
-
Peter Eisentraut authored
-
Peter Eisentraut authored
Based on patch by Thomas Munro <thomas.munro@enterprisedb.com>, although I rephrased most of the initial work.
-
- 21 Sep, 2015 5 commits
-
-
Peter Eisentraut authored
-
Tom Lane authored
mul_var() postpones propagating carries until it risks overflow in its internal digit array. However, the logic failed to account for the possibility of overflow in the carry propagation step, allowing wrong results to be generated in corner cases. We must slightly reduce the when-to-propagate-carries threshold to avoid that. Discovered and fixed by Dean Rasheed, with small adjustments by me. This has been wrong since commit d72f6c75, so back-patch to all supported branches.
-
Noah Misch authored
This commit's parent made superfluous the bit's sole usage. Referential integrity checks have long run as the subject table's owner, and that now implies RLS bypass. Safe use of the bit was tricky, requiring strict control over the SQL expressions evaluating therein. Back-patch to 9.5, where the bit was introduced. Based on a patch by Stephen Frost.
-
Noah Misch authored
Every query of a single ENABLE ROW SECURITY table has two meanings, with the row_security GUC selecting between them. With row_security=force available, every function author would have been advised to either set the GUC locally or test both meanings. Non-compliance would have threatened reliability and, for SECURITY DEFINER functions, security. Authors already face an obligation to account for search_path, and we should not mimic that example. With this change, only BYPASSRLS roles need exercise the aforementioned care. Back-patch to 9.5, where the row_security GUC was introduced. Since this narrows the domain of pg_db_role_setting.setconfig and pg_proc.proconfig, one might bump catversion. A row_security=force setting in one of those columns will elicit a clear message, so don't.
-
Noah Misch authored
Per Coverity. Back-patch to 9.0 (all supported versions). Michael Paquier, reviewed (in earlier versions) by Heikki Linnakangas.
-
- 20 Sep, 2015 1 commit
-
-
Tom Lane authored
RemoveLocalLock() must consider the possibility that LockAcquireExtended() failed to palloc the initial space for a locallock's lockOwners array. I had evidently meant to cope with this hazard when the code was originally written (commit 1785aceb), but missed that the pfree needed to be protected with an if-test. Just to make sure things are left in a clean state, reset numLockOwners as well. Per low-memory testing by Andreas Seltenreich. Back-patch to all supported branches.
-
- 19 Sep, 2015 4 commits
-
-
Peter Eisentraut authored
-
Peter Eisentraut authored
-
Peter Eisentraut authored
BSD find is not very smart and ends up writing double slashes into the output in those cases. Also, xgettext is not very smart and splits the file names incorrectly in those cases, resulting in slightly incorrect file names being written into the POT file.
-
Robert Haas authored
The shm_mq mechanism was built to send error (and notice) messages and tuples between backends. However, shm_mq itself only deals in raw bytes. Since commit 2bd9e412, we have had infrastructure for one message to redirect protocol messages to a queue and for another backend to parse them and do useful things with them. This commit introduces a somewhat analogous facility for tuples by adding a new type of DestReceiver, DestTupleQueue, which writes each tuple generated by a query into a shm_mq, and a new TupleQueueFunnel facility which reads raw tuples out of the queue and reconstructs the HeapTuple format expected by the executor. The TupleQueueFunnel abstraction supports reading from multiple tuple streams at the same time, but only in round-robin fashion. Someone could imaginably want other policies, but this should be good enough to meet our short-term needs related to parallel query, and we can always extend it later. This also makes one minor addition to the shm_mq API that didn' seem worth breaking out as a separate patch. Extracted from Amit Kapila's parallel sequential scan patch. This code was originally written by me, and then it was revised by Amit, and then it was revised some more by me.
-
- 18 Sep, 2015 4 commits
-
-
Andrew Dunstan authored
These functions have been looking up type info for every row they process. Instead of doing that we only look them up the first time through and stash the information in the aggregate state object. Affects json_agg, json_object_agg, jsonb_agg and jsonb_object_agg. There is plenty more work to do in making these more efficient, especially the jsonb functions, but this is a virtually cost free improvement that can be done right away. Backpatch to 9.5 where the jsonb variants were introduced.
-
Tom Lane authored
After an internal failure in shortest() or longest() while pinning down the exact location of a match, find() forgot to free the DFA structure before returning. This is pretty unlikely to occur, since we just successfully ran the "search" variant of the DFA; but it could happen, and it would result in a session-lifespan memory leak since this code uses malloc() directly. Problem seems to have been aboriginal in Spencer's library, so back-patch all the way. In passing, correct a thinko in a comment I added awhile back about the meaning of the "ntree" field. I happened across these issues while comparing our code to Tcl's version of the library.
-
Teodor Sigaev authored
Report from Peter Eisentraut
-
Peter Eisentraut authored
-
- 17 Sep, 2015 5 commits
-
-
Teodor Sigaev authored
Use IsBinaryCoercible() method instead of custom is_expected_type/is_text_type functions which was introduced when tsearch2 was moved into core. Per report by David E. Wheeler Analysis by Tom Lane Patch by me
-
Andrew Dunstan authored
This setting contains extra configuration for the temp instance, as used in pg_regress' --temp-config flag. Backpatch to 9.2 where test.sh was introduced.
-
Robert Haas authored
ExplainPreScanNode knows how to iterate over a generic tree of plan states; factor that logic out into a separate walker function so that other code, such as upcoming patches for parallel query, can also use it. Patch by me, reviewed by Tom Lane.
-
Michael Meskes authored
Back in the day this did not work, but modern compilers should handle it themselves.
-
Teodor Sigaev authored
Commit 013ebc0a introduces microvacuum for GiST, deletetion of tuple marked LP_DEAD uses IndexPageMultiDelete while recovery code uses IndexPageTupleDelete in loop. This causes a difference in offset numbers of tuples to delete. Patch introduces usage of IndexPageMultiDelete in GiST except gistplacetopage() where only one tuple is deleted at once. That also slightly improve performance, because IndexPageMultiDelete is more effective. Patch changes WAL format, so bump wal page magic. Bug report from Jeff Janes Diagnostic and patch by Anastasia Lubennikova and me
-
- 16 Sep, 2015 2 commits
-
-
Robert Haas authored
Commit 924bcf4f introduced a framework for parallel computation in PostgreSQL that makes most but not all built-in functions safe to execute in parallel mode. In order to have parallel query, we'll need to be able to determine whether that query contains functions (either built-in or user-defined) that cannot be safely executed in parallel mode. This requires those functions to be labeled, so this patch introduces an infrastructure for that. Some functions currently labeled as safe may need to be revised depending on how pending issues related to heavyweight locking under paralllelism are resolved. Parallel plans can't be used except for the case where the query will run to completion. If portal execution were suspended, the parallel mode restrictions would need to remain in effect during that time, but that might make other queries fail. Therefore, this patch introduces a framework that enables consideration of parallel plans only when it is known that the plan will be run to completion. This probably needs some refinement; for example, at bind time, we do not know whether a query run via the extended protocol will be execution to completion or run with a limited fetch count. Having the client indicate its intentions at bind time would constitute a wire protocol break. Some contexts in which parallel mode would be safe are not adjusted by this patch; the default is not to try parallel plans except from call sites that have been updated to say that such plans are OK. This commit doesn't introduce any parallel paths or plans; it just provides a way to determine whether they could potentially be used. I'm committing it on the theory that the remaining parallel sequential scan patches will also get committed to this release, hopefully in the not-too-distant future. Robert Haas and Amit Kapila. Reviewed (in earlier versions) by Noah Misch.
-
Tom Lane authored
Sync our regex code with upstream changes since last time we did this, which was Tcl 8.5.11 (see commit 08fd6ff3). The only functional change here is to disbelieve that an octal escape is three digits long if it would exceed \377. That's a bug fix, but it's a minor one and could change the interpretation of working regexes, so don't back-patch. In addition to that, s/INFINITY/DUPINF/ to eliminate the risk of collisions with <math.h>'s macro, and s/LOCAL/NOPROP/ because that also seems like an unnecessarily collision-prone macro name. There were some other cosmetic changes in their copy that I did not adopt, notably a rather half-hearted attempt at renaming some of the C functions in a more verbose style. (I'm not necessarily against the concept, but renaming just a few functions in the package is not an improvement.)
-