- 30 Mar, 2012 1 commit
-
-
Robert Haas authored
Fujii Masao, plus a comment by me. While I'm at it, correctly tabify this chunk of code.
-
- 29 Mar, 2012 9 commits
-
-
Tom Lane authored
The DBLINK_GET_CONN and DBLINK_GET_NAMED_CONN macros did not set the surrounding function's conname variable, causing errors to be incorrectly reported as having occurred on the "unnamed" connection in some cases. This bug was actually visible in two cases in the regression tests, but apparently whoever added those cases wasn't paying attention. Noted by Kyotaro Horiguchi, though this is different from his proposed patch. Back-patch to 8.4; 8.3 does not have the same type of error reporting so the patch is not relevant.
-
Tom Lane authored
It's actually more useful for the module to ignore these. Ignoring EXECUTE (and not incrementing the nesting level) allows the executor hooks to charge the time to the underlying prepared query, which shows up as a stats entry with the original PREPARE as query string (possibly modified by suppression of constants, which might not be terribly useful here but it's not worth avoiding). This is much more useful than cluttering the stats table with a distinct entry for each textually distinct EXECUTE. Experimentation with this idea shows that it's also preferable to ignore PREPARE. If we don't, we get two stats table entries, one with the query string hash and one with the jumble-derived hash, but with the same visible query string (modulo those constants). This is confusing and not very helpful, since the first entry will only receive costs associated with initial planning of the query, which is not something counted at all normally by pg_stat_statements. (And if we do start tracking planning costs, we'd want them blamed on the other hash table entry anyway.)
-
Tom Lane authored
When tracking nested statements, contrib/pg_stat_statements formerly double-counted the execution costs of utility statements that directly contain an executable statement, such as EXPLAIN and DECLARE CURSOR. This was not obvious since the ProcessUtility and Executor hooks would each add their measured costs to the same stats table entry. However, with the new implementation that hashes utility and plannable statements differently, this showed up as seemingly-duplicate stats entries. Fix that by disabling the Executor hooks when the query has a queryId of zero, which was the case already for such statements but is now more clearly specified in the code. (The zero queryId was causing problems anyway because all such statements would add to a single bogus entry.) The PREPARE/EXECUTE case still results in counting the same execution in two different stats table entries, but it should be much less surprising to users that there are two entries in such cases. In passing, include a CommonTableExpr's ctename in the query hash. I had left it out originally on the grounds that we wanted to omit all inessential aliases, but since RTE_CTE RTEs are hashing their referenced names, we'd better hash the CTE names too to make sure we don't hash semantically different queries the same.
-
Peter Eisentraut authored
Some Windows-only messages had apparently been forgotten so far. Also make the wording of the messages more consistent with similar messages other parts, such as pg_ctl and pg_regress.
-
Simon Riggs authored
Initialise ckptXidEpoch from starting checkpoint and maintain the correct value as we roll forwards. This allows GetNextXidAndEpoch() to return the correct epoch when executed during recovery. Backpatch to 9.0 when the problem is first observable by a user. Bug report from Daniel Farina
-
Andrew Dunstan authored
-
Heikki Linnakangas authored
Postmaster sets max_safe_fds by testing how many open file descriptors it can open, and that is normally inherited by all child processes at fork(). Not so on EXEC_BACKEND, ie. Windows, however. Because of that, we effectively ignored max_files_per_process on Windows, and always assumed a conservative default of 32 simultaneous open files. That could have an impact on performance, if you need to access a lot of different files in a query. After this patch, the value is passed to child processes by save/restore_backend_variables() among many other global variables. It has been like this forever, but given the lack of complaints about it, I'm not backpatching this.
-
Andrew Dunstan authored
-
Tom Lane authored
pg_stat_statements now hashes selected fields of the analyzed parse tree to assign a "fingerprint" to each query, and groups all queries with the same fingerprint into a single entry in the pg_stat_statements view. In practice it is expected that queries with the same fingerprint will be equivalent except for values of literal constants. To make the display more useful, such constants are replaced by "?" in the displayed query strings. This mechanism currently supports only optimizable queries (SELECT, INSERT, UPDATE, DELETE). Utility commands are still matched on the basis of their literal query strings. There remain some open questions about how to deal with utility statements that contain optimizable queries (such as EXPLAIN and SELECT INTO) and how to deal with expiring speculative hashtable entries that are made to save the normalized form of a query string. However, fixing these issues should require only localized changes, and since there are other open patches involving contrib/pg_stat_statements, it seems best to go ahead and commit what we've got. Peter Geoghegan, reviewed by Daniel Farina
-
- 28 Mar, 2012 6 commits
-
-
Peter Eisentraut authored
The intent is to allow configure --enable-nls=xx for installation speed and size, but have maintainer-check check all source files regardless.
-
Peter Eisentraut authored
-
Robert Haas authored
Per buildfarm, and Álvaro Herrera.
-
Robert Haas authored
Thomas Ogrisegg and Fujii Masao
-
Robert Haas authored
Fujii Masao
-
Robert Haas authored
Fujii Masao
-
- 27 Mar, 2012 7 commits
-
-
Robert Haas authored
Ants Aasma, Greg Smith
-
Robert Haas authored
Ants Aasma, reviewed by Greg Smith, with very minor tweaks by me.
-
Tom Lane authored
Generally, the parse location assigned to a multiple-token construct is the location of its leftmost token. This commit breaks that rule for the syntaxes TYPENAME 'LITERAL' and CAST(CONSTANT AS TYPENAME) --- the resulting Const will have the location of the literal string, not the typename or CAST keyword. The cases where this matters are pretty thin on the ground (no error messages in the regression tests change, for example), and it's unlikely that any user would be confused anyway by an error cursor pointing at the literal. But still it's less than consistent. The reason for changing it is that contrib/pg_stat_statements wants to know the parse location of the original literal, and it was agreed that this is the least unpleasant way to preserve that information through parse analysis. Peter Geoghegan
-
Tom Lane authored
Add a queryId field to Query and PlannedStmt. This is not used by the core backend, except for being copied around at appropriate times. It's meant to allow plug-ins to track a particular query forward from parse analysis to execution. The queryId is intentionally not dumped into stored rules (and hence this commit doesn't bump catversion). You could argue that choice either way, but it seems better that stored rule strings not have any dependency on plug-ins that might or might not be present. Also, add a post_parse_analyze_hook that gets invoked at the end of parse analysis (but only for top-level analysis of complete queries, not cases such as analyzing a domain's default-value expression). This is mainly meant to be used to compute and assign a queryId, but it could have other applications. Peter Geoghegan
-
Robert Haas authored
Currently, the only way to see the numbers this gathers is via EXPLAIN (ANALYZE, BUFFERS), but the plan is to add visibility through the stats collector and pg_stat_statements in subsequent patches. Ants Aasma, reviewed by Greg Smith, with some further changes by me.
-
Tom Lane authored
-
Peter Eisentraut authored
-
- 26 Mar, 2012 5 commits
-
-
Peter Eisentraut authored
Organize the function descriptions as a list instead of running text, for easier access.
-
Peter Eisentraut authored
found by Coverity
-
Robert Haas authored
It used to be case that lazy vacuum could call this function with only a shared lock on the buffer, but neither lazy vacuum nor any other code path does that any more. Simplify the code accordingly and clean up some related, obsolete comments.
-
Tom Lane authored
The COPY documentation says "COPY FROM matches the input against the null string before removing backslashes". It is therefore reasonable to presume that null markers like E'\\0' will work ... and they did, until someone put the tests in the wrong order during microoptimization-driven rewrites. Since then, we've been failing if the null marker is something that would de-escape to an invalidly-encoded string. Since null markers generally need to be something that can't appear in the data, this represents a nontrivial loss of functionality; surprising nobody noticed it earlier. Per report from Jeff Davis. Backpatch to 8.4 where this got broken.
-
Tom Lane authored
setlocale() accepts locale name "" as meaning "the locale specified by the process's environment variables". Historically we've accepted that for Postgres' locale settings, too. However, it's fairly unsafe to store an empty string in a new database's pg_database.datcollate or datctype fields, because then the interpretation could vary across postmaster restarts, possibly resulting in index corruption and other unpleasantness. Instead, we should expand "" to whatever it means at the moment of calling CREATE DATABASE, which we can do by saving the value returned by setlocale(). For consistency, make initdb set up the initial lc_xxx parameter values the same way. initdb was already doing the right thing for empty locale names, but it did not replace non-empty names with setlocale results. On a platform where setlocale chooses to canonicalize the spellings of locale names, this would result in annoying inconsistency. (It seems that popular implementations of setlocale don't do such canonicalization, which is a pity, but the POSIX spec certainly allows it to be done.) The same risk of inconsistency leads me to not venture back-patching this, although it could certainly be seen as a longstanding bug. Per report from Jeff Davis, though this is not his proposed patch.
-
- 24 Mar, 2012 2 commits
-
-
Tom Lane authored
For some reason, in the original coding of the PlaceHolderVar mechanism I had supposed that PlaceHolderVars couldn't propagate into subqueries. That is of course entirely possible. When it happens, we need to treat an outer-level PlaceHolderVar much like an outer Var or Aggref, that is SS_replace_correlation_vars() needs to replace the PlaceHolderVar with a Param, and then when building the finished SubPlan we have to provide the PlaceHolderVar expression as an actual parameter for the SubPlan. The handling of the contained expression is a bit delicate but it can be treated exactly like an Aggref's expression. In addition to the missing logic in subselect.c, prepjointree.c was failing to search subqueries for PlaceHolderVars that need their relids adjusted during subquery pullup. It looks like everyplace else that touches PlaceHolderVars got it right, though. Per report from Mark Murawski. In 9.1 and HEAD, queries affected by this oversight would fail with "ERROR: Upper-level PlaceHolderVar found where not expected". But in 9.0 and 8.4, you'd silently get possibly-wrong answers, since the value transmitted into the subquery wouldn't go to null when it should.
-
Tom Lane authored
Per compiler warnings on buildfarm member black_firefly.
-
- 23 Mar, 2012 3 commits
-
-
Tom Lane authored
We were doing the recursive simplification of function/operator arguments in half a dozen different places, with rather baroque logic to ensure it didn't get done multiple times on some arguments. This patch improves that by postponing argument simplification until after we've dealt with named parameters and added any needed default expressions. Marti Raudsepp, somewhat hacked on by me
-
Tom Lane authored
Fix loss of previous expression-simplification work when a transform function fires: we must not simply revert to untransformed input tree. Instead build a dummy FuncExpr node to pass to the transform function. This has the additional advantage of providing a simpler, more uniform API for transform functions. Move documentation to a somewhat less buried spot, relocate some poorly-placed code, be more wary of null constants and invalid typmod values, add an opr_sanity check on protransform function signatures, and some other minor cosmetic adjustments. Note: although this patch touches pg_proc.h, no need for catversion bump, because the changes are cosmetic and don't actually change the intended catalog contents.
-
Robert Haas authored
Per discussion with Dmitriy Igrishin and Tom Lane.
-
- 22 Mar, 2012 6 commits
-
-
Robert Haas authored
Jay Levitt, reviewed by Tom Lane.
-
Robert Haas authored
Since 9.1, the minimum overhead is three bytes, not five. Fujii Masao
-
Tom Lane authored
An incorrect and entirely unnecessary "safety check" in exec_stmt_getdiag() caused the code to treat an assignment to a variable with dno zero as a no-op. Unfortunately, that's a perfectly valid dno. This has been broken since GET DIAGNOSTICS was invented. It's not terribly surprising that the bug went unnoticed for so long, since in most cases you probably wouldn't use the function's first-created variable (normally its first parameter) as a GET DIAGNOSTICS target. Nonetheless, it's broken. Per bug #6551 from Adam Buraczewski.
-
Tom Lane authored
Alex Shulgin, lightly edited by me
-
Tom Lane authored
Per a suggestion from Euler Taveira, it seems like a good idea to include this information in \du (and \dg) output. This costs nothing for people who are not using the VALID UNTIL feature, while for those who are, it's rather critical information. Fabrízio de Royes Mello
-
Tom Lane authored
PGAC_PATH_COLLATEINDEX supposed that it could use AC_PATH_PROGS to search for collateindex.pl, but that macro will only accept files that are marked executable, and at least some DocBook installations don't mark the script executable (a case the docs Makefile was already prepared for). Accept the script if it's present and readable in $DOCBOOKSTYLE/bin, and otherwise search the PATH as before. Having fixed that up, we don't need the fallback case that was in the docs Makefile, and instead can throw an understandable error if configure didn't find the script. Per recent trouble report from John Lumby.
-
- 21 Mar, 2012 1 commit
-
-
Peter Eisentraut authored
For those variables only used when asserts are enabled, use a new macro PG_USED_FOR_ASSERTS_ONLY, which expands to __attribute__((unused)) when asserts are not enabled.
-