aboutsummaryrefslogtreecommitdiff
path: root/src/backend
Commit message (Collapse)AuthorAge
* Fix updating of pg_subscription_rel from workersPeter Eisentraut2017-06-07
| | | | | | | | | | A logical replication worker should not insert new rows into pg_subscription_rel, only update existing rows, so that there are no races if a concurrent refresh removes rows. Adjust the API to be able to choose that behavior. Author: Masahiko Sawada <sawada.mshk@gmail.com> Reported-by: tushar <tushar.ahuja@enterprisedb.com>
* Prevent BEFORE triggers from violating partitioning constraints.Robert Haas2017-06-07
| | | | | | | | | | | | | | | | | | | | | Since tuple-routing implicitly checks the partitioning constraints at least for the levels of the partitioning hierarchy it traverses, there's normally no need to revalidate the partitioning constraint after performing tuple routing. However, if there's a BEFORE trigger on the target partition, it could modify the tuple, causing the partitioning constraint to be violated. Catch that case. Also, instead of checking the root table's partition constraint after tuple-routing, check it beforehand. Otherwise, the rules for when the partitioning constraint gets checked get too complicated, because you sometimes have to check part of the constraint but not all of it. This effectively reverts commit 39162b2030fb0a35a6bb28dc636b5a71b8df8d1c in favor of a different approach altogether. Report by me. Initial debugging by Jeevan Ladhe. Patch by Amit Langote, reviewed by me. Discussion: http://postgr.es/m/CA+Tgmoa9DTgeVOqopieV8d1QRpddmP65aCdxyjdYDoEO5pS5KA@mail.gmail.com
* Consistently use subscription name as application namePeter Eisentraut2017-06-06
| | | | | | | The logical replication apply worker uses the subscription name as application name, except for table sync. This was incorrectly set to use the replication slot name, which might be different, in one case. Also add a comment why the other case is different.
* Clean up latch related code.Andres Freund2017-06-06
| | | | | | | | | | | | | | | | | | | | | | The larger part of this patch replaces usages of MyProc->procLatch with MyLatch. The latter works even early during backend startup, where MyProc->procLatch doesn't yet. While the affected code shouldn't run in cases where it's not initialized, it might get copied into places where it might. Using MyLatch is simpler and a bit faster to boot, so there's little point to stick with the previous coding. While doing so I noticed some weaknesses around newly introduced uses of latches that could lead to missed events, and an omitted CHECK_FOR_INTERRUPTS() call in worker_spi. As all the actual bugs are in v10 code, there doesn't seem to be sufficient reason to backpatch this. Author: Andres Freund Discussion: https://postgr.es/m/20170606195321.sjmenrfgl2nu6j63@alap3.anarazel.de https://postgr.es/m/20170606210405.sim3yl6vpudhmufo@alap3.anarazel.de Backpatch: -
* Improve handover logic between sync and apply workersPeter Eisentraut2017-06-06
| | | | | | | | | | | | Make apply busy wait check the catalog instead of shmem state to ensure that next transaction will see the expected table synchronization state. Also make the handover always go through same set of steps to make the overall process easier to understand and debug. Author: Petr Jelinek <petr.jelinek@2ndquadrant.com> Tested-by: Mark Kirkwood <mark.kirkwood@catalyst.net.nz> Tested-by: Erik Rijkers <er@xs4all.nl>
* Use NIL rather than NULL to represent an empty list.Robert Haas2017-06-06
| | | | | | | | Just to be tidy. Amit Langote Discussion: http://postgr.es/m/9297f80f-e4ab-7dda-33d4-8580bab6d634@lab.ntt.co.jp
* Clean up partcollation handling for OID 0.Robert Haas2017-06-06
| | | | | | | | | | | | Consistent with what we do for indexes, we shouldn't try to record dependencies on collation OID 0 or the default collation OID (which is pinned). Also, the fact that indcollation and partcollation can contain zero OIDs when the data type is not collatable should be documented. Amit Langote, per a complaint from me. Discussion: http://postgr.es/m/CA+Tgmoba5mtPgM3NKfG06vv8na5gGbVOj0h4zvivXQwLw8wXXQ@mail.gmail.com
* Wire up query cancel interrupt for walsender backends.Andres Freund2017-06-05
| | | | | | | | | | | This allows to cancel commands run over replication connections. While it might have some use before v10, it has become important now that normal SQL commands are allowed in database connected walsender connections. Author: Petr Jelinek Reviewed-By: Andres Freund, Michael Paquier Discussion: https://postgr.es/m/7966f454-7cd7-2b0c-8b70-cdca9d5a8c97@2ndquadrant.com
* Unify SIGHUP handling between normal and walsender backends.Andres Freund2017-06-05
| | | | | | | | | | | | | | | | | | | | Because walsender and normal backends share the same main loop it's problematic to have two different flag variables, set in signal handlers, indicating a pending configuration reload. Only certain walsender commands reach code paths checking for the variable (START_[LOGICAL_]REPLICATION, CREATE_REPLICATION_SLOT ... LOGICAL, notably not base backups). This is a bug present since the introduction of walsender, but has gotten worse in releases since then which allow walsender to do more. A later patch, not slated for v10, will similarly unify SIGHUP handling in other types of processes as well. Author: Petr Jelinek, Andres Freund Reviewed-By: Michael Paquier Discussion: https://postgr.es/m/20170423235941.qosiuoyqprq4nu7v@alap3.anarazel.de Backpatch: 9.2-, bug is present since 9.0
* Prevent possibility of panics during shutdown checkpoint.Andres Freund2017-06-05
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When the checkpointer writes the shutdown checkpoint, it checks afterwards whether any WAL has been written since it started and throws a PANIC if so. At that point, only walsenders are still active, so one might think this could not happen, but walsenders can also generate WAL, for instance in BASE_BACKUP and logical decoding related commands (e.g. via hint bits). So they can trigger this panic if such a command is run while the shutdown checkpoint is being written. To fix this, divide the walsender shutdown into two phases. First, checkpointer, itself triggered by postmaster, sends a PROCSIG_WALSND_INIT_STOPPING signal to all walsenders. If the backend is idle or runs an SQL query this causes the backend to shutdown, if logical replication is in progress all existing WAL records are processed followed by a shutdown. Otherwise this causes the walsender to switch to the "stopping" state. In this state, the walsender will reject any further replication commands. The checkpointer begins the shutdown checkpoint once all walsenders are confirmed as stopping. When the shutdown checkpoint finishes, the postmaster sends us SIGUSR2. This instructs walsender to send any outstanding WAL, including the shutdown checkpoint record, wait for it to be replicated to the standby, and then exit. Author: Andres Freund, based on an earlier patch by Michael Paquier Reported-By: Fujii Masao, Andres Freund Reviewed-By: Michael Paquier Discussion: https://postgr.es/m/20170602002912.tqlwn4gymzlxpvs2@alap3.anarazel.de Backpatch: 9.4, where logical decoding was introduced
* Have walsenders participate in procsignal infrastructure.Andres Freund2017-06-05
| | | | | | | | | | | | | | | | | The non-participation in procsignal was a problem for both changes in master, e.g. parallelism not working for normal statements run in walsender backends, and older branches, e.g. recovery conflicts and catchup interrupts not working for logical decoding walsenders. This commit thus replaces the previous WalSndXLogSendHandler with procsignal_sigusr1_handler. In branches since db0f6cad48 that can lead to additional SetLatch calls, but that only rarely seems to make a difference. Author: Andres Freund Reviewed-By: Michael Paquier Discussion: https://postgr.es/m/20170421014030.fdzvvvbrz4nckrow@alap3.anarazel.de Backpatch: 9.4, earlier commits don't seem to benefit sufficiently
* Revert "Prevent panic during shutdown checkpoint"Andres Freund2017-06-05
| | | | | | | | | | | This reverts commit 086221cf6b1727c2baed4703c582f657b7c5350e, which was made to master only. The approach implemented in the above commit has some issues. While those could easily be fixed incrementally, doing so would make backpatching considerably harder, so instead first revert this patch. Discussion: https://postgr.es/m/20170602002912.tqlwn4gymzlxpvs2@alap3.anarazel.de
* Don't set application_name in logical replication workersPeter Eisentraut2017-06-05
| | | | | | This was bothering some people because it's not the intended use of application_name and it makes the default view of pg_stat_activity bulky.
* Fix ALTER SUBSCRIPTION grammar ambiguityPeter Eisentraut2017-06-05
| | | | | | | | | There was a grammar ambiguity between SET PUBLICATION name REFRESH and SET PUBLICATION SKIP REFRESH, because SKIP is not a reserved word. To resolve that, fold the refresh choice into the WITH options. Refreshing is the default now. Reported-by: tushar <tushar.ahuja@enterprisedb.com>
* Ignore WL_POSTMASTER_DEATH latch event in single user modePeter Eisentraut2017-06-05
| | | | | | | Otherwise code that uses this will abort with an assertion failure, because postmaster_alive_fds are not initialized. Reported-by: tushar <tushar.ahuja@enterprisedb.com>
* Code review for shm_toc.h/.c.Tom Lane2017-06-05
| | | | | | | | | | | | | | | | | | | | Declare the toc_nentry field as uint32 not Size. Since shm_toc_lookup() reads the field without any lock, it has to be atomically readable, and we do not assume that for fields wider than 32 bits. Performance would be impossibly bad for entry counts approaching 2^32 anyway, so there is no need to try to preserve maximum width here. This is probably an academic issue, because even if reading int64 isn't atomic, the high order half would never change in practice. Still, it's a coding rule violation, so let's fix it. Adjust some other not-terribly-well-chosen data types too, and copy-edit some comments. Make shm_toc_attach's Asserts consistent with shm_toc_create's. None of this looks to be a live bug, so no need for back-patch. Discussion: https://postgr.es/m/16984.1496679541@sss.pgh.pa.us
* Don't be so trusting that shm_toc_lookup() will always succeed.Tom Lane2017-06-05
| | | | | | | | | | | | | | | | | | Given the possibility of race conditions and so on, it seems entirely unsafe to just assume that shm_toc_lookup() always finds the key it's looking for --- but that was exactly what all but one call site were doing. To fix, add a "bool noError" argument, similarly to what we have in many other functions, and throw an error on an unexpected lookup failure. Remove now-redundant Asserts that a rather random subset of call sites had. I doubt this will throw any light on buildfarm member lorikeet's recent failures, because if an unnoticed lookup failure were involved, you'd kind of expect a null-pointer-dereference crash rather than the observed symptom. But you never know ... and this is better coding practice even if it never catches anything. Discussion: https://postgr.es/m/9697.1496675981@sss.pgh.pa.us
* Fix typo in error message.Heikki Linnakangas2017-06-05
| | | | | | Daniele Varrazzo Discussion: https://www.postgresql.org/message-id/CA+mi_8bqY5THP8hLKKSdMEr5GCz6M=hD6_uLbvFeyEBfwqUxeA@mail.gmail.com
* Replace over-optimistic Assert in partitioning code with a runtime test.Tom Lane2017-06-04
| | | | | | | | | | | | | | | | get_partition_parent felt that it could simply Assert that systable_getnext found a tuple. This is unlike any other caller of that function, and it's unsafe IMO --- in fact, the reason I noticed it was that the Assert failed. (OK, I was working with known-inconsistent catalog contents, but I wasn't expecting the DB to fall over quite that violently. The behavior in a non-assert-enabled build wouldn't be very nice, either.) Fix it to do what other callers do, namely an actual runtime-test-and-elog. Also, standardize the wording of elog messages that are complaining about unexpected failure of systable_getnext. 90% of them say "could not find tuple for <object>", so make the remainder do likewise. Many of the holdouts were using the phrasing "cache lookup failed", which is outright misleading since no catcache search is involved.
* #ifdef out assorted unused GEQO code.Tom Lane2017-06-04
| | | | | | | | | | | | | | I'd always assumed that backend/optimizer/geqo/'s remarkably poor showing on code coverage metrics was because we weren't exercising it much in the regression tests. But it turns out that a good chunk of the problem is that there's a bunch of code that is physically unreachable (because the calls to it are #ifdef'd out in geqo_main.c) but is being built anyway. Making the called code have #if guards similar to the calling code saves a couple of kilobytes of executable size and should make the coverage numbers more reflective of reality. It's arguable that we should just delete all the unused recombination mechanisms altogether, but I didn't feel a need to go that far today.
* Disallow CREATE INDEX if table is already in use in current session.Tom Lane2017-06-04
| | | | | | | | | | | | | | If we allow this, whatever outer command has the table open will not know about the new index and may fail to update it as needed, as shown in a report from Laurenz Albe. We already had such a prohibition in place for ALTER TABLE, but the CREATE INDEX syntax missed the check. Fixing it requires an API change for DefineIndex(), which conceivably would break third-party extensions if we were to back-patch it. Given how long this problem has existed without being noticed, fixing it in the back branches doesn't seem worth that risk. Discussion: https://postgr.es/m/A737B7A37273E048B164557ADEF4A58B53A4DC9A@ntex2010i.host.magwien.gv.at
* Assorted translatable string fixesAlvaro Herrera2017-06-04
| | | | | Mark our rusage reportage string translatable; remove quotes from type names; unify formatting of very similar messages.
* Remove dead variables.Tom Lane2017-06-03
| | | | | Commit 512c7356b left a couple of variables unused except for being set. My compiler didn't whine about this, but some buildfarm members did.
* Fix <> and pattern-NOT-match estimators to handle nulls correctly.Tom Lane2017-06-03
| | | | | | | | | | | | | | | | | | | | | | | | These estimators returned 1 minus the corresponding equality/match estimate, which is incorrect: we need to subtract off the fraction of nulls in the column, since those are neither equal nor not equal to the comparison value. The error only becomes obvious if the nullfrac is large, but it could be very bad in a mostly-nulls column, as reported in bug #14676 from Marko Tiikkaja. To fix the <> case, refactor eqsel() and neqsel() to call a common support routine, which can be made to account for nullfrac correctly. The pattern-match cases were already factored that way, and it was simply an oversight that patternsel() wasn't subtracting off nullfrac. neqjoinsel() has a similar problem, but since we're elsewhere discussing changing its behavior entirely, I left it alone for now. This is a very longstanding bug, but I'm hesitant to back-patch a fix for it. Given the lack of prior complaints, such cases must not come up often, so it's probably not worth the risk of destabilizing plans in stable branches. Discussion: https://postgr.es/m/20170529153847.4275.95416@wrigleys.postgresql.org
* Fix old corner-case logic error in final_cost_nestloop().Tom Lane2017-06-03
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When costing a nestloop with stop-at-first-inner-match semantics, and a non-indexscan inner path, final_cost_nestloop() wants to charge the full scan cost of the inner rel at least once, with additional scans charged at inner_rescan_run_cost which might be less. However the logic for doing this effectively assumed that outer_matched_rows is at least 1. If it's zero, which is not unlikely for a small outer rel, we ended up charging inner_run_cost plus N times inner_rescan_run_cost, as much as double the correct charge for an outer rel with only one row that we're betting won't be matched. (Unless the inner rel is materialized, in which case it has very small inner_rescan_run_cost and the cost is not so far off what it should have been.) The upshot of this was that the planner had a tendency to select plans that failed to make effective use of the stop-at-first-inner-match semantics, and that might have Materialize nodes in them even when the predicted number of executions of the Materialize subplan was only 1. This was not so obvious before commit 9c7f5229a, because the case only arose in connection with semi/anti joins where there's not freedom to reverse the join order. But with the addition of unique-inner joins, it could result in some fairly bad planning choices, as reported by Teodor Sigaev. Indeed, some of the test cases added by that commit have plans that look dubious on closer inspection, and are changed by this patch. Fix the logic to ensure that we don't charge for too many inner scans. I chose to adjust it so that the full-freight scan cost is associated with an unmatched outer row if possible, not a matched one, since that seems like a better model of what would happen at runtime. This is a longstanding bug, but given the lesser impact in back branches, and the lack of field complaints, I won't risk a back-patch. Discussion: https://postgr.es/m/CAKJS1f-LzkUsFxdJ_-Luy38orQ+AdEXM5o+vANR+-pHAWPSecg@mail.gmail.com
* Receive invalidation messages correctly in tablesync workerPeter Eisentraut2017-06-03
| | | | | | | | | | | We didn't accept any invalidation messages until the whole sync process had finished (because it flattens all the remote transactions in the single one). So the sync worker didn't learn about subscription changes/drop until it has finished. This could lead to "orphaned" sync workers. Author: Petr Jelinek <petr.jelinek@2ndquadrant.com> Reported-by: Masahiko Sawada <sawada.mshk@gmail.com>
* Make tablesync worker exit when apply dies while it was waiting for itPeter Eisentraut2017-06-03
| | | | | | | | | This avoids "orphaned" sync workers. This was caused by a thinko in wait_for_sync_status_change. Author: Petr Jelinek <petr.jelinek@2ndquadrant.com> Reported-by: Masahiko Sawada <sawada.mshk@gmail.com>
* Allow parallelism in COPY (query) TO ...;Andres Freund2017-06-02
| | | | | | | | | | | | | Previously this was not allowed, as copy.c didn't set the CURSOR_OPT_PARALLEL_OK flag when planning the query. Set it. While the lack of parallel query for COPY isn't strictly speaking a bug, it does prevent parallelism from being used in a facility commonly used to run long running queries. Thus backpatch to 9.6. Author: Andres Freund Discussion: https://postgr.es/m/20170531231958.ihanapplorptykzm@alap3.anarazel.de Backpatch: 9.6, where parallelism was introduced.
* Remove replication slot name check from ReplicationSlotAcquire()Peter Eisentraut2017-06-02
| | | | | | | When trying to access a replication slot that is supposed to already exist, we don't need to check the naming rules again. If the slot does not exist, we will then get a "does not exist" error message, which is generally more useful from the perspective of an end user.
* Fix signal handling in logical replication workersPeter Eisentraut2017-06-02
| | | | | | | | | | The logical replication worker processes now use the normal die() handler for SIGTERM and CHECK_FOR_INTERRUPTS() instead of custom code. One problem before was that the apply worker would not exit promptly when a subscription was dropped, which could lead to deadlocks. Author: Petr Jelinek <petr.jelinek@2ndquadrant.com> Reported-by: Masahiko Sawada <sawada.mshk@gmail.com>
* Fix copy/paste mistake in commentMagnus Hagander2017-06-02
| | | | Amit Langote
* Fix typo in commentMagnus Hagander2017-06-02
| | | | Masahiko Sawada
* Reorganize logical replication worker disconnect codePeter Eisentraut2017-06-01
| | | | | | | | | Move the walrcv_disconnect() calls into the before_shmem_exit handler. This makes sure the call is always made even during exit by signal, it saves some duplicate code, and it makes the logic more similar to walreceiver.c. Author: Petr Jelinek <petr.jelinek@2ndquadrant.com>
* Modify sequence catalog tuple before invoking post alter hook.Andres Freund2017-06-01
| | | | | | | | | This seems to have been broken in the commit (1753b1b027035029) that moved the sequence definition into pg_sequence. Author: Andres Freund Discussion: https://postgr.es/m/20170601000716.qxg7c46ukkiljjb3@alap3.anarazel.de Backpatch: Bug is in master/v10 only
* Make ALTER SEQUENCE, including RESTART, fully transactional.Andres Freund2017-06-01
| | | | | | | | | | | | | | | | | | | | | | | | Previously the changes to the "data" part of the sequence, i.e. the one containing the current value, were not transactional, whereas the definition, including minimum and maximum value were. That leads to odd behaviour if a schema change is rolled back, with the potential that out-of-bound sequence values can be returned. To avoid the issue create a new relfilenode fork whenever ALTER SEQUENCE is executed, similar to how TRUNCATE ... RESTART IDENTITY already is already handled. This commit also makes ALTER SEQUENCE RESTART transactional, as it seems to be too confusing to have some forms of ALTER SEQUENCE behave transactionally, some forms not. This way setval() and nextval() are not transactional, but DDL is, which seems to make sense. This commit also rolls back parts of the changes made in 3d092fe540 and f8dc1985f as they're now not needed anymore. Author: Andres Freund Discussion: https://postgr.es/m/20170522154227.nvafbsm62sjpbxvd@alap3.anarazel.de Backpatch: Bug is in master/v10 only
* Restore accidentally-removed line.Robert Haas2017-05-31
| | | | | | | | Commit 88e66d193fbaf756b3cc9bf94cad116aacbb355b is to blame. Masahiko Sawada Discussion: http://postgr.es/m/CAD21AoAXeb7O4hgg+efs8JT_SxpR4doAH5c5s-Z5WoRLstBZJA@mail.gmail.com
* Sort syscache identifiers into alphabetical order.Tom Lane2017-05-30
| | | | | | | | Not much point in having a convention about this if we don't enforce it. Mark Dilger Discussion: https://postgr.es/m/7F67FBEF-C3B3-404E-8EC6-E02ACB15D894@gmail.com
* brin: Don't crash on auto-summarizationAlvaro Herrera2017-05-30
| | | | | | | | | | | | | | We were trying to free a pointer into a shared buffer, which never works; and we were failing to release the buffer lock appropriately. Fix those omissions. While at it, improve documentation for brinGetTupleForHeapBlock, the inadequacy of which evidently caused these bugs in the first place. Reported independently by Zhou Digoal (bug #14668) and Alexander Sosna. Discussion: https://postgr.es/m/8c31c11b-6adb-228d-22c2-4ace89fc9209@credativ.de Discussion: https://postgr.es/m/20170524063323.29941.46339@wrigleys.postgresql.org
* Fix wording in amvalidate error messagesAlvaro Herrera2017-05-30
| | | | | | | | Remove some gratuituous message differences by making the AM name previously embedded in each message be a %s instead. While at it, get rid of terminology that's unclear and unnecessary in one message. Discussion: https://postgr.es/m/20170523001557.bq2hbq7hxyvyw62q@alvherre.pgsql
* Fix omission of locations in outfuncs/readfuncs partitioning node support.Tom Lane2017-05-30
| | | | | | | | | | | | | We could have limped along without this for v10, which was my intention when I annotated the bug in commit 76a3df6e5. But consensus is that it's better to fix it now and take the cost of a post-beta1 initdb (which is needed because these node types are stored in pg_class.relpartbound). Since we're forcing initdb anyway, take the opportunity to make the node type identification strings match the node struct names, instead of being randomly different from them. Discussion: https://postgr.es/m/E1dFBEX-0004wt-8t@gemulon.postgresql.org
* Fix improper quoting of format_type_be() output.Tom Lane2017-05-29
| | | | | | | Per our message style guidelines, error messages incorporating the results of format_type_be() and its siblings should not add quotes around those results, because those functions already add quotes at need. Fix a few places that hadn't gotten that memo.
* Make edge-case behavior of jsonb_populate_record match json_populate_recordTom Lane2017-05-29
| | | | | | | | | | | json_populate_record throws an error if asked to convert a JSON scalar or array into a composite type. jsonb_populate_record was returning a record full of NULL fields instead. It seems better to make it throw an error for this case as well. Nikita Glukhov Discussion: https://postgr.es/m/fbd1d566-bba0-a3de-d6d0-d3b1d7c24ff2@postgrespro.ru
* Fix thinko in JsObjectSize() macro.Tom Lane2017-05-29
| | | | | | | | | | | | | | | | The macro gave the wrong answers for a JsObject with is_json == 0: it would return 1 if jsonb_cont == NULL, or if that wasn't NULL, it would return 1 for any non-zero size. We could fix that, but the only use of this macro at present is in the JsObjectIsEmpty() macro, so it seems simpler and clearer to get rid of JsObjectSize() and put corrected logic into JsObjectIsEmpty(). Thinko in commit cf35346e8, so no need for back-patch. Nikita Glukhov Discussion: https://postgr.es/m/fbd1d566-bba0-a3de-d6d0-d3b1d7c24ff2@postgrespro.ru
* Allow NumericOnly to be "+ FCONST".Tom Lane2017-05-29
| | | | | | | | | | | | | | | | | | | | | The NumericOnly grammar production accepted ICONST, + ICONST, - ICONST, FCONST, and - FCONST, but for some reason not + FCONST. This led to strange inconsistencies like regression=# set random_page_cost = +4; SET regression=# set random_page_cost = 4000000000; SET regression=# set random_page_cost = +4000000000; ERROR: syntax error at or near "4000000000" (because 4000000000 is too large to be an ICONST). While there's no actual functional reason to need to write a "+", if we allow it for integers it seems like we should allow it for numerics too. It's been like that forever, so back-patch to all supported branches. Discussion: https://postgr.es/m/30908.1496006184@sss.pgh.pa.us
* More code review for get_qual_for_list().Tom Lane2017-05-29
| | | | | | | | | | | | | | | | | | | | Avoid trashing the input PartitionBoundSpec; while that might be safe for current callers, it's certainly trouble waiting to happen. In the same vein, make sure that all of the result data structure is freshly palloc'd, rather than some of it being pointers into the input data structures (which we don't know the lifespans of). Simplify the logic for tacking on IS NULL or IS NOT NULL conditions some more; commit 85c2b9a15 left a lot on the table there. And rearrange the construction of the nodes into (what seems to me) a more logical order. In passing, make sure that get_qual_for_range() also returns a freshly palloc'd structure, since there's no value in having that guarantee for only one kind of partitioning. And improve some comments there. Jeevan Ladhe, with further tweaking by me Discussion: https://postgr.es/m/CAOgcT0MAcYoMs93W80iTUf_dP36=1mZQzeUk+nnwY_-qWDrCfw@mail.gmail.com
* Fix typo in commentMagnus Hagander2017-05-29
| | | | Masahiko Sawada
* Code review focused on new node types added by partitioning support.Tom Lane2017-05-28
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Fix failure to check that we got a plain Const from const-simplification of a coercion request. This is the cause of bug #14666 from Tian Bing: there is an int4 to money cast, but it's only stable not immutable (because of dependence on lc_monetary), resulting in a FuncExpr that the code was miserably unequipped to deal with, or indeed even to notice that it was failing to deal with. Add test cases around this coercion behavior. In view of the above, sprinkle the code liberally with castNode() macros, in hope of catching the next such bug a bit sooner. Also, change some functions that were randomly declared to take Node* to take more specific pointer types. And change some struct fields that were declared Node* but could be given more specific types, allowing removal of assorted explicit casts. Place PARTITION_MAX_KEYS check a bit closer to the code it's protecting. Likewise check only-one-key-for-list-partitioning restriction in a less random place. Avoid not-per-project-style usages like !strcmp(...). Fix assorted failures to avoid scribbling on the input of parse transformation. I'm not sure how necessary this is, but it's entirely silly for these functions to be expending cycles to avoid that and not getting it right. Add guards against partitioning on system columns. Put backend/nodes/ support code into an order that matches handling of these node types elsewhere. Annotate the fact that somebody added location fields to PartitionBoundSpec and PartitionRangeDatum but forgot to handle them in outfuncs.c/readfuncs.c. This is fairly harmless for production purposes (since readfuncs.c would just substitute -1 anyway) but it's still bogus. It's not worth forcing a post-beta1 initdb just to fix this, but if we have another reason to force initdb before 10.0, we should go back and clean this up. Contrariwise, somebody added location fields to PartitionElem and PartitionSpec but forgot to teach exprLocation() about them. Consolidate duplicative code in transformPartitionBound(). Improve a couple of error messages. Improve assorted commentary. Re-pgindent the files touched by this patch; this affects a few comment blocks that must have been added quite recently. Report: https://postgr.es/m/20170524024550.29935.14396@wrigleys.postgresql.org
* Move autogenerated array types out of the way during ALTER ... RENAME.Tom Lane2017-05-26
| | | | | | | | | | | | Commit 9aa3c782c added code to allow CREATE TABLE/CREATE TYPE to not fail when the desired type name conflicts with an autogenerated array type, by dint of renaming the array type out of the way. But I (tgl) overlooked that the same case arises in ALTER TABLE/TYPE RENAME. Fix that too. Back-patch to all supported branches. Report and patch by Vik Fearing, modified a bit by me Discussion: https://postgr.es/m/0f4ade49-4f0b-a9a3-c120-7589f01d1eb8@2ndquadrant.com
* Abort authentication if the client selected an invalid SASL mechanism.Heikki Linnakangas2017-05-25
| | | | | | | | | Previously, the server would log an error, but then try to continue with SCRAM-SHA-256 anyway. Michael Paquier Discussion: https://www.postgresql.org/message-id/CAB7nPqR0G5aF2_kc_LH29knVqwvmBc66TF5DicvpGVdke68nKw@mail.gmail.com
* Fix table syncing with different column orderPeter Eisentraut2017-05-24
| | | | | | | | | | | | Logical replication supports replicating between tables with different column order. But this failed for the initial table sync because of a logic error in how the column list for the internal COPY command was composed. Fix that and also add a test. Also fix a minor omission in the column name mapping cache. When creating the mapping list, it would not skip locally dropped columns. So if a remote column had the same name as a locally dropped column (...pg.dropped...), then the expected error would not occur.
generated by cgit v1.2.3 (git 2.25.1) at 2025-08-07 12:01:26 +0000