Re: [HACKERS] pg_hba_file_settings view patch
On Mon, Oct 3, 2016 at 3:25 PM, Vitaly Burovoy wrote: > I guess for ability to use filtering like: > > SELECT * FROM pg_hba_rules WHERE options->>radiusserver LIKE '%.example.com'; > > I think it would be harder if options is an array of strings... With unnest() and a matching pattern, not that hard but.. -- 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] pg_hba_file_settings view patch
On 10/2/16, Michael Paquier wrote: > + push_jsonb_string_key(&parseState, "map"); > + push_jsonb_string_value(&parseState, hba->usermap); > [...] > + > + options > + jsonb > + Configuration options set for authentication method > + > Why is it an advantage to use jsonb here instead of a simple array > made of name=value? If they were nested I'd see a case for it but it > seems to me that as presented this is just an overkill. I guess for ability to use filtering like: SELECT * FROM pg_hba_rules WHERE options->>radiusserver LIKE '%.example.com'; I think it would be harder if options is an array of strings... -- Best regards, Vitaly Burovoy -- 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] GiST penalty functions [PoC]
> This thread has basically died, so marking as returned with feedback. Well, not exactly died, but it's kind of in blind lead. I'll summarize a bit for future references: 1. cube extension for indexing lowered dimensionality data (2d in 3d) is broken. Queries effectively will degrade to full index scan. 2. In existing GiST API this can be fixed only with technique, implemented in this patch. But this technique seems controversial. 3. This patch also brings 30%-40% speedup, but expected speedup of advanced GIST API techniques is much higher. As Norbert Beckmann put it in other discussion: > Overlap optimization [requires API advancement] is one of the main elements, > if not the main query performance tuning element of the RR*-tree. You would > fall back to old R-Tree times if that would be left off. Outcome: I'm planning to rollout another patch for both GiST and cube, probably to next CF, which will render this patch unnecessary. Thanks to everyone for discussion. Best regards, Andrey Borodin. -- 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] Commit fest 2016-09 is now closed
On Mon, Oct 3, 2016 at 2:01 PM, Michael Paquier wrote: > There was something like 60~70 patches still listed as active in the > CF app on Friday my time, so doing the vacuum cleanup has taken some > time by marking patches as returned with feedback, rejected (few) and > moved to next CF. A couple of things I noted when going through the > remaining entries: > - a lot of folks have registered to review a patch and did not post a > review. Please avoid putting your name on a patch if you can't afford > the time to look at a patch. > - The rule one patch sent = one patch review of equal difficulty still works. > - Be careful of the status of the patch, and update it correctly. I > have noticed a lot of improvements in this area actually! Forgot something: if you feel that your patch did not receive the correct treatment, of course feel free to update its status in the CF app, feel free as well to complain at me and/or on this thread if necessary. I am sure we will be able to sort things out :) -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Commit fest 2016-09 is now closed
Hi all, Per $subject, I would like to thank on behalf of Fabrizio everybody who has taken the time to send patches, review them and argue about them. This has been by experience the largest commit fest ever, with more than 200 entries in total. The delay in stabilizing 9.6 is likely the explanation behind such a high number, a lot of patches have been parked there for a couple of months. There was something like 60~70 patches still listed as active in the CF app on Friday my time, so doing the vacuum cleanup has taken some time by marking patches as returned with feedback, rejected (few) and moved to next CF. A couple of things I noted when going through the remaining entries: - a lot of folks have registered to review a patch and did not post a review. Please avoid putting your name on a patch if you can't afford the time to look at a patch. - The rule one patch sent = one patch review of equal difficulty still works. - Be careful of the status of the patch, and update it correctly. I have noticed a lot of improvements in this area actually! Here is the final score of this session: - Committed: 103. - Moved to next CF: 51. - Rejected: 12. - Returned with Feedback: 53. Total: 219. Thanks, -- 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] pg_hba_file_settings view patch
On Mon, Sep 5, 2016 at 4:09 PM, Haribabu Kommi wrote: > On Sun, Sep 4, 2016 at 1:44 AM, Simon Riggs wrote: >> On 15 August 2016 at 12:17, Haribabu Kommi >> wrote: >> >> > comments? >> >> This looks like a good feature contribution, thanks. >> >> At present the patch doesn't apply cleanly, please rebase. > > > Rebased patch is attached. Moved to next CF as there is a patch and no reviews. + push_jsonb_string_key(&parseState, "map"); + push_jsonb_string_value(&parseState, hba->usermap); [...] + + options + jsonb + Configuration options set for authentication method + Why is it an advantage to use jsonb here instead of a simple array made of name=value? If they were nested I'd see a case for it but it seems to me that as presented this is just an overkill. In short, I think that this patch needs a bit of rework, so I am marking it as returned with feedback. -- 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] Declarative partitioning - another take
On 2016/10/03 13:26, Michael Paquier wrote: > On Fri, Sep 30, 2016 at 9:10 AM, Robert Haas wrote: >> On Thu, Sep 29, 2016 at 8:09 AM, Amit Langote >> wrote: >>> I removed DEPENDENCY_IGNORE. Does the following look good or am I still >>> missing something? >> >> You missed your commit message, but otherwise looks fine. > > I have moved this patch to next CF because that's fresh, but switched > the patch as "waiting on author". Be careful, the patch was in "needs > review". Thanks Michael. I was going to post the patch addressing Robert's comments today, which I will anyway. Then I will switch the status back to "Needs Review", albeit as part of the next CF. 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] pg_xlogdump follow into the future
On Mon, Jul 18, 2016 at 8:10 PM, Magnus Hagander wrote: > > > On Thu, Jul 14, 2016 at 8:27 PM, Andres Freund wrote: >> >> On 2016-07-14 13:46:23 +0200, Magnus Hagander wrote: >> > Currently, if you run pg_xlogdump with -f, you have to specify an end >> > position in an existing file, or if you don't it will only follow until >> > the >> > end of the current file. >> >> That's because specifying a file explicitly says that you only want to >> look at that file, specifying two files that you want the range >> inclusively between the two files. -f works if you just use -s. > > > Hmm. It does now. I'm *sure* it didn't when I was testing it. It must've > been something else that was broken at that point :) Same as Andres here, my understanding is that one file means that you just want to look at this file, and two files permits to look at a range of changes. But I have no argument against changing the current behavior either. >> > I'd appreciate a review of that by someone who's done more work on the >> > xlog >> > stuff, but it seems trivial to me. Not sure I can argue it's a bugfix >> > though, since the usecase simply did not work... >> >> I'd say it's working as intended, and you want to change that >> intent. That's fair, but I'd not call it a bug, and I'd say it's not >> really 9.6 material. > > Based on that, I agree that it's working as intended. > > And definitely that it's not 9.6 material. > > I'll stick it on the CF page so I don't forget about it. Moved to next CF. Magnus, you may want to finish wrapping that if you still intend to change the current behavior. -- 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] Patch to implement pg_current_logfile() function
On Sat, Jul 2, 2016 at 8:56 AM, Karl O. Pinc wrote: > On Tue, 28 Jun 2016 11:06:24 +0200 > Gilles Darold wrote: > >> Thank you very much for the patch review and please apologies this too >> long response delay. I was traveling since end of April and totally >> forgotten this patch. I have applied all your useful feedbacks on the >> patch and attached a new one (v4) to this email : > > My schedule is really full at the moment. I'll get to this > when I have a bit of time. If you get anxious please write > and I'll see about making faster progress. Moved to next CF, the patch still applies. Karl, you have registered to review this patch a couple of months back but nothing happened. I have removed your name for now. If you have time, don't hesitate to come back to it. -- 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] sequence data type
On Sun, Sep 11, 2016 at 12:38 PM, Vitaly Burovoy wrote: > On 9/10/16, Jim Nasby wrote: >> On 9/3/16 6:01 PM, Gavin Flower wrote: >>> I am curious as to the use cases for other possibilities. >> >> A hex or base64 type might be interesting, should those ever get created... >> -- >> Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX > > Hex or base64 are not data types. They are just different > representation types of binary sequences. > Even for bigints these representations are done after writing numbers > as byte sequences. Moved to next CF. The patch did not actually receive that much reviews. -- 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] raw output from copy
On Sat, Jul 16, 2016 at 5:55 PM, Pavel Stehule wrote: > I am sending fresh version of COPY RAW patch. Moved to next CF per this status. +++ b/src/interfaces/libpq/test/copy-raw-regress.pl @@ -0,0 +1,48 @@ +#!/usr/bin/perl -w + +use strict; I don't understand why this is shaped this way, I mean the perl part if we have the TAP infra in place. MSVC is not testing it as well. -- 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] Declarative partitioning - another take
On Fri, Sep 30, 2016 at 9:10 AM, Robert Haas wrote: > On Thu, Sep 29, 2016 at 8:09 AM, Amit Langote > wrote: >> I removed DEPENDENCY_IGNORE. Does the following look good or am I still >> missing something? > > You missed your commit message, but otherwise looks fine. I have moved this patch to next CF because that's fresh, but switched the patch as "waiting on author". Be careful, the patch was in "needs review". -- 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] NOT EXIST for PREPARE
On Sun, Jun 5, 2016 at 11:33 AM, David G. Johnston wrote: > As an aside; most (all?) of our INEs apply to persistent schema objects. > Extending that to session objects is a conceptual leap. There is close to no activity here, so I marked the patch as returned with feedback. I am doubtful about the performance btw.. -- 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] asynchronous and vectorized execution
At Mon, 3 Oct 2016 13:14:23 +0900, Michael Paquier wrote in > On Thu, Sep 29, 2016 at 5:50 PM, Kyotaro HORIGUCHI > wrote: > > Sorry for no response, but, the answer is yes. We could be able > > to avoid the problem by managing execution state for every > > node. But it needs an additional flag in *State structs and > > manipulating on the way shuttling up and down around the > > execution tree. > > Moved to next CF. Thank you. -- Kyotaro Horiguchi NTT Open Source Software Center -- 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 Sat, Sep 10, 2016 at 7:28 AM, Kevin Grittner wrote: > v6 fixes recently-introduced bit-rot. Not as big as I thought, only 2k when both patches are combined... The patch without noapi in its name needs to be applied first, and after the patch with noapi can be applied. 60 files changed, 2073 insertions(+), 63 deletions(-) Moved to next CF. -- 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] sequences and pg_upgrade
On Sat, Oct 1, 2016 at 1:50 AM, Anastasia Lubennikova wrote: > 23.09.2016 21:06, Peter Eisentraut: >> >> Here is an updated patch set. Compared to the initial set, I have >> changed pg_dump's sorting priorities so that sequence data is always >> after table data. This would otherwise have introduced a problem >> because sortDataAndIndexObjectsBySize() only considers consecutive >> DO_TABLE_DATA entries. Also, I have removed the separate >> --sequence-data switch from pg_dump and made it implicit in >> --binary-upgrade. (So the previous patches 0002 and 0003 have been >> combined, because it's no longer a separate feature.) >> > > The patches are good, no complaints. > But again, I have the same question. > I was confused, why do we always dump sequence data, > because I'd overlooked the --sequence-data key. I'd rather leave this > option, > because it's quite non intuitive behaviour... > /* dump sequence data even in schema-only mode */ Moved to next CF. This is fresh. -- 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] asynchronous and vectorized execution
On Thu, Sep 29, 2016 at 5:50 PM, Kyotaro HORIGUCHI wrote: > Sorry for no response, but, the answer is yes. We could be able > to avoid the problem by managing execution state for every > node. But it needs an additional flag in *State structs and > manipulating on the way shuttling up and down around the > execution tree. Moved to next CF. -- 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] Aggregate Push Down - Performing aggregation on foreign server
On Fri, Sep 30, 2016 at 8:58 PM, Jeevan Chalke wrote: > On Mon, Sep 26, 2016 at 6:15 PM, Ashutosh Bapat > wrote: >> >> This patch will need some changes to conversion_error_callback(). That >> function reports an error in case there was an error converting the >> result obtained from the foreign server into an internal datum e.g. >> when the string returned by the foreign server is not acceptable by >> local input function for the expected datatype. In such cases, the >> input function will throw error and conversion_error_callback() will >> provide appropriate context for that error. postgres_fdw.sql has tests >> to test proper context >> We need to fix the error context to provide meaningful information or >> at least not crash. This has been discussed briefly in [1]. > > Oops. I had that in mind when working on this. Somehow skipped checking > for conversion error context. I have fixed that in v3 patch. > Removed assert and for non Var expressions, printing generic context. > This context is almost in-line with the discussion you referred here. Moved to next CF. -- 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] Macro customizable hashtable / bitmapscan & aggregation perf
On 2016-10-02 02:59:04 +0200, Tomas Vondra wrote: > On 10/02/2016 02:17 AM, Andres Freund wrote: > > Hi, > > ... > > > > > > > I think a crucial part of the benchmarking will be identifying and > > > > > measuring corner cases, e.g. a lot of rows with the same key, etc. > > > > > Although that probably is not a major issue for the two places > > > > > switched to the new implementation (e.g. execGrouping merges the > > > > > duplicates to a single group, by definition). > > > > > > > > Hm. I don't really see a case where that's going to cause issues - all > > > > the execGrouping.c users only store one key, and either ignore later > > > > ones, or add the data to the initial tuple in the group. I don't really > > > > see any case where repeated keys should cause an issue for a hash table? > > > > > > > > > > Yep, that's pretty much what I suggested. I was wondering more about the > > > other places where this hash table might be used - I've been thinking > > > about > > > hashjoins, but now that I think of it - that's a fairly specialized and > > > tuned code. In any case, let's not complicate the discussion for now. > > > > My point is that I don't really see any potential usecase where > > that's an issue. > > > > OK, I think we agree the two places modified by the submitted patch series > are fine. Let's leave discussion about places modified by future patches for > the future ;-) I think my problem here is that I just can't see a potential use-case for hashtables where that's an issue ;) > > > 5) SH_RESIZE > > > > > > Do we expect to shrink hash tables? > > > > Not at the moment. Should be doable, but I'm not sure it's worth > > developing and testing - I don't see any usecases in pg atm. > > > > OK, sure. Then what about adding this to SH_RESIZE? > > Assert(oldsize <= newsize); Yes. And potentially rename it to SH_GROW. > I assumed we might shrink the catcache after invalidation, for example (but > maybe I don't really know how that works). They don't shrink so far, and in most invalidation cases we don't delete entries, just mark them as invalid (as they might be referenced on the outside at that moment). > > > It's not entirely clear why is it guaranteed that there's always > > > an element with optimal position (when load factor < 1)? Adding an > > > explanation or a link to a paper would be helpful, I guess. > > > > As the load factor always has to be below 1, there always has to be > > an empty bucket. Every bucket after an empty bucket has to be at > > it's optimal position. > Hmmm, thanks to moving the entries after delete? Pretty much, yes. > Adding an assert() enforcing this seems like a good idea. Probably requires some additional state to be meaningfully enforced. Not sure that's worth the effort. If that invariant is broken, not a lot of stuff works (believe me, I have broken it ;)). Otherwise searches won't necessarily work anymore, if they didn't end up in their original position (as the cell would now be empty, a lookup/insert/delete would not find the existing element). > > > 7) Should hash agg size estimation for the planner consider the > > > fillfactor? > > > > > > I think we should account for fillfactor - we should account for the > > > allocated memory, even if there's free space in the hash table. After all, > > > that's what hashjoins do. > > > > We didn't really for dynahash (as it basically assumed a fillfactor of 1 > > all the time), not sure if changing it won't also cause issues. > > > > That's a case of glass half-full vs. half-empty, I guess. If we assumed load > factor 1.0, then I see it as accounting for load factor (while you see it as > not accounting of it). Well, even before we grow by factors of two, so 1 wasn't accurate for most of the time. My issue isn't really that I don't want to do it, but that I'm not entirely sure that there's a good way. TupleHashEntryData itself isn't exactly large, and we'd have to multiply it by the load factor. At the same time, after the patch, AggStatePerGroupData isn't allocated for empty elements anymore, and that's the largest part of the memory. So I'm kind of inclined to just disregard the fillfactor (and document that). > Also, with load factor 0.8 the table is only ~20% larger, so even if we > don't include that into work_mem it's a reasonably small error (easily > smaller than errors in the pre-9.5 HashJoin accounting, which did not > include chunk headers etc.). I think in either cases the entries themselves are smaller after the patch by enough that the overhead doesn't matter once you crossed a few dozen entries. Regards, Andres -- 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] Multi-tenancy with RLS
On Tue, Jul 19, 2016 at 3:42 PM, Haribabu Kommi wrote: > The above changes are based on my understanding to the discussion occurred in > this mail. In case if I miss anything, please let me know, i will > correct the same. The patch series still apply. + " ((classid = (select oid from pg_class where relname = 'pg_aggregate'))" + " OR (classid = (select oid from pg_class where relname = 'pg_cast') AND has_cast_privilege(objid, 'any'))" + " OR (classid = (select oid from pg_class where relname = 'pg_collation'))" [... long list ...] That's quite hard to digest... +static bool +get_catalog_policy_string(Oid relationid, Form_pg_class pg_class_tuple, char *buf) This is an exceptionally weak interface at quick glance. This is using SQL strings, and nothing is actually done regarding potentially conflicting name types... The number of new files included in policy.c is impressive as well.. This does not count as a full review though, so I am moving it to next CF. Perhaps it will find its audience. -- 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] pg_sequence catalog
On Sun, Sep 11, 2016 at 2:15 PM, Amit Kapila wrote: > On Sun, Sep 11, 2016 at 12:39 AM, Andres Freund wrote: >> On 2016-09-10 17:23:21 +0530, Amit Kapila wrote: >>> > >>> >>> I may be missing something here, but why would it contend on a lock, >>> as per locking scheme proposed by Alvaro, access to sequence object >>> will need a share lock on buffer page. >> >> To make checkpointing/bgwriter work correctly we need exclusive locks on >> pages while writing..., or some new lock level preventing the page from >> being written out, while "shared dirtying" locks are being held. >> > > Right and I think you have a very valid concern, but if we think that > storing multiple sequences on a same page is a reasonable approach, > then we can invent some locking mechanism as indicated by you such > that two writes on same page won't block each other, but they will be > blocked with bgwriter/checkpointer. This thread has died a couple of weeks back, so I am marking it as returned with feedback by seeing the discussion that has been done. Feel free to update the patch if you think that's not adapted. -- 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] [PATCH] Logical decoding timeline following take II
On Thu, Sep 1, 2016 at 1:08 PM, Craig Ringer wrote: > Attached is a rebased and updated logical decoding timeline following > patch for 10.0. Moved to next CF, nothing has happened since submission. -- 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] Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)
On Fri, Sep 16, 2016 at 6:12 AM, Tom Lane wrote: > I'm happy to get rid of the LCM behavior, I just want to have some wiggle > room to be able to get it back if somebody really needs it. Er, actually no that's this thread for this CF entry: https://commitfest.postgresql.org/10/759/ Still there has not been much activity. -- 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] Changed SRF in targetlist handling
On Wed, Aug 24, 2016 at 3:55 AM, Andres Freund wrote: > Comments? This thread has no activity for some time now and it is linked to this CF entry: https://commitfest.postgresql.org/10/759/ I am marking it as returned with feedback.. -- 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] Stopping logical replication protocol
On 3 Oct. 2016 10:15, "Michael Paquier" wrote: > > On Tue, Sep 27, 2016 at 11:05 AM, Craig Ringer wrote: > > On 26 September 2016 at 21:52, Vladimir Gordiychuk wrote: > >>>You should rely on time I tests as little as possible. Some of the test > >>> hosts are VERY slow. If possible something deterministic should be used. > >> > >> That's why this changes difficult to verify. Maybe in that case we should > >> write benchmark but not integration test? > >> In that case we can say, before this changes stopping logical replication > >> gets N ms but after apply changes it gets NN ms where NN ms less than N ms. > >> Is it available add this kind of benchmark to postgresql? I will be grateful > >> for links. > > > > It's for that reason that I added a message printed only in verbose > > mode that pg_recvlogical emits when it's exiting after a > > client-initiated copydone. > > > > You can use the TAP tests, print diag messages, etc. But we generally > > want them to run fairly quickly, so big benchmark runs aren't > > desirable. You'll notice that I left diag messages in to report the > > timing for the results in your tests, I just changed the tests so they > > didn't depend on very tight timing for success/failure anymore. > > > > We don't currently have any automated benchmarking infrastructure. > > Which seems like this patch is not complete yet. Personally I think it is. I'm just explaining why I adjusted Vladimir's tests to be less timing sensitive and rely on a qualitative property instead. That said, it's had recent change and it isn't a big intrusive change that really need attention this cf. > I am marking it as > returned with feedback, but it may be a better idea to move it to next > CF once a new version with updated tests shows up. I'll move it now. I think the tests are fine, if Vladimir agrees, so IMO it's ready to go. More eyes wouldn't hurt though. If Vladimir wants benchmarking based tests that's a whole separate project IMO. Something that will work robustly on the weird slow machines we have isn't simple. Probably a new buildfarm option etc since we won't want to clutter the really slow old niche boxes with it. Vladimir, can I have your opinion on the latest patch or if you want to make changes, an updated patch?
Re: [HACKERS] Pinning a buffer in TupleTableSlot is unnecessary
On Wed, Aug 31, 2016 at 6:03 AM, Andres Freund wrote: > On 2016-08-30 21:59:44 +0100, Greg Stark wrote: >> On Tue, Aug 30, 2016 at 11:12 AM, Heikki Linnakangas wrote: >> > While profiling some queries and looking at executor overhead, I realized >> > that we're not making much use of TupleTableSlot's ability to hold a buffer >> > pin. In a SeqScan, the buffer is held pinned by the underlying heap-scan >> > anyway. Same with an IndexScan, and the SampleScan. The only thing that >> > relies on that feature is TidScan, but we could easily teach TidScan to >> > hold >> > the buffer pin directly. >> > >> > So, how about we remove the ability of a TupleTableSlot to hold a buffer >> > pin, per the attached patch? It shaves a few cycles from a ExecStoreTuple() >> > and ExecClearTuple(), which get called a lot. I couldn't measure any actual >> > difference from that, though, but it seems like a good idea from a >> > readability point of view, anyway. >> >> Out of curiosity why go in this direction and not the other? Why not >> move those other scans to use the TupleTableSlot API to manage the >> pins. Offhand it sounds more readable not less to have the >> TupleTableSlot be a self contained data structure that guarantees the >> lifetime of the pins matches the slots rather than have a dependency >> on the code structure in some far-away scan. > > At least for heap scans the pins are page level, and thus longer lived > than the data in a slot. It's important that a scan holds a pin, because > it needs to rely on the page not being hot pruned etc.. I have moved this patch to next CF. Heikki, are you still planning to work on it? -- 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] PATCH: Batch/pipelining support for libpq
On 3 October 2016 at 10:10, Michael Paquier wrote: > On Tue, Sep 6, 2016 at 8:01 PM, Craig Ringer wrote: >> On 6 September 2016 at 16:10, Daniel Verite wrote: >>> Craig Ringer wrote: >>> Updated patch attached. >>> >>> Please find attached a couple fixes for typos I've came across in >>> the doc part. >> >> Thanks, will apply and post a rebased patch soon, or if someone picks >> this up in the mean time they can apply your diff on top of the patch. > > Could you send an updated patch then? At the same time I am noticing > that git --check is complaining... This patch has tests and a > well-documented feature, so I'll take a look at it soon at the top of > my list. Moved to next CF for now. Thanks. I'd really like to teach psql in non-interactive mode to use it, but (a) I'm concerned about possible subtle behaviour differences arising if we do that and (b) I won't have the time. I think it's mostly of interest to app authors and driver developers and that's what it's aimed at. pg_restore could benefit a lot too. -- Craig Ringer 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
Re: [HACKERS] GiST penalty functions [PoC]
On Thu, Sep 22, 2016 at 3:45 AM, Andrew Borodin wrote: > [blah] > > Practically, this algorithm cannot be implemented in current GiST API > (only if we provide non-penalty-based choose subtree function, > optional for GiST extension), but it certainly has scientific value. This thread has basically died, so marking as returned with feedback. If you feel that's not adapted, feel free to move it to next CF. -- 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] Cache Hash Index meta page.
On Thu, Sep 29, 2016 at 12:55 AM, Mithun Cy wrote: > On Tue, Sep 27, 2016 at 1:53 AM, Jeff Janes wrote: > > I think that this needs to be updated again for v8 of concurrent and v5 > of wal > > Adding the rebased patch over [1] + [2] > > [1] Concurrent Hash index. > [2] Wal for hash index. Moved to next CF. -- 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: speeding up GIN build with parallel workers
On Wed, Sep 14, 2016 at 3:48 PM, Heikki Linnakangas wrote: > If we flushed the tree to a tape instead, then we could perhaps use the > machinery that Peter's parallel B-tree patch is adding to tuplesort.c, to > merge the tapes. I'm not sure if that works out, but I think it's worth some > experimentation. Marking as returned with feedback per mainly this comment. -- 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] Tracking wait event for latches
On Fri, Sep 30, 2016 at 5:47 PM, Michael Paquier wrote: > On Fri, Sep 30, 2016 at 1:48 AM, Robert Haas wrote: >> The way that we're constructing the wait event ID that ultimately gets >> advertised in pg_stat_activity is a bit silly in this patch. We start >> with an event ID (which is an integer) and we then do an array lookup >> (via GetWaitEventClass) to get a second integer which is then XOR'd >> back into the first integer (via pgstat_report_wait_start) before >> storing it in the PGPROC. New idea: let's change >> pgstat_report_wait_start() to take a single integer argument which it >> stores without interpretation. Let's separate the construction of the >> wait event into a separate macro, say make_wait_event(). Then >> existing callers of pgstat_report_wait_start(a, b) will instead do >> pgstat_report_wait_start(make_wait_event(a, b)), but callers that want >> to construct the wait event and push it through a few levels of the >> call stack before advertising it only need to pass one integer. If >> done correctly, this should be cleaner and faster than what you've got >> right now. > > Hm, OK So ProcSleep() and ProcWaitForSignal() need an additional > argument in the shape of a uint32 wait_event. So we need to change the > shape of WaitLatch&friends to have also just a uint32 as an extra > argument. This has as result to force all the callers of WaitLatch to > use the new routine pgstat_make_wait_event() (renamed it), so pgstat.h > needs to be included where WaitLatch() is called. Hmm. I like the use of pgstat in that name. It helps with the confusion created by the overloading of the term 'wait event' in the pg_stat_activity view and the WaitEventSet API, by putting it into a different pseudo-namespace. + uint32 wait_event; How about a typedef for that instead of using raw uint32 everywhere? Something like pgstat_wait_descriptor? Then a variable name like pgstat_desc, since this is most definitely not just a wait_event anymore. + /* Define event to wait for */ It's not defining the event to wait for at all, it's building a description for pgstat. + wait_event = pgstat_make_wait_event(WAIT_EXTENSION, + WAIT_EVENT_EXTENSION); It's not making a wait event, it's combining a class and an event. How about something like this: pgstat_desc = pgstat_make_wait_descriptor(WAIT_CLASS_EXTENSION, WAIT_EVENT_EXTENSION)? /* Sleep until there's something to do */ wc = WaitLatchOrSocket(MyLatch, WL_LATCH_SET | WL_SOCKET_READABLE, PQsocket(conn), - -1L); + -1L, + wait_event); ... then use 'pgstat_desc' here. -- 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] [COMMITTERS] pgsql: Copy-editing for contrib/pg_visibility documentation.
On Mon, Oct 3, 2016 at 11:06 AM, Robert Haas wrote: > On Sun, Oct 2, 2016 at 9:00 PM, Thomas Munro > wrote: >> On Mon, Oct 3, 2016 at 1:36 PM, Robert Haas wrote: >>> On Sat, Oct 1, 2016 at 3:33 PM, Tom Lane wrote: Copy-editing for contrib/pg_visibility documentation. Add omitted names for some function parameters. Fix some minor grammatical issues. >>> >>> Why do you keep insisting on changing case where I've written "which" >>> to instead say "that" in situations where AFAIK either is perfectly >>> correct? I find such changes at best neutral, and in some cases >>> worse. >> >> 'Which' looks OK to me too here, but I speak some kind of British >> English, and it looks like at least some writers of formal American >> English prefer 'that' here: >> >> * >> http://www.chicagomanualofstyle.org/qanda/data/faq/topics/Whichvs.That.html?old=Whichvs.That01.html >> * https://en.oxforddictionaries.com/usage/that-or-which > > Gosh, I think "she held out the hand that was hurt" is clearly worse > than "she held out the hand which was hurt", but even if someone > prefers the opposite, correcting it as a supposed grammatical error > strikes me as arrant pedantry. You know, the kind up with which I > don't particularly wish to put. Interesting, "which" seems more natural to me, perhaps because of classes at school based on "Britain English". My teachers sometimes referred to "American English" as something to run away from as far as possible. -- 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] Notice lock waits
On Mon, Oct 3, 2016 at 11:40 AM, Michael Paquier wrote: > On Fri, Sep 30, 2016 at 2:00 AM, Jeff Janes wrote: >> What do you think of Jim Nasby's idea of making a settable level, rather >> just on or off? > > [reading the code] > That would be a better idea. The interface proposed, aka 2 GUCs doing > basically the same thing is quite confusing I think. I am marking the > patch as returned with feedback for now. Forgot to mention that I also found myself enforcing client_min_messages to warning to avoid annoying NOTICE messages. -- 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] Logical Replication WIP
On Wed, Sep 28, 2016 at 10:12 PM, Peter Eisentraut wrote: > On 9/23/16 9:28 PM, Petr Jelinek wrote: >>> Document to what extent other relation types are supported (e.g., >>> > materialized views as source, view or foreign table or temp table as >>> > target). Suggest an updatable view as target if user wants to have >>> > different table names or write into a different table structure. >>> > >> I don't think that's good suggestion, for one it won't work for UPDATEs >> as we have completely different path for finding the tuple to update >> which only works on real data, not on view. I am thinking of even just >> allowing table to table replication in v1 tbh, but yes it should be >> documented what target relation types can be. > > I'll generalize this then to: Determine which relation types should be > supported at either end, document that, and then make sure it works that > way. A restrictive implementation is OK for the first version, as long > as it keeps options open. The newest patch is 3-week old, so marking this entry as returned with feedback. -- 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] Add support for restrictive RLS policies
On Thu, Sep 29, 2016 at 7:18 PM, Jeevan Chalke wrote: > Hi Stephen, > > >> > 4. It will be good if we have an example for this in section >> > "5.7. Row Security Policies" >> >> I haven't added one yet, but will plan to do so. >> > I think you are going to add this in this patch itself, right? > > I have reviewed your latest patch and it fixes almost all my review > comments. > Also I am agree with your responses for couple of comments like response on > ALTER POLICY and tab completion. No issues with that. > > However in documentation, PERMISSIVE and RESTRICTIVE are actually literals > and not parameters as such. Also can we combine these two options into one > like below (similar to how we document CASCADE and RESTRICT for DROP > POLICY): > > > PERMISSIVE > RESTRICTIVE > > > > ... explain PERMISSIVE ... > > > ... explain RESTRICTIVE ... > > > > > Apart from this changes look excellent to me. I have moved that to next CF, my guess is that Stephen is going to update soon and the activity is fresh. -- 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] Notice lock waits
On Fri, Sep 30, 2016 at 2:00 AM, Jeff Janes wrote: > What do you think of Jim Nasby's idea of making a settable level, rather > just on or off? [reading the code] That would be a better idea. The interface proposed, aka 2 GUCs doing basically the same thing is quite confusing I think. I am marking the patch as returned with feedback for now. -- 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] Bug in to_timestamp().
On Fri, Sep 30, 2016 at 12:34 PM, Amul Sul wrote: > The new status of this patch is: Ready for Committer And moved to next CF with same status. -- 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] WIP: Covering + unique indexes.
On Tue, Sep 27, 2016 at 12:17 AM, Anastasia Lubennikova wrote: > Ok, I'll write it in a few days. Marked as returned with feedback per last emails exchanged. -- 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] Rename max_parallel_degree?
On Sat, Oct 1, 2016 at 1:23 AM, Julien Rouhaud wrote: > On 23/09/2016 21:10, Robert Haas wrote: >> On Fri, Sep 23, 2016 at 8:54 AM, Peter Eisentraut >> wrote: >>> On 9/20/16 4:07 PM, Robert Haas wrote: No, I'm assuming that the classes would be built-in. A string tag seems like over-engineering to me, particularly because the postmaster needs to switch on the tag, and we need to be very careful about the degree to which the postmaster trusts the contents of shared memory. >>> >>> I'm hoping that we can come up with something that extensions can >>> participate in, without the core having to know ahead of time what those >>> extensions are or how they would be categorized. >>> >>> My vision is something like >>> >>> max_processes = 512 # requires restart >>> >>> process_allowances = 'connection:300 superuser:10 autovacuum:10 >>> parallel:30 replication:10 someextension:20 someotherextension:20' >>> # does not require restart >> >> I don't think it's going to be very practical to allow extensions to >> participate in the mechanism because there have to be a finite number >> of slots that is known at the time we create the main shared memory >> segment. >> >> Also, it's really important that we don't add lots more surface area >> for the postmaster to crash and burn. >> > > It seems that there's no objection on Robert's initial proposal, so I'll > try to implement it. > > I've already fixed every other issues mentioned upthread, but I'm facing > a problem for this one. Assuming that the bgworker classes are supposed > to be mutually exclusive, I don't see a simple and clean way to add such > a check in SanityCheckBackgroundWorker(). Am I missing something > obvious, or can someone give me some advice for this? Okay, so marking it as returned with feedback is adapted? I have done so but feel free to contradict me. -- 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] patch: function xmltable
On Tue, Sep 27, 2016 at 2:29 PM, Pavel Stehule wrote: > 2016-09-27 5:53 GMT+02:00 Craig Ringer : >> >> [...] >> ... so please delete that text. I thought I'd tested it but the state >> of my tests dir says I just got distracted by another task at the >> wrong time. Moved patch to next CF with same status: ready for committer. -- 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] Transactions involving multiple postgres foreign servers
On Wed, Sep 28, 2016 at 3:30 PM, Ashutosh Bapat wrote: > I agree, but we need to cope with above two problems. I have marked the patch as returned with feedback per the last output Ashutosh has provided. -- 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] Stopping logical replication protocol
On Tue, Sep 27, 2016 at 11:05 AM, Craig Ringer wrote: > On 26 September 2016 at 21:52, Vladimir Gordiychuk wrote: >>>You should rely on time I tests as little as possible. Some of the test >>> hosts are VERY slow. If possible something deterministic should be used. >> >> That's why this changes difficult to verify. Maybe in that case we should >> write benchmark but not integration test? >> In that case we can say, before this changes stopping logical replication >> gets N ms but after apply changes it gets NN ms where NN ms less than N ms. >> Is it available add this kind of benchmark to postgresql? I will be grateful >> for links. > > It's for that reason that I added a message printed only in verbose > mode that pg_recvlogical emits when it's exiting after a > client-initiated copydone. > > You can use the TAP tests, print diag messages, etc. But we generally > want them to run fairly quickly, so big benchmark runs aren't > desirable. You'll notice that I left diag messages in to report the > timing for the results in your tests, I just changed the tests so they > didn't depend on very tight timing for success/failure anymore. > > We don't currently have any automated benchmarking infrastructure. Which seems like this patch is not complete yet. I am marking it as returned with feedback, but it may be a better idea to move it to next CF once a new version with updated tests shows up. -- 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] PL/Python adding support for multi-dimensional arrays
On Sat, Oct 1, 2016 at 8:45 AM, Jim Nasby wrote: > On 9/29/16 1:51 PM, Heikki Linnakangas wrote: >> >> Jim, I was confused, but you agreed with me. Were you also confused, or >> am I missing something? > > > I was confused by inputs: I have marked the patch as returned with feedback. Or Heikki, do you plan on looking at it more and commit soon? -- 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] PATCH: Batch/pipelining support for libpq
On Tue, Sep 6, 2016 at 8:01 PM, Craig Ringer wrote: > On 6 September 2016 at 16:10, Daniel Verite wrote: >> Craig Ringer wrote: >> >>> Updated patch attached. >> >> Please find attached a couple fixes for typos I've came across in >> the doc part. > > Thanks, will apply and post a rebased patch soon, or if someone picks > this up in the mean time they can apply your diff on top of the patch. Could you send an updated patch then? At the same time I am noticing that git --check is complaining... This patch has tests and a well-documented feature, so I'll take a look at it soon at the top of my list. Moved to next CF for now. -- 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] [COMMITTERS] pgsql: Copy-editing for contrib/pg_visibility documentation.
On Sun, Oct 2, 2016 at 9:00 PM, Thomas Munro wrote: > On Mon, Oct 3, 2016 at 1:36 PM, Robert Haas wrote: >> On Sat, Oct 1, 2016 at 3:33 PM, Tom Lane wrote: >>> Copy-editing for contrib/pg_visibility documentation. >>> >>> Add omitted names for some function parameters. >>> Fix some minor grammatical issues. >> >> Why do you keep insisting on changing case where I've written "which" >> to instead say "that" in situations where AFAIK either is perfectly >> correct? I find such changes at best neutral, and in some cases >> worse. > > 'Which' looks OK to me too here, but I speak some kind of British > English, and it looks like at least some writers of formal American > English prefer 'that' here: > > * > http://www.chicagomanualofstyle.org/qanda/data/faq/topics/Whichvs.That.html?old=Whichvs.That01.html > * https://en.oxforddictionaries.com/usage/that-or-which Gosh, I think "she held out the hand that was hurt" is clearly worse than "she held out the hand which was hurt", but even if someone prefers the opposite, correcting it as a supposed grammatical error strikes me as arrant pedantry. You know, the kind up with which I don't particularly wish to put. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL 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] Block level parallel vacuum WIP
On Fri, Sep 16, 2016 at 6:56 PM, Masahiko Sawada wrote: > Yeah, I don't have a good solution for this problem so far. > We might need to improve group locking mechanism for the updating > operation or came up with another approach to resolve this problem. > For example, one possible idea is that the launcher process allocates > vm and fsm enough in advance in order to avoid extending fork relation > by parallel workers, but it's not resolve fundamental problem. Marked as returned with feedback because of lack of activity and... Feedback provided. -- 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] pg_upgade vs config
On 10/02/2016 07:21 PM, Tom Lane wrote: I wrote: It occurs to me that a back-patchable workaround for this would be to make get_loadable_libraries sort the library names in order by length (and I guess we might as well sort same-length names alphabetically). This would for example guarantee that hstore_plpython is probed after both hstore and plpython. Admittedly, this is a kluge of the first water. But I see no prospect of back-patching any real fix, and it would definitely be better if pg_upgrade didn't fail on these modules. I've tested the attached and verified that it allows pg_upgrade'ing of the hstore_plpython regression DB --- or, if I reverse the sort order, that it reproducibly fails. I propose back-patching this at least as far as 9.5, where the transform modules came in. It might be a good idea to go all the way back, just so that the behavior is predictable. Yeah, it's a really ugly kluge, but I don't have a better idea. cheers andrew -- 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] Optimization for lazy_scan_heap
On Mon, Sep 26, 2016 at 5:26 AM, Rahila Syed wrote: > Some initial comments on optimize_lazy_scan_heap_v2.patch. Seems worth pursuing. Marking as returned with feedback because of lack of activity and some basic reviews sent. -- 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] Patch: Write Amplification Reduction Method (WARM)
On Mon, Sep 5, 2016 at 1:53 PM, Pavan Deolasee wrote: > 0001_track_root_lp_v4.patch: This patch uses a free t_infomask2 bit to track > latest tuple in an update chain. The t_ctid.ip_posid is used to track the > root line pointer of the update chain. We do this only in the latest tuple > in the chain because most often that tuple will be updated and we need to > quickly find the root only during update. > > 0002_warm_updates_v4.patch: This patch implements the core of WARM logic. > During WARM update, we only insert new entries in the indexes whose key has > changed. But instead of indexing the real TID of the new tuple, we index the > root line pointer and then use additional recheck logic to ensure only > correct tuples are returned from such potentially broken HOT chains. Each > index AM must implement a amrecheck method to support WARM. The patch > currently implements this for hash and btree indexes. Moved to next CF, I was surprised to see that it is not *that* large: 43 files changed, 1539 insertions(+), 199 deletions(-) -- 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] Hash Indexes
On Mon, Oct 3, 2016 at 12:42 AM, Pavel Stehule wrote: > > > 2016-10-02 12:40 GMT+02:00 Michael Paquier : >> >> On Sun, Oct 2, 2016 at 3:31 AM, Greg Stark wrote: >> > On Fri, Sep 30, 2016 at 2:11 AM, Robert Haas >> > wrote: >> >> For one thing, we can stop shipping a totally broken feature in release >> >> after release >> > >> > For what it's worth I'm for any patch that can accomplish that. >> > >> > In retrospect I think we should have done the hash-over-btree thing >> > ten years ago but we didn't and if Amit's patch makes hash indexes >> > recoverable today then go for it. >> >> +1. > > +1 And moved to next CF to make it breath. -- 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] PoC: Partial sort
On Tue, Sep 13, 2016 at 5:32 PM, Alexander Korotkov wrote: > On Fri, Apr 8, 2016 at 10:09 PM, Peter Geoghegan wrote: >> >> On Wed, Mar 30, 2016 at 8:02 AM, Alexander Korotkov >> wrote: >> > Hmm... I'm not completely agree with that. In typical usage partial sort >> > should definitely use quicksort. However, fallback to other sort >> > methods is >> > very useful. Decision of partial sort usage is made by planner. But >> > planner makes mistakes. For example, our HashAggregate is purely >> > in-memory. >> > In the case of planner mistake it causes OOM. I met such situation in >> > production and not once. This is why I'd like partial sort to have >> > graceful >> > degradation for such cases. >> >> I think that this should be moved to the next CF, unless a committer >> wants to pick it up today. > > > Patch was rebased to current master. Applies on HEAD at e8bdee27 and passes make-check, now I am seeing zero documentation so it is a bit hard to see what this patch is achieving without reading the thread. $ git diff master --check src/backend/optimizer/prep/prepunion.c:967: trailing whitespace. + cost_sort(&sorted_p, root, NIL, 0, src/backend/utils/sort/tuplesort.c:1244: trailing whitespace. + * tuplesort_updatemax + * Returns true if the plan node isautomatically materializes its output Typo here. Still, this has received to reviews, so moved to next CF. -- 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] amcheck (B-Tree integrity checking tool)
On Thu, Sep 8, 2016 at 3:44 AM, Peter Geoghegan wrote: > On Fri, Sep 2, 2016 at 11:19 AM, Kevin Grittner wrote: >> IMV the process is to post a patch to this list to certify that it >> is yours to contribute and free of IP encumbrances that would >> prevent us from using it. I will wait for that post. > > I attach my V3. There are only very minor code changes here, such as > breaking out a separate elevel constant for DEBUG2 traces that show > information of how the B-Tree is accessed (e.g. current level, block > number). Tiny tweaks to about 3 comments were also performed. These > changes are trivial. > > I've also removed the tests added, since external sort regression test > coverage is in a much better place now. Okay, moved to next CF. I may look at it finally I got some use-cases for it, similar to yours I guess.. -- 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] multivariate statistics (v19)
On Fri, Sep 30, 2016 at 8:10 PM, Heikki Linnakangas wrote: > This patch set is in pretty good shape, the only problem is that it's so big > that no-one seems to have the time or courage to do the final touches and > commit it. Did you see my suggestions about simplifying its SQL structure? You could shave some code without impacting the base set of features. > I fear that using "statistics" as the name of the new object might get a bit > awkward. "statistics" is a plural, but we use it as the name of a single > object, like "pants" or "scissors". Not sure I have any better ideas though. > "estimator"? "statistics collection"? Or perhaps it should be singular, > "statistic". I note that you actually called the system table > "pg_mv_statistic", in singular. > > I'm not a big fan of storing the stats as just a bytea blob, and having to > use special functions to interpret it. By looking at the patch, it's not > clear to me what we actually store for functional dependencies. A list of > attribute numbers? Could we store them simply as an int[]? (I'm not a big > fan of the hack in pg_statistic, that allows storing arrays of any data type > in the same column, though. But for functional dependencies, I don't think > we need that.) I am marking this patch as returned with feedback for now. > Overall, this is going to be a great feature! +1. -- 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] PATCH: two slab-like memory allocators
On Sun, Oct 2, 2016 at 10:15 AM, Tomas Vondra wrote: > One more comment about GenSlab, particularly about unpredictability of the > repalloc() behavior. It works for "large" chunks allocated in the AllocSet > part, and mostly does not work for "small" chunks allocated in the > SlabContext. Moreover, the chunkSize changes over time, so for two chunks of > the same size, repalloc() may work on one of them and fail on the other, > depending on time of allocation. > > With SlabContext it's perfectly predictable - repalloc() call fails unless > the chunk size is exactly the same as before (which is perhaps a bit > pointless, but if we decide to fail even in this case it'll work 100% time). > > But with GenSlabContext it's unclear whether the call fails or not. > > I don't like this unpredictability - I'd much rather have consistent > failures (making sure people don't do repalloc() on with GenSlab). But I > don't see a nice way to achieve that - the repalloc() call does not go > through GenSlabRealloc() at all, but directly to SlabRealloc() or > AllocSetRealloc(). > > The best solution I can think of is adding an alternate version of > AllocSetMethods, pointing to a different AllocSetReset implementation. You guys are still playing with this patch, so moved to next CF with "waiting on author". -- 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] GiST support for UUIDs
On Wed, Aug 19, 2015 at 1:25 PM, Paul A Jungwirth wrote: > This is my first patch, so my apologies if anything is missing. I went > the guidelines and I think I have everything covered. :-) I am moving this patch to next CF, removing Julien Rouhaud and Teodor Sigaev as reviewers because they have not showed up once on this thread for reviews. -- 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] pg_upgrade documentation improvement patch
On Tue, Apr 19, 2016 at 3:42 AM, Yuri Niyazov wrote: > Should I update the documentation patch to instruct the use of > pg_controldata exclusively? I guess so. Marked as returned with feedback because the thread has died. -- 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] "Re: Question about grant create on database and pg_dump/pg_dumpall
On Fri, Sep 30, 2016 at 12:29 AM, Tom Lane wrote: > The fundamental thing we have to do in order to move forward on this is > to rethink what's the division of labor between pg_dump and pg_dumpall. > I find the patch as presented quite unacceptable because it's made no > effort to do that (or even to touch the documentation). Patch marked as returned with feedback for now. > 1. pg_dump without --create continues to do what it does today, ie it just > dumps objects within the database, assuming that database-level properties > will already be set correctly for the target database. > > 2. pg_dump with --create creates the target database and also sets all > database-level properties (ownership, ACLs, ALTER DATABASE SET, etc etc). > > 3. pg_dumpall loses all code relating to individual-database creation > and property setting and instead relies on pg_dump --create to do that. > This would leave only the code relating to "pg_dumpall -g" (ie, dump roles > and tablespaces) within pg_dumpall itself. I saw a couple of people willing to have that on those mailling lists over the years... So +1 for the idea. > One thing that would still be messy is that presumably "pg_dumpall -g" > would issue ALTER ROLE SET commands, but it's unclear what to do with > ALTER ROLE IN DATABASE SET commands. Should those become part of > "pg_dump --create"'s charter? It seems like not, but I'm not certain. I guess that we need to think about simplifying the restore flow if we have the occasion: 1) First restore the result of pg_dumpall -g 2) Restore the state of each database's dump --create pg_dump --create assigns the created database to its rightful owner, so it assumes that the role has been already created. If we do not include those commands in pg_dump --create we make the whole restore scenario more complicated. > Another thing that requires some thought is that pg_dumpall is currently > willing to dump ACLs and other properties for template1/template0, though > it does not invoke pg_dump on them. If we wanted to preserve that > behavior while still moving the code that does those things to pg_dump, > pg_dump would have to grow an option that would let it do that. But > I'm not sure how much of that behavior is actually sensible. An extra option looks like the way to go. There are people that willingly set up ACLs and we could just document that something like pg_dump --template needs to be run to generate the template's dump values. > This would probably take a pg_dump archive version bump, since I think > we don't currently record enough information for --create to do this > (and we can't just cram the extra commands into the DATABASE entry, > since we don't know whether --create will be specified to pg_restore). > But we've done those before. Yep. (By the way, last time I saw dump/restore integration into an internal system we have finished by using pg_dumpall -g and then pg_dump on a given DB for simplicity) -- 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] [COMMITTERS] pgsql: Copy-editing for contrib/pg_visibility documentation.
On Mon, Oct 3, 2016 at 1:36 PM, Robert Haas wrote: > On Sat, Oct 1, 2016 at 3:33 PM, Tom Lane wrote: >> Copy-editing for contrib/pg_visibility documentation. >> >> Add omitted names for some function parameters. >> Fix some minor grammatical issues. > > Why do you keep insisting on changing case where I've written "which" > to instead say "that" in situations where AFAIK either is perfectly > correct? I find such changes at best neutral, and in some cases > worse. 'Which' looks OK to me too here, but I speak some kind of British English, and it looks like at least some writers of formal American English prefer 'that' here: * http://www.chicagomanualofstyle.org/qanda/data/faq/topics/Whichvs.That.html?old=Whichvs.That01.html * https://en.oxforddictionaries.com/usage/that-or-which -- 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] [PATCH] add option to pg_dumpall to exclude tables from the dump
On Thu, Sep 29, 2016 at 2:16 AM, Robert Haas wrote: > On Tue, Sep 6, 2016 at 9:37 PM, Gerdan Rezende dos Santos > wrote: >> After review, I realized that there is a call to the function: >> doShellQuoting (pgdumpopts, OPTARG), which no longer seems to exist ... >> After understand the code, I saw that the call is appendShellString >> (pgdumpopts, OPTARG). >> >> Follow the patches already with the necessary corrections. > > This doesn't seem to take into account the discussion between Tom Lane > and Jim Nasby about how this feature should work. So, Juergen, it would be nice if you could participate in the discussion and get a consensus on the patch. Until then, I am marking this patch as returned with feedback. -- 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] Patch: Implement failover on libpq connect level.
On Fri, Sep 30, 2016 at 5:44 PM, Victor Wagner wrote: > But backward compatibility is more > important thing, so I now assume, that user tries to connect just one > node, and this node is read only, user knows what he is doing. Moved this patch to next CF. (Note that I am in CF-vacuuming mode this morning, I'll try to close it asap.) -- 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] Forbid use of LF and CR characters in database and role names
On Sun, Oct 2, 2016 at 10:47 PM, Michael Paquier wrote: > And seeing nothing happening here, I still don't know what to do with > this patch. Thoughts? If we are going to do nothing I would suggest to > just remove the comment in string_utils.c saying that such LF and CR > will be unsupported in a future release and move on. And so I am just marking this patch as returned with feedback. -- 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] [COMMITTERS] pgsql: Copy-editing for contrib/pg_visibility documentation.
On Sat, Oct 1, 2016 at 3:33 PM, Tom Lane wrote: > Copy-editing for contrib/pg_visibility documentation. > > Add omitted names for some function parameters. > Fix some minor grammatical issues. Why do you keep insisting on changing case where I've written "which" to instead say "that" in situations where AFAIK either is perfectly correct? I find such changes at best neutral, and in some cases worse. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL 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] pg_upgade vs config
I wrote: > It occurs to me that a back-patchable workaround for this would be to > make get_loadable_libraries sort the library names in order by length > (and I guess we might as well sort same-length names alphabetically). > This would for example guarantee that hstore_plpython is probed after > both hstore and plpython. Admittedly, this is a kluge of the first > water. But I see no prospect of back-patching any real fix, and it > would definitely be better if pg_upgrade didn't fail on these modules. I've tested the attached and verified that it allows pg_upgrade'ing of the hstore_plpython regression DB --- or, if I reverse the sort order, that it reproducibly fails. I propose back-patching this at least as far as 9.5, where the transform modules came in. It might be a good idea to go all the way back, just so that the behavior is predictable. regards, tom lane diff --git a/src/bin/pg_upgrade/function.c b/src/bin/pg_upgrade/function.c index b432b54..5435bff 100644 *** a/src/bin/pg_upgrade/function.c --- b/src/bin/pg_upgrade/function.c *** *** 15,20 --- 15,44 /* + * qsort comparator for pointers to library names + * + * We sort first by name length, then alphabetically for names of the same + * length. This is to ensure that, eg, "hstore_plpython" sorts after both + * "hstore" and "plpython"; otherwise transform modules will probably fail + * their LOAD tests. (The backend ought to cope with that consideration, + * but it doesn't yet, and even when it does it'd be a good idea to have + * a predictable order of probing here.) + */ + static int + library_name_compare(const void *p1, const void *p2) + { + const char *str1 = *(const char *const *) p1; + const char *str2 = *(const char *const *) p2; + int slen1 = strlen(str1); + int slen2 = strlen(str2); + + if (slen1 != slen2) + return slen1 - slen2; + return strcmp(str1, str2); + } + + + /* * get_loadable_libraries() * * Fetch the names of all old libraries containing C-language functions. *** get_loadable_libraries(void) *** 38,47 PGconn *conn = connectToServer(&old_cluster, active_db->db_name); /* ! * Fetch all libraries referenced in this DB. We can't exclude the ! * "pg_catalog" schema because, while such functions are not ! * explicitly dumped by pg_dump, they do reference implicit objects ! * that pg_dump does dump, e.g. CREATE LANGUAGE plperl. */ ress[dbnum] = executeQueryOrDie(conn, "SELECT DISTINCT probin " --- 62,68 PGconn *conn = connectToServer(&old_cluster, active_db->db_name); /* ! * Fetch all libraries referenced in this DB. */ ress[dbnum] = executeQueryOrDie(conn, "SELECT DISTINCT probin " *** get_loadable_libraries(void) *** 69,76 res = executeQueryOrDie(conn, "SELECT 1 " ! "FROM pg_catalog.pg_proc JOIN pg_namespace " ! " ON pronamespace = pg_namespace.oid " "WHERE proname = 'plpython_call_handler' AND " "nspname = 'public' AND " "prolang = 13 /* C */ AND " --- 90,98 res = executeQueryOrDie(conn, "SELECT 1 " ! "FROM pg_catalog.pg_proc p " ! "JOIN pg_catalog.pg_namespace n " ! "ON pronamespace = n.oid " "WHERE proname = 'plpython_call_handler' AND " "nspname = 'public' AND " "prolang = 13 /* C */ AND " *** get_loadable_libraries(void) *** 112,124 if (found_public_plpython_handler) pg_fatal("Remove the problem functions from the old cluster to continue.\n"); - /* Allocate what's certainly enough space */ - os_info.libraries = (char **) pg_malloc(totaltups * sizeof(char *)); - /* ! * Now remove duplicates across DBs. This is pretty inefficient code, but ! * there probably aren't enough entries to matter. */ totaltups = 0; for (dbnum = 0; dbnum < old_cluster.dbarr.ndbs; dbnum++) --- 134,151 if (found_public_plpython_handler) pg_fatal("Remove the problem functions from the old cluster to continue.\n"); /* ! * Now we want to remove duplicates across DBs and sort the library names ! * into order. This avoids multiple probes of the same library, and ! * ensures that libraries are probed in a consistent order, which is ! * important for reproducible behavior if one library depends on another. ! * ! * First transfer all the names into one array, then sort, then remove ! * duplicates. Note: we strdup each name in the first loop so that we can ! * safely clear the PGresults in the same loop. This is a bit wasteful ! * but it's unlikely there are enough names to matter. */ + os_info.libraries = (char **) pg_malloc(totaltups * sizeof(char *)); totaltups = 0; for (dbnum = 0; dbnum < old_cluster.dbarr.ndbs; dbnum++) *** get_loadable_libraries(void) *** 131,157
Re: [HACKERS] pg_upgade vs config
Andres Freund writes: > On 2016-10-02 17:59:47 -0400, Tom Lane wrote: >> So now I'm thinking you're right, it'd be better to have some solution >> whereby dfmgr.c knows about cross-module dependencies and loads the >> dependencies first. Not sure how to approach that. The extension >> "requires" mechanism is tantalizingly close to providing the data >> we need, but dfmgr.c doesn't know about that, and there's no concept >> of a reverse mapping from .so names to extensions anyway. > One, kind of extreme, way to get there would be to resolve the hstore > symbols hstore_plpython needs with load_external_function, during > _PG_init(). Most of these files don't actually depend on a large number > of symbols, so that should actually be doable. Hm. That would actually not be a bad idea, perhaps, because the method we're using right now requires that the linker not bitch about unresolved symbols at build time, which is a really bad thing that I'd prefer to turn off. It's still not a very back-patchable answer, but it's something that we could get to in HEAD without a huge amount of work. 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] pg_upgade vs config
On 2016-10-02 17:59:47 -0400, Tom Lane wrote: > I've found that the Linux '-l:hstore.so' solution works on HPUX as well, > at least to the extent of being able to run LOAD. However, it doesn't > seem to be possible to make it work on macOS, which has a hard distinction > between "loadable modules" and "shared libraries". Our extensions are > the former, which means that e.g. hstore.so is not a shlib and the linker > will refuse to consider it as a linkable library. I tried converting > them to true shlibs, which is just a matter of changing "-bundle" to > "-dynamiclib" in the link command and dropping "-bundle_loader postgres" > because that's not accepted with "-dynamiclib". That works about 90%, > but I found that plpython failed its regression test for a very scary > reason: there's a "hash_search()" somewhere in libc on this platform > and the linker was preferentially resolving plpython's call to that > rather than to the one in Postgres. I don't think we want to risk > that sort of platform dependency :-( ISTM that that's a risk independent of this specific issue? On other platforms, I mean? > So now I'm thinking you're right, it'd be better to have some solution > whereby dfmgr.c knows about cross-module dependencies and loads the > dependencies first. Not sure how to approach that. The extension > "requires" mechanism is tantalizingly close to providing the data > we need, but dfmgr.c doesn't know about that, and there's no concept > of a reverse mapping from .so names to extensions anyway. One, kind of extreme, way to get there would be to resolve the hstore symbols hstore_plpython needs with load_external_function, during _PG_init(). Most of these files don't actually depend on a large number of symbols, so that should actually be doable. 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] pg_upgade vs config
I wrote: > So it doesn't seem that we've broken anything since 9.5 --- it didn't > work before either. The seeming successes may have been due to chance, > i.e. pg_upgrade probing the libraries in an order that happened to work. > I see no evidence that get_loadable_libraries/check_loadable_libraries > are paying any attention to what order the libraries are checked in. It occurs to me that a back-patchable workaround for this would be to make get_loadable_libraries sort the library names in order by length (and I guess we might as well sort same-length names alphabetically). This would for example guarantee that hstore_plpython is probed after both hstore and plpython. Admittedly, this is a kluge of the first water. But I see no prospect of back-patching any real fix, and it would definitely be better if pg_upgrade didn't fail on these modules. 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] pg_upgade vs config
Andrew Dunstan writes: > On 10/02/2016 12:54 PM, Tom Lane wrote: >> [ digs more deeply ... ] Oh, weird: it looks like this succeeded in >> every case except 9.6->HEAD upgrade. Did we break something recently? > Yeah, my latest version of the test module (soon to hit githyb) also > does a self upgrade, and these modules pass that on 9.5, whereas they > fail on 9.6, as well as the 9.6->HEAD and HEAD self-tests failing. So > indeed it looks like we've broken something. I've now checked that simply doing LOAD 'hstore_plpython2.so' in a fresh session fails, in both HEAD and 9.5, on both Linux and current macOS. So it doesn't seem that we've broken anything since 9.5 --- it didn't work before either. The seeming successes may have been due to chance, i.e. pg_upgrade probing the libraries in an order that happened to work. I see no evidence that get_loadable_libraries/check_loadable_libraries are paying any attention to what order the libraries are checked in. I've found that the Linux '-l:hstore.so' solution works on HPUX as well, at least to the extent of being able to run LOAD. However, it doesn't seem to be possible to make it work on macOS, which has a hard distinction between "loadable modules" and "shared libraries". Our extensions are the former, which means that e.g. hstore.so is not a shlib and the linker will refuse to consider it as a linkable library. I tried converting them to true shlibs, which is just a matter of changing "-bundle" to "-dynamiclib" in the link command and dropping "-bundle_loader postgres" because that's not accepted with "-dynamiclib". That works about 90%, but I found that plpython failed its regression test for a very scary reason: there's a "hash_search()" somewhere in libc on this platform and the linker was preferentially resolving plpython's call to that rather than to the one in Postgres. I don't think we want to risk that sort of platform dependency :-( On further reflection, though, this approach is probably a dead end even without any platform concerns. The reason is that if we do 'LOAD foo' and the dynamic linker then pulls in 'bar.so', bar.so will be present in the address space all right, but we will not have checked for a compatible magic block, nor have called its _PG_init function if any. In general, if calls into a module occur without having called its _PG_init, it might misbehave very badly. So now I'm thinking you're right, it'd be better to have some solution whereby dfmgr.c knows about cross-module dependencies and loads the dependencies first. Not sure how to approach that. The extension "requires" mechanism is tantalizingly close to providing the data we need, but dfmgr.c doesn't know about that, and there's no concept of a reverse mapping from .so names to extensions anyway. 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
[HACKERS] Non-empty default log_line_prefix
Re: Tom Lane 2016-09-29 <18642.1475159...@sss.pgh.pa.us> > > Possibly the longer version could be added as an example in the > > documentation. > > I suspect that simply having a nonempty default in the first place > is going to do more to raise peoples' awareness than anything we > could do in the documentation. But perhaps an example along these > lines would be useful for showing proper use of %q. Patch attached. (Still using %t, I don't think %m makes sense for the default.) Christoph diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml new file mode 100644 index e826c19..a4d8b74 *** a/doc/src/sgml/config.sgml --- b/doc/src/sgml/config.sgml *** local0.*/var/log/postgresql *** 5004,5010 value will pad on the left. Padding can be useful to aid human readability in log files. This parameter can only be set in the postgresql.conf ! file or on the server command line. The default is an empty string. --- 5004,5011 value will pad on the left. Padding can be useful to aid human readability in log files. This parameter can only be set in the postgresql.conf ! file or on the server command line. The default is ! %t [%p] which logs a time stamp and the process ID. *** FROM pg_stat_activity; *** 5142,5147 --- 5143,5159 include those escapes if you are logging to syslog. + + + + The %q escape is useful when including information that is + only available in session (backend) context like user or database + name. An example would be: + + log_line_prefix = '%t [%p] %q%u@%d/%a ' + + + diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c new file mode 100644 index cced814..b71fa93 *** a/src/backend/utils/misc/guc.c --- b/src/backend/utils/misc/guc.c *** static struct config_string ConfigureNam *** 3014,3020 gettext_noop("If blank, no prefix is used.") }, &Log_line_prefix, ! "", NULL, NULL, NULL }, --- 3014,3020 gettext_noop("If blank, no prefix is used.") }, &Log_line_prefix, ! "%t [%p] ", NULL, NULL, NULL }, diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample new file mode 100644 index 05b1373..9eaa23e *** a/src/backend/utils/misc/postgresql.conf.sample --- b/src/backend/utils/misc/postgresql.conf.sample *** *** 430,436 #log_duration = off #log_error_verbosity = default# terse, default, or verbose messages #log_hostname = off ! #log_line_prefix = '' # special values: # %a = application name # %u = user name # %d = database name --- 430,436 #log_duration = off #log_error_verbosity = default# terse, default, or verbose messages #log_hostname = off ! #log_line_prefix = '%t [%p] ' # special values: # %a = application name # %u = user name # %d = database name -- 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_upgade vs config
On 10/02/2016 12:54 PM, Tom Lane wrote: Andrew Dunstan writes: The biggest issue is this: the upgrade fails completely on ltree-plpython and hstore-plpython, presumably because these modules rely on the plpython module being loaded first. pg_upgrade rather simple-mindedly calls LOAD on the object library to test if it's usable. FWIW, that seems to have worked fine yesterday on prairiedog. I suspect the explanation is that macOS's dynamic linker is smart enough to pull in plpython when one of those modules is LOAD'ed. The ideal fix would be to make that happen on all platforms. I'm not actually sure why it doesn't already; surely every dynamic linker in existence has such a capability. [ digs more deeply ... ] Oh, weird: it looks like this succeeded in every case except 9.6->HEAD upgrade. Did we break something recently? Yeah, my latest version of the test module (soon to hit githyb) also does a self upgrade, and these modules pass that on 9.5, whereas they fail on 9.6, as well as the 9.6->HEAD and HEAD self-tests failing. So indeed it looks like we've broken something. Yet another example of why I need to get this test module production ready :-) cheers andrew -- 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_upgade vs config
Andrew Dunstan writes: > On 10/02/2016 01:53 PM, Tom Lane wrote: >> Because pg_dump with --binary-upgrade neglects to emit >> ALTER EXTENSION bloom ADD ACCESS METHOD bloom; >> which it would need to do in order to make this work right. The other >> small problem is that there is no such ALTER EXTENSION syntax in the >> backend. This is a rather major oversight in the patch that added DDL >> support for access methods, if you ask me. > I agree. Remarkably enough, it seems that only a gram.y production need be added --- the only other code involved is objectaddress.c, which does seem to have gotten extended sufficiently. 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] pg_upgade vs config
On 10/02/2016 01:53 PM, Tom Lane wrote: So then why are the pre-upgrade and post-upgrade dumps different? Because pg_dump with --binary-upgrade neglects to emit ALTER EXTENSION bloom ADD ACCESS METHOD bloom; That's what I suspected. which it would need to do in order to make this work right. The other small problem is that there is no such ALTER EXTENSION syntax in the backend. This is a rather major oversight in the patch that added DDL support for access methods, if you ask me. I agree. cheers andrew -- 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_upgade vs config
Andrew Dunstan writes: > On 10/02/2016 09:50 AM, Michael Paquier wrote: >> On Sun, Oct 2, 2016 at 10:40 PM, Andrew Dunstan wrote: >>> It looks like we have some work to do to teach pg_dump about handling access >>> methods in extensions. This doesn't look quite as bad as the first issue, >>> but it's a pity 9.6 escaped into the wild with this issue. >> 562f06f3 has addressed this issue 3 months ago, and there is a test in >> src/test/modules/test_pg_dump. > So then why are the pre-upgrade and post-upgrade dumps different? Because pg_dump with --binary-upgrade neglects to emit ALTER EXTENSION bloom ADD ACCESS METHOD bloom; which it would need to do in order to make this work right. The other small problem is that there is no such ALTER EXTENSION syntax in the backend. This is a rather major oversight in the patch that added DDL support for access methods, if you ask me. (Also, I didn't look at what test_pg_dump is testing, but I bet it isn't attempting to cover --binary-upgrade behavior.) 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] pg_upgade vs config
I wrote: > I suspect the explanation is that macOS's dynamic linker is smart enough > to pull in plpython when one of those modules is LOAD'ed. The ideal fix > would be to make that happen on all platforms. I'm not actually sure > why it doesn't already; surely every dynamic linker in existence has > such a capability. Some experimentation says that this is indeed possible on Linux, at least. It's a bit of a pain because the .so's we need to reference are not named "libsomething.so" and thus a straight -l switch doesn't work. The Linux ld(1) man page documents that you can write "-l:filename" to override the addition of "lib", but I have no idea how portable that is to other toolchains. (On macOS, and maybe other BSD-derived systems, it looks like you can do this without the colon, ie -lhstore.so will work.) Also, it seems that -L../hstore -l:hstore$(DLSUFFIX) which was my first attempt, doesn't work because you end up with a hard-coded reference to "../hstore/hstore.so", which rpath searching doesn't cope with. I was able to make it work by copying hstore.so and plpython2.so into contrib/hstore_plpython and then just writing SHLIB_LINK += -L. -l:hstore$(DLSUFFIX) \ -l:plpython$(python_majorversion)$(DLSUFFIX) $(python_libspec) That results in undecorated references that do work with the rpath. This all seems depressingly platform-specific, but maybe we can make it work on enough platforms to be satisfactory. 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] pg_upgade vs config
Andrew Dunstan writes: > The biggest issue is this: the upgrade fails completely on > ltree-plpython and hstore-plpython, presumably because these modules > rely on the plpython module being loaded first. pg_upgrade rather > simple-mindedly calls LOAD on the object library to test if it's usable. FWIW, that seems to have worked fine yesterday on prairiedog. I suspect the explanation is that macOS's dynamic linker is smart enough to pull in plpython when one of those modules is LOAD'ed. The ideal fix would be to make that happen on all platforms. I'm not actually sure why it doesn't already; surely every dynamic linker in existence has such a capability. [ digs more deeply ... ] Oh, weird: it looks like this succeeded in every case except 9.6->HEAD upgrade. Did we break something recently? 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] Hash Indexes
2016-10-02 12:40 GMT+02:00 Michael Paquier : > On Sun, Oct 2, 2016 at 3:31 AM, Greg Stark wrote: > > On Fri, Sep 30, 2016 at 2:11 AM, Robert Haas > wrote: > >> For one thing, we can stop shipping a totally broken feature in release > after release > > > > For what it's worth I'm for any patch that can accomplish that. > > > > In retrospect I think we should have done the hash-over-btree thing > > ten years ago but we didn't and if Amit's patch makes hash indexes > > recoverable today then go for it. > > +1. > +1 Pavel > -- > 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] pg_upgade vs config
On 10/02/2016 09:50 AM, Michael Paquier wrote: On Sun, Oct 2, 2016 at 10:40 PM, Andrew Dunstan wrote: It looks like we have some work to do to teach pg_dump about handling access methods in extensions. This doesn't look quite as bad as the first issue, but it's a pity 9.6 escaped into the wild with this issue. 562f06f3 has addressed this issue 3 months ago, and there is a test in src/test/modules/test_pg_dump. So then why are the pre-upgrade and post-upgrade dumps different? cheers andrew -- 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] Confusing docs about GetForeignUpperPaths in fdwhandler.sgml
On Thu, Sep 22, 2016 at 7:48 PM, Rushabh Lathia wrote: > I performed basic test with patch, > > a) patch get applied cleanly on latest source, > b) able to build documentation cleanly. > > Marking this as ready for committer. Oops, incorrect patch... I am moving it to next CF. Discussion can continue there. -- 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] Minor improvement to fdwhandler.sgml
On Wed, Jan 27, 2016 at 7:52 PM, Etsuro Fujita wrote: > Here is a small patch to do s/for/For/ to two section titles in > fdwhandlers.sgml, for consistency. I am grepping 54 places where "for" is used in a , and none of them use an upper case for its first letter. I am marking this patch as rejected. -- 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] Quorum commit for multiple synchronous replication.
On Wed, Sep 28, 2016 at 5:14 PM, Michael Paquier wrote: > OK, so I have done a review of this patch keeping that in mind as > that's the consensus. I am still getting familiar with the code... Returned with feedback for now. This just needs polishing so feel free to move it to the next CF once you have a new patch. -- 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] Showing parallel status in \df+
On Sun, Oct 2, 2016 at 10:55 PM, Michael Paquier wrote: > Let's remove it and move on then. By looking again at this thread and > particularly > https://www.postgresql.org/message-id/20160926190618.gh5...@tamriel.snowman.net > (thanks Stephen for the summary) that's where we are heading to. (Moved to next CF) -- 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] Showing parallel status in \df+
On Sat, Oct 1, 2016 at 9:47 AM, Tom Lane wrote: > Jim Nasby writes: >> On 9/28/16 2:59 PM, Alvaro Herrera wrote: >>> My vote (which was not counted by Stephen) was to remove it from \df+ >>> altogether. I stand by that. People who are used to seeing the output >>> in \df+ will wonder "where the heck did it go" and eventually figure it >>> out, at which point it's no longer a problem. We're not breaking >>> anyone's scripts, that's for sure. >>> >>> If we're not removing it, I +0 support the option of moving it to >>> footers. I'm -1 on doing nothing. > >> I agree with everything Alvaro just said. > > Well, alternatively, can we get a consensus for doing that? People > did speak against removing PL source code from \df+ altogether, but > maybe they're willing to reconsider if the alternative is doing nothing. > > Personally I'm on the edge of washing my hands of the whole thing... Let's remove it and move on then. By looking again at this thread and particularly https://www.postgresql.org/message-id/20160926190618.gh5...@tamriel.snowman.net (thanks Stephen for the summary) that's where we are heading to. -- 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] pg_upgade vs config
On Sun, Oct 2, 2016 at 10:40 PM, Andrew Dunstan wrote: > It looks like we have some work to do to teach pg_dump about handling access > methods in extensions. This doesn't look quite as bad as the first issue, > but it's a pity 9.6 escaped into the wild with this issue. 562f06f3 has addressed this issue 3 months ago, and there is a test in src/test/modules/test_pg_dump. -- 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] Forbid use of LF and CR characters in database and role names
On Mon, Sep 12, 2016 at 11:38 AM, Michael Paquier wrote: > On Mon, Sep 12, 2016 at 10:01 AM, Noah Misch wrote: >> I discourage documenting LF/CR restrictions. For the epsilon of readers who >> would benefit from this knowledge, the error message suffices. For everyone >> else, it would just dilute the text. (One could argue against other parts of >> our documentation on this basis, but I'm not calling for such a study. I'm >> just saying that today's lack of documentation on this topic is optimal.) > > Okay. > >>> > > I think the way forward here, if any, is to work on removing these >>> > > restrictions, not to keep sprinkling them around. >>> > >>> > If we were talking about pathnames containing spaces, I would agree, >>> > but I've never heard of a legitimate pathname containing CR or LF. I >>> > can't see us losing much by refusing to allow such pathnames, except >>> > for security holes. >> >> (In other words, +1 to that.) > > Yep. To be honest, I see value in preventing directly the use of > newlines and carriage returns in database and role names to avoid > users to be bitten by custom scripts, things for example written in > bash that scan manually for pg_database, pg_roles, etc. And I have > seen such things over the years. Now it is true that the safeguards in > core are proving to be enough, if only the in-core tools are used, but > that's not necessarily the case with all the things gravitating around > this community. And seeing nothing happening here, I still don't know what to do with this patch. Thoughts? If we are going to do nothing I would suggest to just remove the comment in string_utils.c saying that such LF and CR will be unsupported in a future release and move on. -- 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] pgbench - allow to store select results into variables
On Tue, Sep 27, 2016 at 10:41 AM, Amit Langote wrote: > On 2016/09/26 20:27, Fabien COELHO wrote: >> >> Hello Amit, >> I am marking the pgbench-into-5.patch [1] as "Ready for Committer" as I have no further comments at the moment. >>> >>> Wait... Heikki's latest commit now requires this patch to be rebased. >> >> Indeed. Here is the rebased version, which still get through my various >> tests. > > Thanks, Fabien. It seems to work here too. Moved to next CF with same status, ready for committer. -- 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] pgbench more operators & functions
On Sat, Oct 1, 2016 at 11:35 PM, Fabien COELHO wrote: > Attached version changes: > - removes C operators not present in psql > - document operators one per line Moved to next CF with same status: "Ready for committer". -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] pg_upgade vs config
I'm working on updating and making production ready my experimental cross version pg_upgrade testing module for the buildfarm. A couple of things have emerged that are of concern. This module does a much more complete test that our normal test for pg_upgrade, which only checks upgrading the standard regression database. This tests all the databases the buildfarm creates for testing, including those made by modules in contrib. The biggest issue is this: the upgrade fails completely on ltree-plpython and hstore-plpython, presumably because these modules rely on the plpython module being loaded first. pg_upgrade rather simple-mindedly calls LOAD on the object library to test if it's usable. It's a bit embarrassing that we can't upgrade a database using one of our own modules. At the very least we should hard-code a way around this (e.g. have it load the relevant plpython module first), but more generally I think we need a way to tell pg_upgrade that module X relies on module Y. In the past ISTR we've said we don't support having dependencies between loadable modules, but that ship now seems to have sailed. Second, we get an unexpected difference between the pre-upgrade and post-upgrade dumps for the bloom module: --- /home/bf/bfr/root/upgrade/HEAD/origin-REL9_6_STABLE.sql 2016-10-02 09:16:03.298341639 -0400 +++ /home/bf/bfr/root/upgrade/HEAD/converted-REL9_6_STABLE-to-HEAD.sql 2016-10-02 09:16:54.889343991 -0400 @@ -7413,6 +7413,20 @@ COMMENT ON EXTENSION bloom IS 'bloom access method - signature file based index'; +-- +-- Name: bloom; Type: ACCESS METHOD; Schema: -; Owner: +-- + +CREATE ACCESS METHOD bloom TYPE INDEX HANDLER public.blhandler; + + +-- +-- Name: ACCESS METHOD bloom; Type: COMMENT; Schema: -; Owner: +-- + +COMMENT ON ACCESS METHOD bloom IS 'bloom index access method'; + + SET search_path = public, pg_catalog; SET default_tablespace = ''; It looks like we have some work to do to teach pg_dump about handling access methods in extensions. This doesn't look quite as bad as the first issue, but it's a pity 9.6 escaped into the wild with this issue. cheers andrew -- 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_dump / copy bugs with "big lines" ?
On Fri, Sep 30, 2016 at 10:12 PM, Daniel Verite wrote: > Tomas Vondra wrote: > >> A few minor comments regarding the patch: >> >> 1) CopyStartSend seems pretty pointless - It only has one function call >> in it, and is called on exactly one place (and all other places simply >> call allowLongStringInfo directly). I'd get rid of this function and >> replace the call in CopyOneRowTo(() with allowLongStringInfo(). >> >> 2) allowlong seems awkward, allowLong or allow_long would be better >> >> 3) Why does resetStringInfo reset the allowLong flag? Are there any >> cases when we want/need to forget the flag value? I don't think so, so >> let's simply not reset it and get rid of the allowLongStringInfo() >> calls. Maybe it'd be better to invent a new makeLongStringInfo() method >> instead, which would make it clear that the flag value is permanent. >> >> 4) HandleParallelMessage needs a tweak, as it uses msg->len in a log >> message, but with '%d' and not '%ld' (as needed after changing the type >> to Size). >> >> 5) The comment at allowLongStringInfo talks about allocLongStringInfo >> (i.e. wrong function name). > > Here's an updated patch. Compared to the previous version: > > - removed CopyStartSend (per comment #1 in review) > > - renamed flag to allow_long (comment #2) > > - resetStringInfo no longer resets the flag (comment #3). > > - allowLongStringInfo() is removed (comment #3 and #5), > makeLongStringInfo() and initLongStringInfo() are provided > instead, as alternatives to makeStringInfo() and initStringInfo(). > > - StringInfoData.len is back to int type, 2GB max. > (addresses comment #4 incidentally). > This is safer because many routines peeking > into StringInfoData use int for offsets into the buffer, > for instance most of the stuff in backend/libpq/pqformat.c > Altough these routines are not supposed to be called on long > buffers, this assertion was not enforced in the code, and > with a 64-bit length effectively over 2GB, they would misbehave > pretty badly. The patch status is "Waiting on Author" in the CF app, but a new patch has been sent 2 days ago, so this entry has been moved to next CF with "Needs Review". -- 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] [BUG] pg_basebackup from disconnected standby fails
> So, if I understand correctly, then we can mark the version posted by > you upthread [1] which includes a test along with Kyotaro's fix can be > marked as Ready for committer. If so, then please change the status > of patch accordingly. Patch moved to next CF 2016-11, still with status "ready for committer". IMO, this thread has reached as conclusion to use this patch to address the problem reported: cab7npqtv5gmkqcndofgtgqoqxz2xlz4rrw247oqojzztvy6...@mail.gmail.com -- 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] pgsql: Add putenv support for msvcrt from Visual Studio 2013
On Tue, Sep 6, 2016 at 9:36 PM, Michael Paquier wrote: > OK, let's get to the next step of the game and get a committer to look > at this patch. Moved to next CF. It would be good to get a committer on this one. We have come on a conclusion on what to do. Actually, 0001 can be just HEAD-only per the lack of complaints. -- 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] postgres_fdw : altering foreign table not invalidating prepare statement execution plan.
On Fri, Sep 30, 2016 at 1:09 AM, Ashutosh Bapat wrote: >> >>> The attached patch adds the >>> dependencies from create_foreignscan_plan() which is called for any >>> foreign access. I have also added testcases to test the functionality. >>> Let me know your comments on the patch. >> >> >> Hmm. I'm not sure that that's a good idea. >> >> I was thinking the changes to setrefs.c proposed by Amit to collect that >> dependencies would be probably OK, but I wasn't sure that it's a good idea >> that he used PlanCacheFuncCallback as the syscache inval callback function >> for the foreign object caches because it invalidates not only generic plans >> but query trees, as you mentioned downthread. So, I was thinking to modify >> his patch so that we add a new syscache inval callback function for the >> caches that is much like PlanCacheFuncCallback but only invalidates generic >> plans. > > PlanCacheFuncCallback() invalidates the query tree only when > invalItems are added to the plan source. The patch adds the > dependencies in root->glob->invalItems, which standard_planner() > copies into PlannedStmt::invalItems. This is then copied into the > gplan->stmt_list. Thus PlanCacheFuncCallback never invalidates the > query tree. I have verified this under the debugger. Am I missing > something? Moved to next CF. Feel free to continue the work on this item. -- 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] WAL logging problem in 9.4.3?
On Thu, Sep 29, 2016 at 10:02 PM, Kyotaro HORIGUCHI wrote: > Hello, > > At Thu, 29 Sep 2016 16:59:55 +0900, Michael Paquier > wrote in > >> On Mon, Sep 26, 2016 at 5:03 PM, Kyotaro HORIGUCHI >> wrote: >> > Hello, I return to this before my things:) >> > >> > Though I haven't played with the patch yet.. >> >> Be sure to run the test cases in the patch or base your tests on them then! > > All items of 006_truncate_opt fail on ed0b228 and they are fixed > with the patch. > >> > Though I don't know how it actually impacts the perfomance, it >> > seems to me that we can live with truncated_to and sync_above in >> > RelationData and BufferNeedsWAL(rel, buf) instead of >> > HeapNeedsWAL(rel, buf). Anyway up to one entry for one relation >> > seems to exist at once in the hash. >> >> TBH, I still think that the design of this patch as proposed is pretty >> cool and easy to follow. > > It is clean from certain viewpoint but additional hash, > especially hash-searching on every HeapNeedsWAL seems to me to be > unacceptable. Do you see it accetable? > > > The attached patch is quiiiccck-and-dirty-hack of Michael's patch > just as a PoC of my proposal quoted above. This also passes the > 006 test. The major changes are the following. > > - Moved sync_above and truncted_to into RelationData. > > - Cleaning up is done in AtEOXact_cleanup instead of explicit > calling to smgrDoPendingSyncs(). > > * BufferNeedsWAL (replace of HeapNeedsWAL) no longer requires > hash_search. It just refers to the additional members in the > given Relation. > > X I feel that I have dropped one of the features of the origitnal > patch during the hack, but I don't recall it clearly now:( > > X I haven't consider relfilenode replacement, which didn't matter > for the original patch. (but there's few places to consider). > > What do you think about this? I have moved this patch to next CF. (I still need to look at your patch.) -- 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] [GENERAL] pg_upgrade from 9.5 to 9.6 fails with "invalid argument"
On 10/01/2016 02:01 PM, Tom Lane wrote: Andrew Dunstan writes: On 09/30/2016 12:24 PM, Tom Lane wrote: Seems to be some additional prep work needed somewhere ... No upgrade_install_root at /home/bfarm/bf-scripts/build-farm-4.17/PGBuild/Modules/TestUpgradeXversion.pm line 51. Oh sorry, you also need an entry for that in the config file. Mine has: upgrade_install_root => '/home/bf/bfr/root/upgrade', Yesterday's runs of prairiedog had this turned on. I'm not sure how to interpret the output (because there isn't any, ahem), but it does not appear that the test detected anything wrong. I'm working on collecting the log files and making the module more useful. But you can see pretty much all the logs (lots of them!) after a run inside the upgrade directories. cheers andrew -- 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 Indexes
On Sun, Oct 2, 2016 at 3:31 AM, Greg Stark wrote: > On Fri, Sep 30, 2016 at 2:11 AM, Robert Haas wrote: >> For one thing, we can stop shipping a totally broken feature in release >> after release > > For what it's worth I'm for any patch that can accomplish that. > > In retrospect I think we should have done the hash-over-btree thing > ten years ago but we didn't and if Amit's patch makes hash indexes > recoverable today then go for it. +1. -- 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] On conflict update & hint bits
On Sat, Oct 1, 2016 at 1:15 PM, Peter Geoghegan wrote: > It also looks like the DO NOTHING variant is similarly affected, even > when the isolation level is READ COMMITTED.:-( Actually, the DO NOTHING variant is also only affected in isolation levels greater than READ COMMITTED. Not sure why I thought otherwise. -- Peter Geoghegan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers