aboutsummaryrefslogtreecommitdiff
path: root/src/backend/utils/adt
Commit message (Collapse)AuthorAge
...
* Fix yet another corner case in dumping rules/views with USING clauses.Tom Lane2014-05-01
| | | | | | | | | | | | | | | | ruleutils.c tries to cope with additions/deletions/renamings of columns in tables referenced by views, by means of adding machine-generated aliases to the printed form of a view when needed to preserve the original semantics. A recent blog post by Marko Tiikkaja pointed out a case I'd missed though: if one input of a join with USING is itself a join, there is nothing to stop the user from adding a column of the same name as the USING column to whichever side of the sub-join didn't provide the USING column. And then there'll be an error when the view is re-parsed, since now the sub-join exposes two columns matching the USING specification. We were catching a lot of related cases, but not this one, so add some logic to cope with it. Back-patch to 9.3, which is the first release that makes any serious attempt to cope with such cases (cf commit 2ffa740be and follow-ons).
* Fix failure to detoast fields in composite elements of structured types.Tom Lane2014-05-01
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | If we have an array of records stored on disk, the individual record fields cannot contain out-of-line TOAST pointers: the tuptoaster.c mechanisms are only prepared to deal with TOAST pointers appearing in top-level fields of a stored row. The same applies for ranges over composite types, nested composites, etc. However, the existing code only took care of expanding sub-field TOAST pointers for the case of nested composites, not for other structured types containing composites. For example, given a command such as UPDATE tab SET arraycol = ARRAY[(ROW(x,42)::mycompositetype] ... where x is a direct reference to a field of an on-disk tuple, if that field is long enough to be toasted out-of-line then the TOAST pointer would be inserted as-is into the array column. If the source record for x is later deleted, the array field value would become a dangling pointer, leading to errors along the line of "missing chunk number 0 for toast value ..." when the value is referenced. A reproducible test case for this was provided by Jan Pecek, but it seems likely that some of the "missing chunk number" reports we've heard in the past were caused by similar issues. Code-wise, the problem is that PG_DETOAST_DATUM() is not adequate to produce a self-contained Datum value if the Datum is of composite type. Seen in this light, the problem is not just confined to arrays and ranges, but could also affect some other places where detoasting is done in that way, for example form_index_tuple(). I tried teaching the array code to apply toast_flatten_tuple_attribute() along with PG_DETOAST_DATUM() when the array element type is composite, but this was messy and imposed extra cache lookup costs whether or not any TOAST pointers were present, indeed sometimes when the array element type isn't even composite (since sometimes it takes a typcache lookup to find that out). The idea of extending that approach to all the places that currently use PG_DETOAST_DATUM() wasn't attractive at all. This patch instead solves the problem by decreeing that composite Datum values must not contain any out-of-line TOAST pointers in the first place; that is, we expand out-of-line fields at the point of constructing a composite Datum, not at the point where we're about to insert it into a larger tuple. This rule is applied only to true composite Datums, not to tuples that are being passed around the system as tuples, so it's not as invasive as it might sound at first. With this approach, the amount of code that has to be touched for a full solution is greatly reduced, and added cache lookup costs are avoided except when there actually is a TOAST pointer that needs to be inlined. The main drawback of this approach is that we might sometimes dereference a TOAST pointer that will never actually be used by the query, imposing a rather large cost that wasn't there before. On the other side of the coin, if the field value is used multiple times then we'll come out ahead by avoiding repeat detoastings. Experimentation suggests that common SQL coding patterns are unaffected either way, though. Applications that are very negatively affected could be advised to modify their code to not fetch columns they won't be using. In future, we might consider reverting this solution in favor of detoasting only at the point where data is about to be stored to disk, using some method that can drill down into multiple levels of nested structured types. That will require defining new APIs for structured types, though, so it doesn't seem feasible as a back-patchable fix. Note that this patch changes HeapTupleGetDatum() from a macro to a function call; this means that any third-party code using that macro will not get protection against creating TOAST-pointer-containing Datums until it's recompiled. The same applies to any uses of PG_RETURN_HEAPTUPLEHEADER(). It seems likely that this is not a big problem in practice: most of the tuple-returning functions in core and contrib produce outputs that could not possibly be toasted anyway, and the same probably holds for third-party extensions. This bug has existed since TOAST was invented, so back-patch to all supported branches.
* Rationalize common/relpath.[hc].Tom Lane2014-04-30
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Commit a73018392636ce832b09b5c31f6ad1f18a4643ea created rather a mess by putting dependencies on backend-only include files into include/common. We really shouldn't do that. To clean it up: * Move TABLESPACE_VERSION_DIRECTORY back to its longtime home in catalog/catalog.h. We won't consider this symbol part of the FE/BE API. * Push enum ForkNumber from relfilenode.h into relpath.h. We'll consider relpath.h as the source of truth for fork numbers, since relpath.c was already partially serving that function, and anyway relfilenode.h was kind of a random place for that enum. * So, relfilenode.h now includes relpath.h rather than vice-versa. This direction of dependency is fine. (That allows most, but not quite all, of the existing explicit #includes of relpath.h to go away again.) * Push forkname_to_number from catalog.c to relpath.c, just to centralize fork number stuff a bit better. * Push GetDatabasePath from catalog.c to relpath.c; it was rather odd that the previous commit didn't keep this together with relpath(). * To avoid needing relfilenode.h in common/, redefine the underlying function (now called GetRelationPath) as taking separate OID arguments, and make the APIs using RelFileNode or RelFileNodeBackend into macro wrappers. (The macros have a potential multiple-eval risk, but none of the existing call sites have an issue with that; one of them had such a risk already anyway.) * Fix failure to follow the directions when "init" fork type was added; specifically, the errhint in forkname_to_number wasn't updated, and neither was the SGML documentation for pg_relation_size(). * Fix tablespace-path-too-long check in CreateTableSpace() to account for fork-name component of maximum-length pathnames. This requires putting FORKNAMECHARS into a header file, but it was rather useless (and actually unreferenced) where it was. The last couple of items are potentially back-patchable bug fixes, if anyone is sufficiently excited about them; but personally I'm not. Per a gripe from Christoph Berg about how include/common wasn't self-contained.
* Check for interrupts and stack overflow during rule/view dumps.Tom Lane2014-04-30
| | | | | | | | | Since ruleutils.c recurses, it could be driven to stack overflow by deeply nested constructs. Very large queries might also take long enough to deparse that a check for interrupts seems like a good idea. Stick appropriate tests into a couple of key places. Noted by Greg Stark. Back-patch to all supported branches.
* Reduce indentation/parenthesization of set operations in rule/view dumps.Tom Lane2014-04-30
| | | | | | | | | | | | | | | | | | | | | | | | | A query such as "SELECT x UNION SELECT y UNION SELECT z UNION ..." produces a left-deep nested parse tree, which we formerly showed in its full nested glory and with all the possible parentheses. This does little for readability, though, and long UNION lists resulting in excessive indentation are common. Instead, let's omit parentheses and indent all the subqueries at the same level in such cases. This patch skips indentation/parenthesization whenever the lefthand input of a SetOperationStmt is another SetOperationStmt of the same kind and ALL/DISTINCT property. We could teach the code the exact syntactic precedence of set operations and thereby avoid parenthesization in some more cases, but it's not clear that that'd be a readability win: it seems better to parenthesize if the set operation changes. (As an example, if there's one UNION in a long list of UNION ALL, it now stands out like a sore thumb, which seems like a good thing.) Back-patch to 9.3. This completes our response to a complaint from Greg Stark that since commit 62e666400d there's a performance problem in pg_dump for views containing long UNION sequences (or other types of deeply nested constructs). The previous commit 0601cb54dac14d979d726ab2ebeda251ae36e857 handles the general problem, but this one makes the specific case of UNION lists look a lot nicer.
* Limit overall indentation in rule/view dumps.Tom Lane2014-04-30
| | | | | | | | | | | | | | | | | Continuing to indent no matter how deeply nested we get doesn't really do anything for readability; what's worse, it results in O(N^2) total whitespace, which can become a performance and memory-consumption issue. To address this, once we get past 40 characters of indentation, reduce the indentation step distance 4x, and also limit the maximum indentation by reducing it modulo 40. This latter choice is a bit weird at first glance, but it seems to preserve readability better than a simple cap would do. Back-patch to 9.3, because since commit 62e666400d the performance issue is a hazard for pg_dump. Greg Stark and Tom Lane
* Fix indentation of JOIN clauses in rule/view dumps.Tom Lane2014-04-30
| | | | | | | | | | | | | | | | | | | | The code attempted to outdent JOIN clauses further left than the parent FROM keyword, which was odd in any case, and led to inconsistent formatting since in simple cases the clauses couldn't be moved any further left than that. And it left a permanent decrement of the indentation level, causing subsequent lines to be much further left than they should be (again, this couldn't be seen in simple cases for lack of indentation to give up). After a little experimentation I chose to make it indent JOIN keywords two spaces from the parent FROM, which is one space more than the join's lefthand input in cases where that appears on a different line from FROM. Back-patch to 9.3. This is a purely cosmetic change, and the bug is quite old, so that may seem arbitrary; but we are going to be making some other changes to the indentation behavior in both HEAD and 9.3, so it seems reasonable to include this in 9.3 too. I committed this one first because its effects are more visible in the regression test results as they currently stand than they will be later.
* Add to_regprocedure() and to_regoperator().Robert Haas2014-04-16
| | | | | | | | | These are natural complements to the functions added by commit 0886fc6a5c75b294544263ea979b9cf6195407d9, but they weren't included in the original patch for some reason. Add them. Patch by me, per a complaint by Tom Lane. Review by Tatsuo Ishii.
* Provide moving-aggregate support for boolean aggregates.Tom Lane2014-04-13
| | | | David Rowley and Florian Pflug, reviewed by Dean Rasheed
* Provide moving-aggregate support for a bunch of numerical aggregates.Tom Lane2014-04-12
| | | | | | | | | | | | | First installment of the promised moving-aggregate support in built-in aggregates: count(), sum(), avg(), stddev() and variance() for assorted datatypes, though not for float4/float8. In passing, remove a 2001-vintage kluge in interval_accum(): interval array elements have been properly aligned since around 2003, but nobody remembered to take out this workaround. Also, fix a thinko in the opr_sanity tests for moving-aggregate catalog entries. David Rowley and Florian Pflug, reviewed by Dean Rasheed
* Add an in-core GiST index opclass for inet/cidr types.Tom Lane2014-04-08
| | | | | | | | | | | | | | | | | | | | | | This operator class can accelerate subnet/supernet tests as well as btree-equivalent ordered comparisons. It also handles a new network operator inet && inet (overlaps, a/k/a "is supernet or subnet of"), which is expected to be useful in exclusion constraints. Ideally this opclass would be the default for GiST with inet/cidr data, but we can't mark it that way until we figure out how to do a more or less graceful transition from the current situation, in which the really-completely-bogus inet/cidr opclasses in contrib/btree_gist are marked as default. Having the opclass in core and not default is better than not having it at all, though. While at it, add new documentation sections to allow us to officially document GiST/GIN/SP-GiST opclasses, something there was never a clear place to do before. I filled these in with some simple tables listing the existing opclasses and the operators they support, but there's certainly scope to put more information there. Emre Hasegeli, reviewed by Andreas Karlsson, further hacking by me
* Add new to_reg* functions for error-free OID lookups.Robert Haas2014-04-08
| | | | | | | | | These functions won't throw an error if the object doesn't exist, or if (for functions and operators) there's more than one matching object. Yugo Nagata and Nozomi Anzai, reviewed by Amit Khandekar, Marti Raudsepp, Amit Kapila, and me.
* Reduce lock levels of some ALTER TABLE cmdsSimon Riggs2014-04-06
| | | | | | | | | | | | | | | | | VALIDATE CONSTRAINT CLUSTER ON SET WITHOUT CLUSTER ALTER COLUMN SET STATISTICS ALTER COLUMN SET () ALTER COLUMN RESET () All other sub-commands use AccessExclusiveLock Simon Riggs and Noah Misch Reviews by Robert Haas and Andres Freund
* Fix non-equivalence of VARIADIC and non-VARIADIC function call formats.Tom Lane2014-04-03
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | For variadic functions (other than VARIADIC ANY), the syntaxes foo(x,y,...) and foo(VARIADIC ARRAY[x,y,...]) should be considered equivalent, since the former is converted to the latter at parse time. They have indeed been equivalent, in all releases before 9.3. However, commit 75b39e790 made an ill-considered decision to record which syntax had been used in FuncExpr nodes, and then to make equal() test that in checking node equality --- which caused the syntaxes to not be seen as equivalent by the planner. This is the underlying cause of bug #9817 from Dmitry Ryabov. It might seem that a quick fix would be to make equal() disregard FuncExpr.funcvariadic, but the same commit made that untenable, because the field actually *is* semantically significant for some VARIADIC ANY functions. This patch instead adopts the approach of redefining funcvariadic (and aggvariadic, in HEAD) as meaning that the last argument is a variadic array, whether it got that way by parser intervention or was supplied explicitly by the user. Therefore the value will always be true for non-ANY variadic functions, restoring the principle of equivalence. (However, the planner will continue to consider use of VARIADIC as a meaningful difference for VARIADIC ANY functions, even though some such functions might disregard it.) In HEAD, this change lets us simplify the decompilation logic in ruleutils.c, since the funcvariadic/aggvariadic flag tells directly whether to print VARIADIC. However, in 9.3 we have to continue to cope with existing stored rules/views that might contain the previous definition. Fortunately, this just means no change in ruleutils.c, since its existing behavior effectively ignores funcvariadic for all cases other than VARIADIC ANY functions. In HEAD, bump catversion to reflect the fact that FuncExpr.funcvariadic changed meanings; this is sort of pro forma, since I don't believe any built-in views are affected. Unfortunately, this patch doesn't magically fix everything for affected 9.3 users. After installing 9.3.5, they might need to recreate their rules/views/indexes containing variadic function calls in order to get everything consistent with the new definition. As in the cited bug, the symptom of a problem would be failure to use a nominally matching index that has a variadic function call in its definition. We'll need to mention this in the 9.3.5 release notes.
* Code review for commit d26888bc4d1e539a82f21382b0000fe5bbf889d9.Tom Lane2014-04-03
| | | | | Mostly, copy-edit the comments; but also fix it to not reject domains over arrays.
* De-anonymize the union in JsonbValue.Tom Lane2014-04-02
| | | | Needed for strict C89 compliance.
* Fix bugs in manipulation of PgBackendStatus.st_clienthostname.Tom Lane2014-04-01
| | | | | | | | | | Initialization of this field was not being done according to the st_changecount protocol (it has to be done within the changecount increment range, not outside). And the test to see if the value should be reported as null was wrong. Noted while perusing uses of Port.remote_hostname. This was wrong from the introduction of this code (commit 4a25bc145), so back-patch to 9.1.
* Rename GinLogicValue to GinTernaryValue.Heikki Linnakangas2014-03-31
| | | | | It's more descriptive. Also, get rid of the enum, and use #defines instead, per Greg Stark's suggestion.
* Silence compiler warnings in new jsonb code.Heikki Linnakangas2014-03-27
| | | | Amit Kapila.
* Fix uninitialized variables in json's populate_record_worker().Andrew Dunstan2014-03-26
| | | | Peter Geoghegan.
* Cleanup around json_to_record/json_to_recordsetAndrew Dunstan2014-03-26
| | | | | | | | | Set function parameter names and defaults. Add jsonb versions (which the code already provided for so the actual new code is trivial). Add jsonb regression tests and docs. Bump catalog version (which I apparently forgot to do when jsonb was committed).
* Fix 'recheck' flag in tsquery's GIN tri-consistent function.Heikki Linnakangas2014-03-26
| | | | | | | It needs to be initialized, like in the boolean gin_tsquery_consistent version. Peter Geoghegan.
* Tidy up the populate/to_record{set} code for json a bit.Andrew Dunstan2014-03-25
| | | | In the process fix a small bug.
* Fix ts_rank_cd() to ignore stripped lexemesBruce Momjian2014-03-24
| | | | | | | Previously, stripped lexemes got a default location and could be considered if mixed with non-stripped lexemes. BACKWARD INCOMPATIBILITY CHANGE
* Introduce jsonb, a structured format for storing json.Andrew Dunstan2014-03-23
| | | | | | | | | | | | | | | | | | | | | | The new format accepts exactly the same data as the json type. However, it is stored in a format that does not require reparsing the orgiginal text in order to process it, making it much more suitable for indexing and other operations. Insignificant whitespace is discarded, and the order of object keys is not preserved. Neither are duplicate object keys kept - the later value for a given key is the only one stored. The new type has all the functions and operators that the json type has, with the exception of the json generation functions (to_json, json_agg etc.) and with identical semantics. In addition, there are operator classes for hash and btree indexing, and two classes for GIN indexing, that have no equivalent in the json type. This feature grew out of previous work by Oleg Bartunov and Teodor Sigaev, which was intended to provide similar facilities to a nested hstore type, but which in the end proved to have some significant compatibility issues. Authors: Oleg Bartunov, Teodor Sigaev, Peter Geoghegan and Andrew Dunstan. Review: Andres Freund
* Fix typos in comments.Fujii Masao2014-03-17
| | | | Thom Brown
* C comments: remove odd blank lines after #ifdef WIN32 linesBruce Momjian2014-03-13
|
* Items on GIN data pages are no longer always 6 bytes; update gincostestimate.Heikki Linnakangas2014-03-12
| | | | Also improve the comments a bit.
* Allow opclasses to provide tri-valued GIN consistent functions.Heikki Linnakangas2014-03-12
| | | | | | | | | | | | | | | With the GIN "fast scan" feature, GIN can skip items without fetching all the keys for them, if it can prove that they don't match regardless of those keys. So far, it has done the proving by calling the boolean consistent function with all combinations of TRUE/FALSE for the unfetched keys, but since that's O(n^2), it becomes unfeasible with more than a few keys. We can avoid calling consistent with all the combinations, if we can tell the operator class implementation directly which keys are unknown. This commit includes a triConsistent function for the built-in array and tsvector opclasses. Alexander Korotkov, with some changes by me.
* Avoid memcpy() with same source and destination address.Heikki Linnakangas2014-03-07
| | | | | | | The behavior of that is undefined, although unlikely to lead to problems in practice. Found by running regression tests with Valgrind.
* Avoid getting more than AccessShareLock when deparsing a query.Tom Lane2014-03-06
| | | | | | | | | | | | | | | | | | | | In make_ruledef and get_query_def, we have long used AcquireRewriteLocks to ensure that the querytree we are about to deparse is up-to-date and the schemas of the underlying relations aren't changing. Howwever, that function thinks the query is about to be executed, so it acquires locks that are stronger than necessary for the purpose of deparsing. Thus for example, if pg_dump asks to deparse a rule that includes "INSERT INTO t", we'd acquire RowExclusiveLock on t. That results in interference with concurrent transactions that might for example ask for ShareLock on t. Since pg_dump is documented as being purely read-only, this is unexpected. (Worse, it used to actually be read-only; this behavior dates back only to 8.1, cf commit ba4200246.) Fix this by adding a parameter to AcquireRewriteLocks to tell it whether we want the "real" execution locks or only AccessShareLock. Report, diagnosis, and patch by Dean Rasheed. Back-patch to all supported branches.
* isdigit() needs an unsigned char argument.Heikki Linnakangas2014-03-06
| | | | | | | Per the C standard, the routine should be passed an int, with a value that's representable as an unsigned char or EOF. Passing a signed char is wrong, because a negative value is not representable as an unsigned char. Unfortunately no compiler warns about that.
* Fix portability issues in recently added make_timestamp/make_interval code.Tom Lane2014-03-05
| | | | | | | | Explicitly reject infinity/NaN inputs, rather than just assuming that something else will do it for us. Per buildfarm. While at it, make some over-parenthesized and under-legible code more readable.
* Constructors for interval, timestamp, timestamptzAlvaro Herrera2014-03-04
| | | | | | Author: Pavel Stěhule, editorialized somewhat by Álvaro Herrera Reviewed-by: Tomáš Vondra, Marko Tiikkaja With input from Fabrízio de Royes Mello, Jim Nasby
* Another round of Coverity fixesStephen Frost2014-03-03
| | | | | | | | | | | | | | | | | | | | | | | Additional non-security issues/improvements spotted by Coverity. In backend/libpq, no sense trying to protect against port->hba being NULL after we've already dereferenced it in the switch() statement. Prevent against possible overflow due to 32bit arithmitic in basebackup throttling (not yet released, so no security concern). Remove nonsensical check of array pointer against NULL in procarray.c, looks to be a holdover from 9.1 and earlier when there were pointers being used but now it's just an array. Remove pointer check-against-NULL in tsearch/spell.c as we had already dereferenced it above (in the strcmp()). Remove dead code from adt/orderedsetaggs.c, isnull is checked immediately after each tuplesort_getdatum() call and if true we return, so no point checking it again down at the bottom. Remove recently added minor error-condition memory leak in pg_regress.
* Allow regex operations to be terminated early by query cancel requests.Tom Lane2014-03-01
| | | | | | | | | | | | | | | | | | | | | | | | | The regex code didn't have any provision for query cancel; which is unsurprising given its non-Postgres origin, but still problematic since some operations can take a long time. Introduce a callback function to check for a pending query cancel or session termination request, and call it in a couple of strategic spots where we can make the regex code exit with an error indicator. If we ever actually split out the regex code as a standalone library, some additional work will be needed to let the cancel callback function be specified externally to the library. But that's straightforward (certainly so by comparison to putting the locale-dependent character classification logic on a similar arms-length basis), and there seems no need to do it right now. A bigger issue is that there may be more places than these two where we need to check for cancels. We can always add more checks later, now that the infrastructure is in place. Since there are known examples of not-terribly-long regexes that can lock up a backend for a long time, back-patch to all supported branches. I have hopes of fixing the known performance problems later, but adding query cancel ability seems like a good idea even if they were all fixed.
* Fix crash in json_to_record().Jeff Davis2014-02-26
| | | | | | | | | | | | json_to_record() depends on get_call_result_type() for the tuple descriptor of the record that should be returned, but in some cases that cannot be determined. Add a guard to check if the tuple descriptor has been properly resolved, similar to other callers of get_call_result_type(). Also add guard for two other callers of get_call_result_type() in jsonfuncs.c. Although json_to_record() is the only actual bug, it's a good idea to follow convention.
* Use SnapshotDirty rather than an active snapshot to probe index endpoints.Tom Lane2014-02-25
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | If there are lots of uncommitted tuples at the end of the index range, get_actual_variable_range() ends up fetching each one and doing an MVCC visibility check on it, until it finally hits a visible tuple. This is bad enough in isolation, considering that we don't need an exact answer only an approximate one. But because the tuples are not yet committed, each visibility check does a TransactionIdIsInProgress() test, which involves scanning the ProcArray. When multiple sessions do this concurrently, the ensuing contention results in horrid performance loss. 20X overall throughput loss on not-too-complicated queries is easy to demonstrate in the back branches (though someone's made it noticeably less bad in HEAD). We can dodge the problem fairly effectively by using SnapshotDirty rather than a normal MVCC snapshot. This will cause the index probe to take uncommitted tuples as good, so that we incur only one tuple fetch and test even if there are many such tuples. The extent to which this degrades the estimate is debatable: it's possible the result is actually a more accurate prediction than before, if the endmost tuple has become committed by the time we actually execute the query being planned. In any case, it's not very likely that it makes the estimate a lot worse. SnapshotDirty will still reject tuples that are known committed dead, so we won't give bogus answers if an invalid outlier has been deleted but not yet vacuumed from the index. (Because btrees know how to mark such tuples dead in the index, we shouldn't have a big performance problem in the case that there are many of them at the end of the range.) This consideration motivates not using SnapshotAny, which was also considered as a fix. Note: the back branches were using SnapshotNow instead of an MVCC snapshot, but the problem and solution are the same. Per performance complaints from Bartlomiej Romanski, Josh Berkus, and others. Back-patch to 9.0, where the issue was introduced (by commit 40608e7f949fb7e4025c0ddd5be01939adc79eec).
* Show xid and xmin in pg_stat_activity and pg_stat_replication.Robert Haas2014-02-25
| | | | | Christian Kruse, reviewed by Andres Freund and myself, with further minor adjustments by me.
* Allow single-point polygons to be converted to circlesBruce Momjian2014-02-24
| | | | | | | This allows finding the center of a single-point polygon and converting it to a point. Per report from Josef Grahn
* docs: document behavior of CHAR() comparisons with chars < spaceBruce Momjian2014-02-24
| | | | | | | Space trimming rather than space-padding causes unusual behavior, which might not be standards-compliant. Also remove recently-added now-redundant C comment.
* Prefer pg_any_to_server/pg_server_to_any over pg_do_encoding_conversion.Tom Lane2014-02-23
| | | | | | | | | | | | | | | | | | | | A large majority of the callers of pg_do_encoding_conversion were specifying the database encoding as either source or target of the conversion, meaning that we can use the less general functions pg_any_to_server/pg_server_to_any instead. The main advantage of using the latter functions is that they can make use of a cached conversion-function lookup in the common case that the other encoding is the current client_encoding. It's notationally cleaner too in most cases, not least because of the historical artifact that the latter functions use "char *" rather than "unsigned char *" in their APIs. Note that pg_any_to_server will apply an encoding verification step in some cases where pg_do_encoding_conversion would have just done nothing. This seems to me to be a good idea at most of these call sites, though it partially negates the performance benefit. Per discussion of bug #9210.
* Do ScalarArrayOp estimation correctly when array is a stable expression.Tom Lane2014-02-21
| | | | | | | | | | | | | | Most estimation functions apply estimate_expression_value to see if they can reduce an expression to a constant; the key difference is that it allows evaluation of stable as well as immutable functions in hopes of ending up with a simple Const node. scalararraysel didn't get the memo though, and neither did gincost_opexpr/gincost_scalararrayopexpr. Fix that, and remove a now-unnecessary estimate_expression_value step in the subsidiary function scalararraysel_containment. Per complaint from Alexey Klyukin. Back-patch to 9.3. The problem goes back further, but I'm hesitant to change estimation behavior in long-stable release branches.
* Further code review for pg_lsn data type.Robert Haas2014-02-19
| | | | | | | | | Change input function error messages to be more consistent with what is done elsewhere. Remove a bunch of redundant type casts, so that the compiler will warn us if we screw up. Don't pass LSNs by value on platforms where a Datum is only 32 bytes, per buildfarm. Move macros for packing and unpacking LSNs to pg_lsn.h so that we can include access/xlogdefs.h, to avoid an unsatisfied dependency on XLogRecPtr.
* pg_lsn macro naming and type behavior revisions.Robert Haas2014-02-19
| | | | | Change pg_lsn_mi so that it can return negative values when subtracting LSNs, and clean up some perhaps ill-considered macro names.
* Add a pg_lsn data type, to represent an LSN.Robert Haas2014-02-19
| | | | Robert Haas and Michael Paquier
* Prevent potential overruns of fixed-size buffers.Tom Lane2014-02-17
| | | | | | | | | | | | | | | | | | | | | | | Coverity identified a number of places in which it couldn't prove that a string being copied into a fixed-size buffer would fit. We believe that most, perhaps all of these are in fact safe, or are copying data that is coming from a trusted source so that any overrun is not really a security issue. Nonetheless it seems prudent to forestall any risk by using strlcpy() and similar functions. Fixes by Peter Eisentraut and Jozef Mlich based on Coverity reports. In addition, fix a potential null-pointer-dereference crash in contrib/chkpass. The crypt(3) function is defined to return NULL on failure, but chkpass.c didn't check for that before using the result. The main practical case in which this could be an issue is if libc is configured to refuse to execute unapproved hashing algorithms (e.g., "FIPS mode"). This ideally should've been a separate commit, but since it touches code adjacent to one of the buffer overrun changes, I included it in this commit to avoid last-minute merge issues. This issue was reported by Honza Horak. Security: CVE-2014-0065 for buffer overruns, CVE-2014-0066 for crypt()
* Predict integer overflow to avoid buffer overruns.Noah Misch2014-02-17
| | | | | | | | | | | | | | | | | Several functions, mostly type input functions, calculated an allocation size such that the calculation wrapped to a small positive value when arguments implied a sufficiently-large requirement. Writes past the end of the inadvertent small allocation followed shortly thereafter. Coverity identified the path_in() vulnerability; code inspection led to the rest. In passing, add check_stack_depth() to prevent stack overflow in related functions. Back-patch to 8.4 (all supported versions). The non-comment hstore changes touch code that did not exist in 8.4, so that part stops at 9.0. Noah Misch and Heikki Linnakangas, reviewed by Tom Lane. Security: CVE-2014-0064
* Shore up ADMIN OPTION restrictions.Noah Misch2014-02-17
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | Granting a role without ADMIN OPTION is supposed to prevent the grantee from adding or removing members from the granted role. Issuing SET ROLE before the GRANT bypassed that, because the role itself had an implicit right to add or remove members. Plug that hole by recognizing that implicit right only when the session user matches the current role. Additionally, do not recognize it during a security-restricted operation or during execution of a SECURITY DEFINER function. The restriction on SECURITY DEFINER is not security-critical. However, it seems best for a user testing his own SECURITY DEFINER function to see the same behavior others will see. Back-patch to 8.4 (all supported versions). The SQL standards do not conflate roles and users as PostgreSQL does; only SQL roles have members, and only SQL users initiate sessions. An application using PostgreSQL users and roles as SQL users and roles will never attempt to grant membership in the role that is the session user, so the implicit right to add or remove members will never arise. The security impact was mostly that a role member could revoke access from others, contrary to the wishes of his own grantor. Unapproved role member additions are less notable, because the member can still largely achieve that by creating a view or a SECURITY DEFINER function. Reviewed by Andres Freund and Tom Lane. Reported, independently, by Jonas Sundman and Noah Misch. Security: CVE-2014-0060
* Add C comment about problems with CHAR() space trimmingBruce Momjian2014-02-13
|