- 29 1月, 2015 2 次提交
-
-
由 Stephen Frost 提交于
Commit 804b6b6d added the build of a range table in copy.c to initialize the EState es_range_table since it can be needed in error paths. Unfortunately, that commit didn't appreciate that some code paths might end up not initializing the rte which is used to build the range table. Fix that and clean up a couple others things along the way- build it only once and don't explicitly set it on the !is_from path as it doesn't make any sense there (cstate is palloc0'd, so this isn't an issue from an initializing standpoint either). The prior commit went back to 9.0, but this only goes back to 9.1 as prior to that the range table build happens immediately after building the RTE and therefore doesn't suffer from this issue. Pointed out by Robert.
-
由 Stephen Frost 提交于
While building error messages to return to the user, BuildIndexValueDescription, ExecBuildSlotValueDescription and ri_ReportViolation would happily include the entire key or entire row in the result returned to the user, even if the user didn't have access to view all of the columns being included. Instead, include only those columns which the user is providing or which the user has select rights on. If the user does not have any rights to view the table or any of the columns involved then no detail is provided and a NULL value is returned from BuildIndexValueDescription and ExecBuildSlotValueDescription. Note that, for key cases, the user must have access to all of the columns for the key to be shown; a partial key will not be returned. Further, in master only, do not return any data for cases where row security is enabled on the relation and row security should be applied for the user. This required a bit of refactoring and moving of things around related to RLS- note the addition of utils/misc/rls.c. Back-patch all the way, as column-level privileges are now in all supported versions. This has been assigned CVE-2014-8161, but since the issue and the patch have already been publicized on pgsql-hackers, there's no point in trying to hide this commit.
-
- 28 1月, 2015 3 次提交
-
-
由 Heikki Linnakangas 提交于
-
由 Heikki Linnakangas 提交于
gist_poly_compress() and gist_circle_compress() checked for a NULL-pointer key argument, but that was dead code; the gist code never passes a NULL-pointer to the "compress" method. This commit also removes a documentation note added in commit a0a3883d, about doing NULL-pointer checks in the "compress" method. It was added based on the fact that some implementations were doing NULL-pointer checks, but those checks were unnecessary in the first place. The NULL-pointer check in gbt_var_same() function was also unnecessary. The arguments to the "same" method come from the "compress", "union", or "picksplit" methods, but none of them return a NULL pointer. None of this is to be confused with SQL NULL values. Those are dealt with by the gist machinery, and are never passed to the GiST opclass methods. Michael Paquier
-
由 Tom Lane 提交于
Commit 14534353 arranged to store numeric NaN values as short-header numerics, but the field access macros did not get the memo: they thought only "SHORT" numerics have short headers. Most of the time this makes no difference because we don't access the weight or dscale of a NaN; but numeric_send does that. As pointed out by Andrew Gierth, this led to fetching uninitialized bytes. AFAICS this could not have any worse consequences than that; in particular, an unaligned stored numeric would have been detoasted by PG_GETARG_NUMERIC, so that there's no risk of a fetch off the end of memory. Still, the code is wrong on its own terms, and it's not hard to foresee future changes that might expose us to real risks. So back-patch to all affected branches.
-
- 27 1月, 2015 5 次提交
-
-
由 Tom Lane 提交于
We had better memorialize what the actual requirements are for this.
-
由 Tom Lane 提交于
Some fields of the sinfo struct are modified within PG_TRY and then referenced within PG_CATCH, so as with recent patch to async.c, "volatile" is necessary for strict POSIX compliance; and that propagates to a couple of subroutines as well as materializeQueryResult() itself. I think the risk of actual issues here is probably higher than in async.c, because storeQueryResult() is likely to get inlined into materializeQueryResult(), leaving the compiler free to conclude that its stores into sinfo fields are dead code.
-
由 Robert Haas 提交于
Commit 1be4eb1b disabled this, but I think the real problem here was fixed by commit b181a919 and commit d060e07f. So let's try re-enabling it now and see what happens.
-
由 Tom Lane 提交于
The "callargs" variable is modified within PG_TRY and then referenced within PG_CATCH, which is exactly the coding pattern we've now found to be unsafe. Marking "callargs" volatile would be problematic because it is passed by reference to some Tcl functions, so fix the problem by not modifying it within PG_TRY. We can just postpone the free() till we exit the PG_TRY construct, as is already done elsewhere in this same file. Also, fix failure to free(callargs) when exiting on too-many-arguments error. This is only a minor memory leak, but a leak nonetheless. In passing, remove some unnecessary "volatile" markings in the same function. Those doubtless are there because gcc 2.95.3 whinged about them, but we now know that its algorithm for complaining is many bricks shy of a load. This is certainly a live bug with compilers that optimize similarly to current gcc, so back-patch to all active branches.
-
由 Tom Lane 提交于
The "pos" variable is modified within PG_TRY and then referenced within PG_CATCH, so for strict POSIX conformance it must be marked volatile. Superficially the code looked safe because pos's address was taken, which was sufficient to force it into memory ... but it's not sufficient to ensure that the compiler applies updates exactly where the program text says to. The volatility marking has to extend into a couple of subroutines too, but I think that's probably a good thing because the risk of out-of-order updates is mostly in those subroutines not asyncQueueReadAllNotifications() itself. In principle the compiler could have re-ordered operations such that an error could be thrown while "pos" had an incorrect value. It's unclear how real the risk is here, but for safety back-patch to all active branches.
-
- 26 1月, 2015 2 次提交
-
-
由 Tom Lane 提交于
On closer inspection, we can remove the "volatile" qualifier on "using_subtxn" so long as we initialize that before the PG_TRY block, which there's no particularly good reason not to do. Also, push the "change" variable inside the PG_TRY so as to remove all question of whether it needs "volatile", and remove useless early initializations of "snapshow_now" and "using_subtxn".
-
由 Tom Lane 提交于
Fix unsafe use of a non-volatile variable in PG_TRY/PG_CATCH in AlterSystemSetConfigFile(). While at it, clean up a bundle of other infelicities and outright bugs, including corner-case-incorrect linked list manipulation, a poorly designed and worse documented parse-and-validate function (which even included some randomly chosen hard-wired substitutes for the specified elevel in one code path ... wtf?), direct use of open() instead of fd.c's facilities, inadequate checking of write()'s return value, and generally poorly written commentary.
-
- 25 1月, 2015 4 次提交
-
-
由 Tom Lane 提交于
Fix unsafe coding around PG_TRY in RelationBuildRowSecurity: can't change a variable inside PG_TRY and then use it in PG_CATCH without marking it "volatile". In this case though it seems saner to avoid that by doing a single assignment before entering the TRY block. I started out just intending to fix that, but the more I looked at the row-security code the more distressed I got. This patch also fixes incorrect construction of the RowSecurityPolicy cache entries (there was not sufficient care taken to copy pass-by-ref data into the cache memory context) and a whole bunch of sloppiness around the definition and use of pg_policy.polcmd. You can't use nulls in that column because initdb will mark it NOT NULL --- and I see no particular reason why a null entry would be a good idea anyway, so changing initdb's behavior is not the right answer. The internal value of '\0' wouldn't be suitable in a "char" column either, so after a bit of thought I settled on using '*' to represent ALL. Chasing those changes down also revealed that somebody wasn't paying attention to what the underlying values of ACL_UPDATE_CHR etc really were, and there was a great deal of lackadaiscalness in the catalogs.sgml documentation for pg_policy and pg_policies too. This doesn't pretend to be a complete code review for the row-security stuff, it just fixes the things that were in my face while dealing with the bugs in RelationBuildRowSecurity.
-
由 Tom Lane 提交于
"iterstate" must be marked volatile since it's changed inside the PG_TRY block and then used in the PG_CATCH stanza. Noted by Mark Wilding of Salesforce. (We really need to see if we can't get the C compiler to warn about this.) Also, reset iterstate to NULL after the mainline ReorderBufferIterTXNFinish call, to ensure the PG_CATCH block doesn't try to do that a second time.
-
由 Tom Lane 提交于
strncpy() has a well-deserved reputation for being unsafe, so make an effort to get rid of nearly all occurrences in HEAD. A large fraction of the remaining uses were passing length less than or equal to the known strlen() of the source, in which case no null-padding can occur and the behavior is equivalent to memcpy(), though doubtless slower and certainly harder to reason about. So just use memcpy() in these cases. In other cases, use either StrNCpy() or strlcpy() as appropriate (depending on whether padding to the full length of the destination buffer seems useful). I left a few strncpy() calls alone in the src/timezone/ code, to keep it in sync with upstream (the IANA tzcode distribution). There are also a few such calls in ecpg that could possibly do with more analysis. AFAICT, none of these changes are more than cosmetic, except for the four occurrences in fe-secure-openssl.c, which are in fact buggy: an overlength source leads to a non-null-terminated destination buffer and ensuing misbehavior. These don't seem like security issues, first because no stack clobber is possible and second because if your values of sslcert etc are coming from untrusted sources then you've got problems way worse than this. Still, it's undesirable to have unpredictable behavior for overlength inputs, so back-patch those four changes to all active branches.
-
由 Tom Lane 提交于
This file hasn't been part of any build since 2005, and even before that wasn't used unless you configured --with-krb4 (and had a machine without gethostname(2), obviously). What's more, we haven't actually called gethostname anywhere since then, either (except in thread_test.c, whose testing of this function is probably pointless). So we don't need it.
-
- 24 1月, 2015 5 次提交
-
-
由 Alvaro Herrera 提交于
Pointed out by Michael Paquier
-
由 Robert Haas 提交于
Peter Geoghegan
-
由 Alvaro Herrera 提交于
This mode allows vacuumdb to open several server connections to vacuum or analyze several tables simultaneously. Author: Dilip Kumar. Some reworking by Álvaro Herrera Reviewed by: Jeff Janes, Amit Kapila, Magnus Hagander, Andres Freund
-
由 Robert Haas 提交于
When we write tuples out to disk and read them back in, the abbreviated keys become non-abbreviated, because the readtup routines don't know anything about abbreviation. But without this fix, the rest of the code still thinks the abbreviation-aware compartor should be used, so chaos ensues. Report by Andrew Gierth; patch by Peter Geoghegan.
-
由 Robert Haas 提交于
MSVC generates a warning here; we hope this will make it happy. Report by Michael Paquier. Patch by David Rowley.
-
- 23 1月, 2015 7 次提交
-
-
由 Tom Lane 提交于
contrib/pg_stat_statements will sometimes run the core lexer a second time on submitted statements. Formerly, if you had standard_conforming_strings turned off, this led to sometimes getting two copies of any warnings enabled by escape_string_warning. While this is probably no longer a big deal in the field, it's a pain for regression testing. To fix, change the lexer so it doesn't consult the escape_string_warning GUC variable directly, but looks at a copy in the core_yy_extra_type state struct. Then, pg_stat_statements can change that copy to disable warnings while it's redoing the lexing. It seemed like a good idea to make this happen for all three of the GUCs consulted by the lexer, not just escape_string_warning. There's not an immediate use-case for callers to adjust the other two AFAIK, but making it possible is easy enough and seems like good future-proofing. Arguably this is a bug fix, but there doesn't seem to be enough interest to justify a back-patch. We'd not be able to back-patch exactly as-is anyway, for fear of breaking ABI compatibility of the struct. (We could perhaps back-patch the addition of only escape_string_warning by adding it at the end of the struct, where there's currently alignment padding space.)
-
由 Peter Eisentraut 提交于
-
由 Alvaro Herrera 提交于
In the union support proc, we were not checking the hasnulls flag of value A early enough, so it could be skipped if the "allnulls" flag in value B is set. Also, a check on the allnulls flag of value "B" was redundant, so remove it. Also change inet_minmax_ops to not be the default opclass for type inet, as a future inclusion operator class would be more useful and it's pretty difficult to change default opclass for a datatype later on. (There is no catversion bump for this catalog change; this shouldn't be a problem.) Extracted from a larger patch to add an "inclusion" operator class. Author: Emre Hasegeli
-
由 Bruce Momjian 提交于
Clarify the meaning of libpq return values for PQputCopyData and PQputCopyEnd, particularly in non-blocking mode. Report by Robert Haas
-
由 Robert Haas 提交于
The split between which things need to happen in the C-locale case and which needed to happen in the locale-aware case was a few bricks short of a load. Try to fix that.
-
由 Bruce Momjian 提交于
When REASSIGN and ALTER OWNER TO are used, both the object owner and ACL list should be changed from the old owner to the new owner. This patch fixes types, foreign data wrappers, and foreign servers to change their ACL list properly; they already changed owners properly. BACKWARD INCOMPATIBILITY? Report by Alexey Bashtanov
-
由 Robert Haas 提交于
First, when LC_COLLATE = C, bttext_abbrev_convert should use memcpy() rather than strxfrm() to construct the abbreviated key, because the authoritative comparator uses memcpy(). If we do anything else here, we might get inconsistent answers, and the buildfarm says this risk is not theoretical. It should be faster this way, too. Second, while I'm looking at bttext_abbrev_convert, convert a needless use of goto into the loop it's trying to implement into an actual loop. Both of the above problems date to the original commit of abbreviated keys, commit 4ea51cdf. Third, fix a bogus assignment to tss->locale before tss is set up. That's a new goof in commit b529b65d.
-
- 22 1月, 2015 2 次提交
-
-
由 Robert Haas 提交于
Prior to commit 4ea51cdf, this function only had one job, which was to decide whether we could avoid trampolining through the fmgr layer when performing sort comparisons. As of that commit, it has a second job, which is to decide whether we can use abbreviated keys. Unfortunately, those two tasks are somewhat intertwined in the existing coding, which is likely why neither Peter Geoghegan nor I noticed prior to commit that this calls pg_newlocale_from_collation() in cases where it didn't previously. The buildfarm noticed, though. To fix, rewrite the logic so that the decision as to which comparator to use is more cleanly separated from the decision about abbreviation.
-
由 Alvaro Herrera 提交于
Author: Sawada Masahiko
-
- 21 1月, 2015 2 次提交
-
-
由 Robert Haas 提交于
Most of the Windows buildfarm members (bowerbird, hamerkop, currawong, jacana, brolga) are unhappy with yesterday's abbreviated keys patch, although there are some (narwhal, frogmouth) that seem OK with it. Since there's no obvious pattern to explain why some are working and others are failing, just disable this across-the-board on Windows for now. This is a bit unfortunate since the optimization will be a big win in some cases, but we can't leave the buildfarm broken.
-
由 Bruce Momjian 提交于
This dumps the predefined preprocessor macros
-
- 20 1月, 2015 6 次提交
-
-
由 Robert Haas 提交于
Per buildfarm failure on bowerbird after last night's commit 4ea51cdf. Peter Geoghegan
-
由 Tom Lane 提交于
This results in a very substantial reduction in disk space usage during "make check-world", since that sequence involves creation of numerous temporary installations. It should also help a bit in the buildfarm, even though the buildfarm script doesn't create as many temp installations, because the current script misses deleting some of them; and anyway it seems better to do this once in one place rather than expecting that script to get it right every time. In 9.4 and HEAD, also undo the unwise choice in commit b1aebbb6 to report strerror(errno) after a rmtree() failure. rmtree has already reported that, possibly for multiple failures with distinct errnos; and what's more, by the time it returns there is no good reason to assume that errno still reflects the last reportable error. So reporting errno here is at best redundant and at worst badly misleading. Back-patch to all supported branches, so that future revisions of the buildfarm script can rely on this behavior.
-
由 Tom Lane 提交于
Per discussion, change the log level of this message to be LOG not WARNING. The main point of this change is to avoid causing buildfarm run failures when the stats collector is exceptionally slow to respond, which it not infrequently is on some of the smaller/slower buildfarm members. This change does lose notice to an interactive user when his stats query is looking at out-of-date stats, but the majority opinion (not necessarily that of yours truly) is that WARNING messages would probably not get noticed anyway on heavily loaded production systems. A LOG message at least ensures that the problem is recorded somewhere where bulk auditing for the issue is possible. Also, instead of an untranslated "pgstat wait timeout" message, provide a translatable and hopefully more understandable message "using stale statistics instead of current ones because stats collector is not responding". The original text was written hastily under the assumption that it would never really happen in practice, which we now know to be unduly optimistic. Back-patch to all active branches, since we've seen the buildfarm issue in all branches.
-
由 Andres Freund 提交于
As noted by Tom Lane the improvements in 4b4b680c had the problem that in some situations we searched, entered and modified entries in the private refcount hash while holding a spinlock. I had tried to keep the logic entirely local to PinBuffer_Locked(), but that's not really possible given it's called with a spinlock held... Besides being disadvantageous from a performance point of view, this also has problems with error handling safety. If we failed inserting an entry into the hashtable due to an out of memory error, we'd error out with a held spinlock. Not good. Change the way private refcounts are manipulated: Before a buffer can be tracked an entry has to be reserved using ReservePrivateRefCountEntry(); then, if a entry is not found using GetPrivateRefCountEntry(), it can be entered with NewPrivateRefCountEntry(). Also take advantage of the fact that PinBuffer_Locked() currently is never called for buffers that already have been pinned by the current backend and don't search the private refcount entries for preexisting local pins. That results in a small, but measurable, performance improvement. Additionally make ReleaseBuffer() always call UnpinBuffer() for shared buffers. That avoids duplicating work in an eventual UnpinBuffer() call that already has been done in ReleaseBuffer() and also saves some code. Per discussion with Tom Lane. Discussion: 15028.1418772313@sss.pgh.pa.us
-
由 Robert Haas 提交于
This commit extends the SortSupport infrastructure to allow operator classes the option to provide abbreviated representations of Datums; in the case of text, we abbreviate by taking the first few characters of the strxfrm() blob. If the abbreviated comparison is insufficent to resolve the comparison, we fall back on the normal comparator. This can be much faster than the old way of doing sorting if the first few bytes of the string are usually sufficient to resolve the comparison. There is the potential for a performance regression if all of the strings to be sorted are identical for the first 8+ characters and differ only in later positions; therefore, the SortSupport machinery now provides an infrastructure to abort the use of abbreviation if it appears that abbreviation is producing comparatively few distinct keys. HyperLogLog, a streaming cardinality estimator, is included in this commit and used to make that determination for text. Peter Geoghegan, reviewed by me.
-
由 Robert Haas 提交于
Etsuro Fujita
-
- 19 1月, 2015 2 次提交
-
-
由 Alvaro Herrera 提交于
Pointed out by Alan Mogi (bug #12571)
-
由 Robert Haas 提交于
Amit Langote
-