aboutsummaryrefslogtreecommitdiff
path: root/src
Commit message (Collapse)AuthorAge
...
* In libpq, free any partial query result before collecting a server error.Tom Lane2018-04-13
| | | | | | | | | | | | | | | | | | | | We'd throw away the partial result anyway after parsing the error message. Throwing it away beforehand costs nothing and reduces the risk of out-of-memory failure. Also, at least in systems that behave like glibc/Linux, if the partial result was very large then the error PGresult would get allocated at high heap addresses, preventing the heap storage used by the partial result from being released to the OS until the error PGresult is freed. In psql >= 9.6, we hold onto the error PGresult until another error is received (for \errverbose), so that this behavior causes a seeming memory leak to persist for awhile, as in a recent complaint from Darafei Praliaskouski. This is a potential performance regression from older versions, justifying back-patching at least that far. But similar behavior may occur in other client applications, so it seems worth just back-patching to all supported branches. Discussion: https://postgr.es/m/CAC8Q8tJ=7cOkPePyAbJE_Pf691t8nDFhJp0KZxHvnq_uicfyVg@mail.gmail.com
* Use custom hash opclass for hash partition pruningAlvaro Herrera2018-04-13
| | | | | | | | | | | | This custom opclass was already in use in other tests -- defined independently in every such file. Move the definition to the earliest test that uses it, and keep it around so that later tests can reuse it. Use it in the tests for pruning of hash partitioning, and since this makes the second expected file unnecessary, put those tests back in partition_prune.sql whence they sprang. Author: Amit Langote Discussion: https://postgr.es/m/CA%2BTgmoZ0D5kJbt8eKXtvVdvTcGGWn6ehWCRSZbWytD-uzH92mQ%40mail.gmail.com
* Attempt to stabilize partition_prune test output (2)Alvaro Herrera2018-04-13
| | | | | | | | | | Environmental conditions might cause parallel workers to be scheduled in different ways in this test, destabilizing the EXPLAIN output. Disable use of workers in an attempt to make output stable. Author: David Rowley Diagnosed-by: Thomas Munro Discussion: https://postgr.es/m/CAKJS1f8j24tUX_nOwACiM=UO5jrMrDz8ca0xbG0vhVgfWph0ZA@mail.gmail.com
* Fix bogus affix-merging code.Tom Lane2018-04-12
| | | | | | | | | | | | | | NISortAffixes() compared successive compound affixes incorrectly, thus possibly failing to merge identical affixes, or (less likely) merging ones that shouldn't be merged. The user-visible effects of this are unclear, to me anyway. Per bug #15150 from Alexander Lakhin. It's been broken for a long time, so back-patch to all supported branches. Arthur Zakirov Discussion: https://postgr.es/m/152353327780.31225.13445405496721177988@wrigleys.postgresql.org
* Revert lowering of lock level for ATTACH PARTITIONAlvaro Herrera2018-04-12
| | | | | | | | | I lowered the lock level for partitions being scanned from AccessExclusive to ShareLock in the course of 72cf7f310c07, but that was bogus, as pointed out by Robert Haas. Revert that bit. Doing this is possible, but requires more work. Discussion: https://postgr.es/m/CA+TgmobV7Nfmqv+TZXcdSsb9Bjc-OL-Anv6BNmCbfJVZLYPE4Q@mail.gmail.com
* Add comment about default partition in check_new_partition_boundAlvaro Herrera2018-04-12
| | | | | The intention of the test is not immediately obvious, so we need this much.
* Use the right memory context for partkey's FmgrInfoAlvaro Herrera2018-04-12
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We were using CurrentMemoryContext to put the partsupfunc fmgr_info into, which isn't right, because we want the PartitionKey as a whole to be in the isolated Relation->rd_partkeycxt context. This can cause a crash with user-defined support functions in the operator classes used by partitioning keys. (Maybe this can cause problems with core-supplied opclasses too, not sure.) This is demonstrably broken in Postgres 10, too, but the initial proposed fix runs afoul of a problem discussed back when 8a0596cb656e ("Get rid of copy_partition_key") reorganized that code: namely that it is possible to jump out of RelationBuildPartitionKey because of some error and leave a dangling memory context child of CacheMemoryContext. Also, while reviewing this I noticed that the removed-in-pg11 copy_partition_key was doing something wrong, unfixed in pg10, namely doing memcpy() on the FmgrInfo, which is bogus (should be doing fmgr_info_copy). Therefore, in branch pg10, the sane fix seems to be to backpatch both the aforementioned 8a0596cb656e and its followup be2343221fb7 ("Protect against hypothetical memory leaks in RelationGetPartitionKey"), so do that, then apply the fmgr_info memcxt bugfix on top. Add a test case exercising btree-based custom operator classes, which causes a crash prior to this fix. This is not a security problem, because in order to create an operator class you need superuser privileges anyway. Authors: Álvaro Herrera and Amit Langote Reported and diagnosed by: Amit Langote Discussion: https://postgr.es/m/3041e853-b1dd-a0c6-ff21-7cc5633bffd0@lab.ntt.co.jp
* Fix YA parallel-make hazard, this one in "make check" in plpython.Tom Lane2018-04-12
| | | | | | | We have to ensure that submake-generated-headers is finished before the topmost make run launches any child makes. Discussion: https://postgr.es/m/20180411235843.GG32449@paquier.xyz
* Fix interference between covering indexes and partitioned tablesTeodor Sigaev2018-04-12
| | | | | | | | | | | | | The bug is caused due to the original IndexStmt that DefineIndex receives being overwritten when processing the INCLUDE columns. Use separate list of index params to propagate to child tables. Add tests covering this case. Amit Langote and Alexander Korotkov. Re-commit 5c6110c6a960ad6fe1b0d0fec6ae36ef4eb913f5 because it discovered a bug fixed in c266ed31a8a3beed3533e6a78faeca78234cbd43 Discussion: https://www.postgresql.org/message-id/CAJGNTeO%3DBguEyG8wxMpU_Vgvg3nGGzy71zUQ0RpzEn_mb0bSWA%40mail.gmail.com
* Cleanup covering infrastructureTeodor Sigaev2018-04-12
| | | | | | | | | | | - Explicitly forbids opclass, collation and indoptions (like DESC/ASC etc) for including columns. Throw an error if user points that. - Truncated storage arrays for such attributes to store only key atrributes, added assertion checks. - Do not check opfamily and collation for including columns in CompareIndexInfo() Discussion: https://www.postgresql.org/message-id/5ee72852-3c4e-ee35-e2ed-c1d053d45c08@sigaev.ru
* Revert MERGE patchSimon Riggs2018-04-12
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This reverts commits d204ef63776b8a00ca220adec23979091564e465, 83454e3c2b28141c0db01c7d2027e01040df5249 and a few more commits thereafter (complete list at the end) related to MERGE feature. While the feature was fully functional, with sufficient test coverage and necessary documentation, it was felt that some parts of the executor and parse-analyzer can use a different design and it wasn't possible to do that in the available time. So it was decided to revert the patch for PG11 and retry again in the future. Thanks again to all reviewers and bug reporters. List of commits reverted, in reverse chronological order: f1464c5380 Improve parse representation for MERGE ddb4158579 MERGE syntax diagram correction 530e69e59b Allow cpluspluscheck to pass by renaming variable 01b88b4df5 MERGE minor errata 3af7b2b0d4 MERGE fix variable warning in non-assert builds a5d86181ec MERGE INSERT allows only one VALUES clause 4b2d44031f MERGE post-commit review 4923550c20 Tab completion for MERGE aa3faa3c7a WITH support in MERGE 83454e3c2b New files for MERGE d204ef6377 MERGE SQL Command following SQL:2016 Author: Pavan Deolasee Reviewed-by: Michael Paquier
* Rename IndexInfo.ii_KeyAttrNumbers arrayTeodor Sigaev2018-04-12
| | | | | | | | Rename ii_KeyAttrNumbers to ii_IndexAttrNumbers to prevent confusion with ii_NumIndexAttrs/ii_NumIndexKeyAttrs. ii_IndexAttrNumbers contains all attributes including "including" columns, not only key attribute. Discussion: https://www.postgresql.org/message-id/13123421-1d52-d0e4-c95c-6d69011e0595%40sigaev.ru
* Set relispartition correctly for index partitionsAlvaro Herrera2018-04-11
| | | | | | | | | | | Oversight in commit 8b08f7d4820f: pg_class.relispartition was not being set for index partitions, which is a bit odd, and was also causing the code to unnecessarily call has_superclass() when simply checking the flag was enough. Author: Álvaro Herrera Reported-by: Amit Langote Discussion: https://postgr.es/m/12085bc4-0bc6-0f3a-4c43-57fe0681772b@lab.ntt.co.jp
* Ignore nextOid when replaying an ONLINE checkpoint.Tom Lane2018-04-11
| | | | | | | | | | | | | | | The nextOid value is from the start of the checkpoint and may well be stale compared to values from more recent XLOG_NEXTOID records. Previously, we adopted it anyway, allowing the OID counter to go backwards during a crash. While this should be harmless, it contributed to the severity of the bug fixed in commit 0408e1ed5, by allowing duplicate TOAST OIDs to be assigned immediately following a crash. Without this error, that issue would only have arisen when TOAST objects just younger than a multiple of 2^32 OIDs were deleted and then not vacuumed in time to avoid a conflict. Pavan Deolasee Discussion: https://postgr.es/m/CABOikdOgWT2hHkYG3Wwo2cyZJq2zfs1FH0FgX-=h4OLosXHf9w@mail.gmail.com
* Do not select new object OIDs that match recently-dead entries.Tom Lane2018-04-11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | When selecting a new OID, we take care to avoid picking one that's already in use in the target table, so as not to create duplicates after the OID counter has wrapped around. However, up to now we used SnapshotDirty when scanning for pre-existing entries. That ignores committed-dead rows, so that we could select an OID matching a deleted-but-not-yet-vacuumed row. While that mostly worked, it has two problems: * If recently deleted, the dead row might still be visible to MVCC snapshots, creating a risk for duplicate OIDs when examining the catalogs within our own transaction. Such duplication couldn't be visible outside the object-creating transaction, though, and we've heard few if any field reports corresponding to such a symptom. * When selecting a TOAST OID, deleted toast rows definitely *are* visible to SnapshotToast, and will remain so until vacuumed away. This leads to a conflict that will manifest in errors like "unexpected chunk number 0 (expected 1) for toast value nnnnn". We've been seeing reports of such errors from the field for years, but the cause was unclear before. The fix is simple: just use SnapshotAny to search for conflicting rows. This results in a slightly longer window before object OIDs can be recycled, but that seems unlikely to create any large problems. Pavan Deolasee Discussion: https://postgr.es/m/CABOikdOgWT2hHkYG3Wwo2cyZJq2zfs1FH0FgX-=h4OLosXHf9w@mail.gmail.com
* Allocate enough shared string memory for stats of auxiliary processes.Heikki Linnakangas2018-04-11
| | | | | | | | | | | | | | This fixes a bug whereby the st_appname, st_clienthostname, and st_activity_raw fields for auxiliary processes point beyond the end of their respective shared memory segments. As a result, the application_name of a backend might show up as the client hostname of an auxiliary process. Backpatch to v10, where this bug was introduced, when the auxiliary processes were added to the array. Author: Edmund Horner Reviewed-by: Michael Paquier Discussion: https://www.postgresql.org/message-id/CAMyN-kA7aOJzBmrYFdXcc7Z0NmW%2B5jBaf_m%3D_-77uRNyKC9r%3DA%40mail.gmail.com
* Make local copy of client hostnames in backend status array.Heikki Linnakangas2018-04-11
| | | | | | | | | | | | | The other strings, application_name and query string, were snapshotted to local memory in pgstat_read_current_status(), but we forgot to do that for client hostnames. As a result, the client hostname would appear to change in the local copy, if the client disconnected. Backpatch to all supported versions. Author: Edmund Horner Reviewed-by: Michael Paquier Discussion: https://www.postgresql.org/message-id/CAMyN-kA7aOJzBmrYFdXcc7Z0NmW%2B5jBaf_m%3D_-77uRNyKC9r%3DA%40mail.gmail.com
* Fix ALTER TABLE .. ATTACH PARTITION ... DEFAULTAlvaro Herrera2018-04-11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | If the table being attached contained values that contradict the default partition's partition constraint, it would fail to complain, because CommandCounterIncrement changes in 4dba331cb3dc coupled with some bogus coding in the existing ValidatePartitionConstraints prevented the partition constraint from being validated after all -- or rather, it caused to constraint to become an empty one, always succeeding. Fix by not re-reading the OID of the default partition in ATExecAttachPartition. To forestall similar problems, revise the existing code: * rename routine from ValidatePartitionConstraints() to QueuePartitionConstraintValidation, to better represent what it actually does. * add an Assert() to make sure that when queueing a constraint for a partition we're not overwriting a constraint previously queued. * add an Assert() that we don't try to invoke the special-purpose validation of the default partition when attaching the default partition itself. While at it, change some loops to obtain partition OIDs from partdesc->oids rather than find_all_inheritors; reduce the lock level of partitions being scanned from AccessExclusiveLock to ShareLock; rewrite QueuePartitionConstraintValidation in a recursive fashion rather than repetitive. Author: Álvaro Herrera. Tests written by Amit Langote Reported-by: Rushabh Lathia Diagnosed-by: Kyotaro HORIGUCHI, who also provided the initial fix. Reviewed-by: Kyotaro HORIGUCHI, Amit Langote, Jeevan Ladhe Discussion: https://postgr.es/m/CAGPqQf0W+v-Ci_qNV_5R3A=Z9LsK4+jO7LzgddRncpp_rrnJqQ@mail.gmail.com
* Invoke submake-generated-headers during "make check", too.Tom Lane2018-04-11
| | | | | | | | | | | | | | | | The MAKELEVEL hack to prevent submake-generated-headers from doing anything in child make runs means that we have to explicitly invoke it at top level for "make check", too, in case somebody proceeds directly to that without an explicit "make all". (I think this usage had parallel-make hazards even before the addition of more generated headers; but it was totally broken as of 3b8f6e75f.) Out of paranoia, force the submake-libpq target to depend on submake-generated-headers, too. This seems to not be absolutely necessary today, but it's not really saving us anything to omit the ordering dependency, and it'll likely break someday without it. Discussion: https://postgr.es/m/20180411103930.GB31461@momjian.us
* Temporary revert 5c6110c6a960ad6fe1b0d0fec6ae36ef4eb913f5Teodor Sigaev2018-04-11
| | | | It discovers one more bug in CompareIndexInfo(), should be fixed first.
* Fix interference between cavering indexes and partitioned tablesTeodor Sigaev2018-04-11
| | | | | | | | The bug is caused due to the original IndexStmt that DefineIndex receives being overwritten when processing the INCLUDE columns. Use separate list of index params to propagate to child tables. Add tests covering this case. Amit Langote and Alexander Korotkov.
* minor comment fixes in nbtinsert.cAndrew Dunstan2018-04-10
|
* Fix incorrect close() call in dsm_impl_mmap().Tom Lane2018-04-10
| | | | | | | | | | | | One improbable error-exit path in this function used close() where it should have used CloseTransientFile(). This is unlikely to be hit in the field, and I think the consequences wouldn't be awful (just an elog(LOG) bleat later). But a bug is a bug, so back-patch to 9.4 where this code came in. Pan Bian Discussion: https://postgr.es/m/152056616579.4966.583293218357089052@wrigleys.postgresql.org
* Adjustments to the btree fastpath optimization.Andrew Dunstan2018-04-10
| | | | | | | | | | | | | | | This optimization was introduced in commit 2b272734. The changes include some additional comments and documentation, and also these more substantive changes: . ensure the optimization is only applied on the leaf node of a tree whose root is on level 2 or more. It's of little value on small trees. . Delay calling RelationSetTargetBlock() until after the critical section of _bt_insertonpg . ensure the optimization is also applied to unlogged tables. Pavan Deolasee and Peter Geoghegan with some very light editing from me. Discussion: https://postgr.es/m/CABOikdO8jhRarNC60nZLktZYhxt+TK8z_V97+Ny499YQdyAfug@mail.gmail.com
* Put back parallel-safety guards in plpython and src/test/regress/.Tom Lane2018-04-10
| | | | | | | | | | | | | | | | | | | | | | | | I'd hoped that commit 3b8f6e75f was sufficient to ensure parallel safety even when a build started in a subdirectory requires rebuilding of generated headers. This isn't so, because making submake-generated-headers a prerequisite of "all" isn't enough to ensure it's completed before starting on "all"'s other prerequisites. The explicit dependencies we put on the recursive make targets ensure safe ordering before we recurse into child directories, but they don't protect targets to be made in the current directory. Hence, put back some ordering dependencies in directories that we've traditionally expected to be starting points for "standalone" builds, to wit src/pl/plpython and src/test/regress. (The former needs this in order to minimize the work involved in building for both python 2 and python 3; the latter to support packagings that make the regression tests available for out-of-build-tree execution.) Adjust some other dependencies so that these two cases work correctly even at high -j settings. I'm not terribly happy with this partial solution, but I don't see a way to do better without massive makefile restructuring, which we surely aren't doing at this point in the development cycle. In any case, it's little if any worse than what we had in prior releases. Discussion: https://postgr.es/m/1523353963.8169.26.camel@gunduz.org
* Fix IndexOnlyScan counter for heap fetches in parallel modeAlvaro Herrera2018-04-10
| | | | | | | | | | | | The HeapFetches counter was using a simple value in IndexOnlyScanState, which fails to propagate values from parallel workers; so the counts are wrong when IndexOnlyScan runs in parallel. Move it to Instrumentation, like all the other counters. While at it, change INSERT ON CONFLICT conflicting tuple counter to use the new ntuples2 instead of nfiltered2, which is a blatant misuse. Discussion: https://postgr.es/m/20180409215851.idwc75ct2bzi6tea@alvherre.pgsql
* Fix pgxs.mk to not try to build generated headers in external builds.Tom Lane2018-04-10
| | | | | | | | | | | | | | | | Per Julien Rouhaud and the buildfarm. This is not quite Julien's patch: there's no need to lobotomize this build rule when building contrib modules in-tree, so set NO_GENERATED_HEADERS only if PGXS. In passing, also set NO_TEMP_INSTALL in external builds. This doesn't seem to be fixing any live bug, because "make check" in an external build just produces the expected error message without first trying to make a temp install ... but it's far from obvious why it doesn't, so this change seems like good future-proofing. Julien Rouhaud and Tom Lane Discussion: https://postgr.es/m/CAOBaU_YH=g68opbbMk8is3jNwhoXGa8ckRSre1nx0Obe1C7i-Q@mail.gmail.com
* Fix comment on B-tree insertion fastpath condition.Heikki Linnakangas2018-04-10
| | | | | | The comment earlier in the function correctly states "and the insertion key is strictly greater than the first key in this page". That is what we check here, not "greater than or equal".
* Fix partial-build problems introduced by having more generated headers.Tom Lane2018-04-09
| | | | | | | | | | | | | | | | | | | | Commit 372728b0d created some problems for usages like building a subdirectory without having first done "make all" at the top level, or for proceeding directly to "make install" without "make all". The only reasonably clean way to fix this seems to be to force the submake-generated-headers rule to fire in *any* "make all" or "make install" command anywhere in the tree. To avoid lots of redundant work, as well as parallel make jobs possibly clobbering each others' output, we still need to be sure that the rule fires only once in a recursive build. For that, adopt the same MAKELEVEL hack previously used for "temp-install". But try to document it a bit better. The submake-errcodes mechanism previously used in src/port/ and src/common/ is subsumed by this, so we can get rid of those special cases. It was inadequate for src/common/ anyway after the aforesaid commit, and it always risked parallel attempts to build errcodes.h. Discussion: https://postgr.es/m/E1f5FAB-0006LU-MB@gemulon.postgresql.org
* Fix incorrect logic for choosing the next Parallel Append subplanAlvaro Herrera2018-04-09
| | | | | | | | | | | | | | | | | In 499be013de support for pruning unneeded Append subnodes was added. The logic in that commit was not correctly checking if the next subplan was in fact a valid subplan. This could cause parallel workers processes to be given a subplan to work on which didn't require any work. Per code review following an otherwise unexplained regression failure in buildfarm member Pademelon. (We haven't been able to reproduce the failure, so this is a bit of a blind fix in terms of whether it'll actually fix it; but it is a clear bug nonetheless). In passing, also add a comment to explain what first_partial_plan means. Author: David Rowley Discussion: https://postgr.es/m/CAKJS1f_E5r05hHUVG3UmCQJ49DGKKHtN=SHybD44LdzBn+CJng@mail.gmail.com
* Silence some warnings in TAP testsMagnus Hagander2018-04-09
| | | | Author: Michael Paquier
* Make sure pg_rewind can't run as rootMagnus Hagander2018-04-09
| | | | | | | | Previously a warning was printed, but the tool actually kept running even when running as root. This is something we definitely want to prevent, but since this means a behavior change, not backpatching. Author: Michael Paquier
* Reduce chattiness of genbki.pl and Gen_fmgrtab.pl.Tom Lane2018-04-09
| | | | | | | | | | | Make these scripts emit just one log message when they run, not one per output file. The latter is way too verbose in the wake of commit 372728b0d. The specific wording used is what already existed in the MSVC scripts. John Naylor Discussion: https://postgr.es/m/11103.1523208822@sss.pgh.pa.us
* Make reformat_dat_file.pl preserve all blank lines.Tom Lane2018-04-09
| | | | | | | | | | | | | | | In its original form, reformat_dat_file.pl smashed consecutive blank lines to a single blank line, which was helpful for mopping up excess whitespace during the bootstrap data format conversion. But going forward, there seems little reason to do that; if developers want to put in multiple blank lines, let 'em. This makes it conform to the documentation I (tgl) wrote, too. In passing, clean up some sloppy markup choices in bki.sgml. John Naylor Discussion: https://postgr.es/m/28827.1523039259@sss.pgh.pa.us
* Further cleanup of client dependencies on src/include/catalog headers.Tom Lane2018-04-09
| | | | | | | | | | | | | | | | | In commit 9c0a0de4c, I'd failed to notice that catalog/catalog.h should also be considered a frontend-unsafe header, because it includes (and needs) the full form of pg_class.h, not to mention relcache.h. However, various frontend code was depending on it to get TABLESPACE_VERSION_DIRECTORY, so refactoring of some sort is called for. The cleanest answer seems to be to move TABLESPACE_VERSION_DIRECTORY, as well as the OIDCHARS symbol, to common/relpath.h. Do that, and mop up inclusions as necessary. (I found that quite a few current users of catalog/catalog.h don't seem to need it at all anymore, apparently as a result of the refactorings that created common/relpath.[hc]. And initdb.c needed it only as a route to pg_class_d.h.) Discussion: https://postgr.es/m/6629.1523294509@sss.pgh.pa.us
* catversion bump for online-checksums revertMagnus Hagander2018-04-09
| | | | Lack thereof pointed out by Tom Lane.
* Revert "Allow on-line enabling and disabling of data checksums"Magnus Hagander2018-04-09
| | | | | | | | This reverts the backend sides of commit 1fde38beaa0c3e66c340efc7cc0dc272d6254bb0. I have, at least for now, left the pg_verify_checksums tool in place, as this tool can be very valuable without the rest of the patch as well, and since it's a read-only tool that only runs when the cluster is down it should be a lot safer.
* Minor comment updatesAlvaro Herrera2018-04-09
| | | | | | | | Fix a couple of typos, and update a comment about why we set a BMS to NULL. Author: David Rowley Discussion: http://postgr.es/m/CAKJS1f-tux=KdUz6ENJ9GHM_V2qgxysadYiOyQS9Ko9PTteVhQ@mail.gmail.com
* Add missed bms_copy() in perform_pruning_combine_stepAlvaro Herrera2018-04-09
| | | | | | | | | | | We were initializing a BMS to merely reference an existing one, which would cause a double-free (and a crash) when the recursive algorithm tried to intersect it with an empty one. Fix it by creating a copy at initialization time. Reported-by: sqlsmith (by way of Andreas Seltenreich) Author: Amit Langote Discussion: https://postgr.es/m/87in923lyw.fsf@ansel.ydns.eu
* Fix typo in comment.Heikki Linnakangas2018-04-09
| | | | Author: Kyotaro Horiguchi
* Skip permissions test under MINGW/WindowsStephen Frost2018-04-08
| | | | | | | We don't support the same kind of permissions tests on Windows/MINGW, so these tests really shouldn't be getting run on that platform. Per buildfarm.
* Fix additional breakage in covering-index patch.Tom Lane2018-04-08
| | | | | | | | | | | | | | | | | | | | | | | CheckIndexCompatible() misused ComputeIndexAttrs() by not bothering to fill ii_NumIndexAttrs and ii_NumIndexKeyAttrs in the passed IndexInfo. Omission of ii_NumIndexAttrs was previously unimportant, but now this matters because ComputeIndexAttrs depends on ii_NumIndexKeyAttrs to decide how many columns it needs to report on. (BTW, the fact that this oversight wasn't detected earlier implies that we have no regression test verifying whether CheckIndexCompatible ever succeeds. Bad dog. Not the job of this patch to fix it, though.) Also, change the API of ComputeIndexAttrs so that it fills the opclass output array for all column positions, as it does for the options output array; positions for non-key index columns are filled with zeroes. This isn't directly fixing any bug, but it seems like a good idea. Per valgrind failure reports from buildfarm. Alexander Korotkov, tweaked a bit by me Discussion: https://postgr.es/m/CAPpHfduWrysrT-qAhn+3Ea5+Mg6Vhc-oA6o2Z-hRCPRdvf3tiw@mail.gmail.com
* Cosmetic cleanups in initial catalog data.Tom Lane2018-04-08
| | | | | | | | | | | | | | Write ',' and ';' for typdelim values instead of the obscurantist ASCII octal equivalents. Not sure why anybody ever thought the latter were better; maybe it had something to do with lack of a better quoting convention, twenty-plus years ago? Reassign a couple of high-numbered OIDs that were left in during yesterday's mad rush to commit stuff of uncertain internal temperature. The latter requires a catversion bump, though the former wouldn't since the end-result catalog data is unchanged.
* Reduce worst-case shell command line length during "make install".Tom Lane2018-04-08
| | | | | | | | | | | | | Addition of the catalog/pg_foo_d.h headers seems to have pushed us over the brink of the maximum command line length for some older platforms during "make install" for our header files. The main culprit here is repetition of the target directory path, which could be long. Rearrange so that we don't repeat that once per file, but only once per subdirectory. Per buildfarm. Discussion: https://postgr.es/m/E1f5Dwm-0004n5-7O@gemulon.postgresql.org
* Merge catalog/pg_foo_fn.h headers back into pg_foo.h headers.Tom Lane2018-04-08
| | | | | | | | | | | | | Traditionally, include/catalog/pg_foo.h contains extern declarations for functions in backend/catalog/pg_foo.c, in addition to its function as the authoritative definition of the pg_foo catalog's rowtype. In some cases, we'd been forced to split out those extern declarations into separate pg_foo_fn.h headers so that the catalog definitions could be #include'd by frontend code. That problem is gone as of commit 9c0a0de4c, so let's undo the splits to make things less confusing. Discussion: https://postgr.es/m/23690.1523031777@sss.pgh.pa.us
* Switch client-side code to include catalog/pg_foo_d.h not pg_foo.h.Tom Lane2018-04-08
| | | | | | | | | | | | | | Everything of use to frontend code should now appear in the _d.h files, and making this change frees us from needing to worry about whether the catalog header files proper are frontend-safe. Remove src/interfaces/ecpg/ecpglib/pg_type.h entirely, as the previous commit reduced it to a confusingly-named wrapper around pg_type_d.h. In passing, make test_rls_hooks.c follow project convention of including our own files with #include "" not <>. Discussion: https://postgr.es/m/23690.1523031777@sss.pgh.pa.us
* Replace our traditional initial-catalog-data format with a better design.Tom Lane2018-04-08
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Historically, the initial catalog data to be installed during bootstrap has been written in DATA() lines in the catalog header files. This had lots of disadvantages: the format was badly underdocumented, it was very difficult to edit the data in any mechanized way, and due to the lack of any abstraction the data was verbose, hard to read/understand, and easy to get wrong. Hence, move this data into separate ".dat" files and represent it in a way that can easily be read and rewritten by Perl scripts. The new format is essentially "key => value" for each column; while it's a bit repetitive, explicit labeling of each value makes the data far more readable and less error-prone. Provide a way to abbreviate entries by omitting field values that match a specified default value for their column. This allows removal of a large amount of repetitive boilerplate and also lowers the barrier to adding new columns. Also teach genbki.pl how to translate symbolic OID references into numeric OIDs for more cases than just "regproc"-like pg_proc references. It can now do that for regprocedure-like references (thus solving the problem that regproc is ambiguous for overloaded functions), operators, types, opfamilies, opclasses, and access methods. Use this to turn nearly all OID cross-references in the initial data into symbolic form. This represents a very large step forward in readability and error resistance of the initial catalog data. It should also reduce the difficulty of renumbering OID assignments in uncommitted patches. Also, solve the longstanding problem that frontend code that would like to use OID macros and other information from the catalog headers often had difficulty with backend-only code in the headers. To do this, arrange for all generated macros, plus such other declarations as we deem fit, to be placed in "derived" header files that are safe for frontend inclusion. (Once clients migrate to using these pg_*_d.h headers, it will be possible to get rid of the pg_*_fn.h headers, which only exist to quarantine code away from clients. That is left for follow-on patches, however.) The now-automatically-generated macros include the Anum_xxx and Natts_xxx constants that we used to have to update by hand when adding or removing catalog columns. Replace the former manual method of generating OID macros for pg_type entries with an automatic method, ensuring that all built-in types have OID macros. (But note that this patch does not change the way that OID macros for pg_proc entries are built and used. It's not clear that making that match the other catalogs would be worth extra code churn.) Add SGML documentation explaining what the new data format is and how to work with it. Despite being a very large change in the catalog headers, there is no catversion bump here, because postgres.bki and related output files haven't changed at all. John Naylor, based on ideas from various people; review and minor additional coding by me; previous review by Alvaro Herrera Discussion: https://postgr.es/m/CAJVSVGWO48JbbwXkJz_yBFyGYW-M9YWxnPdxJBUosDC9ou_F0Q@mail.gmail.com
* match_clause_to_index should check only key columnsTeodor Sigaev2018-04-08
| | | | | Alexander Korotkov per gripe from Tom Lane noticed on valgrind-enabled buildfarm members
* Remove unused variable in non-assert-enabled buildTeodor Sigaev2018-04-08
| | | | | | Use field of structure in Assert directly Jeff Janes
* Attempt to stabilize partition_prune test output.Andrew Gierth2018-04-08
| | | | | | | | | Disable index-only scan for tests that might report variable results for "Heap Fetches" statistic due to concurrent transactions affecting whether all-visible flags can be set. Author: David Rowley Discussion: https://postgr.es/m/CAKJS1f_yjtHDJnDzx1uuR_3D7beDVAkNQfWJhRLA1gvPCzkAhg@mail.gmail.com