aboutsummaryrefslogtreecommitdiff
path: root/src/backend
Commit message (Collapse)AuthorAge
* Remove superfluous tqual.h includes.Andres Freund2019-01-21
| | | | | | | | | | | | Most of these had been obsoleted by 568d4138c / the SnapshotNow removal. This is is preparation for moving most of tqual.[ch] into either snapmgr.h or heapam.h, which in turn is in preparation for pluggable table AMs. Author: Andres Freund Discussion: https://postgr.es/m/20180703070645.wchpu5muyto5n647@alap3.anarazel.de
* Replace uses of heap_open et al with the corresponding table_* function.Andres Freund2019-01-21
| | | | | Author: Andres Freund Discussion: https://postgr.es/m/20190111000539.xbv7s6w7ilcvm7dp@alap3.anarazel.de
* Replace heapam.h includes with {table, relation}.h where applicable.Andres Freund2019-01-21
| | | | | | | | | A lot of files only included heapam.h for relation_open, heap_open etc - replace the heapam.h include in those files with the narrower header. Author: Andres Freund Discussion: https://postgr.es/m/20190111000539.xbv7s6w7ilcvm7dp@alap3.anarazel.de
* Introduce access/{table.h, relation.h}, for generic functions from heapam.h.Andres Freund2019-01-21
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | access/heapam contains functions that are very storage specific (say heap_insert() and a lot of lower level functions), and fairly generic infrastructure like relation_open(), heap_open() etc. In the upcoming pluggable storage work we're introducing a layer between table accesses in general and heapam, to allow for different storage methods. For a bit cleaner separation it thus seems advantageous to move generic functions like the aforementioned to their own headers. access/relation.h will contain relation_open() etc, and access/table.h will contain table_open() (formerly known as heap_open()). I've decided for table.h not to include relation.h, but we might change that at a later stage. relation.h already exists in another directory, but the other plausible name (rel.h) also conflicts. It'd be nice if there were a non-conflicting name, but nobody came up with a suggestion. It's possible that the appropriate way to address the naming conflict would be to rename nodes/relation.h, which isn't particularly well named. To avoid breaking a lot of extensions that just use heap_open() etc, table.h has macros mapping the old names to the new ones, and heapam.h includes relation, table.h. That also allows to keep the bulk renaming of existing callers in a separate commit. Author: Andres Freund Discussion: https://postgr.es/m/20190111000539.xbv7s6w7ilcvm7dp@alap3.anarazel.de
* Sort the dependent objects before recursing in findDependentObjects().Tom Lane2019-01-21
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Historically, the notices output by DROP CASCADE tended to come out in uncertain order, and in some cases you might get different claims about which object depends on which other one. This is because we just traversed the dependency tree in the order in which pg_depend entries are seen, and nbtree has never promised anything about the order of equal-keyed index entries. We've put up with that for years, hacking regression tests when necessary to prevent them from emitting unstable output. However, it's a problem for pending work that will change nbtree's behavior for equal keys, as that causes unexpected changes in the regression test results. Hence, adjust findDependentObjects to sort the results of each indexscan before processing them. The sort is on descending OID of the dependent objects, hence more or less reverse creation order. While this rule could still result in bogus regression test failures if an OID wraparound occurred mid-test, that seems unlikely to happen in any plausible development or packaging-test scenario. This is enough to ensure output stability for ordinary DROP CASCADE commands, but not for DROP OWNED BY, because that has a different code path with the same problem. We might later choose to sort in the DROP OWNED BY code as well, but this patch doesn't do so. I've also not done anything about reverting the existing hacks to suppress unstable DROP CASCADE output in specific regression tests. It might be worth undoing those, but it seems like a distinct question. The first indexscan loop in findDependentObjects is not touched, meaning there is a hazard of unstable error reports from that too. However, said hazard is not the fault of that code: it was designed on the assumption that there could be at most one "owning" object to complain about, and that assumption does not seem unreasonable. The recent patch that added the possibility of multiple DEPENDENCY_INTERNAL_AUTO links broke that assumption, but we should fix that situation not band-aid around it. That's a matter for another patch, though. Discussion: https://postgr.es/m/12244.1547854440@sss.pgh.pa.us
* Fix ALTER TRIGGER ... RENAME, broken in WITH OIDS removal.Andres Freund2019-01-21
| | | | | | | I (Andres) broke this in 578b229718e. Author: Rushabh Lathia Discussion: https://postgr.es/m/CAGPqQf04PywZX3sVQaF6H=oLiW9GJncRW+=e78vTy4MokEWcZw@mail.gmail.com
* Adjust some more comments for WITH OIDS removal.Andres Freund2019-01-21
| | | | | | I missed these in 578b229718e8f. Author: Andres Freund
* Postpone generating tlists and EC members for inheritance dummy children.Etsuro Fujita2019-01-21
| | | | | | | | | | | | | | | | | | | | | Previously, in set_append_rel_size(), we generated tlists and EC members for dummy children for possible use by partition-wise join, even if partition-wise join was disabled or the top parent was not a partitioned table, but adding such EC members causes noticeable planning speed degradation for queries with certain kinds of join quals like "(foo.x + bar.y) = constant" where foo and bar are partitioned tables in cases where there are lots of dummy children, as the EC members lists grow huge, especially for the ECs derived from such join quals, which makes the search for the parent EC members in add_child_rel_equivalences() very time-consuming. Postpone the work until such children are actually involved in a partition-wise join. Reported-by: Sanyo Capobiango Analyzed-by: Justin Pryzby and Alvaro Herrera Author: Amit Langote, with a few additional changes by me Reviewed-by: Ashutosh Bapat Backpatch-through: v11 where partition-wise join was added Discussion: https://postgr.es/m/CAO698qZnrxoZu7MEtfiJmpmUtz3AVYFVnwzR%2BpqjF%3DrmKBTgpw%40mail.gmail.com
* Allow COPY FROM to filter data using WHERE conditionsTomas Vondra2019-01-20
| | | | | | | | | | | | | Extends the COPY FROM command with a WHERE condition, which allows doing various types of filtering while importing the data (random sampling, condition on a data column, etc.). Until now such filtering required either preprocessing of the input data, or importing all data and then filtering in the database. COPY FROM ... WHERE is an easy-to-use and low-overhead alternative for most simple cases. Author: Surafel Temesgen Reviewed-by: Tomas Vondra, Masahiko Sawada, Lim Myungkyu Discussion: https://www.postgresql.org/message-id/flat/CALAY4q_DdpWDuB5-Zyi-oTtO2uSk8pmy+dupiRe3AvAc++1imA@mail.gmail.com
* Replace @postgresql.org with @lists.postgresql.org for mailinglistsMagnus Hagander2019-01-19
| | | | | | Commit c0d0e54084 replaced the ones in the documentation, but missed out on the ones in the code. Replace those as well, but unlike c0d0e54084, don't backpatch the code changes to avoid breaking translations.
* Fix outdated commentPeter Eisentraut2019-01-19
| | | | | The issue the comment is referring to was fixed by 08859bb5c2cebc132629ca838113d27bb31b990c.
* Fix creation of duplicate foreign keys on partitionsAlvaro Herrera2019-01-18
| | | | | | | | | | | | | | | | | | When creating a foreign key in a partitioned table, if some partitions already have equivalent constraints, we wastefully create duplicates of the constraints instead of attaching to the existing ones. That's inconsistent with the de-duplication that is applied when a table is attached as a partition. To fix, reuse the FK-cloning code instead of having a separate code path. Backpatch to Postgres 11. This is a subtle behavior change, but surely a welcome one since there's no use in having duplicate foreign keys. Discovered by Álvaro Herrera while thinking about a different problem reported by Jesper Pedersen (bug #15587). Author: Álvaro Herrera Discussion: https://postgr.es/m/201901151935.zfadrzvyof4k@alvherre.pgsql
* Move CloneForeignKeyConstraints to tablecmds.cAlvaro Herrera2019-01-18
| | | | | | | | | | | | | | | My commit 3de241dba86f introduced some code to create a clone of a foreign key to a partition, but I put it in pg_constraint.c because it was too close to the contents of the pg_constraint row. With the previous commit that split out the constraint tuple deconstruction into its own routine, it makes more sense to have the FK-cloning function in tablecmds.c, mostly because its static subroutine can then be used by a future bugfix. My initial posting of this patch had this routine as static in tablecmds.c, but sadly this function is already part of the Postgres 11 ABI as exported from pg_constraint.c, so keep it as exported also just to avoid breaking any possible users of it.
* Refactor duplicate code into DeconstructFkConstraintRowAlvaro Herrera2019-01-18
| | | | | | | | | | | My commit 3de241dba86f introduced some code (in tablecmds.c) to obtain data from a pg_constraint row for a foreign key, that already existed in ri_triggers.c. Split it out into its own routine in pg_constraint.c, where it naturally belongs. No functional code changes, only code movement. Backpatch to pg11, because a future bugfix is simpler after this.
* Avoid sometimes printing both tables and their columns in DROP CASCADE.Tom Lane2019-01-18
| | | | | | | | | | | | | | | | | A cascaded drop might find independent reasons to drop both a table and some column of the table (for instance, a schema drop might include dropping a data type used in some table in the schema). Depending on the order of visitation of pg_depend entries, we might report the table column and the whole table as separate objects-to-be-dropped, or we might only report the table. This is confusing and leads to unstable regression test output, so fix it to report only the table regardless of visitation order. Per gripe from Peter Geoghegan. This is just cosmetic from a user's standpoint, and we haven't actually seen regression test problems in practice (yet), so I'll refrain from back-patching. Discussion: https://postgr.es/m/15908.1547762076@sss.pgh.pa.us
* Fix incorrect relation name in comment of vacuumlazy.cMichael Paquier2019-01-18
| | | | | Author: Masahiko Sawada Discussion: https://postgr.es/m/CAD21AoBiOiapB7YGbWRfNZji3cs1gkEwv=uGLTemaZ9yNKK1DA@mail.gmail.com
* Restrict the use of temporary namespace in two-phase transactionsMichael Paquier2019-01-18
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Attempting to use a temporary table within a two-phase transaction is forbidden for ages. However, there have been uncovered grounds for a couple of other object types and commands which work on temporary objects with two-phase commit. In short, trying to create, lock or drop an object on a temporary schema should not be authorized within a two-phase transaction, as it would cause its state to create dependencies with other sessions, causing all sorts of side effects with the existing session or other sessions spawned later on trying to use the same temporary schema name. Regression tests are added to cover all the grounds found, the original report mentioned function creation, but monitoring closer there are many other patterns with LOCK, DROP or CREATE EXTENSION which are involved. One of the symptoms resulting in combining both is that the session which used the temporary schema is not able to shut down completely, waiting for being able to drop the temporary schema, something that it cannot complete because of the two-phase transaction involved with temporary objects. In this case the client is able to disconnect but the session remains alive on the backend-side, potentially blocking connection backend slots from being used. Other problems reported could also involve server crashes. This is back-patched down to v10, which is where 9b013dc has introduced MyXactFlags, something that this patch relies on. Reported-by: Alexey Bashtanov Author: Michael Paquier Reviewed-by: Masahiko Sawada Discussion: https://postgr.es/m/5d910e2e-0db8-ec06-dd5f-baec420513c3@imap.cc Backpatch-through: 10
* Postpone aggregate checks until after collation is assigned.Andrew Gierth2019-01-17
| | | | | | | | | | | | | | | | | | | | | | | Previously, parseCheckAggregates was run before assign_query_collations, but this causes problems if any expression has already had a collation assigned by some transform function (e.g. transformCaseExpr) before parseCheckAggregates runs. The differing collations would cause expressions not to be recognized as equal to the ones in the GROUP BY clause, leading to spurious errors about unaggregated column references. The result was that CASE expr WHEN val ... would fail when "expr" contained a GROUPING() expression or matched one of the group by expressions, and where collatable types were involved; whereas the supposedly identical CASE WHEN expr = val ... would succeed. Backpatch all the way; this appears to have been wrong ever since collations were introduced. Per report from Guillaume Lelarge, analysis and patch by me. Discussion: https://postgr.es/m/CAECtzeVSO_US8C2Khgfv54ZMUOBR4sWq+6_bLrETnWExHT=rFg@mail.gmail.com Discussion: https://postgr.es/m/87muo0k0c7.fsf@news-spur.riddles.org.uk
* Reorganize planner code moved in b60c39759908Alvaro Herrera2019-01-16
| | | | | | | | It seems modules are better defined like this instead of the original split. Per complaints from David Rowley as well as Amit Langote's self review. Discussion: https://postgr.es/m/CAKJS1f988rsyhwvLgfT-y1UCYUfXDOv67ENQk=v24OxhsZOzZw@mail.gmail.com
* Don't duplicate parallel seqscan shmem sizing logic in nbtree.Andres Freund2019-01-15
| | | | | | | | | | | This is architecturally mildly problematic, which becomes more pronounced with the upcoming introduction of pluggable storage. To fix, teach heap_parallelscan_estimate() to deal with SnapshotAny snapshots, and then use it from _bt_parallel_estimate_shared(). Author: Andres Freund Discussion: https://postgr.es/m/20180703070645.wchpu5muyto5n647@alap3.anarazel.de
* Move vacuumlazy.c into access/heap.Andres Freund2019-01-15
| | | | | | | | It's heap table storage specific code that can't realistically be generalized into table AM agnostic code. Author: Andres Freund Discussion: https://postgr.es/m/20180703070645.wchpu5muyto5n647@alap3.anarazel.de
* Fix parent of WCO qual.Andres Freund2019-01-15
| | | | | | | | | | | | | | The parent of some WCO expressions was, apparently by accident, set to the the source of DML queries, rather than the target table. This causes problems for the upcoming pluggable storage work, because the target and source table might be of different storage types. It's possible that this is already problematic, but neither experimenting nor inquiries on -hackers have found them. So don't backpatch for now. Author: Andres Freund Discussion: https://postgr.es/m/20181205225213.hiwa3kgoxeybqcqv@alap3.anarazel.de
* Finish reverting "recheck_on_update" patch.Tom Lane2019-01-15
| | | | | | | | | | | | 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
* Don't include genam.h from execnodes.h and relscan.h anymore.Andres Freund2019-01-14
| | | | | | | | | | | | | | | | | | | This is the genam.h equivalent of 4c850ecec649c (which removed heapam.h from a lot of other headers). There's still a few header includes of genam.h, but not from central headers anymore. As a few headers are not indirectly included anymore, execnodes.h and relscan.h need a few additional includes. Some of the depended on types were replacable by using the underlying structs, but e.g. for Snapshot in execnodes.h that'd have gotten more invasive than reasonable in this commit. Like the aforementioned commit 4c850ecec649c, this requires adding new genam.h includes to a number of backend files, which likely is also required in a few external projects. Author: Andres Freund Discussion: https://postgr.es/m/20190114000701.y4ttcb74jpskkcfb@alap3.anarazel.de
* Make naming of tupdesc related structs more consistent with the rest of PG.Andres Freund2019-01-14
| | | | | | | | | | | | | | We usually don't change the name of structs between the struct name itself and the name of the typedef. Additionally, structs that are usually used via a typedef that hides being a pointer, are commonly suffixed Data. Change tupdesc code to follow those convention. This is triggered by a future patch that intends to forward declare TupleDescData in another header - keeping with the naming scheme makes that easier to understand. Author: Andres Freund Discussion: https://postgr.es/m/20190114000701.y4ttcb74jpskkcfb@alap3.anarazel.de
* Don't include heapam.h from others headers.Andres Freund2019-01-14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | heapam.h previously was included in a number of widely used headers (e.g. execnodes.h, indirectly in executor.h, ...). That's problematic on its own, as heapam.h contains a lot of low-level details that don't need to be exposed that widely, but becomes more problematic with the upcoming introduction of pluggable table storage - it seems inappropriate for heapam.h to be included that widely afterwards. heapam.h was largely only included in other headers to get the HeapScanDesc typedef (which was defined in heapam.h, even though HeapScanDescData is defined in relscan.h). The better solution here seems to be to just use the underlying struct (forward declared where necessary). Similar for BulkInsertState. Another problem was that LockTupleMode was used in executor.h - parts of the file tried to cope without heapam.h, but due to the fact that it indirectly included it, several subsequent violations of that goal were not not noticed. We could just reuse the approach of declaring parameters as int, but it seems nicer to move LockTupleMode to lockoptions.h - that's not a perfect location, but also doesn't seem bad. As a number of files relied on implicitly included heapam.h, a significant number of files grew an explicit include. It's quite probably that a few external projects will need to do the same. Author: Andres Freund Reviewed-By: Alvaro Herrera Discussion: https://postgr.es/m/20190114000701.y4ttcb74jpskkcfb@alap3.anarazel.de
* Fix typos in documentation and for one wait eventMichael Paquier2019-01-15
| | | | | | | | | These have been found while cross-checking for the use of unique words in the documentation, and a wait event was not getting generated in a way consistent to what the documentation provided. Author: Alexander Lakhin Discussion: https://postgr.es/m/9b5a3a85-899a-ae62-dbab-1e7943aa5ab1@gmail.com
* Re-add default_with_oids GUC to avoid breaking old dump files.Andres Freund2019-01-14
| | | | | | | | | | | | | | After 578b229718 / the removal of WITH OIDS support, older dump files containing SET default_with_oids = false; either report unnecessary errors (as the subsequent tables have no oids) or even fail to restore entirely (when using transaction mode). To avoid that, re-add the GUC, but don't allow setting it to true. Per complaint from Tom Lane. Author: Amit Khandekar, editorialized by me Discussion: https://postgr.es/m/CAJ3gD9dZyxrtL0rJfoNoOj6v7fJSDaXBngi9wy5XU8m-ioXhAA@mail.gmail.com
* Fix unique INCLUDE indexes on partitioned tablesAlvaro Herrera2019-01-14
| | | | | | | | | | We were considering the INCLUDE columns as part of the key, allowing unicity-violating rows to be inserted in different partitions. Concurrent development conflict in eb7ed3f30634 and 8224de4f42cc. Reported-by: Justin Pryzby Discussion: https://postgr.es/m/20190109065109.GA4285@telsasoft.com
* Fix error message for logical replication targetsMichael Paquier2019-01-13
| | | | | | This fixes an oversight from 373bda6. Noted by Erik Rijkers.
* Change default of recovery_target_timeline to 'latest'Peter Eisentraut2019-01-13
| | | | | | | | | This is what one usually wants for recovery and almost always wants for a standby. Discussion: https://www.postgresql.org/message-id/flat/6dd2c23a-4162-8469-410f-bfe146e28c0c@2ndquadrant.com/ Reviewed-by: David Steele <david@pgmasters.net> Reviewed-by: Michael Paquier <michael@paquier.xyz>
* Improve error messages for incorrect types of logical replication targetsMichael Paquier2019-01-13
| | | | | | | | | | | If trying to use something else than a plain table as logical replication target, a rather-generic error message gets used to report the problem. This can be confusing when it comes to foreign tables and partitioned tables, so use more dedicated messages in these cases. Author: Amit Langote Reviewed-by: Peter Eisentraut, Magnus Hagander, Michael Paquier Discussion: https://postgr.es/m/41799bee-40eb-7bb5-80b1-325ce17518bc@lab.ntt.co.jp
* Remove heapam.h include made superfluous by b60c3975990.Andres Freund2019-01-12
| | | | | | Noticed this while working on another patch. Author: Andres Freund
* Free pre-modification HeapTuple in ALTER TABLE ... TYPE ...Andrew Dunstan2019-01-11
| | | | | | | | This was an oversight in commit 3b174b1a3. Per offline gripe from Alvaro Herrera Backpatch to release 11.
* Avoid sharing PARAM_EXEC slots between different levels of NestLoop.Tom Lane2019-01-11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Up to now, createplan.c attempted to share PARAM_EXEC slots for NestLoopParams across different plan levels, if the same underlying Var was being fed down to different righthand-side subplan trees by different NestLoops. This was, I think, more of an artifact of using subselect.c's PlannerParamItem infrastructure than an explicit design goal, but anyway that was the end result. This works well enough as long as the plan tree is executing synchronously, but the feature whereby Gather can execute the parallelized subplan locally breaks it. An upper NestLoop node might execute for a row retrieved from a parallel worker, and assign a value for a PARAM_EXEC slot from that row, while the leader's copy of the parallelized subplan is suspended with a different active value of the row the Var comes from. When control eventually returns to the leader's subplan, it gets the wrong answers if the same PARAM_EXEC slot is being used within the subplan, as reported in bug #15577 from Bartosz Polnik. This is pretty reminiscent of the problem fixed in commit 46c508fbc, and the proper fix seems to be the same: don't try to share PARAM_EXEC slots across different levels of controlling NestLoop nodes. This requires decoupling NestLoopParam handling from PlannerParamItem handling, although the logic remains somewhat similar. To avoid bizarre division of labor between subselect.c and createplan.c, I decided to move all the param-slot-assignment logic for both cases out of those files and put it into a new file paramassign.c. Hopefully it's a bit better documented now, too. A regression test case for this might be nice, but we don't know a test case that triggers the problem with a suitably small amount of data. Back-patch to 9.6 where we added Gather nodes. It's conceivable that related problems exist in older branches; but without some evidence for that, I'll leave the older branches alone. Discussion: https://postgr.es/m/15577-ca61ab18904af852@postgresql.org
* Add value 'current' for recovery_target_timelinePeter Eisentraut2019-01-11
| | | | | | | | | | | | | This value represents the default behavior of using the current timeline. Previously, this was represented by an empty string. (Before the removal of recovery.conf, this setting could not be chosen explicitly but was used when recovery_target_timeline was not mentioned at all.) Discussion: https://www.postgresql.org/message-id/flat/6dd2c23a-4162-8469-410f-bfe146e28c0c@2ndquadrant.com/ Reviewed-by: David Steele <david@pgmasters.net> Reviewed-by: Michael Paquier <michael@paquier.xyz>
* Fix missing values when doing ALTER TABLE ALTER COLUMN TYPEAndrew Dunstan2019-01-10
| | | | | | | | | | | | | | This was an oversight in commit 16828d5c. If the table is going to be rewritten, we simply clear all the missing values from all the table's attributes, since there will no longer be any rows with the attributes missing. Otherwise, we repackage the missing value in an array constructed with the new type specifications. Backpatch to release 11. This fixes bug #15446, reported by Dmitry Molotkov Reviewed by Dean Rasheed
* Fix C++ compile failures in headers.Tom Lane2019-01-10
| | | | | | | | | | | | | 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.
* Move inheritance expansion code into its own fileAlvaro Herrera2019-01-10
| | | | | | | | | | | | | | | | | This commit moves expand_inherited_tables and underlings from optimizer/prep/prepunionc.c to optimizer/utils/inherit.c. Also, all of the AppendRelInfo-based expression manipulation routines are moved to optimizer/utils/appendinfo.c. No functional code changes. One exception is the introduction of make_append_rel_info, but that's still just moving around code. Also, stop including <limits.h> in prepunion.c, which no longer needs it since 3fc6e2d7f5b6. I (Álvaro) noticed this because Amit was copying that to inherit.c, which likewise doesn't need it. Author: Amit Langote Discussion: https://postgr.es/m/3be67028-a00a-502c-199a-da00eec8fb6e@lab.ntt.co.jp
* Fix grammar mistakes in md.cMichael Paquier2019-01-10
| | | | | Author: Kirk Jamison Discussion: https://postgr.es/m/D09B13F772D2274BB348A310EE3027C640AC54@g01jpexmbkw24
* Reduce the size of the fmgr_builtin_oid_index[] array.Tom Lane2019-01-09
| | | | | | | | | | | | | | | | | 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
* Replace the data structure used for keyword lookup.Tom Lane2019-01-06
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Previously, ScanKeywordLookup was passed an array of string pointers. This had some performance deficiencies: the strings themselves might be scattered all over the place depending on the compiler (and some quick checking shows that at least with gcc-on-Linux, they indeed weren't reliably close together). That led to very cache-unfriendly behavior as the binary search touched strings in many different pages. Also, depending on the platform, the string pointers might need to be adjusted at program start, so that they couldn't be simple constant data. And the ScanKeyword struct had been designed with an eye to 32-bit machines originally; on 64-bit it requires 16 bytes per keyword, making it even more cache-unfriendly. Redesign so that the keyword strings themselves are allocated consecutively (as part of one big char-string constant), thereby eliminating the touch-lots-of-unrelated-pages syndrome. And get rid of the ScanKeyword array in favor of three separate arrays: uint16 offsets into the keyword array, uint16 token codes, and uint8 keyword categories. That reduces the overhead per keyword to 5 bytes instead of 16 (even less in programs that only need one of the token codes and categories); moreover, the binary search only touches the offsets array, further reducing its cache footprint. This also lets us put the token codes somewhere else than the keyword strings are, which avoids some unpleasant build dependencies. While we're at it, wrap the data used by ScanKeywordLookup into a struct that can be treated as an opaque type by most callers. That doesn't change things much right now, but it will make it less painful to switch to a hash-based lookup method, as is being discussed in the mailing list thread. Most of the change here is associated with adding a generator script that can build the new data structure from the same list-of-PG_KEYWORD header representation we used before. The PG_KEYWORD lists that plpgsql and ecpg used to embed in their scanner .c files have to be moved into headers, and the Makefiles have to be taught to invoke the generator script. This work is also necessary if we're to consider hash-based lookup, since the generator script is what would be responsible for constructing a hash table. Aside from saving a few kilobytes in each program that includes the keyword table, this seems to speed up raw parsing (flex+bison) by a few percent. So it's worth doing even as it stands, though we think we can gain even more with a follow-on patch to switch to hash-based lookup. John Naylor, with further hacking by me Discussion: https://postgr.es/m/CAJVSVGXdFVU2sgym89XPL=Lv1zOS5=EHHQ8XWNzFL=mTXkKMLw@mail.gmail.com
* Don't create relfilenode for relations without storageAlvaro Herrera2019-01-04
| | | | | | | | | | | | 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
* Rename macro to RELKIND_HAS_STORAGEAlvaro Herrera2019-01-04
| | | | | | The original name was an unfortunate choice. Discussion: https://postgr.es/m/20181218.145600.172055615.horiguchi.kyotaro@lab.ntt.co.jp
* Move the built-in conversions into the initial catalog data.Tom Lane2019-01-03
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Instead of running a SQL script to create the standard conversion functions and pg_conversion entries, put those entries into the initial data in postgres.bki. This shaves a few percent off the runtime of initdb, and also allows accurate comments to be attached to the conversion functions; the previous script labeled them with machine-generated comments that were not quite right for multi-purpose conversion functions. Also, we can get rid of the duplicative Makefile and MSVC perl implementations of the generation code for that SQL script. A functional change is that these pg_proc and pg_conversion entries are now "pinned" by initdb. Leaving them unpinned was perhaps a good thing back while the conversions feature was under development, but there seems no valid reason for it now. Also, the conversion functions are now marked as immutable, where before they were volatile by virtue of lacking any explicit specification. That seems like it was just an oversight. To avoid using magic constants in pg_conversion.dat, extend genbki.pl to allow encoding names to be converted, much as it does for language, access method, etc names. John Naylor Discussion: https://postgr.es/m/CAJVSVGWtUqxpfAaxS88vEGvi+jKzWZb2EStu5io-UPc4p9rSJg@mail.gmail.com
* Use symbolic references for pg_language OIDs in the bootstrap data.Tom Lane2019-01-03
| | | | | | | | | | | | | | | | | | | This patch teaches genbki.pl to replace pg_language names by OIDs in much the same way as it already does for pg_am names etc, and converts pg_proc.dat to use such symbolic references in the prolang column. Aside from getting rid of a few more magic numbers in the initial catalog data, this means that Gen_fmgrtab.pl no longer needs to read pg_language.dat, since it doesn't have to know the OID of the "internal" language; now it's just looking for the string "internal". No need for a catversion bump, since the contents of postgres.bki don't actually change at all. John Naylor Discussion: https://postgr.es/m/CAJVSVGWtUqxpfAaxS88vEGvi+jKzWZb2EStu5io-UPc4p9rSJg@mail.gmail.com
* Improve ANALYZE's handling of concurrent-update scenarios.Tom Lane2019-01-03
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This patch changes the rule for whether or not a tuple seen by ANALYZE should be included in its sample. When we last touched this logic, in commit 51e1445f1, we weren't thinking very hard about tuples being UPDATEd by a long-running concurrent transaction. In such a case, we might see the pre-image as either LIVE or DELETE_IN_PROGRESS depending on timing; and we might see the post-image not at all, or as INSERT_IN_PROGRESS. Since the existing code will not sample either DELETE_IN_PROGRESS or INSERT_IN_PROGRESS tuples, this leads to concurrently-updated rows being omitted from the sample entirely. That's not very helpful, and it's especially the wrong thing if the concurrent transaction ends up rolling back. The right thing seems to be to sample DELETE_IN_PROGRESS rows just as if they were live. This makes the "sample it" and "count it" decisions the same, which seems good for consistency. It's clearly the right thing if the concurrent transaction ends up rolling back; in effect, we are sampling as though IN_PROGRESS transactions haven't happened yet. Also, this combination of choices ensures maximum robustness against the different combinations of whether and in which state we might see the pre- and post-images of an update. It's slightly annoying that we end up recording immediately-out-of-date stats in the case where the transaction does commit, but on the other hand the stats are fine for columns that didn't change in the update. And the alternative of sampling INSERT_IN_PROGRESS rows instead seems like a bad idea, because then the sampling would be inconsistent with the way rows are counted for the stats report. Per report from Mark Chambers; thanks to Jeff Janes for diagnosing what was happening. Back-patch to all supported versions. Discussion: https://postgr.es/m/CAFh58O_Myr6G3tcH3gcGrF-=OExB08PJdWZcSBcEcovaiPsrHA@mail.gmail.com
* Don't believe MinMaxExpr is leakproof without checking.Tom Lane2019-01-02
| | | | | | | | | | | | | | | | MinMaxExpr invokes the btree comparison function for its input datatype, so it's only leakproof if that function is. Many such functions are indeed leakproof, but others are not, and we should not just assume that they are. Hence, adjust contain_leaked_vars to verify the leakproofness of the referenced function explicitly. I didn't add a regression test because it would need to depend on some particular comparison function being leaky, and that's a moving target, per discussion. This has been wrong all along, so back-patch to supported branches. Discussion: https://postgr.es/m/31042.1546194242@sss.pgh.pa.us
* Ensure link commands list *.o files before LDFLAGS.Tom Lane2019-01-02
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | It's important for link commands to list *.o input files before -l switches for libraries, as library code may not get pulled into the link unless referenced by an earlier command-line entry. This is certainly necessary for static libraries (.a style). Apparently on some platforms it is also necessary for shared libraries, as reported by Donald Dong. We often put -l switches for within-tree libraries into LDFLAGS, meaning that link commands that list *.o files after LDFLAGS are hazardous. Most of our link commands got this right, but a few did not. In particular, places that relied on gmake's default implicit link rule failed, because that puts LDFLAGS first. Fix that by overriding the built-in rule with our own. The implicit link rules in src/makefiles/Makefile.* for single-.o-file shared libraries mostly got this wrong too, so fix them. I also changed the link rules for the backend and a couple of other places for consistency, even though they are not (currently) at risk because they aren't adding any -l switches to LDFLAGS. Arguably, the real problem here is that we're abusing LDFLAGS by putting -l switches in it and we should stop doing that. But changing that would be quite invasive, so I'm not eager to do so. Perhaps this is a candidate for back-patching, but so far it seems that problems can only be exhibited in test code we don't normally build, and at least some of the problems are new in HEAD anyway. So I'll refrain for now. Donald Dong and Tom Lane Discussion: https://postgr.es/m/CAKABAquXn-BF-vBeRZxhzvPyfMqgGuc74p8BmQZyCFDpyROBJQ@mail.gmail.com
* Update copyright for 2019Bruce Momjian2019-01-02
| | | | Backpatch-through: certain files through 9.4
generated by cgit v1.2.3 (git 2.25.1) at 2025-08-05 08:18:04 +0000