2070 Commits (c0d0e540847d24d9dfe374549fb4cfd5ca40d050)

Author	SHA1	Message	Date
Tom Lane	1c53c4dec3	Finish reverting "recheck_on_update" patch. ... This reverts commit c203d6cf8 and some follow-on fixes, completing the task begun in commit 5d28c9bd7. If that feature is ever resurrected, the code will look quite a bit different from this, so it seems best to start from a clean slate. The v11 branch is not touched; in that branch, the recheck_on_update storage option remains present, but nonfunctional and undocumented. Discussion: https://postgr.es/m/20190114223409.3tcvejfhlvbucrv5@alap3.anarazel.de	7 years ago
Tom Lane	eaf0380ecc	Fix C++ compile failures in headers. ... Avoid using "typeid" as a parameter name in header files, since that is a C++ keyword. These cases were introduced recently, in 04fe805a1 and 586b98fdf. Since I'm an incurable neatnik, also rename these parameters in the underlying function definitions. That's not really necessary per project rules, but I don't like function declarations that don't quite agree with the underlying definitions. Per src/tools/pginclude/cpluspluscheck.	7 years ago
Tom Lane	8ff5f824dc	Reduce the size of the fmgr_builtin_oid_index[] array. ... This index array was originally defined to have 10000 entries (ranging up to FirstGenbkiObjectId), but we really only need entries up to the last existing builtin function OID, currently 6121. That saves close to 8K of never-accessed space in the server executable, at the small price of one more fetch in fmgr_isbuiltin(). We could reduce the array size still further by renumbering a few of the highest-numbered builtin functions; but there's a small risk of breaking clients that have chosen to hardwire those function OIDs, so it's not clear if it'd be worth the trouble. (We should, however, discourage future patches from choosing function OIDs above 6K as long as there's still lots of space below that.) Discussion: https://postgr.es/m/12359.1547063064@sss.pgh.pa.us	7 years ago
Alvaro Herrera	807ae415c5	Don't create relfilenode for relations without storage ... Some relation kinds had relfilenode set to some non-zero value, but apparently the actual files did not really exist because creation was prevented elsewhere. Get rid of the phony pg_class.relfilenode values. Catversion bumped, but only because the sanity_test check will fail if run in a system initdb'd with the previous version. Reviewed-by: Kyotaro HORIGUCHI, Michael Paquier Discussion: https://postgr.es/m/20181206215552.fm2ypuxq6nhpwjuc@alvherre.pgsql	7 years ago
Bruce Momjian	97c39498e5	Update copyright for 2019 ... Backpatch-through: certain files through 9.4	7 years ago
Michael Paquier	1707a0d2aa	Remove configure switch --disable-strong-random ... This removes a portion of infrastructure introduced by fe0a0b5 to allow compilation of Postgres in environments where no strong random source is available, meaning that there is no linking to OpenSSL and no /dev/urandom (Windows having its own CryptoAPI). No systems shipped this century lack /dev/urandom, and the buildfarm is actually not testing this switch at all, so just remove it. This simplifies particularly some backend code which included a fallback implementation using shared memory, and removes a set of alternate regression output files from pgcrypto. Author: Michael Paquier Reviewed-by: Tom Lane Discussion: https://postgr.es/m/20181230063219.GG608@paquier.xyz	7 years ago
Tom Lane	586b98fdf1	Make type "name" collation-aware. ... The "name" comparison operators now all support collations, making them functionally equivalent to "text" comparisons, except for the different physical representation of the datatype. They do, in fact, mostly share the varstr_cmp and varstr_sortsupport infrastructure, which has been slightly enlarged to handle the case. To avoid changes in the default behavior of the datatype, set name's typcollation to C_COLLATION_OID not DEFAULT_COLLATION_OID, so that by default comparisons to a name value will continue to use strcmp semantics. (This would have been the case for system catalog columns anyway, because of commit 6b0faf723, but doing this makes it true for user-created name columns as well. In particular, this avoids locale-dependent changes in our regression test results.) In consequence, tweak a couple of places that made assumptions about collatable base types always having typcollation DEFAULT_COLLATION_OID. I have not, however, attempted to relax the restriction that user- defined collatable types must have that. Hence, "name" doesn't behave quite like a user-defined type; it acts more like a domain with COLLATE "C". (Conceivably, if we ever get rid of the need for catalog name columns to be fixed-length, "name" could actually become such a domain over text. But that'd be a pretty massive undertaking, and I'm not volunteering.) Discussion: https://postgr.es/m/15938.1544377821@sss.pgh.pa.us	7 years ago
Tom Lane	5e09280057	Make pg_statistic and related code account more honestly for collations. ... When we first put in collations support, we basically punted on teaching pg_statistic, ANALYZE, and the planner selectivity functions about that. They've just used DEFAULT_COLLATION_OID independently of the actual collation of the data. It's time to improve that, so: * Add columns to pg_statistic that record the specific collation associated with each statistics slot. * Teach ANALYZE to use the column's actual collation when comparing values for statistical purposes, and record this in the appropriate slot. (Note that type-specific typanalyze functions are now expected to fill stats->stacoll with the appropriate collation, too.) * Teach assorted selectivity functions to use the actual collation of the stats they are looking at, instead of just assuming it's DEFAULT_COLLATION_OID. This should give noticeably better results in selectivity estimates for columns with nondefault collations, at least for query clauses that use that same collation (which would be the default behavior in most cases). It's still true that comparisons with explicit COLLATE clauses different from the stored data's collation won't be well-estimated, but that's no worse than before. Also, this patch does make the first step towards doing better with that, which is that it's now theoretically possible to collect stats for a collation other than the column's own collation. Patch by me; thanks to Peter Eisentraut for review. Discussion: https://postgr.es/m/14706.1544630227@sss.pgh.pa.us	7 years ago
Andres Freund	09568ec3d3	Create a separate oid range for oids assigned by genbki.pl. ... The changes I made in 578b229718 assigned oids below FirstBootstrapObjectId to objects in include/catalog/*.dat files that did not have an oid assigned, starting at the max oid explicitly assigned. Tom criticized that for mainly two reasons: 1) It's not clear which values are manually and which explicitly assigned. 2) The space below FirstBootstrapObjectId gets pretty crowded, and some PostgreSQL forks have used oids >= 9000 for their own objects, to avoid conflicting. Thus create a new range for objects not assigned explicit oids, but assigned by genbki.pl. For now 1-9999 is for explicitly assigned oids, FirstGenbkiObjectId (10000) to FirstBootstrapObjectId (1200) -1 is for genbki.pl assigned oids, and < FirstNormalObjectId (16384) is for oids assigned during bootstrap. It's possible that we'll have to adjust these boundaries, but there's some headroom for now. Add a note suggesting that oids in forks should be assigned in the 9000-9999 range. Catversion bump for obvious reasons. Per complaint from Tom Lane. Author: Andres Freund Discussion: https://postgr.es/m/16845.1544393682@sss.pgh.pa.us	7 years ago
Tom Lane	04fe805a17	Drop no-op CoerceToDomain nodes from expressions at planning time. ... If a domain has no constraints, then CoerceToDomain doesn't really do anything and can be simplified to a RelabelType. This not only eliminates cycles at execution, but allows the planner to optimize better (for instance, match the coerced expression to an index on the underlying column). However, we do have to support invalidating the plan later if a constraint gets added to the domain. That's comparable to the case of a change to a SQL function that had been inlined into a plan, so all the necessary logic already exists for plans depending on functions. We need only duplicate or share that logic for domains. ALTER DOMAIN ADD/DROP CONSTRAINT need to be taught to send out sinval messages for the domain's pg_type entry, since those operations don't update that row. (ALTER DOMAIN SET/DROP NOT NULL do update that row, so no code change is needed for them.) Testing this revealed what's really a pre-existing bug in plpgsql: it caches the SQL-expression-tree expansion of type coercions and had no provision for invalidating entries in that cache. Up to now that was only a problem if such an expression had inlined a SQL function that got changed, which is unlikely though not impossible. But failing to track changes of domain constraints breaks an existing regression test case and would likely cause practical problems too. We could fix that locally in plpgsql, but what seems like a better idea is to build some generic infrastructure in plancache.c to store standalone expressions and track invalidation events for them. (It's tempting to wonder whether plpgsql's "simple expression" stuff could use this code with lower overhead than its current use of the heavyweight plancache APIs. But I've left that idea for later.) Other stuff fixed in passing: * Allow estimate_expression_value() to drop CoerceToDomain unconditionally, effectively assuming that the coercion will succeed. This will improve planner selectivity estimates for cases involving estimatable expressions that are coerced to domains. We could have done this independently of everything else here, but there wasn't previously any need for eval_const_expressions_mutator to know about CoerceToDomain at all. * Use a dlist for plancache.c's list of cached plans, rather than a manually threaded singly-linked list. That eliminates a potential performance problem in DropCachedPlan. * Fix a couple of inconsistencies in typecmds.c about whether operations on domains drop RowExclusiveLock on pg_type. Our common practice is that DDL operations do drop catalog locks, so standardize on that choice. Discussion: https://postgr.es/m/19958.1544122124@sss.pgh.pa.us	7 years ago
Alvaro Herrera	88bdbd3f74	Add log_statement_sample_rate parameter ... This allows to set a lower log_min_duration_statement value without incurring excessive log traffic (which reduces performance). This can be useful to analyze workloads with lots of short queries. Author: Adrien Nayrat Reviewed-by: David Rowley, Vik Fearing Discussion: https://postgr.es/m/c30ee535-ee1e-db9f-fa97-146b9f62caed@anayrat.info	7 years ago
Peter Eisentraut	2dedf4d9a8	Integrate recovery.conf into postgresql.conf ... recovery.conf settings are now set in postgresql.conf (or other GUC sources). Currently, all the affected settings are PGC_POSTMASTER; this could be refined in the future case by case. Recovery is now initiated by a file recovery.signal. Standby mode is initiated by a file standby.signal. The standby_mode setting is gone. If a recovery.conf file is found, an error is issued. The trigger_file setting has been renamed to promote_trigger_file as part of the move. The documentation chapter "Recovery Configuration" has been integrated into "Server Configuration". pg_basebackup -R now appends settings to postgresql.auto.conf and creates a standby.signal file. Author: Fujii Masao <masao.fujii@gmail.com> Author: Simon Riggs <simon@2ndquadrant.com> Author: Abhijit Menon-Sen <ams@2ndquadrant.com> Author: Sergei Kornilov <sk@zsrv.org> Discussion: https://www.postgresql.org/message-id/flat/607741529606767@web3g.yandex.ru/	7 years ago
Andres Freund	578b229718	Remove WITH OIDS support, change oid catalog column visibility. ... Previously tables declared WITH OIDS, including a significant fraction of the catalog tables, stored the oid column not as a normal column, but as part of the tuple header. This special column was not shown by default, which was somewhat odd, as it's often (consider e.g. pg_class.oid) one of the more important parts of a row. Neither pg_dump nor COPY included the contents of the oid column by default. The fact that the oid column was not an ordinary column necessitated a significant amount of special case code to support oid columns. That already was painful for the existing, but upcoming work aiming to make table storage pluggable, would have required expanding and duplicating that "specialness" significantly. WITH OIDS has been deprecated since 2005 (commit ff02d0a05280e0). Remove it. Removing includes: - CREATE TABLE and ALTER TABLE syntax for declaring the table to be WITH OIDS has been removed (WITH (oids[ = true]) will error out) - pg_dump does not support dumping tables declared WITH OIDS and will issue a warning when dumping one (and ignore the oid column). - restoring an pg_dump archive with pg_restore will warn when restoring a table with oid contents (and ignore the oid column) - COPY will refuse to load binary dump that includes oids. - pg_upgrade will error out when encountering tables declared WITH OIDS, they have to be altered to remove the oid column first. - Functionality to access the oid of the last inserted row (like plpgsql's RESULT_OID, spi's SPI_lastoid, ...) has been removed. The syntax for declaring a table WITHOUT OIDS (or WITH (oids = false) for CREATE TABLE) is still supported. While that requires a bit of support code, it seems unnecessary to break applications / dumps that do not use oids, and are explicit about not using them. The biggest user of WITH OID columns was postgres' catalog. This commit changes all 'magic' oid columns to be columns that are normally declared and stored. To reduce unnecessary query breakage all the newly added columns are still named 'oid', even if a table's column naming scheme would indicate 'reloid' or such. This obviously requires adapting a lot code, mostly replacing oid access via HeapTupleGetOid() with access to the underlying Form_pg_*->oid column. The bootstrap process now assigns oids for all oid columns in genbki.pl that do not have an explicit value (starting at the largest oid previously used), only oids assigned later by oids will be above FirstBootstrapObjectId. As the oid column now is a normal column the special bootstrap syntax for oids has been removed. Oids are not automatically assigned during insertion anymore, all backend code explicitly assigns oids with GetNewOidWithIndex(). For the rare case that insertions into the catalog via SQL are called for the new pg_nextoid() function can be used (which only works on catalog tables). The fact that oid columns on system tables are now normal columns means that they will be included in the set of columns expanded by * (i.e. SELECT * FROM pg_class will now include the table's oid, previously it did not). It'd not technically be hard to hide oid column by default, but that'd mean confusing behavior would either have to be carried forward forever, or it'd cause breakage down the line. While it's not unlikely that further adjustments are needed, the scope/invasiveness of the patch makes it worthwhile to get merge this now. It's painful to maintain externally, too complicated to commit after the code code freeze, and a dependency of a number of other patches. Catversion bump, for obvious reasons. Author: Andres Freund, with contributions by John Naylor Discussion: https://postgr.es/m/20180930034810.ywp2c7awz7opzcfr@alap3.anarazel.de	7 years ago
Peter Eisentraut	7ac0069fb8	A small tweak to some comments for PartitionKeyData ... It was not really that obvious that there's meant to be exactly 1 item in the partexprs List for each zero-valued partattrs element. Some incorrect code using these fields was the cause of CVE-2018-1052, so it's worthwhile to mention how they should be used in the comments. Author: David Rowley <david.rowley@2ndquadrant.com>	7 years ago
Tom Lane	fa2952d8eb	Fix missing role dependencies for some schema and type ACLs. ... This patch fixes several related cases in which pg_shdepend entries were never made, or were lost, for references to roles appearing in the ACLs of schemas and/or types. While that did no immediate harm, if a referenced role were later dropped, the drop would be allowed and would leave a dangling reference in the object's ACL. That still wasn't a big problem for normal database usage, but it would cause obscure failures in subsequent dump/reload or pg_upgrade attempts, taking the form of attempts to grant privileges to all-numeric role names. (I think I've seen field reports matching that symptom, but can't find any right now.) Several cases are fixed here: 1. ALTER DOMAIN SET/DROP DEFAULT would lose the dependencies for any existing ACL entries for the domain. This case is ancient, dating back as far as we've had pg_shdepend tracking at all. 2. If a default type privilege applies, CREATE TYPE recorded the ACL properly but forgot to install dependency entries for it. This dates to the addition of default privileges for types in 9.2. 3. If a default schema privilege applies, CREATE SCHEMA recorded the ACL properly but forgot to install dependency entries for it. This dates to the addition of default privileges for schemas in v10 (commit ab89e465c). Another somewhat-related problem is that when creating a relation rowtype or implicit array type, TypeCreate would apply any available default type privileges to that type, which we don't really want since such an object isn't supposed to have privileges of its own. (You can't, for example, drop such privileges once they've been added to an array type.) ab89e465c is also to blame for a race condition in the regression tests: privileges.sql transiently installed globally-applicable default privileges on schemas, which sometimes got absorbed into the ACLs of schemas created by concurrent test scripts. This should have resulted in failures when privileges.sql tried to drop the role holding such privileges; but thanks to the bug fixed here, it instead led to dangling ACLs in the final state of the regression database. We'd managed not to notice that, but it became obvious in the wake of commit da906766c, which allowed the race condition to occur in pg_upgrade tests. To fix, add a function recordDependencyOnNewAcl to encapsulate what callers of get_user_default_acl need to do; while the original call sites got that right via ad-hoc code, none of the later-added ones have. Also change GenerateTypeDependencies to generate these dependencies, which requires adding the typacl to its parameter list. (That might be annoying if there are any extensions calling that function directly; but if there are, they're most likely buggy in the same way as the core callers were, so they need work anyway.) While I was at it, I changed GenerateTypeDependencies to accept most of its parameters in the form of a Form_pg_type pointer, making its parameter list a bit less unwieldy and mistake-prone. The test race condition is fixed just by wrapping the addition and removal of default privileges into a single transaction, so that that state is never visible externally. We might eventually prefer to separate out tests of default privileges into a script that runs by itself, but that would be a bigger change and would make the tests run slower overall. Back-patch relevant parts to all supported branches. Discussion: https://postgr.es/m/15719.1541725287@sss.pgh.pa.us	7 years ago
Peter Eisentraut	5d7c703a44	Remove get_attidentity() ... All existing uses can get this information more easily from the relation descriptor, so the detour through the syscache is not necessary. Reviewed-by: Michael Paquier <michael@paquier.xyz>	7 years ago
Peter Eisentraut	c903bb7b1c	Remove get_atttypmod() ... This has been unused since 2004. get_atttypetypmodcoll() is often a better alternative. Reviewed-by: Michael Paquier <michael@paquier.xyz>	7 years ago
Andres Freund	28d750c0cd	Reorder FmgrBuiltin members, saving 25% in size. ... That's worth it, as fmgr_builtins is frequently accessed, and as fmgr_builtins is one of the biggest constant variables in a backend. On most 64bit systems this will change the size of the struct from 32byte to 24bytes. While that could make indexing into the array marginally more expensive, the higher cache hit ratio is worth more, especially because these days fmgr_builtins isn't searched with a binary search anymore (c.f. 212e6f34d5). Discussion: https://postgr.es/m/20181016201145.aa2dfeq54rhqzron@alap3.anarazel.de	7 years ago
Andres Freund	02a30a09f9	Correct constness of system attributes in heap.c & prerequisites. ... This allows the compiler / linker to mark affected pages as read-only. There's a fair number of pre-requisite changes, to allow the const properly be propagated. Most of consts were already required for correctness anyway, just not represented on the type-level. Arguably we could be more aggressive in using consts in related code, but.. This requires using a few of the types underlying typedefs that removes pointers (e.g. const NameData *) as declaring the typedefed type constant doesn't have the same meaning (it makes the variable const, not what it points to). Discussion: https://postgr.es/m/20181015200754.7y7zfuzsoux2c4ya@alap3.anarazel.de	7 years ago
Tom Lane	13cd7209f7	Simplify use of AllocSetContextCreate() wrapper macro. ... We can allow this macro to accept either abbreviated or non-abbreviated allocation parameters by making use of __VA_ARGS__. As noted by Andres Freund, it's unlikely that any compiler would have __builtin_constant_p but not __VA_ARGS__, so this gives up little or no error checking, and it avoids a minor but annoying API break for extensions. With this change, there is no reason for anybody to call AllocSetContextCreateExtended directly, so in HEAD I renamed it to AllocSetContextCreateInternal. It's probably too late for an ABI break like that in 11, though. Discussion: https://postgr.es/m/20181012170355.bhxi273skjt6sag4@alap3.anarazel.de	7 years ago
Alvaro Herrera	c7d43c4d8a	Correct attach/detach logic for FKs in partitions ... There was no code to handle foreign key constraints on partitioned tables in the case of ALTER TABLE DETACH; and if you happened to ATTACH a partition that already had an equivalent constraint, that one was ignored and a new constraint was created. Adding this to the fact that foreign key cloning reuses the constraint name on the partition instead of generating a new name (as it probably should, to cater to SQL standard rules about constraint naming within schemas), the result was a pretty poor user experience -- the most visible failure was that just detaching a partition and re-attaching it failed with an error such as ERROR: duplicate key value violates unique constraint "pg_constraint_conrelid_contypid_conname_index" DETAIL: Key (conrelid, contypid, conname)=(26702, 0, test_result_asset_id_fkey) already exists. because it would try to create an identically-named constraint in the partition. To make matters worse, if you tried to drop the constraint in the now-independent partition, that would fail because the constraint was still seen as dependent on the constraint in its former parent partitioned table: ERROR: cannot drop inherited constraint "test_result_asset_id_fkey" of relation "test_result_cbsystem_0001_0050_monthly_2018_09" This fix attacks the problem from two angles: first, when the partition is detached, the constraint is also marked as independent, so the drop now works. Second, when the partition is re-attached, we scan existing constraints searching for one matching the FK in the parent, and if one exists, we link that one to the parent constraint. So we don't end up with a duplicate -- and better yet, we don't need to scan the referenced table to verify that the constraint holds. To implement this I made a small change to previously planner-only struct ForeignKeyCacheInfo to contain the constraint OID; also relcache now maintains the list of FKs for partitioned tables too. Backpatch to 11. Reported-by: Michael Vitale (bug #15425) Discussion: https://postgr.es/m/15425-2dbc9d2aa999f816@postgresql.org	7 years ago
Andres Freund	cda6a8d01d	Remove deprecated abstime, reltime, tinterval datatypes. ... These types have been deprecated for a *long* time. Catversion bump, for obvious reasons. Author: Andres Freund Discussion: https://postgr.es/m/20181009192237.34wjp3nmw7oynmmr@alap3.anarazel.de https://postgr.es/m/20171213080506.cwjkpcz3bkk6yz2u@alap3.anarazel.de https://postgr.es/m/25615.1513115237@sss.pgh.pa.us	7 years ago
Tom Lane	c87cb5f7a6	Allow btree comparison functions to return INT_MIN. ... Historically we forbade datatype-specific comparison functions from returning INT_MIN, so that it would be safe to invert the sort order just by negating the comparison result. However, this was never really safe for comparison functions that directly return the result of memcmp(), strcmp(), etc, as POSIX doesn't place any such restriction on those library functions. Buildfarm results show that at least on recent Linux on s390x, memcmp() actually does return INT_MIN sometimes, causing sort failures. The agreed-on answer is to remove this restriction and fix relevant call sites to not make such an assumption; code such as "res = -res" should be replaced by "INVERT_COMPARE_RESULT(res)". The same is needed in a few places that just directly negated the result of memcmp or strcmp. To help find places having this problem, I've also added a compile option to nbtcompare.c that causes some of the commonly used comparators to return INT_MIN/INT_MAX instead of their usual -1/+1. It'd likely be a good idea to have at least one buildfarm member running with "-DSTRESS_SORT_INT_MIN". That's far from a complete test of course, but it should help to prevent fresh introductions of such bugs. This is a longstanding portability hazard, so back-patch to all supported branches. Discussion: https://postgr.es/m/20180928185215.ffoq2xrq5d3pafna@alap3.anarazel.de	7 years ago
Tom Lane	aaf10f32a3	Fix assorted bugs in pg_get_partition_constraintdef(). ... It failed if passed a nonexistent relation OID, or one that was a non-heap relation, because of blindly applying heap_open to a user-supplied OID. This is not OK behavior for a SQL-exposed function; we have a project policy that we should return NULL in such cases. Moreover, since pg_get_partition_constraintdef ought now to work on indexes, restricting it to heaps is flat wrong anyway. The underlying function generate_partition_qual() wasn't on board with indexes having partition quals either, nor for that matter with rels having relispartition set but yet null relpartbound. (One wonders whether the person who wrote the function comment blocks claiming that these functions allow a missing relpartbound had ever tested it.) Fix by testing relispartition before opening the rel, and by using relation_open not heap_open. (If any other relkinds ever grow the ability to have relispartition set, the code will work with them automatically.) Also, don't reject null relpartbound in generate_partition_qual. Back-patch to v11, and all but the null-relpartbound change to v10. (It's not really necessary to change generate_partition_qual at all in v10, but I thought s/heap_open/relation_open/ would be a good idea anyway just to keep the code in sync with later branches.) Per report from Justin Pryzby. Discussion: https://postgr.es/m/20180927200020.GJ776@telsasoft.com	7 years ago
Thomas Munro	f025bd2ddd	Use size_t consistently in dsa.{ch}. ... Takeshi Ideriha complained that there is a mixture of Size and size_t in dsa.c and corresponding header. Let's use size_t. Back-patch to 10 where dsa.c landed, to make future back-patching easy. Discussion: https://postgr.es/m/4E72940DA2BF16479384A86D54D0988A6F19ABD9%40G01JPEXMBKW04	7 years ago
Alexander Korotkov	2a6368343f	Add support for nearest-neighbor (KNN) searches to SP-GiST ... Currently, KNN searches were supported only by GiST. SP-GiST also capable to support them. This commit implements that support. SP-GiST scan stack is replaced with queue, which serves as stack if no ordering is specified. KNN support is provided for three SP-GIST opclasses: quad_point_ops, kd_point_ops and poly_ops (catversion is bumped). Some common parts between GiST and SP-GiST KNNs are extracted into separate functions. Discussion: https://postgr.es/m/570825e8-47d0-4732-2bf6-88d67d2d51c8%40postgrespro.ru Author: Nikita Glukhov, Alexander Korotkov based on GSoC work by Vlad Sterzhanov Review: Andrey Borodin, Alexander Korotkov	7 years ago
Peter Eisentraut	842cb9fa62	Refactor dlopen() support ... Nowadays, all platforms except Windows and older HP-UX have standard dlopen() support. So having a separate implementation per platform under src/backend/port/dynloader/ is a bit excessive. Instead, treat dlopen() like other library functions that happen to be missing sometimes and put a replacement implementation under src/port/. Discussion: https://www.postgresql.org/message-id/flat/e11a49cb-570a-60b7-707d-7084c8de0e61%402ndquadrant.com#54e735ae37476a121abb4e33c2549b03	7 years ago
Andres Freund	8ecdefc261	Remove test for VA_ARGS, implied by C99. ... This simplifies logic / reduces duplication in a few headers. Author: Andres Freund Discussion: https://postgr.es/m/97d4b165-192d-3605-749c-f614a0c4e783@2ndquadrant.com	7 years ago
Tomas Vondra	c4c3400885	Use the built-in float datatypes to implement geometric types ... This patch makes the geometric operators and functions use the exported function of the float4/float8 datatypes. The main reason of doing so is to check for underflow and overflow, and to handle NaNs consciously. The float datatypes consider NaNs values to be equal and greater than all non-NaN values. This change considers NaNs equal only for equality operators. The placement operators, contains, overlaps, left/right of etc. continue to return false when NaNs are involved. We don't need to worry about them being considered greater than any-NaN because there aren't any basic comparison operators like less/greater than for the geometric datatypes. The changes may be summarised as: * Check for underflow, overflow and division by zero * Consider NaN values to be equal * Return NULL when the distance is NaN for all closest point operators * Favour not-NaN over NaN where it makes sense The patch also replaces all occurrences of "double" as "float8". They are the same, but were used inconsistently in the same file. Author: Emre Hasegeli Reviewed-by: Kyotaro Horiguchi, Tomas Vondra Discussion: https://www.postgresql.org/message-id/CAE2gYzxF7-5djV6-cEvqQu-fNsnt%3DEqbOURx7ZDg%2BVv6ZMTWbg%40mail.gmail.com	7 years ago
Tom Lane	46b5e7c4b5	Revert "Distinguish printf-like functions that support %m from those that don't." ... This reverts commit 3a60c8ff89. Buildfarm results show that that caused a whole bunch of new warnings on platforms where gcc believes the local printf to be non-POSIX-compliant. This problem outweighs the hypothetical-anyway possibility of getting warnings for misuse of %m. We could use gnu_printf archetype when we've substituted src/port/snprintf.c, but that brings us right back to the problem of not getting warnings for %m. A possible answer is to attack it in the other direction by insisting that %m support be included in printf's feature set, but that will take more investigation. In the meantime, revert the previous change, and update the comment for PGAC_C_PRINTF_ARCHETYPE to more fully explain what's going on. Discussion: https://postgr.es/m/2975.1526862605@sss.pgh.pa.us	8 years ago
Tom Lane	a2a8acd152	Produce compiler errors if errno is referenced inside elog/ereport calls. ... It's often unsafe to reference errno within an elog/ereport call, because there are a lot of sub-functions involved and they might not all preserve errno. (This is why we support the %m format spec: it works off a value of errno captured before we execute any potentially-unsafe functions in the arguments.) Therefore, we have a project policy not to use errno there. This patch adds a hack to cause an (admittedly obscure) compiler error for such unsafe usages. With the current code, the error will only be seen on Linux, macOS, and FreeBSD, but that should certainly be enough to catch mistakes in the buildfarm if they somehow get missed earlier. In addition, fix some places in src/common/exec.c that trip the error. I think these places are actually all safe, but it's simple enough to avoid the error by capturing errno manually, and doing so is good future-proofing in case these call sites get any more complicated. Thomas Munro (exec.c fixes by me) Discussion: https://postgr.es/m/2975.1526862605@sss.pgh.pa.us	8 years ago
Tom Lane	3a60c8ff89	Distinguish printf-like functions that support %m from those that don't. ... The elog/ereport family of functions certainly support the %m format spec, because they implement it "by hand". But elsewhere we have printf wrappers that might or might not allow it depending on whether the platform's printf does. (Most non-glibc versions don't, and notably, src/port/snprintf.c doesn't.) Hence, rather than using the gnu_printf format archetype interchangeably for all these functions, use it only for elog/ereport. This will allow us to get compiler warnings for mistakes like the ones fixed in commit a13b47a59, at least on platforms where printf doesn't take %m and gcc is correctly configured to know it. (Unfortunately, that won't happen on Linux, nor on macOS according to my testing. It remains to be seen what the buildfarm's gcc-on-Windows animals will think of this, but we may well have to rely on less-popular platforms to warn us about unportable code of this kind.) Discussion: https://postgr.es/m/2975.1526862605@sss.pgh.pa.us	8 years ago
Peter Geoghegan	4974d7f87e	Handle parallel index builds on mapped relations. ... Commit 9da0cc3528, which introduced parallel CREATE INDEX, failed to propagate relmapper.c backend local cache state to parallel worker processes. This could result in parallel index builds against mapped catalog relations where the leader process (participating as a worker) scans the new, pristine relfilenode, while worker processes scan the obsolescent relfilenode. When this happened, the final index structure was typically not consistent with the owning table's structure. The final index structure could contain entries formed from both heap relfilenodes. Only rebuilds on mapped catalog relations that occur as part of a VACUUM FULL or CLUSTER could become corrupt in practice, since their mapped relation relfilenode swap is what allows the inconsistency to arise. On master, fix the problem by propagating the required relmapper.c backend state as part of standard parallel initialization (Cf. commit 29d58fd3). On v11, simply disallow builds against mapped catalog relations by deeming them parallel unsafe. Author: Peter Geoghegan Reported-By: "death lock" Reviewed-By: Tom Lane, Amit Kapila Bug: #15309 Discussion: https://postgr.es/m/153329671686.1405.18298309097348420351@wrigleys.postgresql.org Backpatch: 11-, where parallel CREATE INDEX was introduced.	8 years ago
Tom Lane	f3eb76b399	Further fixes for quoted-list GUC values in pg_dump and ruleutils.c. ... Commits 742869946 et al turn out to be a couple bricks shy of a load. We were dumping the stored values of GUC_LIST_QUOTE variables as they appear in proconfig or setconfig catalog columns. However, although that quoting rule looks a lot like SQL-identifier double quotes, there are two critical differences: empty strings ("") are legal, and depending on which variable you're considering, values longer than NAMEDATALEN might be valid too. So the current technique fails altogether on empty-string list entries (as reported by Steven Winfield in bug #15248) and it also risks truncating file pathnames during dump/reload of GUC values that are lists of pathnames. To fix, split the stored value without any downcasing or truncation, and then emit each element as a SQL string literal. This is a tad annoying, because we now have three copies of the comma-separated-string splitting logic in varlena.c as well as a fourth one in dumputils.c. (Not to mention the randomly-different-from-those splitting logic in libpq...) I looked at unifying these, but it would be rather a mess unless we're willing to tweak the API definitions of SplitIdentifierString, SplitDirectoriesString, or both. That might be worth doing in future; but it seems pretty unsafe for a back-patched bug fix, so for now accept the duplication. Back-patch to all supported branches, as the previous fix was. Discussion: https://postgr.es/m/7585.1529435872@sss.pgh.pa.us	8 years ago
Alvaro Herrera	23ca82d7ef	Fix typo in file identification and copyright year	8 years ago
Tomas Vondra	6bf0bc842b	Provide separate header file for built-in float types ... Some data types under adt/ have separate header files, but most simple ones do not, and their public functions are defined in builtins.h. As the patches improving geometric types will require making additional functions public, this seems like a good opportunity to create a header for floats types. Commit 1acf757255 made _cmp functions public to solve NaN issues locally for GiST indexes. This patch reworks it in favour of a more widely applicable API. The API uses inline functions, as they are easier to use compared to macros, and avoid double-evaluation hazards. Author: Emre Hasegeli Reviewed-by: Kyotaro Horiguchi Discussion: https://www.postgresql.org/message-id/CAE2gYzxF7-5djV6-cEvqQu-fNsnt%3DEqbOURx7ZDg%2BVv6ZMTWbg%40mail.gmail.com	8 years ago
Tomas Vondra	a7dc63d904	Refactor geometric functions and operators ... The primary goal of this patch is to eliminate duplicate code and share code between different geometric data types more often, to prepare the ground for additional patches. Until now the code reuse was limited, probably because the simpler types (line and point) were implemented after the more complex ones. The changes are quite extensive and can be summarised as: * Eliminate SQL-level function calls. * Re-use more functions to implement others. * Unify internal function names and signatures. * Remove private functions from geo_decls.h. * Replace should-not-happen checks with assertions. * Add comments describe for various functions. * Remove some unreachable code. * Define delimiter symbols of line datatype like the other ones. * Remove the GEODEBUG macro and printf() calls. * Unify code style of a few oddly formatted lines. While the goal was to cause minimal user-visible changes, it was not possible to keep the original behavior in all cases - for example when handling NaN values, or when reusing code makes the functions return consistent results. Author: Emre Hasegeli Reviewed-by: Kyotaro Horiguchi, me Discussion: https://www.postgresql.org/message-id/CAE2gYzxF7-5djV6-cEvqQu-fNsnt%3DEqbOURx7ZDg%2BVv6ZMTWbg%40mail.gmail.com	8 years ago
Andres Freund	86eaf208ea	Hand code string to integer conversion for performance. ... As benchmarks show, using libc's string-to-integer conversion is pretty slow. At least part of the reason for that is that strtol[l] have to be more generic than what largely is required inside pg. This patch considerably speeds up int2/int4 input (int8 already was already using hand-rolled code). Most of the existing pg_atoi callers have been converted. But as one requires pg_atoi's custom delimiter functionality, and as it seems likely that there's external pg_atoi users, it seems sensible to just keep pg_atoi around. Author: Andres Freund Reviewed-By: Robert Haas Discussion: https://postgr.es/m/20171208214437.qgn6zdltyq5hmjpk@alap3.anarazel.de	8 years ago
Tom Lane	3cb646264e	Use a ResourceOwner to track buffer pins in all cases. ... Historically, we've allowed auxiliary processes to take buffer pins without tracking them in a ResourceOwner. However, that creates problems for error recovery. In particular, we've seen multiple reports of assertion crashes in the startup process when it gets an error while holding a buffer pin, as for example if it gets ENOSPC during a write. In a non-assert build, the process would simply exit without releasing the pin at all. We've gotten away with that so far just because a failure exit of the startup process translates to a database crash anyhow; but any similar behavior in other aux processes could result in stuck pins and subsequent problems in vacuum. To improve this, institute a policy that we must *always* have a resowner backing any attempt to pin a buffer, which we can enforce just by removing the previous special-case code in resowner.c. Add infrastructure to make it easy to create a process-lifespan AuxProcessResourceOwner and clear out its contents at appropriate times. Replace existing ad-hoc resowner management in bgwriter.c and other aux processes with that. (Thus, while the startup process gains a resowner where it had none at all before, some other aux process types are replacing an ad-hoc resowner with this code.) Also use the AuxProcessResourceOwner to manage buffer pins taken during StartupXLOG and ShutdownXLOG, even when those are being run in a bootstrap process or a standalone backend rather than a true auxiliary process. In passing, remove some other ad-hoc resource owner creations that had gotten cargo-culted into various other places. As far as I can tell that was all unnecessary, and if it had been necessary it was incomplete, due to lacking any provision for clearing those resowners later. (Also worth noting in this connection is that a process that hasn't called InitBufferPoolBackend has no business accessing buffers; so there's more to do than just add the resowner if we want to touch buffers in processes not covered by this patch.) Although this fixes a very old bug, no back-patch, because there's no evidence of any significant problem in non-assert builds. Patch by me, pursuant to a report from Justin Pryzby. Thanks to Robert Haas and Kyotaro Horiguchi for reviews. Discussion: https://postgr.es/m/20180627233939.GA10276@telsasoft.com	8 years ago
Heikki Linnakangas	6b387179ba	Fix misc typos, mostly in comments. ... A collection of typos I happened to spot while reading code, as well as grepping for common mistakes. Backpatch to all supported versions, as applicable, to avoid conflicts when backporting other commits in the future.	8 years ago
Peter Eisentraut	f7cb2842bf	Add plan_cache_mode setting ... This allows overriding the choice of custom or generic plan. Author: Pavel Stehule <pavel.stehule@gmail.com> Discussion: https://www.postgresql.org/message-id/flat/CAFj8pRAGLaiEm8ur5DWEBo7qHRWTk9HxkuUAz00CZZtJj-LkCA%40mail.gmail.com	8 years ago
Alexander Korotkov	edf59c40dd	Fix more wrong paths in header comments ... It appears that there are more files, whose header comment paths are wrong. So, fix those paths. No backpatching per proposal of Tom Lane. Discussion: https://postgr.es/m/CAPpHfdsJyYbOj59MOQL%2B4XxdcomLSLfLqBtAvwR%2BpsCqj3ELdQ%40mail.gmail.com	8 years ago
Alvaro Herrera	0c8910a0ca	Teach SHOW ALL to honor pg_read_all_settings membership ... Also, fix the pg_settings view to display source filename and line number when invoked by a pg_read_all_settings member. This addition by me (Álvaro). Also, fix wording of the comment in GetConfigOption regarding the restriction it implements, renaming the parameter for extra clarity. Noted by Michaël. These were all oversight in commit 25fff40798fc; backpatch to pg10, where that commit first appeared. Author: Laurenz Albe Reviewed-by: Michaël Paquier, Álvaro Herrera Discussion: https://postgr.es/m/1519917758.6586.8.camel@cybertec.at	8 years ago
Tom Lane	2efc924180	Detoast plpgsql variables if they might live across a transaction boundary. ... Up to now, it's been safe for plpgsql to store TOAST pointers in its variables because the ActiveSnapshot for whatever query called the plpgsql function will surely protect such TOAST values from being vacuumed away, even if the owning table rows are committed dead. With the introduction of procedures, that assumption is no longer good in "non atomic" executions of plpgsql code. We adopt the slightly brute-force solution of detoasting all TOAST pointers at the time they are stored into variables, if we're in a non-atomic context, just in case the owning row goes away. Some care is needed to avoid long-term memory leaks, since plpgsql tends to run with CurrentMemoryContext pointing to its call-lifespan context, but we shouldn't assume that no memory is leaked by heap_tuple_fetch_attr. In plpgsql proper, we can do the detoasting work in the "eval_mcontext". Most of the code thrashing here is due to the need to add this capability to expandedrecord.c as well as plpgsql proper. In expandedrecord.c, we can't assume that the caller's context is short-lived, so make use of the short-term sub-context that was already invented for checking domain constraints. In view of this repurposing, it seems good to rename that variable and associated code from "domain_check_cxt" to "short_term_cxt". Peter Eisentraut and Tom Lane Discussion: https://postgr.es/m/5AC06865.9050005@anastigmatix.net	8 years ago
Tom Lane	9bf28f96c7	Rearrange makefile rules for running Gen_fmgrtab.pl. ... Make these rules look more like the ones associated with genbki.pl, to wit: * Use a stamp file to record when we last ran the script, instead of relying on the timestamps of the individual output files. * Take the knowledge out of backend/Makefile and put it in utils/Makefile where it belongs. I moved down the handling of errcodes.h and probes.h too, although those continue to be built by separate processes. In itself, this is just much-needed cleanup with little practical effect. However, by decoupling these makefile rules from the timestamps of the generated header files, we open the door to not advancing those timestamps unnecessarily, which will be taken advantage of by the next commit. msvc/Solution.pm should be taught to do things similarly, but I'll leave that for another commit. Discussion: https://postgr.es/m/16925.1525376229@sss.pgh.pa.us	8 years ago
Heikki Linnakangas	445e31bdc7	Fix some sloppiness in the new BufFileSize() and BufFileAppend() functions. ... There were three related issues: * BufFileAppend() incorrectly reset the seek position on the 'source' file. As a result, if you had called BufFileRead() on the file before calling BufFileAppend(), it got confused, and subsequent calls would read/write at wrong position. * BufFileSize() did not work with files opened with BufFileOpenShared(). * FileGetSize() only worked on temporary files. To fix, change the way BufFileSize() works so that it works on shared files. Remove FileGetSize() altogether, as it's no longer needed. Remove buffilesize from TapeShare struct, as the leader process can simply call BufFileSize() to get the tape's size, there's no need to pass it through shared memory anymore. Discussion: https://www.postgresql.org/message-id/CAH2-WznEDYe_NZXxmnOfsoV54oFkTdMy7YLE2NPBLuttO96vTQ@mail.gmail.com	8 years ago
Tom Lane	bdf46af748	Post-feature-freeze pgindent run. ... Discussion: https://postgr.es/m/15719.1523984266@sss.pgh.pa.us	8 years ago
Alvaro Herrera	da6f3e45dd	Reorganize partitioning code ... There's been a massive addition of partitioning code in PostgreSQL 11, with little oversight on its placement, resulting in a catalog/partition.c with poorly defined boundaries and responsibilities. This commit tries to set a couple of distinct modules to separate things a little bit. There are no code changes here, only code movement. There are three new files: src/backend/utils/cache/partcache.c src/include/partitioning/partdefs.h src/include/utils/partcache.h The previous arrangement of #including catalog/partition.h almost everywhere is no more. Authors: Amit Langote and Álvaro Herrera Discussion: https://postgr.es/m/98e8d509-790a-128c-be7f-e48a5b2d8d97@lab.ntt.co.jp https://postgr.es/m/11aa0c50-316b-18bb-722d-c23814f39059@lab.ntt.co.jp https://postgr.es/m/143ed9a4-6038-76d4-9a55-502035815e68@lab.ntt.co.jp https://postgr.es/m/20180413193503.nynq7bnmgh6vs5vm@alvherre.pgsql	8 years ago
Teodor Sigaev	8224de4f42	Indexes with INCLUDE columns and their support in B-tree ... This patch introduces INCLUDE clause to index definition. This clause specifies a list of columns which will be included as a non-key part in the index. The INCLUDE columns exist solely to allow more queries to benefit from index-only scans. Also, such columns don't need to have appropriate operator classes. Expressions are not supported as INCLUDE columns since they cannot be used in index-only scans. Index access methods supporting INCLUDE are indicated by amcaninclude flag in IndexAmRoutine. For now, only B-tree indexes support INCLUDE clause. In B-tree indexes INCLUDE columns are truncated from pivot index tuples (tuples located in non-leaf pages and high keys). Therefore, B-tree indexes now might have variable number of attributes. This patch also provides generic facility to support that: pivot tuples contain number of their attributes in t_tid.ip_posid. Free 13th bit of t_info is used for indicating that. This facility will simplify further support of index suffix truncation. The changes of above are backward-compatible, pg_upgrade doesn't need special handling of B-tree indexes for that. Bump catalog version Author: Anastasia Lubennikova with contribition by Alexander Korotkov and me Reviewed by: Peter Geoghegan, Tomas Vondra, Antonin Houska, Jeff Janes, David Rowley, Alexander Korotkov Discussion: https://www.postgresql.org/message-id/flat/56168952.4010101@postgrespro.ru	8 years ago
Teodor Sigaev	1c1791e000	Add json(b)_to_tsvector function ... Jsonb has a complex nature so there isn't best-for-everything way to convert it to tsvector for full text search. Current to_tsvector(json(b)) suggests to convert only string values, but it's possible to index keys, numerics and even booleans value. To solve that json(b)_to_tsvector has a second required argument contained a list of desired types of json fields. Second argument is a jsonb scalar or array right now with possibility to add new options in a future. Bump catalog version Author: Dmitry Dolgov with some editorization by me Reviewed by: Teodor Sigaev Discussion: https://www.postgresql.org/message-id/CA+q6zcXJQbS1b4kJ_HeAOoOc=unfnOrUEL=KGgE32QKDww7d8g@mail.gmail.com	8 years ago