Re: [HACKERS] Logical Replication WIP
On 2016-12-18 11:12, Petr Jelinek wrote: (now using latest: patchset:) 0001-Add-PUBLICATION-catalogs-and-DDL-v14.patch 0002-Add-SUBSCRIPTION-catalog-and-DDL-v14.patch 0003-Define-logical-replication-protocol-and-output-plugi-v14.patch 0004-Add-logical-replication-workers-v14.patch 0005-Add-separate-synchronous-commit-control-for-logical--v14.patch BTW you don't need to add primary key to pgbench_history. Simply ALTER TABLE pgbench_history REPLICA IDENTITY FULL; should be enough. Either should, but neither is. set-up: Before creating the publication/subscription: On master I run pgbench -qis 1, then set replica identity (and/or add serial column) for pgbench_history, then dump/restore the 4 pgbench tables from master to replica. Then enabling publication/subscription. logs looks well. (Other tests I've devised earlier (on other tables) still work nicely.) Now when I do a pgbench-run on master, something like: pgbench -c 1 -T 20 -P 1 I often see this (when running pgbench): ERROR: publisher does not send replica identity column expected by the logical replication target public.pgbench_tellers or, sometimes (less often) the same ERROR for pgbench_accounts appears (as in the subsciber-log below) -- publisher log 2016-12-19 07:44:22.738 CET 22690 LOG: logical decoding found consistent point at 0/14598C78 2016-12-19 07:44:22.738 CET 22690 DETAIL: There are no running transactions. 2016-12-19 07:44:22.738 CET 22690 LOG: exported logical decoding snapshot: "000130FA-1" with 0 transaction IDs 2016-12-19 07:44:22.886 CET 22729 LOG: starting logical decoding for slot "sub1" 2016-12-19 07:44:22.886 CET 22729 DETAIL: streaming transactions committing after 0/14598CB0, reading WAL from 0/14598C78 2016-12-19 07:44:22.886 CET 22729 LOG: logical decoding found consistent point at 0/14598C78 2016-12-19 07:44:22.886 CET 22729 DETAIL: There are no running transactions. 2016-12-19 07:45:25.568 CET 22729 LOG: could not receive data from client: Connection reset by peer 2016-12-19 07:45:25.568 CET 22729 LOG: unexpected EOF on standby connection 2016-12-19 07:45:25.580 CET 26696 LOG: starting logical decoding for slot "sub1" 2016-12-19 07:45:25.580 CET 26696 DETAIL: streaming transactions committing after 0/1468E0D0, reading WAL from 0/1468DC90 2016-12-19 07:45:25.589 CET 26696 LOG: logical decoding found consistent point at 0/1468DC90 2016-12-19 07:45:25.589 CET 26696 DETAIL: There are no running transactions. -- subscriber log 2016-12-19 07:44:22.878 CET 17027 LOG: starting logical replication worker for subscription 24581 2016-12-19 07:44:22.883 CET 22726 LOG: logical replication apply for subscription sub1 started 2016-12-19 07:45:11.069 CET 22726 WARNING: leaked hash_seq_search scan for hash table 0x2def1a8 2016-12-19 07:45:25.566 CET 22726 ERROR: publisher does not send replica identity column expected by the logical replication target public.pgbench_accounts 2016-12-19 07:45:25.568 CET 16984 LOG: worker process: logical replication worker 24581 (PID 22726) exited with exit code 1 2016-12-19 07:45:25.568 CET 17027 LOG: starting logical replication worker for subscription 24581 2016-12-19 07:45:25.574 CET 26695 LOG: logical replication apply for subscription sub1 started 2016-12-19 07:46:10.950 CET 26695 WARNING: leaked hash_seq_search scan for hash table 0x2def2c8 2016-12-19 07:46:10.950 CET 26695 WARNING: leaked hash_seq_search scan for hash table 0x2def2c8 2016-12-19 07:46:10.950 CET 26695 WARNING: leaked hash_seq_search scan for hash table 0x2def2c8 Sometimes replication (caused by a pgbench run) runs for a few seconds replicating all 4 pgbench tables correctly, but never longer than 10 to 20 seconds. If you cannot reproduce with the provided info it I will make a more precise setup-desciption, but it's so solidly failing here that I hope that won't be necessary. Erik Rijkers -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Declarative partitioning vs. sql_inheritance
On 2016/12/17 10:40, Robert Haas wrote: > On Fri, Dec 16, 2016 at 7:39 PM, Tom Lanewrote: >> Peter Eisentraut writes: >>> On 12/16/16 11:05 AM, Robert Haas wrote: If we were going to do anything about this, my vote would be to remove sql_inheritance. >> >>> Go for it. >> >>> Let's also remove the table* syntax then. >> >> Meh --- that might break existing queries, to what purpose? >> >> We certainly shouldn't remove query syntax without a deprecation period. >> I'm less concerned about that for GUCs. > > I agree. Patch attached, just removing the GUC and a fairly minimal > amount of the supporting infrastructure. +1 to removing the sql_inheritance GUC. The patch looks good to me. Thanks, Amit -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] postgres_fdw bug in 9.6
On Fri, Dec 16, 2016 at 9:43 PM, Tom Lanewrote: > Etsuro Fujita writes: >> On 2016/12/16 11:25, Etsuro Fujita wrote: >>> As I said upthread, an alternative I am thinking is (1) to create an >>> equivalent nestloop join path using inner/outer paths of a foreign join >>> path, except when that join path implements a full join, in which case a >>> merge/hash join path is used, (2) store it in fdw_outerpath as-is, and >>> (3) during an EPQ recheck, apply postgresRecheckForeignScan to the outer >>> subplan created from the fdw_outerpath as-is. What do you think about >>> that? > >> Let me explain about #1 and #2 a bit more. What I have in mind is: > >> * modify postgresGetForeignPaths so that a simple foreign table scan >> path is stored into the base relation's PgFdwRelationInfo. >> * modify postgresGetForeignJoinPaths so that >> (a) a local join path for a 2-way foreign join is created using >> simple foreign table scan paths stored in the base relations' >> PgFdwRelationInfos, and stored into the join relation's PgFdwRelationInfo. >> (b) a local join path for a 3-way foreign join, whose left input is >> a 2-way foreign join, is created using a local join path stored in the >> left input join relation's PgFdwRelationInfo and a simple foreign table >> scan path stored into the right input base relation's PgFdwRelationInfo. >> (c) Likewise for higher level foreign joins. >> (d) local join paths created are passed to create_foreignscan_path >> and stored into the fdw_outerpaths of the resulting ForeignPahts. > > Hm, isn't this overcomplicated? As you said earlier, we don't really > care all that much whether the fdw_outerpath is free of lower foreign > joins, because EPQ setup will select those lower join's substitute EPQ > plans anyway. All that we need is that the EPQ tree be a legal > implementation of the join order with join quals applied at the right > places. So I think the rule could be > > "When first asked to produce a path for a given foreign joinrel, collect > the cheapest paths for its left and right inputs, and make a nestloop path > (or hashjoin path, if full join) from those, using the join quals needed > for the current input relation pair. Use this as the fdw_outerpath for > all foreign paths made for the joinrel." We could use fdw_outerpath of the left and right inputs instead of looking for the cheapest paths. For base relations as left or right relations, use the unparameterized cheapest foreign path. This will ensure that all joins in fdw_outerpath are local joins, making EPQ checks slightly efficient by avoiding redirection at every foreign path. -- Best Wishes, Ashutosh Bapat EnterpriseDB Corporation The Postgres Database Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] postgres_fdw bug in 9.6
On 2016/12/17 1:13, Tom Lane wrote: Etsuro Fujitawrites: On 2016/12/16 11:25, Etsuro Fujita wrote: As I said upthread, an alternative I am thinking is (1) to create an equivalent nestloop join path using inner/outer paths of a foreign join path, except when that join path implements a full join, in which case a merge/hash join path is used, (2) store it in fdw_outerpath as-is, and (3) during an EPQ recheck, apply postgresRecheckForeignScan to the outer subplan created from the fdw_outerpath as-is. What do you think about that? Let me explain about #1 and #2 a bit more. What I have in mind is: * modify postgresGetForeignPaths so that a simple foreign table scan path is stored into the base relation's PgFdwRelationInfo. * modify postgresGetForeignJoinPaths so that (a) a local join path for a 2-way foreign join is created using simple foreign table scan paths stored in the base relations' PgFdwRelationInfos, and stored into the join relation's PgFdwRelationInfo. (b) a local join path for a 3-way foreign join, whose left input is a 2-way foreign join, is created using a local join path stored in the left input join relation's PgFdwRelationInfo and a simple foreign table scan path stored into the right input base relation's PgFdwRelationInfo. (c) Likewise for higher level foreign joins. (d) local join paths created are passed to create_foreignscan_path and stored into the fdw_outerpaths of the resulting ForeignPahts. Hm, isn't this overcomplicated? As you said earlier, we don't really care all that much whether the fdw_outerpath is free of lower foreign joins, because EPQ setup will select those lower join's substitute EPQ plans anyway. All that we need is that the EPQ tree be a legal implementation of the join order with join quals applied at the right places. Exactly. I thought the EPQ trees without lower foreign joins would be better because that would be easier to understand. So I think the rule could be "When first asked to produce a path for a given foreign joinrel, collect the cheapest paths for its left and right inputs, and make a nestloop path (or hashjoin path, if full join) from those, using the join quals needed for the current input relation pair. Seems reasonable. Use this as the fdw_outerpath for all foreign paths made for the joinrel." I'm not sure that would work well for foreign joins with sort orders. Consider a merge join, whose left input is a 2-way foreign join with a sort order that implements a full join and whose right input is a sorted local table scan. If the EPQ subplan for the foreign join wouldn't produce the right sort order, the merge join might break during EPQ rechecks (note that in this case the EPQ subplan for the foreign join might produce more than a single row during an EPQ recheck). So, I think we would need to add an explicit sort to the fdw_outerpath for the foreign join. The important point here is that we avoid using a merge join because that has assumptions about input ordering that likely won't be satisfied by the child paths chosen through this method. (I guess you could fall back to it for the case of no quals in a fulljoin, because then the ordering assumptions are vacuous anyway.) I agree on that point. I'll create a patch. Best regards, Etsuro Fujita -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Creating a DSA area to provide work space for parallel execution
On Sat, Dec 17, 2016 at 5:41 AM, Robert Haaswrote: > On Wed, Dec 14, 2016 at 3:25 PM, Robert Haas wrote: >> Thoughts? > > Hearing no objections, I've gone ahead and committed this. If that > makes somebody really unhappy I can revert it, but I am betting that > the real story is that nobody cares about preserving T_ID(). I suppose LWLock could include a uint16 member 'id' without making LWLock any larger, since it would replace the padding between 'tranche' and 'state'. But I think a better solution, if anyone really wants these T_ID numbers from a dtrace script, would be to add lock address to the existing lwlock probes, and then figure out a way to add some new probes to report the base and stride in the right places so you can do the book keeping in dtrace variables. -- Thomas Munro http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Retire src/backend/port/dynloader/linux.c ?
On 12/18/16 10:19 PM, Tom Lane wrote: > Andres Freundwrites: >> Shouldn't we just remove that code? > > What for? It's maintenance-free ... hasn't been touched since 2004. > While I agree with you that it's *probably* dead code, it's hard to > see much upside from removing it. > > If we want to get into arguing whether code is dead or not, there's > an awful lot of potentially removable stuff in the tree, but I doubt > it's worth the trouble to figure out what's really dead. If someone wants to dive into it, I think you could probably remove most or all of the prehistoric pre-dlopen code for *bsd and darwin as well. The hpux and win32 code could be moved to libpgport, and then we could just call dlopen() etc. directly and remove this whole subdirectory. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Retire src/backend/port/dynloader/linux.c ?
Andres Freundwrites: > Shouldn't we just remove that code? What for? It's maintenance-free ... hasn't been touched since 2004. While I agree with you that it's *probably* dead code, it's hard to see much upside from removing it. If we want to get into arguing whether code is dead or not, there's an awful lot of potentially removable stuff in the tree, but I doubt it's worth the trouble to figure out what's really dead. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Measuring replay lag
On 11/22/16 4:27 AM, Thomas Munro wrote: > Thanks very much for testing! New version attached. I will add this > to the next CF. I don't see it there yet. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Declarative partitioning - another take
On 2016/12/17 11:32, Amit Langote wrote: > On Sat, Dec 17, 2016 at 1:07 AM, Robert Haaswrote: >> On Fri, Dec 16, 2016 at 3:02 AM, Amit Langote >> wrote: >>> Aside from the above, I found few other issues and fixed them in the >>> attached patches. Descriptions follow: >> >> To avoid any further mistakes on my part, can you please resubmit >> these with each patch file containing a proposed commit message >> including patch authorship information, who reported the issue, links >> to relevant discussion if any, and any other attribution information >> which I should not fail to include when committing? > > I think it's a good advice and will keep in mind for any patches I > post henceforth. > > In this particular case, I found all the issues myself while working > with some more esoteric test scenarios, except the first patch (1/7), > where I have mentioned in the description of the patch in the email, > that there were independent reports of the issue by Tomas Vondra and > David Fetter. Here are updated patches including the additional information. Thanks, Amit >From c2afac3447a8bd2f7f9004e8b7e258039ca2296b Mon Sep 17 00:00:00 2001 From: amit Date: Tue, 13 Dec 2016 15:07:06 +0900 Subject: [PATCH 1/7] Invalidate the parent's relcache after partition creation. CREATE TABLE PARTITION OF failed to invalidate the parent table's relcache, causing the subsequent commands in the same transaction to not see the new partition. Reported by: Tomas Vondra and David Fetter Patch by: Amit Langote Reports: https://www.postgresql.org/message-id/22dd313b-d7fd-22b5-0787-654845c8f849%402ndquadrant.com https://www.postgresql.org/message-id/20161215090916.GB20659%40fetter.org --- src/backend/catalog/heap.c | 7 ++- src/backend/commands/tablecmds.c | 13 - src/include/catalog/heap.h | 2 +- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c index c09c9f28a7..e5d6aecc3f 100644 --- a/src/backend/catalog/heap.c +++ b/src/backend/catalog/heap.c @@ -3230,9 +3230,12 @@ RemovePartitionKeyByRelId(Oid relid) * StorePartitionBound * Update pg_class tuple of rel to store the partition bound and set * relispartition to true + * + * Also, invalidate the parent's relcache, so that the next rebuild will load + * the new partition's info into its partition descriptor. */ void -StorePartitionBound(Relation rel, Node *bound) +StorePartitionBound(Relation rel, Relation parent, Node *bound) { Relation classRel; HeapTuple tuple, @@ -3273,4 +3276,6 @@ StorePartitionBound(Relation rel, Node *bound) CatalogUpdateIndexes(classRel, newtuple); heap_freetuple(newtuple); heap_close(classRel, RowExclusiveLock); + + CacheInvalidateRelcache(parent); } diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 7a574dc50d..1c219b03dd 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -777,10 +777,11 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId, * it does not return on error. */ check_new_partition_bound(relname, parent, bound); - heap_close(parent, NoLock); /* Update the pg_class entry. */ - StorePartitionBound(rel, bound); + StorePartitionBound(rel, parent, bound); + + heap_close(parent, NoLock); /* * The code that follows may also update the pg_class tuple to update @@ -13141,7 +13142,7 @@ ATExecAttachPartition(List **wqueue, Relation rel, PartitionCmd *cmd) cmd->bound); /* Update the pg_class entry. */ - StorePartitionBound(attachRel, cmd->bound); + StorePartitionBound(attachRel, rel, cmd->bound); /* * Generate partition constraint from the partition bound specification. @@ -13352,12 +13353,6 @@ ATExecAttachPartition(List **wqueue, Relation rel, PartitionCmd *cmd) } } - /* - * Invalidate the parent's relcache so that the new partition is now - * included its partition descriptor. - */ - CacheInvalidateRelcache(rel); - ObjectAddressSet(address, RelationRelationId, RelationGetRelid(attachRel)); /* keep our lock until commit */ diff --git a/src/include/catalog/heap.h b/src/include/catalog/heap.h index 77dc1983e8..0e4262f611 100644 --- a/src/include/catalog/heap.h +++ b/src/include/catalog/heap.h @@ -143,6 +143,6 @@ extern void StorePartitionKey(Relation rel, Oid *partopclass, Oid *partcollation); extern void RemovePartitionKeyByRelId(Oid relid); -extern void StorePartitionBound(Relation rel, Node *bound); +extern void StorePartitionBound(Relation rel, Relation parent, Node *bound); #endif /* HEAP_H */ -- 2.11.0 >From 21d38e05d1107a88178e9954b1f4a3804cd9e154 Mon Sep 17 00:00:00 2001 From: amit Date: Thu, 15 Dec 2016 16:27:04 +0900 Subject: [PATCH 2/7] Change how RelationGetPartitionQual() and related code works Since we always want to recurse, ie,
[HACKERS] Retire src/backend/port/dynloader/linux.c ?
Hi, I don't think PG works on any linux without dlopen(). And DLD (what's used in the dlopen replacement) hasn't been maintained in a while. See https://www.gnu.org/software/dld/ Shouldn't we just remove that code? Greetings, Andres Freund -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Trouble building uuid-ossp extension in new versions of Postgres
> > On Mac, the recommended thing is to forget about the ossp code and >> use "configure --with-uuid=e2fs". Sorry if that wasn't clear enough. >> >> > Ok thanks, I'll try this. > Thanks Tom, "uuid-ossp" built perfectly with "--with--uuid=e2fs". Cheers and Happy Holidays! Ryan
Re: [HACKERS] delta relations in AFTER triggers
On Sun, Dec 18, 2016 at 3:15 PM, Kevin Grittnerwrote: > Patch v8 ... FWIW here's that plpython patch, adjusted to apply on top of your latest patch. -- Thomas Munro http://www.enterprisedb.com delta-relations-plpython.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Hash tables in dynamic shared memory
On Sun, Nov 20, 2016 at 11:54 AM, John Gormanwrote: > I reviewed the dht-v2.patch and found that it is in excellent shape. Thanks for reviewing! And sorry for the late reply. > Benchmarking shows that this performs somewhat faster than > dynahash which is surprising because it is doing DSA address > translations on the fly. That is indeed surprising and I think warrants more investigation. > One area where this could run faster is to reduce the amount of > time when the hash is in the RESIZE_IN_PROGRESS state. > > When a hash partition reaches 75% full a resize begins by allocating > an empty new hash bucket array with double the number of buckets. > This begins the RESIZE_IN_PROGRESS state where there is both an > old and a new hash bucket array. > > During the RESIZE state all operations such as find, insert, > delete and iterate run slower due to having to look items up in > two hash bucket arrays. > > Whenever a new item is inserted into the hash and the hash is in > the RESIZE state the current code takes the time to move one item > from the old hash partition to the new hash partition. During > insertion an exclusive lock is already held on the partition so > this is an efficient time to do the resize cleanup work. > > However since we only clean up one item per insertion we do not > complete the cleanup and free the old hash bucket array until we > are due to start yet another resize operation. > > So we are effectively always in the RESIZE state which slows down > operations and wastes some space. Right, that is the case in a workload that inserts a bunch of data but then becomes read-only forever. A workload that constantly does a mix of writing and reading should settle down at a reasonable size and stop resizing. > Here are the timings for insert and find in nanoseconds on a > Macbook Pro. The insert_resize figure includes the resize work to > move one item from the old to the new hash bucket array. > > insert_resize: 945.98 > insert_clean: 450.39 > find_resize: 348.90 > find_clean:293.16 > > The attached dht-v2-resize-cleanup.patch patch applies on top of > the dht-v2.patch and speeds up the resize cleanup process so that > hashes spend most of their time in the clean state. > > It does this by cleaning up more than one old item during > inserts. This patch cleans up three old items. > > There is also the case where a hash receives all of its inserts > at the beginning and the rest of the work load is all finds. This > patch also cleans up two items for each find. > > The downside of doing extra cleanup early is some additional > latency. Here are the experimental figures and the approximate > formulas for different numbers of cleanups for inserts. Note that > we cannot go lower than one cleanup per insert. > > Resize work in inserts: 729.79 insert + 216.19 * cleanups > 1 945.98 > 2 1178.13 > 3 1388.73 > 4 1617.04 > 5 1796.91 > > Here are the timings for different numbers of cleanups for finds. > Note that since we do not hold an exclusive partition lock on a find > there is the additional overhead of taking that lock. > > Resize work in finds: 348.90 dirty_find + 233.45 lock + 275.78 * cleanups > 0 348.90 > 1 872.88 > 2 1138.98 > 3 1448.94 > 4 1665.31 > 5 1975.91 > > The new effective latencies during the resize vs. the clean states. > > #define DHT_INSERT_CLEANS 3 > #define DHT_SEARCH_CLEANS 2 > > insert_resize: 1388.73 -- 3 cleaned > insert_clean: 450.39 > find_resize: 1138.98 -- 2 cleaned > find_clean: 293.16 > > The overall performance will be faster due to not having to examine > more than one hash bucket array most of the time. Thanks for doing all these experiments. Yeah, I think it makes sense to merge this change to improve that case. Since Dilip Kumar's Parallel Bitmap Heap Scan project is no longer using this, I think I should park it here unless/until another potential use case pops up. Some interesting candidates have been mentioned already, and I'm fairly sure there are other uses too, but I don't expect anyone to be interested in committing this patch until something concrete shows up, so I'll go work on other things until then. -- Thomas Munro http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_background contrib module proposal
On Mon, Dec 12, 2016 at 10:17:24PM +0500, Andrew Borodin wrote: > Hi! > > Just in case you'd like to include sleepsort as a test, here it is > wrapped as a regression test(see attachment). But it has serious > downside: it runs no less than 5 seconds. Couldn't it sleep in increments smaller than a second? Like maybe milliseconds? Also, it's probably cleaner (or at least more comprehensible) to write something using format() and dollar quoting than the line with the double 's. Best, David. -- David Fetterhttp://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david(dot)fetter(at)gmail(dot)com Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Parallel build with MSVC
On Thu, Sep 08, 2016 at 08:54:08AM +0200, Christian Ullrich wrote: > * Noah Misch wrote: > > >Committed. > > Much apologizings for coming in late again, but I just realized it would be > better if the user-controlled flags came after all predefined options the > user might want to override. Right now that is only /verbosity in both build > and clean operations. > > Patch attached, also reordering the ecpg-clean command line in clean.bat to > match the others that have the project file first. Committed. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] delta relations in AFTER triggers
On Sun, Dec 18, 2016 at 3:15 PM, Kevin Grittnerwrote: > On Sun, Dec 4, 2016 at 11:35 PM, Haribabu Kommi > wrote: > >> Moved to next CF with "waiting on author" status. > > Patch v8 attempts to address the issues explicitly raised in > Thomas Munro's review. An opaque query environment is created > that, for now, only passes through ephemeral named relations, of > which the only initial implementation is named tuplestores; but > the techniques are intended to support both other types of > ephemeral named relations and environmental properties (things > affecting parsing, planning, and execution that are not coming from > the system catalog) besides ENRs. There is no clue in the access > to the QE whether something is, for example, stored in a list or a > hash table. That's on purpose, so that the addition of other > properties or changes to their implementation doesn't affect the > calling code. +1 > There were a few changes Thomas included in the version he posted, > without really delving into an explanation for those changes. Some > or all of them are likely to be worthwhile, but I would rather > incorporate them based on explicit discussion, so this version > doesn't do much other than generalize the interface a little, > change some names, and add more regression tests for the new > feature. (The examples I worked up for the rough proof of concept > of enforcement of RI through set logic rather than row-at-a-time > navigation were the basis for the new tests, so the idea won't get > totally lost.) Thomas, please discuss each suggested change (e.g., > the inclusion of the query environment in the parameter list of a > few more functions). I was looking for omissions that would cause some kind of statements to miss out on ENRs arbitrarily. It seemed to me that parse_analyze_varparams should take a QueryEnvironment, mirroring parse_analyze, so that PrepareQuery could pass it in. Otherwise, PREPARE wouldn't see ENRs. Is there any reason why SPI_prepare should see them but PREPARE not? Should we attempt to detect if the tupledesc changes incompatibly between planning and execution? > Changed to "Needs review" status. + enr->md.enrtuples = tuplestore_tuple_count(trigdata->tg_newtable); I was wondering about this. As I understand it, plans for statements in plpgsql functions are automatically cached. Is it OK for the number of tuples on the first invocation of the trigger in a given session to determine the estimate used for the plan? I suppose that is the case with regular tables, so maybe it is. + register_enr(estate.queryEnv, enr); + SPI_register_relation(enr); Here plpgsql calls both register_enr and SPI_register_relation. Yet SPI_register_relation calls register_enr too, so is this a mistake? Also, the return code isn't checked. -- Thomas Munro http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] building HEAD on macos fails with #error no source of random numbers configured
On 12/11/16 1:52 PM, Jim Nasby wrote: > On 12/9/16 9:53 AM, Heikki Linnakangas wrote: >>> >>> I'm not sure what --no-recursion does, but I would say that we'd >>> consider that unsupported as well. >> >> Interesting. Running config.status adds those --no-create --no-recursion >> flags automatically. You can see them in the command-line at the top of >> config.log, too. I never bothered to check what they do... > > AIUI they have to do with config dependency checking (where a simple > make will detect if config needs to run again or not). I've been bitten > by this in the past as well. Maybe there's a way to get config to spit > out a warning for those options and have make filter the warning out. When config.status is out of date against configure, then Makefile.global runs config.status --recheck, which internally runs the original configure line with --no-create and --no-recursion added. --no-create means not to create any output files, because the makefile rules will create those. --no-recursion means not to run any configure scripts in subdirectories (we don't use that functionality). It's arguably a bit confusing that config.log then records the configure line with --no-create and --no-recursion added. But other than that, everything works as intended. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] dblink get_connect_string() passes FDW option "updatable" to the connect string, connection fails.
On Sun, Dec 18, 2016 at 4:57 PM, Michael Paquierwrote: > On Mon, Dec 19, 2016 at 6:48 AM, Joe Conway wrote: > > Maybe if "CREATE FOREIGN DATA WRAPPER" had a way to specify that the FDW > > supports a libpq connection it would make sense to allows other FDWs > > with this attribute, but since there is none the current state strikes > > me as a bad idea. > > > > Thoughts? > > libpq is proper to the implementation of the FDW, not the wrapper on > top of it, so using in the CREATE FDW command a way to do the > decision-making that does not look right to me. Filtering things at > connection attempt is a better solution. > -- > Michael > The only benefit I see would be giving the user a slightly more clear error message like ('dblink doesn't work with %', 'mysql') instead of the error about the failure of the connect string generated by the options that did overlap. Gaming out the options of a Wrapper X pointing to server Y: 1. Wrapper X does not have enough overlap in options to accidentally create a legit connect string: Connection fails, user gets a message about the incompleteness of the connection. 2. Wrapper X has enough options (i.e. user+host+dbname,etc) to generate a legit connect string (with matching port), but server+port pointed to by X isn't listening or doesn't speak libpq: Connection fails, user gets an error message. 3. Wrapper X has enough options (i.e. user+host+dbname,etc) to generate a legit connect string (without matching port), but server+5432 pointed to by X doesn't speak libpq: Whatever *is* listening on 5432 has a chance to try to handshake libpq-style, and I guess there's a chance a hostile service listening on that port might know enough libpq to siphon off the credentials, but the creds it would get would be to a pgsql service on Y and Y is already compromised. Also, that vulnerability would exist for FDWs that do speak libpq as well. 4. Wrapper X has enough overlap in options to create a legit connect string which happens to speak libpq: Connection succeeds, and it's a feature not a bug.
Re: [HACKERS] dblink get_connect_string() passes FDW option "updatable" to the connect string, connection fails.
On Mon, Dec 19, 2016 at 6:48 AM, Joe Conwaywrote: > Maybe if "CREATE FOREIGN DATA WRAPPER" had a way to specify that the FDW > supports a libpq connection it would make sense to allows other FDWs > with this attribute, but since there is none the current state strikes > me as a bad idea. > > Thoughts? libpq is proper to the implementation of the FDW, not the wrapper on top of it, so using in the CREATE FDW command a way to do the decision-making that does not look right to me. Filtering things at connection attempt is a better solution. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] dblink get_connect_string() passes FDW option "updatable" to the connect string, connection fails.
On 11/21/2016 03:59 PM, Corey Huinker wrote: > On 11/21/2016 02:16 PM, Tom Lane wrote: >> The dblink docs recommend using dblink_fdw as the FDW for this purpose, >> which would only accept legal connstr options. However, I can see the >> point of using a postgres_fdw server instead, and considering that >> dblink isn't actually enforcing use of any particular FDW type, it seems >> like the onus should be on it to be more wary of what the options are. > I have to admit, this was the first I'd heard of dblink_fdw. I'm glad it > exists, though. And yes, I'd like to be able to use postgres_fdw entries > because there's value in knowing that the connection for your ad-hoc SQL > exactly matches the connection used for other FDW tables. > I'm happy to write the patch, for both v10 and any back-patches we feel > are necessary. I looked at Corey's patch, which is straightforward enough, but I was left wondering if dblink should be allowing any FDW at all (as it does currently), or should it be limited to dblink_fdw and postgres_fdw? It doesn't make sense to even try if the FDW does not connect via libpq. Maybe if "CREATE FOREIGN DATA WRAPPER" had a way to specify that the FDW supports a libpq connection it would make sense to allows other FDWs with this attribute, but since there is none the current state strikes me as a bad idea. Thoughts? Joe -- Crunchy Data - http://crunchydata.com PostgreSQL Support for Secure Enterprises Consulting, Training, & Open Source Development signature.asc Description: OpenPGP digital signature
Re: [HACKERS] move collation import to backend
On 11/30/16 8:18 AM, Peter Eisentraut wrote: >> It seems not to be project style to have prototypes in the middle of the >> file... > > OK, will fix. Updated patch with that fix. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services >From 0c17610b698cc335bc0aed1a66d151e96f618537 Mon Sep 17 00:00:00 2001 From: Peter EisentrautDate: Wed, 30 Nov 2016 12:00:00 -0500 Subject: [PATCH v3] Add function to import operation system collations Move this logic out of initdb into a user-callable function. This simplifies the code and makes it possible to update the standard collations later on if additional operating system collations appear. --- src/backend/catalog/pg_collation.c| 18 +++- src/backend/commands/collationcmds.c | 149 +- src/bin/initdb/initdb.c | 164 +- src/include/catalog/pg_collation_fn.h | 3 +- src/include/catalog/pg_proc.h | 3 + src/include/commands/collationcmds.h | 2 + 6 files changed, 172 insertions(+), 167 deletions(-) diff --git a/src/backend/catalog/pg_collation.c b/src/backend/catalog/pg_collation.c index f37cf37c4a..cda64c44a1 100644 --- a/src/backend/catalog/pg_collation.c +++ b/src/backend/catalog/pg_collation.c @@ -41,7 +41,8 @@ Oid CollationCreate(const char *collname, Oid collnamespace, Oid collowner, int32 collencoding, -const char *collcollate, const char *collctype) +const char *collcollate, const char *collctype, +bool if_not_exists) { Relation rel; TupleDesc tupDesc; @@ -72,10 +73,21 @@ CollationCreate(const char *collname, Oid collnamespace, PointerGetDatum(collname), Int32GetDatum(collencoding), ObjectIdGetDatum(collnamespace))) - ereport(ERROR, + { + if (if_not_exists) + { + ereport(NOTICE, (errcode(ERRCODE_DUPLICATE_OBJECT), - errmsg("collation \"%s\" for encoding \"%s\" already exists", + errmsg("collation \"%s\" for encoding \"%s\" already exists, skipping", collname, pg_encoding_to_char(collencoding; + return InvalidOid; + } + else + ereport(ERROR, + (errcode(ERRCODE_DUPLICATE_OBJECT), + errmsg("collation \"%s\" for encoding \"%s\" already exists", + collname, pg_encoding_to_char(collencoding; + } /* * Also forbid matching an any-encoding entry. This test of course is not diff --git a/src/backend/commands/collationcmds.c b/src/backend/commands/collationcmds.c index 9bba748708..eafc0a99fa 100644 --- a/src/backend/commands/collationcmds.c +++ b/src/backend/commands/collationcmds.c @@ -136,7 +136,11 @@ DefineCollation(ParseState *pstate, List *names, List *parameters) GetUserId(), GetDatabaseEncoding(), collcollate, - collctype); + collctype, + false); + + if (!newoid) + return InvalidObjectAddress; ObjectAddressSet(address, CollationRelationId, newoid); @@ -177,3 +181,146 @@ IsThereCollationInNamespace(const char *collname, Oid nspOid) errmsg("collation \"%s\" already exists in schema \"%s\"", collname, get_namespace_name(nspOid; } + + +/* + * "Normalize" a locale name, stripping off encoding tags such as + * ".utf8" (e.g., "en_US.utf8" -> "en_US", but "br_FR.iso885915@euro" + * -> "br_FR@euro"). Return true if a new, different name was + * generated. + */ +static bool +normalize_locale_name(char *new, const char *old) +{ + char *n = new; + const char *o = old; + bool changed = false; + + while (*o) + { + if (*o == '.') + { + /* skip over encoding tag such as ".utf8" or ".UTF-8" */ + o++; + while ((*o >= 'A' && *o <= 'Z') + || (*o >= 'a' && *o <= 'z') + || (*o >= '0' && *o <= '9') + || (*o == '-')) +o++; + changed = true; + } + else + *n++ = *o++; + } + *n = '\0'; + + return changed; +} + + +Datum +pg_import_system_collations(PG_FUNCTION_ARGS) +{ + bool if_not_exists = PG_GETARG_BOOL(0); + Oid nspid = PG_GETARG_OID(1); + + FILE *locale_a_handle; + char localebuf[NAMEDATALEN]; /* we assume ASCII so this is fine */ + int count = 0; + + if (!superuser()) + ereport(ERROR, +(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), + (errmsg("must be superuser to import system collations"; + + locale_a_handle = OpenPipeStream("locale -a", "r"); + if (locale_a_handle == NULL) + ereport(ERROR, +(errcode_for_file_access(), + errmsg("could not execute command \"%s\": %m", + "locale -a"))); + + while (fgets(localebuf, sizeof(localebuf), locale_a_handle)) + { + int i; + size_t len; + int enc; + bool skip; + char alias[NAMEDATALEN]; + + len = strlen(localebuf); + + if (len == 0 || localebuf[len - 1] != '\n') + { + elog(DEBUG1, "locale name too long, skipped: \"%s\"", localebuf); + continue; + } + localebuf[len - 1] = '\0'; + + /* + * Some systems have locale names that don't consist entirely of ASCII + *
Re: [HACKERS] Logical Replication WIP
On 12/18/2016 05:28 AM, Petr Jelinek wrote: On 17/12/16 18:34, Steve Singer wrote: On 12/16/2016 07:49 AM, Petr Jelinek wrote: Yeah subscriptions are per database. I don't want to make v14 just for these 2 changes as that would make life harder for anybody code-reviewing the v13 so attached is diff with above fixes that applies on top of v13. Thanks that fixes those issues. A few more I've noticed pg_dumping subscriptions doesn't seem to work ./pg_dump -h localhost --port 5441 --include-subscriptions test pg_dump: [archiver (db)] query failed: ERROR: missing FROM-clause entry for table "p" LINE 1: ...LECT rolname FROM pg_catalog.pg_roles WHERE oid = p.subowner... ^ pg_dump: [archiver (db)] query was: SELECT s.tableoid, s.oid, s.subname,(SELECT rolname FROM pg_catalog.pg_roles WHERE oid = p.subowner) AS rolname, s.subenabled, s.subconninfo, s.subslotname, s.subpublications FROM pg_catalog.pg_subscription s WHERE s.subdbid = (SELECT oid FROM pg_catalog.pg_database WHERE datname = current_database()) I have attached a patch that fixes this. pg_dump is also generating warnings pg_dump: [archiver] WARNING: don't know how to set owner for object type SUBSCRIPTION I know that the plan is to add proper ACL's for publications and subscriptions later. I don't know if we want to leave the warning in until then or do something about it. Also the tab-competion for create subscription doesn't seem to work as intended. I've attached a patch that fixes it and patches to add tab completion for alter publication|subscription >From 3ed2ad471766490310b1102d8c9166a597ac11af Mon Sep 17 00:00:00 2001 From: Steve SingerDate: Sun, 18 Dec 2016 12:37:29 -0500 Subject: [PATCH 3/4] Fix tab complete for CREATE SUBSCRIPTION --- src/bin/psql/tab-complete.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c index 8816f6c..6dd47f6 100644 --- a/src/bin/psql/tab-complete.c +++ b/src/bin/psql/tab-complete.c @@ -2298,8 +2298,10 @@ psql_completion(const char *text, int start, int end) /* CREATE SUBSCRIPTION */ else if (Matches3("CREATE", "SUBSCRIPTION", MatchAny)) COMPLETE_WITH_CONST("CONNECTION"); + else if (Matches5("CREATE", "SUBSCRIPTION", MatchAny, "CONNECTION",MatchAny)) + COMPLETE_WITH_CONST("PUBLICATION"); /* Complete "CREATE SUBSCRIPTION ... WITH ( " */ - else if (Matches2("CREATE", "SUBSCRIPTION") && TailMatches2("WITH", "(")) + else if (HeadMatches2("CREATE", "SUBSCRIPTION") && TailMatches2("WITH", "(")) COMPLETE_WITH_LIST5("ENABLED", "DISABLED", "CREATE SLOT", "NOCREATE SLOT", "SLOT NAME"); -- 2.1.4 >From 36a0d4382f7ffd2b740298a2bafd49471766bdad Mon Sep 17 00:00:00 2001 From: Steve Singer Date: Sun, 18 Dec 2016 12:51:14 -0500 Subject: [PATCH 2/4] Add tab-complete for ALTER SUBSCRIPTION --- src/bin/psql/tab-complete.c | 11 ++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c index 4cbb848..8816f6c 100644 --- a/src/bin/psql/tab-complete.c +++ b/src/bin/psql/tab-complete.c @@ -1410,7 +1410,7 @@ psql_completion(const char *text, int start, int end) "EVENT TRIGGER", "EXTENSION", "FOREIGN DATA WRAPPER", "FOREIGN TABLE", "FUNCTION", "GROUP", "INDEX", "LANGUAGE", "LARGE OBJECT", "MATERIALIZED VIEW", "OPERATOR", "POLICY", "PUBLICATION", "ROLE", "RULE", "SCHEMA", "SERVER", "SEQUENCE", - "SYSTEM", "TABLE", "TABLESPACE", "TEXT SEARCH", "TRIGGER", "TYPE", + "SUBSCRIPTION", "SYSTEM", "TABLE", "TABLESPACE", "TEXT SEARCH", "TRIGGER", "TYPE", "USER", "USER MAPPING FOR", "VIEW", NULL}; COMPLETE_WITH_LIST(list_ALTER); @@ -1446,6 +1446,15 @@ psql_completion(const char *text, int start, int end) COMPLETE_WITH_LIST6("PUBLISH INSERT", "NOPUBLISH INSERT", "PUBLISH UPDATE", "NOPUBLISH UPDATE", "PUBLISH DELETE", "NOPUBLISH DELETE"); } + /* ALTER SUBSCRIPTION ... */ + else if (Matches3("ALTER","SUBSCRIPTION",MatchAny)) + { + COMPLETE_WITH_LIST5("WITH", "CONNECTION", "SET PUBLICATION", "ENABLE", "DISABLE"); + } + else if (HeadMatches3("ALTER", "SUBSCRIPTION", MatchAny) && TailMatches2("WITH", "(")) + { + COMPLETE_WITH_CONST("SLOT NAME"); + } /* ALTER SCHEMA */ else if (Matches3("ALTER", "SCHEMA", MatchAny)) COMPLETE_WITH_LIST2("OWNER TO", "RENAME TO"); -- 2.1.4 >From d4685b991245270221a33e0cf8e61d00fb0bf67e Mon Sep 17 00:00:00 2001 From: Steve Singer Date: Sun, 18 Dec 2016 12:47:15 -0500 Subject: [PATCH 1/4] Add tab-complete for ALTER PUBLICATION --- src/bin/psql/tab-complete.c | 16 +--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c index 6ad05b7..4cbb848 100644 --- a/src/bin/psql/tab-complete.c +++ b/src/bin/psql/tab-complete.c @@ -1409,8 +1409,8 @@
Re: [HACKERS] Hash Indexes
On Fri, Dec 16, 2016 at 9:57 PM, Robert Haaswrote: > On Thu, Dec 15, 2016 at 11:33 AM, Amit Kapila wrote: >> Attached are the two patches on top of remove-hash-wrtbuf. Patch >> fix_dirty_marking_v1.patch allows to mark the buffer dirty in one of >> the corner cases in _hash_freeovflpage() and avoids to mark dirty >> without need in _hash_squeezebucket(). I think this can be combined >> with remove-hash-wrtbuf patch. fix_lock_chaining_v1.patch fixes the >> chaining behavior (lock next overflow bucket page before releasing the >> previous bucket page) was broken in _hash_freeovflpage(). These >> patches can be applied in series, first remove-hash-wrtbuf, then >> fix_dirst_marking_v1.patch and then fix_lock_chaining_v1.patch. > > I committed remove-hash-wrtbuf and fix_dirty_marking_v1 but I've got > some reservations about fix_lock_chaining_v1. ISTM that the natural > fix here would be to change the API contract for _hash_freeovflpage so > that it doesn't release the lock on the write buffer. Why does it > even do that? I think that the only reason why _hash_freeovflpage > should be getting wbuf as an argument is so that it can handle the > case where wbuf happens to be the previous block correctly. > Yeah, as of now that is the only case, but for WAL patch, I think we need to ensure that the action of moving all the tuples to the page being written and the overflow page being freed needs to be logged together as an atomic operation. Now apart from that, it is theoretically possible that write page will remain locked for multiple overflow pages being freed (when the page being written has enough space that it can accommodate tuples from multiple overflow pages). I am not sure if it is worth worrying about such a case because practically it might happen rarely. So, I have prepared a patch to retain a lock on wbuf in _hash_freeovflpage() as suggested by you. -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com fix_lock_chaining_v2.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Proposal for changes to recovery.conf API
On Sun, Dec 18, 2016 at 02:14:06PM +0100, Magnus Hagander wrote: > > turn > > > into another section that we keep around (whether as part of the > release > > notes, > > > or as a separate "upgrade steps" section or something). > > > > I suggest whatever we do, we place the information in a permanent > > location that isn't moved or removed. > > > > > > > > +1. Absolutely. That's a very important point. > > That is really my only point --- wherever you put it, expect it to live > there forever. > > Then in the end, it turns out we're in strong agreement :) Yes, but there was talk of adding it to the docs, then removing it in later releases --- that's what I think is a bad idea. I think it would be logical to have a sub-document underneath each major release note section with more detailed instructions. This could be done for minor releases as well where necessary. -- Bruce Momjianhttp://momjian.us EnterpriseDB http://enterprisedb.com + As you are, so once was I. As I am, so you will be. + + Ancient Roman grave inscription + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Proposal for changes to recovery.conf API
On Sun, Dec 18, 2016 at 2:11 PM, Bruce Momjianwrote: > On Sun, Dec 18, 2016 at 02:02:58PM +0100, Magnus Hagander wrote: > > > Again, I am fine putting this as a subsection of the release > notes, > > but > > > let's not pretend it is some extra section we can remove in > five > > years. > > > > > > > > > Depends on what we decide to do about it, but sure, it could > certainly > > turn > > > into another section that we keep around (whether as part of the > release > > notes, > > > or as a separate "upgrade steps" section or something). > > > > I suggest whatever we do, we place the information in a permanent > > location that isn't moved or removed. > > > > > > > > +1. Absolutely. That's a very important point. > > That is really my only point --- wherever you put it, expect it to live > there forever. > > Then in the end, it turns out we're in strong agreement :) -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/
Re: [HACKERS] Proposal for changes to recovery.conf API
On Sun, Dec 18, 2016 at 02:02:58PM +0100, Magnus Hagander wrote: > > Again, I am fine putting this as a subsection of the release notes, > but > > let's not pretend it is some extra section we can remove in five > years. > > > > > > Depends on what we decide to do about it, but sure, it could certainly > turn > > into another section that we keep around (whether as part of the release > notes, > > or as a separate "upgrade steps" section or something). > > I suggest whatever we do, we place the information in a permanent > location that isn't moved or removed. > > > > +1. Absolutely. That's a very important point. That is really my only point --- wherever you put it, expect it to live there forever. -- Bruce Momjianhttp://momjian.us EnterpriseDB http://enterprisedb.com + As you are, so once was I. As I am, so you will be. + + Ancient Roman grave inscription + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Trouble building uuid-ossp extension in new versions of Postgres
> On Mac, the recommended thing is to forget about the ossp code and > use "configure --with-uuid=e2fs". Sorry if that wasn't clear enough. > > Ok thanks, I'll try this. > Reading over your post again, it sounds like you're trying to force-build > contrib/uuid-ossp without having used any of the configure options that > are supposed to enable it. I'm not sure that would ever have worked very > reliably. > > Ok, that makes sense. I had never learned the proper procedure for building extensions, but have had success with going into e.g. contrib/pgcrypto and typing make to build the pgcrypto extension. Since things like that have generally worked for me I didn't learn about the proper flags etc. I'll try some more things and get back to you, thanks for the help. Best, Ryan
Re: [HACKERS] Proposal for changes to recovery.conf API
On Sun, Dec 18, 2016 at 1:57 PM, Bruce Momjianwrote: > On Sun, Dec 18, 2016 at 12:41:01PM +0100, Magnus Hagander wrote: > > No, they become uninteresting to anyone who has passed Postgres 10. > I > > would argue they are still required to be around even after we stop > > supporting Postgres 9.6 because we know everyone will not upgrade > off of > > supported releases once we stop supporting them. This means we can > > effectively never remove the information. > > > > > > This is true, but I think it's also safe to say that it's acceptable > that if > > you are upgrading from an unsupported version you need to read more than > one > > set of documentation -- one set to get to a supported one, and one get > on from > > there. > > How do you propose they find that other documentation set if upgrading > from 9.6 to version 16? Do we put the old docs somewhere on our web > site? Do we have them download a tarball and unpack it? How do we > handle old translated doc versions? I realize the wiki isn't > translated, but if we have translators translate it, we should keep it > available. > We have all the old documentation up on the website, yes. So far it goes back to 6.3. There is no reason to stop that. The wiki is not a good location for documentation that needs to be around for a long time. For translated docs, that's really up to each translation team, since we don't have any central repository for it. We do have that for the translation of the code, but not docs. > > Right now if you migrate from Postgres 8.0 to Postgres 9.6, all the > > information you need is in the 9.6 documentation. If you were to > remove > > migration details from 8.4 to 9.0 they would have to look at the 9.0 > > docs to get a full picture of how to migrate. > > > > > > In fairness, all the information you need is definitely not in the > > documentation. You have all the release notes, that is true. But for a > lot of > > people and in the case of a lot of features, that is not at all enough > > information. But it's true in the sense that it's just as much > information as > > Uh, what exactly is this additional information you are talking about? > Blogs? Are you saying we have information about upgrades we have > captured but discard? > Exactly the type of information that Simon is suggested that he writes this time. The *how* to migrate. And no, we don't have this information "officially" today. Some of it is in blogs, most of it is in the mailinglist archives. The proposal here is, AIUI, to formalize that so we have it in the formal documentation in the future. > > Again, I am fine putting this as a subsection of the release notes, > but > > let's not pretend it is some extra section we can remove in five > years. > > > > > > Depends on what we decide to do about it, but sure, it could certainly > turn > > into another section that we keep around (whether as part of the release > notes, > > or as a separate "upgrade steps" section or something). > > I suggest whatever we do, we place the information in a permanent > location that isn't moved or removed. > > +1. Absolutely. That's a very important point. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/
Re: [HACKERS] Proposal for changes to recovery.conf API
On Sun, Dec 18, 2016 at 12:41:01PM +0100, Magnus Hagander wrote: > No, they become uninteresting to anyone who has passed Postgres 10. I > would argue they are still required to be around even after we stop > supporting Postgres 9.6 because we know everyone will not upgrade off of > supported releases once we stop supporting them. This means we can > effectively never remove the information. > > > This is true, but I think it's also safe to say that it's acceptable that if > you are upgrading from an unsupported version you need to read more than one > set of documentation -- one set to get to a supported one, and one get on from > there. How do you propose they find that other documentation set if upgrading from 9.6 to version 16? Do we put the old docs somewhere on our web site? Do we have them download a tarball and unpack it? How do we handle old translated doc versions? I realize the wiki isn't translated, but if we have translators translate it, we should keep it available. > Right now if you migrate from Postgres 8.0 to Postgres 9.6, all the > information you need is in the 9.6 documentation. If you were to remove > migration details from 8.4 to 9.0 they would have to look at the 9.0 > docs to get a full picture of how to migrate. > > > In fairness, all the information you need is definitely not in the > documentation. You have all the release notes, that is true. But for a lot of > people and in the case of a lot of features, that is not at all enough > information. But it's true in the sense that it's just as much information as Uh, what exactly is this additional information you are talking about? Blogs? Are you saying we have information about upgrades we have captured but discard? > Again, I am fine putting this as a subsection of the release notes, but > let's not pretend it is some extra section we can remove in five years. > > > Depends on what we decide to do about it, but sure, it could certainly turn > into another section that we keep around (whether as part of the release > notes, > or as a separate "upgrade steps" section or something). I suggest whatever we do, we place the information in a permanent location that isn't moved or removed. -- Bruce Momjianhttp://momjian.us EnterpriseDB http://enterprisedb.com + As you are, so once was I. As I am, so you will be. + + Ancient Roman grave inscription + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Quorum commit for multiple synchronous replication.
On Fri, Dec 16, 2016 at 10:42 PM, Fujii Masaowrote: > Attached is the modified version of the patch. Barring objections, I will > commit this version. There is a whitespace: $ git diff master --check src/backend/replication/syncrep.c:39: trailing whitespace. + * > Even after committing the patch, there will be still many source comments > and documentations that we need to update, for example, > in high-availability.sgml. We need to check and update them throughly later. The current patch is complicated enough, so that's fine for me. I checked the patch one last time and that looks good. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Proposal for changes to recovery.conf API
On Sat, Dec 17, 2016 at 10:34 PM, Bruce Momjianwrote: > On Sat, Dec 17, 2016 at 02:52:41PM +0100, Magnus Hagander wrote: > > The point is that the documentation about the recovery.conf changes > in > > Postgres are only interesting to people migrating to Postgres 10, > i.e. > > this is not quality documentation for someone going from Postgres 10 > to > > Postgres 11. > > > > > > > > They will also be interesting to people going from 9.4 to 11, or from > 9.3 to > > 12. Or from 9.5 to 13. > > > > They only become uninteresting when we stop supporting 9.6 which is the > last > > version that didn't have that functionality. > > No, they become uninteresting to anyone who has passed Postgres 10. I > would argue they are still required to be around even after we stop > supporting Postgres 9.6 because we know everyone will not upgrade off of > supported releases once we stop supporting them. This means we can > effectively never remove the information. > This is true, but I think it's also safe to say that it's acceptable that if you are upgrading from an unsupported version you need to read more than one set of documentation -- one set to get to a supported one, and one get on from there. > Right now if you migrate from Postgres 8.0 to Postgres 9.6, all the > information you need is in the 9.6 documentation. If you were to remove > migration details from 8.4 to 9.0 they would have to look at the 9.0 > docs to get a full picture of how to migrate. > In fairness, all the information you need is definitely not in the documentation. You have all the release notes, that is true. But for a lot of people and in the case of a lot of features, that is not at all enough information. But it's true in the sense that it's just as much information as you would've had if you'd done the incremental steps of upgrading, because we didn't purge anything. > Again, I am fine putting this as a subsection of the release notes, but > let's not pretend it is some extra section we can remove in five years. Depends on what we decide to do about it, but sure, it could certainly turn into another section that we keep around (whether as part of the release notes, or as a separate "upgrade steps" section or something). //Magnus
Re: [HACKERS] Logical Replication WIP
On 17/12/16 18:34, Steve Singer wrote: > On 12/16/2016 07:49 AM, Petr Jelinek wrote: >> Hi, >> >> attached is version 13 of the patch. >> >> I merged in changes from PeterE. And did following changes: >> - fixed the ownership error messages for both provider and subscriber >> - added ability to send invalidation message to invalidate whole >> relcache and use it in publication code >> - added the post creation/alter/drop hooks >> - removed parts of docs that refer to initial sync (which does not exist >> yet) >> - added timeout handling/retry, etc to apply/launcher based on the GUCs >> that exist for wal receiver (they could use renaming though) >> - improved feedback behavior >> - apply worker now uses owner of the subscription as connection user >> - more tests >> - check for max_replication_slots in launcher >> - clarify the update 'K' sub-message description in protocol > > A few things I've noticed so far > > If I shutdown the publisher I see the following in the log > > 2016-12-17 11:33:49.548 EST [1891] LOG: worker process: ?)G? (PID 1987) > exited with exit code 1 > > but then if I shutdown the subscriber postmaster and restart it switches to > 2016-12-17 11:43:09.628 EST [2373] LOG: worker process: (PID 2393) > exited with exit code 1 > > Not sure where the 'G' was coming from (other times I have seen an 'I' > here or other random characters) > Uninitialized bgw_name for apply worker. Rather silly bug. Fixed. > > I don't think we are cleaning up subscriptions on a drop database > > If I do the following > > 1) Create a subscription in a new database > 2) Stop the publisher > 3) Drop the database on the subscriber > > test=# create subscription mysuba connection 'host=localhost dbname=test > port=5440' publication mypub; > test=# \c b > b=# drop database test; > DROP DATABASE > b=# select * FROM pg_subscription ; > subdbid | subname | subowner | subenabled | subconninfo | > subslotname | subpublications > -+-+--++--+-+- > >16384 | mysuba | 10 | t | host=localhost dbname=test > port=5440 | mysuba | {mypub} > Good one. I added check that prevents dropping database when there is subscription defined for it. I think we can't cascade here as subscription may or may not hold resources (slot) in another instance/database so preventing the drop is best we can do. > > Also I don't think I can now drop mysuba > b=# drop subscription mysuba; > ERROR: subscription "mysuba" does not exist > Yeah subscriptions are per database. I don't want to make v14 just for these 2 changes as that would make life harder for anybody code-reviewing the v13 so attached is diff with above fixes that applies on top of v13. -- Petr Jelinek http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services diff --git a/src/backend/catalog/pg_subscription.c b/src/backend/catalog/pg_subscription.c index 8be9f39..2c58cc6 100644 --- a/src/backend/catalog/pg_subscription.c +++ b/src/backend/catalog/pg_subscription.c @@ -107,7 +107,41 @@ GetSubscription(Oid subid, bool missing_ok) } /* - * Free memory allocated by subscription struct. */ + * Return number of subscriptions defined in given database. + * Used by dropdb() to check if database can indeed be dropped. + */ +int +CountDBSubscriptions(Oid dbid) +{ + intnsubs = 0; + Relation rel; + ScanKeyData scankey; + SysScanDesc scan; + HeapTuple tup; + + rel = heap_open(SubscriptionRelationId, RowExclusiveLock); + + ScanKeyInit(, +Anum_pg_subscription_subdbid, +BTEqualStrategyNumber, F_OIDEQ, +ObjectIdGetDatum(dbid)); + + scan = systable_beginscan(rel, InvalidOid, false, + NULL, 1, ); + + while (HeapTupleIsValid(tup = systable_getnext(scan))) + nsubs++; + + systable_endscan(scan); + + heap_close(rel, NoLock); + + return nsubs; +} + +/* + * Free memory allocated by subscription struct. + */ void FreeSubscription(Subscription *sub) { diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c index 0919ad8..45d152c 100644 --- a/src/backend/commands/dbcommands.c +++ b/src/backend/commands/dbcommands.c @@ -37,6 +37,7 @@ #include "catalog/pg_authid.h" #include "catalog/pg_database.h" #include "catalog/pg_db_role_setting.h" +#include "catalog/pg_subscription.h" #include "catalog/pg_tablespace.h" #include "commands/comment.h" #include "commands/dbcommands.h" @@ -790,6 +791,7 @@ dropdb(const char *dbname, bool missing_ok) int npreparedxacts; int nslots, nslots_active; + int nsubscriptions; /* * Look up the target database's OID, and get exclusive lock on it. We @@ -875,6 +877,21 @@ dropdb(const char *dbname, bool missing_ok) errdetail_busy_db(notherbackends, npreparedxacts))); /* + * Check if there are subscriptions defined in the target database. + * + * We can't drop them automatically because
Re: [HACKERS] Logical Replication WIP
On 17/12/16 13:37, Erik Rijkers wrote: > On 2016-12-16 13:49, Petr Jelinek wrote: >> >> version 13 of the patch. >> >> 0001-Add-PUBLICATION-catalogs-and-DDL-v13.patch.gz (~32 KB) >> 0002-Add-SUBSCRIPTION-catalog-and-DDL-v13.patch.gz (~28 KB) >> 0003-Define-logical-rep...utput-plugi-v13.patch.gz (~13 KB) >> 0004-Add-logical-replication-workers-v13.patch.gz (~44 KB) >> 0005-Add-separate-synch...or-logical--v13.patch.gz (~2 KB) > > Hi, > > You wrote on 2016-08-05: : > >> What's missing: >> - sequences, I'd like to have them in 10.0 but I don't have good >>way to implement it. PGLogical uses periodical syncing with some >>buffer value but that's suboptimal. I would like to decode them >>but that has proven to be complicated due to their sometimes >>transactional sometimes nontransactional nature, so I probably >>won't have time to do it within 10.0 by myself. > > I ran into problems with sequences and I wonder if sequences-problems > are still expected, as the above seems to imply. > > (short story: I tried to run pgbench across logical replication; and > therefore > added a sequence to pgbench_history to give it a replica identity, and > cannot get it to work reliably ). > Sequences are not replicated but that should not prevent pgbench_history itself from being replicated when you add serial to it. BTW you don't need to add primary key to pgbench_history. Simply ALTER TABLE pgbench_history REPLICA IDENTITY FULL; should be enough. -- Petr Jelinek http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers