- 14 Mar, 2015 3 commits
-
-
Peter Eisentraut authored
-
Tom Lane authored
Since commit ba7c5975, port/dirmod.c has contained only Windows-specific functions. Most platforms don't seem to mind uselessly building an empty file, but OS X for one issues warnings. Hence, treat dirmod.c as a Windows-specific file selected by configure rather than one that's always built. We can revert this change if dirmod.c ever gains any non-Windows functionality again. Back-patch to 9.4 where the mentioned commit appeared.
-
Tom Lane authored
GNU readline defines the return value of write_history() as "zero if OK, else an errno code". libedit's version of that function used to have a different definition (to wit, "-1 if error, else the number of lines written to the file"). We tried to work around that by checking whether errno had become nonzero, but this method has never been kosher according to the published API of either library. It's reportedly completely broken in recent Ubuntu releases: psql bleats about "No such file or directory" when saving ~/.psql_history, even though the write worked fine. However, libedit has been following the readline definition since somewhere around 2006, so it seems all right to finally break compatibility with ancient libedit releases and trust that the return value is what readline specifies. (I'm not sure when the various Linux distributions incorporated this fix, but I did find that OS X has been shipping fixed versions since 10.5/Leopard.) If anyone is still using such an ancient libedit, they will find that psql complains it can't write ~/.psql_history at exit, even when the file was written correctly. This is no worse than the behavior we're fixing for current releases. Back-patch to all supported branches.
-
- 13 Mar, 2015 2 commits
-
-
Tatsuo Ishii authored
The message tries to tell the replication apply delay which fails if the first WAL record is not applied yet. Fix is, instead of telling overflowed minus numeric, showing "N/A" which indicates that the delay data is not yet available. Problem reported by me and patch by Fabrízio de Royes Mello. Back patched to 9.4, 9.3 and 9.2 stable branches (9.1 and 9.0 do not have the debug message).
-
Robert Haas authored
These APIs changed somewhat subsequent to the initial commit, and may change further in the future, but let's document what we have today. KaiGai Kohei and Robert Haas, reviewed by Tom Lane and Thom Brown
-
- 12 Mar, 2015 6 commits
-
-
Tom Lane authored
Explain some of the funny conventions used in btree page items. Peter Geoghegan and Jeff Janes
-
Tom Lane authored
The ROW_MARK_COPY path in EvalPlanQualFetchRowMarks() was just setting tableoid to InvalidOid, I think on the assumption that the referenced RTE must be a subquery or other case without a meaningful OID. However, foreign tables also use this code path, and they do have meaningful table OIDs; so failure to set the tuple field can lead to user-visible misbehavior. Fix that by fetching the appropriate OID from the range table. There's still an issue about whether CTID can ever have a meaningful value in this case; at least with postgres_fdw foreign tables, it does. But that is a different problem that seems to require a significantly different patch --- it's debatable whether postgres_fdw really wants to use this code path at all. Simplified version of a patch by Etsuro Fujita, who also noted the problem to begin with. The issue can be demonstrated in all versions having FDWs, so back-patch to 9.1.
-
Heikki Linnakangas authored
Per bug #12850 by Walter Nordmann. Backpatch to 9.4 where the leak was introduced.
-
Tom Lane authored
We can't handle this in the general case due to limitations of the planner's data representations; but we can allow it in many useful cases, by being careful to flatten only when we are pulling a single-row subquery up into a FROM (or, equivalently, inner JOIN) node that will still have at least one remaining relation child. Per discussion of an example from Kyotaro Horiguchi.
-
Tom Lane authored
While poking at David Kubečka's issue I noticed an ancient logic error in get_loop_count(): it used 1.0 as a "no data yet" indicator, but since that is actually a valid rowcount estimate, this doesn't work. If we have one input relation with 1.0 as rowcount and then another one with a larger rowcount, we should use 1.0 as the result, but we picked the larger rowcount instead. (I think when I coded this, I recognized the conflict, but mistakenly thought that the logic would pick the desired count anyway.) Fixing this changed the plan for one existing regression test case. Since the point of that test is to exercise creation of a particular shape of nestloop plan, I tweaked the query a little bit so it still results in the same plan choice. This is definitely a bug, but I'm hesitant to back-patch since it might change plan choices unexpectedly, and anyway failure to implement a heuristic precisely as intended is a pretty low-grade bug.
-
Tom Lane authored
If we have a semijoin, say SELECT * FROM x WHERE x1 IN (SELECT y1 FROM y) and we're estimating the cost of a parameterized indexscan on x, the number of repetitions of the indexscan should not be taken as the size of y; it'll really only be the number of distinct values of y1, because the only valid plan with y on the outside of a nestloop would require y to be unique-ified before joining it to x. Most of the time this doesn't make that much difference, but sometimes it can lead to drastically underestimating the cost of the indexscan and hence choosing a bad plan, as pointed out by David Kubečka. Fixing this is a bit difficult because parameterized indexscans are costed out quite early in the planning process, before we have the information that would be needed to call estimate_num_groups() and thereby estimate the number of distinct values of the join column(s). However we can move the code that extracts a semijoin RHS's unique-ification columns, so that it's done in initsplan.c rather than on-the-fly in create_unique_path(). That shouldn't make any difference speed-wise and it's really a bit cleaner too. The other bit of information we need is the size of the semijoin RHS, which is easy if it's a single relation (we make those estimates before considering indexscan costs) but problematic if it's a join relation. The solution adopted here is just to use the product of the sizes of the join component rels. That will generally be an overestimate, but since estimate_num_groups() only uses this input as a clamp, an overestimate shouldn't hurt us too badly. In any case we don't allow this new logic to produce a value larger than we would have chosen before, so that at worst an overestimate leaves us no wiser than we were before.
-
- 11 Mar, 2015 13 commits
-
-
Peter Eisentraut authored
-
Alvaro Herrera authored
In the spirit of 890192e9, this time add support for the things living in the pg_default_acl catalog. These are not really "objects", but they show up as such in event triggers. There is no "DROP DEFAULT PRIVILEGES" or similar command, so it doesn't look like the new representation given would be useful anywhere else, so I didn't try to use it outside objectaddress.c. (That might be a bug in itself, but that would be material for another commit.) Reviewed by Stephen Frost.
-
Alvaro Herrera authored
Evidently, this test is not run very frequently ...
-
Alvaro Herrera authored
Since commit 72dd233d we were trying to obtain object addressing information in sql_drop event triggers, but that caused failures when the drops involved user mappings. This addition enables that to work again. Naturally, pg_get_object_address can work with these objects now, too. I toyed with the idea of removing DropUserMappingStmt as a node and using DropStmt instead in the DropUserMappingStmt grammar production, but that didn't go very well: for one thing the messages thrown by the specific code are specialized (you get "server not found" if you specify the wrong server, instead of a generic "user mapping for ... not found" which you'd get it we were to merge this with RemoveObjects --- unless we added even more special cases). For another thing, it would require to pass RoleSpec nodes through the objname/objargs representation used by RemoveObjects, which works in isolation, but gets messy when pg_get_object_address is involved. So I dropped this part for now. Reviewed by Stephen Frost.
-
Peter Eisentraut authored
PL/Python uses str() to convert Python values back to PostgreSQL, but str() is lossy for float values, so use repr() instead in that case. Author: Marko Kreen <markokr@gmail.com>
-
Robert Haas authored
Per discussion, it's better to have a consistent coding rule here. Michael Paquier, per a node from Greg Stark referencing an old post from Tom Lane.
-
Tom Lane authored
While the SQL standard is pretty vague on the overall topic of operator precedence (because it never presents a unified BNF for all expressions), it does seem reasonable to conclude from the spec for <boolean value expression> that OR has the lowest precedence, then AND, then NOT, then IS tests, then the six standard comparison operators, then everything else (since any non-boolean operator in a WHERE clause would need to be an argument of one of these). We were only sort of on board with that: most notably, while "<" ">" and "=" had properly low precedence, "<=" ">=" and "<>" were treated as generic operators and so had significantly higher precedence. And "IS" tests were even higher precedence than those, which is very clearly wrong per spec. Another problem was that "foo NOT SOMETHING bar" constructs, such as "x NOT LIKE y", were treated inconsistently because of a bison implementation artifact: they had the documented precedence with respect to operators to their right, but behaved like NOT (i.e., very low priority) with respect to operators to their left. Fixing the precedence issues is just a small matter of rearranging the precedence declarations in gram.y, except for the NOT problem, which requires adding an additional lookahead case in base_yylex() so that we can attach a different token precedence to NOT LIKE and allied two-word operators. The bulk of this patch is not the bug fix per se, but adding logic to parse_expr.c to allow giving warnings if an expression has changed meaning because of these precedence changes. These warnings are off by default and are enabled by the new GUC operator_precedence_warning. It's believed that very few applications will be affected by these changes, but it was agreed that a warning mechanism is essential to help debug any that are.
-
Tom Lane authored
setup_param_list() was allocating a fresh ParamListInfo for each query or expression evaluation requested by a plpgsql function. There was probably once good reason to do it like that, but for a long time we've had a convention that there's a one-to-one mapping between the function's PLpgSQL_datum array and the ParamListInfo slots, which means that a single ParamListInfo can serve all the function's evaluation requests: the data that would need to be passed is the same anyway. In this patch, we retain the pattern of zeroing out the ParamListInfo contents during each setup_param_list() call, because some of the slots may be stale and we don't know exactly which ones. So this patch only saves a palloc/pfree per evaluation cycle and nothing more; still, that seems to be good for a couple percent overall speedup on simple-arithmetic type statements. In future, though, we might be able to improve matters still more by managing the param array contents more carefully. Also, unify the former use of estate->cur_expr with that of paramLI->parserSetupArg; they both were used to point to the active expression, so we can combine the variables into just one.
-
Robert Haas authored
KaiGai Kohei, reviewed by Álvaro Herrera and myself
-
Robert Haas authored
Error messages informing the user that no such column exists can sometimes provoke a perplexed response. This often happens due to a subtle typo in the column name or, perhaps less likely, in the alias name. To speed discovery of what the real issue is in such cases, we'll now search the range table for approximate matches. If there are one or two such matches that are good enough to think that they might be what the user intended to type, and better than all other approximate matches, we'll issue a hint suggesting that the user might have intended to reference those columns. Peter Geoghegan and Robert Haas
-
Andres Freund authored
Until now __attribute__() was defined to be empty for all compilers but gcc. That's problematic because it prevents using it in other compilers; which is necessary e.g. for atomics portability. It's also just generally dubious to do so in a header as widely included as c.h. Instead add pg_attribute_format_arg, pg_attribute_printf, pg_attribute_noreturn macros which are implemented in the compilers that understand them. Also add pg_attribute_noreturn and pg_attribute_packed, but don't provide fallbacks, since they can affect functionality. This means that external code that, possibly unwittingly, relied on __attribute__ defined to be empty on !gcc compilers may now run into warnings or errors on those compilers. But there shouldn't be many occurances of that and it's hard to work around... Discussion: 54B58BA3.8040302@ohmu.fi Author: Oskari Saarenmaa, with some minor changes by me.
-
Alvaro Herrera authored
This is in preparation to "upgrade" some modules from contrib/ to src/bin/, per discussion. Author: Michael Paquier
-
Fujii Masao authored
When newly-added GUC parameter, wal_compression, is on, the PostgreSQL server compresses a full page image written to WAL when full_page_writes is on or during a base backup. A compressed page image will be decompressed during WAL replay. Turning this parameter on can reduce the WAL volume without increasing the risk of unrecoverable data corruption, but at the cost of some extra CPU spent on the compression during WAL logging and on the decompression during WAL replay. This commit changes the WAL format (so bumping WAL version number) so that the one-byte flag indicating whether a full page image is compressed or not is included in its header information. This means that the commit increases the WAL volume one-byte per a full page image even if WAL compression is not used at all. We can save that one-byte by borrowing one-bit from the existing field like hole_offset in the header and using it as the flag, for example. But which would reduce the code readability and the extensibility of the feature. Per discussion, it's not worth paying those prices to save only one-byte, so we decided to add the one-byte flag to the header. This commit doesn't introduce any new compression algorithm like lz4. Currently a full page image is compressed using the existing PGLZ algorithm. Per discussion, we decided to use it at least in the first version of the feature because there were no performance reports showing that its compression ratio is unacceptably lower than that of other algorithm. Of course, in the future, it's worth considering the support of other compression algorithm for the better compression. Rahila Syed and Michael Paquier, reviewed in various versions by myself, Andres Freund, Robert Haas, Abhijit Menon-Sen and many others.
-
- 10 Mar, 2015 4 commits
-
-
Tom Lane authored
Commit 865f14a2 was quite a few bricks shy of a load: psql, ecpg, and plpgsql were all left out-of-step with the core lexer. Of these only the last was likely to be a fatal problem; but still, a minimal amount of grepping, or even just reading the comments adjacent to the places that were changed, would have found the other places that needed to be changed.
-
Alvaro Herrera authored
The introduction in the Shared Library Preloading section already instructs the user to separate multiple library names with commas, so just remove the fragment from here. Author: Dagfinn Ilmari Mannsåker
-
Alvaro Herrera authored
... which is the usual convention among AMs, so that pg_filedump and similar utilities can tell apart pages of different AMs. It was also the intent of the original code, but I failed to realize that alignment considerations would move the whole thing to the previous-to-last word in the page. The new definition of the associated macro makes surrounding code a bit leaner, too. Per note from Heikki at http://www.postgresql.org/message-id/546A16EF.9070005@vmware.com
-
Robert Haas authored
SQL has standardized on => as the use of to specify named parameters, and we've wanted for many years to support the same syntax ourselves, but this has been complicated by the possible use of => as an operator name. In PostgreSQL 9.0, we began emitting a warning when an operator named => was defined, and in PostgreSQL 9.2, we stopped shipping a =>(text, text) operator as part of hstore. By the time the next major version of PostgreSQL is released, => will have been deprecated for a full five years, so hopefully there won't be too many people still relying on it. We continue to support := for compatibility with previous PostgreSQL releases. Pavel Stehule, reviewed by Petr Jelinek, with a few documentation tweaks by me.
-
- 09 Mar, 2015 10 commits
-
-
Alvaro Herrera authored
We allow this module to be turned off on restarts, so a restart time check is enough to activate or deactivate the module; however, if there is a standby replaying WAL emitted from a master which is restarted, but the standby isn't, the state in the standby becomes inconsistent and can easily be crashed. Fix by activating and deactivating the module during WAL replay on parameter change as well as on system start. Problem reported by Fujii Masao in http://www.postgresql.org/message-id/CAHGQGwFhJ3CnHo1CELEfay18yg_RA-XZT-7D8NuWUoYSZ90r4Q@mail.gmail.com Author: Petr Jelínek
-
Alvaro Herrera authored
ALTER DEFAULT PRIVILEGES was trying to decode the list of roles in the FOR clause as a list of names rather than of RoleSpecs; and the IN clause in CREATE ROLE was doing the same thing. This was evidenced by crashes on some buildfarm machines, though on my platform this doesn't cause a failure by mere chance; I can reproduce the failures only by adding some padding in struct RoleSpecs. Fix by dereferencing those lists as being of RoleSpecs, not string Values.
-
Alvaro Herrera authored
Commands such as ALTER USER, ALTER GROUP, ALTER ROLE, GRANT, and the various ALTER OBJECT / OWNER TO, as well as ad-hoc clauses related to roles such as the AUTHORIZATION clause of CREATE SCHEMA, the FOR clause of CREATE USER MAPPING, and the FOR ROLE clause of ALTER DEFAULT PRIVILEGES can now take the keywords CURRENT_USER and SESSION_USER as user specifiers in place of an explicit user name. This commit also fixes some quite ugly handling of special standards- mandated syntax in CREATE USER MAPPING, which in particular would fail to work in presence of a role named "current_user". The special role specifiers PUBLIC and NONE also have more consistent handling now. Also take the opportunity to add location tracking to user specifiers. Authors: Kyotaro Horiguchi. Heavily reworked by Álvaro Herrera. Reviewed by: Rushabh Lathia, Adam Brightwell, Marti Raudsepp.
-
Michael Meskes authored
-
Michael Meskes authored
This reverts commit b9e538b1.
-
Robert Haas authored
Commit 5cefbf5a introduced an assumption that this field would always be non-NULL when doing a merge pass, but that's not true. Without this fix, you can crash the server by building a hash index that is sufficiently large relative to maintenance_work_mem, or by triggering a large datum sort. Commit 5ea86e6e changed the comments for that field to say that it would be set in all cases except for the hash index case, but that wasn't (and still isn't) true. The datum-sort failure was spotted by Tomas Vondra; initial analysis of that failure was by Peter Geoghegan. The remaining issues were spotted by me during review of the surrounding code, and the patch is all my fault.
-
Heikki Linnakangas authored
This makes it easier to write frontend programs that needs to understand the WAL record format of CREATE/DROP DATABASE. dbcommands.h cannot easily be #included in a frontend program, because it pulls in other header files that need backend stuff, but the new dbcommands_xlog.h header file has fewer dependencies.
-
Michael Meskes authored
Patch by Michael Paquier
-
Fujii Masao authored
-
Fujii Masao authored
Spotted by Andres Freund.
-
- 08 Mar, 2015 2 commits
-
-
Peter Eisentraut authored
The previous order appears to have been historically grown randomness.
-
Tom Lane authored
This is a possibly-vain effort to silence a Coverity warning about bogus endianness dependency. The code's fine, because it takes care of endianness issues for itself, but Coverity sees an int64 being passed to an int* argument and not unreasonably suspects something's wrong. I'm not sure if putting the void* cast in the way will shut it up; but it can't hurt and seems better from a documentation standpoint anyway, since the pointer is not used as an int* in this code path. Just for a bit of additional safety, verify that the result length is 8 bytes as expected. Back-patch to 9.3 where the code in question was added.
-