Re: [HACKERS] TAP / recovery-test fs-level backups, psql enhancements etc
I wrote: > Michael Paquierwrites: >> Yes, that's the problem. Instead of using details(), summary() is >> enough actually. And it is enough to let caller know the failure when >> just one test has been found as not passing. See attached. > This one works for me on RHEL6. Pushed; we'll see if the older > buildfarm members like it. I don't normally run the TAP tests on "prairiedog", because it's too $!*&@ slow, but trying that manually seems to work. That's the Perl 5.8.6 that Apple shipped with OS X 10.4 ... if there's anything older in our buildfarm, I don't know about it. 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] [PATCH] Logical decoding support for sequence advances
On 1 March 2016 at 05:30, Petr Jelinekwrote: > > On 29/02/16 03:23, Craig Ringer wrote: >> >> > Sound reasonable? >> > > I wonder if it would be acceptable to create new info flag for RM_SEQ_ID > that would behave just like XLOG_SEQ_LOG but would be used only for the > nontransactional updates (nextval) so that decoding could easily > differentiate between transactional and non-transactional update of > sequence and then just either call the callback immediately or add the > change to reorder buffer based on that. The redo code could just have > simple OR expression to behave same with both of the info flags. > That's much cleaner than trying to keep track of sequence creations and really pretty harmless. I'll give that a go and see how it looks. > Seems like simpler solution than building all the tracking code on the > decoding side to me. +1 -- Craig Ringer http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
Re: [HACKERS][REVIEW]: Password identifiers, protocol aging and SCRAM protocol
On Wed, Mar 2, 2016 at 4:05 AM, Dmitry Dolgov <9erthali...@gmail.com> wrote: > [...] Thanks for the review. > The default value contains "scram". Shouldn't be here also: > >>Specifies a comma-separated list of supported password formats by >>the server. Supported formats are currently plain, >>md5 and scram. > > Or I missed something? Ah, I see. That's in the documentation of password_protocols. Yes scram should be listed there as well. That should be fixed in 0009. >> >>db_user_namespace causes the client's and >>server's user name representation to differ. >>Authentication checks are always done with the server's user name >>so authentication methods must be configured for the >>server's user name, not the client's. Because >>md5 uses the user name as salt on both the >>client and server, md5 cannot be used with >>db_user_namespace. >> > > Looks like the same (pls, correct me if I'm wrong) is applicable for "scram" > as I see from the code below. Shouldn't be "scram" mentioned here also? Oops. Good catch. Yes it should be mentioned as part of the SCRAM patch (0009). -- 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] 2016-03 Commitfest Manager
On Wed, Mar 02, 2016 at 03:34:39PM +0900, Michael Paquier wrote: > On Wed, Mar 2, 2016 at 2:53 PM, David Fetterwrote: > > On Wed, Mar 02, 2016 at 10:49:01AM +0900, Michael Paquier wrote: > >> On Wed, Mar 2, 2016 at 7:22 AM, Michael Paquier > >> wrote: > >> > On Wed, Mar 2, 2016 at 6:15 AM, David Steele wrote: > >> >> On 3/1/16 3:04 PM, Tom Lane wrote: > >> >>> David Steele writes: > >> I volunteered a while back to be the CFM and I haven't seen any other > >> volunteers or objections to my offer. > >> >>> > >> I am still ready, eager, and willing! > >> >>> > >> >>> I haven't heard any other volunteers either. You have the conn, sir. > >> >> > >> >> Aye aye! Once the 2016-03 CF has been closed I'll send out a kickoff > >> >> report. > >> > > >> > Magnus, should David have administration rights in the CF app? I > >> > believe that he cannot change the CF status now. We should switch to > >> > "In Progress". I can do that at least. > >> > >> I am planning to switch 2016-03 from "Open" to "Processing" in a > >> couple of hours, at which stage nobody will be able to register new > >> patches to it. I guess it's more than time. > > > > How do I get my weighted stats patch in? > > Why didn't you register it before and why didn't you send a new > version yet? Looking at this stuff now the last activity for this > patch was the 12th of January, with no new version, just a mention > that the patch will be fixed: > http://www.postgresql.org/message-id/cajrrpgf1kt-5bn+rpunxmkcigxnqs5_ratyqseun-xff9h+...@mail.gmail.com I have been pretty busy with some high-priority family and work stuff, including a terminal illness (not mine) and a new job. None of that is a reason to go around the CF process. I'll see whether I can wheedle a committer's time on this once I have the new patch. If I can't, I'll queue it for the next one. Cheers, David. -- David Fetter http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@gmail.com Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Incorrect error message in InitializeSessionUserId
On Tue, Mar 1, 2016 at 10:21 PM, Dmitriy Sarafannikovwrote: > I have found incorrect error message in InitializeSessionUserId function > if you try to connect to database by role Oid (for example > BackgroundWorkerInitializeConnectionByOid). > If role have no permissions to login, you will see error message like this: > FATAL: role "(null)" is not permitted to log in > > I changed few lines of code and fixed this. > Patch is attached. > I want to add this patch to commitfest. > Any objections? Not really. That looks like an oversight to 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] 2016-03 Commitfest Manager
On Wed, Mar 2, 2016 at 2:53 PM, David Fetterwrote: > On Wed, Mar 02, 2016 at 10:49:01AM +0900, Michael Paquier wrote: >> On Wed, Mar 2, 2016 at 7:22 AM, Michael Paquier >> wrote: >> > On Wed, Mar 2, 2016 at 6:15 AM, David Steele wrote: >> >> On 3/1/16 3:04 PM, Tom Lane wrote: >> >>> David Steele writes: >> I volunteered a while back to be the CFM and I haven't seen any other >> volunteers or objections to my offer. >> >>> >> I am still ready, eager, and willing! >> >>> >> >>> I haven't heard any other volunteers either. You have the conn, sir. >> >> >> >> Aye aye! Once the 2016-03 CF has been closed I'll send out a kickoff >> >> report. >> > >> > Magnus, should David have administration rights in the CF app? I >> > believe that he cannot change the CF status now. We should switch to >> > "In Progress". I can do that at least. >> >> I am planning to switch 2016-03 from "Open" to "Processing" in a >> couple of hours, at which stage nobody will be able to register new >> patches to it. I guess it's more than time. > > How do I get my weighted stats patch in? Why didn't you register it before and why didn't you send a new version yet? Looking at this stuff now the last activity for this patch was the 12th of January, with no new version, just a mention that the patch will be fixed: http://www.postgresql.org/message-id/cajrrpgf1kt-5bn+rpunxmkcigxnqs5_ratyqseun-xff9h+...@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] Publish autovacuum informations
On Tue, Mar 1, 2016 at 11:37 PM, Julien Rouhaud wrote: > I'm not sure what are the fancy things that Michael had in mind with > exposing the private structure. Michael, was it something like having > the ability to change some of these data through an extension? I was referring to you here :) I have witnessed already many fancy things coming out of brain, and I have no doubt you could make something out of just a sampling of data. Jokes apart, what I mean with fancy things here is putting the sampling data in a shape that a user suits to him. It would be easy to exploit that for example in a background worker that scans periodically the shared memory, or have an extension that represents this shared memory data into something that can be queried at SQL level. Now, the main use case that I have with the data available in shared memory is more or less: - represent the current shared memory as SQL - Scan this data, reshape it and report it elsewhere, say a background worker printing out a file. Now, take your set of hooks... There are 4 hooks here: - autovacuum_list_tables_hook, to do something with the list of tables a worker has collected, at the moment they have been collected. - autovacuum_end_table_hook, to do something when knowing that a table is skipped or cancelled - autovacuum_begin_table_hook, to trigger something when a relation is beginning to be processed. - autovacuum_database_finished_hook, to trigger something once a database is done with its processing. The only use cases that I have in mind here for those hooks, which would help in decision-making to tune autovacuum-related parameters would be the following: - Log entries regarding those operations, then why not introducing a GUC parameter that is an on/off switch, like log_autovacuum (this is not a per-relation parameter), defaulting to off. Those could be used by pgbadger. - Have system statistics with a new system relation like pg_stat_autovacuum, and make this information available to user. Are there other things that you think could make use those hooks? Your extension just does pg_stat_autovacuum and emulates the existing pg_stat_* facility when gathering information about the global autovacuum statistics. So it seems to me that those hooks are not that necessary, and that this may help in tuning a system, particularly the number of relations skipped would be interesting to have. The stats could have a delay, the point being to have hints on how autovacuum workers are sorting things out. In short, I am doubtful about the need of hooks in those code paths, the thing that we should try to do instead is to improve native solutions to give user more information regarding how autovacuum works, which help in tuning 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] GetExistingLocalJoinPath() vs. the docs
I think that you need to take a little broader look at this section. > At the top, it says "To use any of these functions, you need to > include the header file foreign/foreign.h in your source file", but > this function is defined in foreign/fdwapi.h. It's not clear to me > whether we should consider moving the prototype, or just document that > this function is someplace else. The other functions prototyped in > fdwapi.h aren't documented at all, except for > IsImportableForeignTable, which is mentioned in passing. > > Further down, the section says "Some object types have name-based > lookup functions in addition to the OID-based ones:" and you propose > to put the documentation for this function after that. But this > comment doesn't actually describe this particular function. > > Actually, this function just doesn't seem to fit into this section at > all. It's really quite different from the others listed there. How > about something like the attached instead? Right. Mentioning the function in the description of relevant function looks better and avoids some duplication. -- Best Wishes, Ashutosh Bapat EnterpriseDB Corporation The Postgres Database Company
Re: [HACKERS] TAP / recovery-test fs-level backups, psql enhancements etc
Michael Paquierwrites: > Yes, that's the problem. Instead of using details(), summary() is > enough actually. And it is enough to let caller know the failure when > just one test has been found as not passing. See attached. This one works for me on RHEL6. Pushed; we'll see if the older buildfarm members like it. 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] 2016-03 Commitfest Manager
On Wed, Mar 02, 2016 at 10:49:01AM +0900, Michael Paquier wrote: > On Wed, Mar 2, 2016 at 7:22 AM, Michael Paquier >wrote: > > On Wed, Mar 2, 2016 at 6:15 AM, David Steele wrote: > >> On 3/1/16 3:04 PM, Tom Lane wrote: > >>> David Steele writes: > I volunteered a while back to be the CFM and I haven't seen any other > volunteers or objections to my offer. > >>> > I am still ready, eager, and willing! > >>> > >>> I haven't heard any other volunteers either. You have the conn, sir. > >> > >> Aye aye! Once the 2016-03 CF has been closed I'll send out a kickoff > >> report. > > > > Magnus, should David have administration rights in the CF app? I > > believe that he cannot change the CF status now. We should switch to > > "In Progress". I can do that at least. > > I am planning to switch 2016-03 from "Open" to "Processing" in a > couple of hours, at which stage nobody will be able to register new > patches to it. I guess it's more than time. How do I get my weighted stats patch in? Cheers, David. -- David Fetter http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@gmail.com Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS]WIP: Covering + unique indexes.
On Wed, Mar 2, 2016 at 2:10 AM, Anastasia Lubennikovawrote: > 01.03.2016 19:55, Anastasia Lubennikova: >> It is not the final version, because it breaks pg_dump for previous >> versions. I need some help from hackers here. >> pgdump. line 5466 >> if (fout->remoteVersion >= 90400) >> >> What does 'remoteVersion' mean? And what is the right way to change it? Or >> it changes between releases? >> I guess that 90400 is for 9.4 and 80200 is for 8.2 but is it really so? >> That is totally new to me. Yes, you got it. That's basically PG_VERSION_NUM as compiled on the server that has been queried, in this case the server from which a dump is taken. If you are changing the system catalog layer, you would need to provide a query at least equivalent to what has been done until now for your patch, the modify pg_dump as follows: if (fout->remoteVersion >= 90600) { query = my_new_query; } else if (fout->remoteVersion >= 90400) { query = the existing 9.4 query } etc. In short you just need to add a new block so as remote servers newer than 9.6 will be able to dump objects correctly. pg_upgrade is a good way to check the validity of pg_dump actually, this explains why some objects are not dropped in the regression tests. Perhaps you'd want to do the same with your patch if the current test coverage of pg_dump is not enough. I have not looked at your patch so I cannot say for sure. -- 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] TAP / recovery-test fs-level backups, psql enhancements etc
On 2 March 2016 at 13:22, Michael Paquierwrote: > On Wed, Mar 2, 2016 at 2:18 PM, Alvaro Herrera > wrote: > > Tom Lane wrote: > >> I wrote: > >> > Can't use string ("Test::Builder") as a HASH ref while "strict refs" > in use at /usr/share/perl5/Test/Builder.pm line 1798. > >> > >> > The referenced line number is the end of the file, > >> > >> Oh, scratch that; I was looking at the wrong file. Actually, > >> /usr/share/perl5/Test/Builder.pm has > >> > >> sub details { > >> my $self = shift; > >> return @{ $self->{Test_Results} }; > >> } > >> > >> and line 1798 is the "return" statement in that. I still lack enough > >> Perl-fu to decipher this, though. > > > > I think it's complaining about being called as a class method. Perhaps > > this would work: > > > > + foreach my $detail (Test::More->builder->details()) > > Yes, that's the problem. Instead of using details(), summary() is > enough actually. And it is enough to let caller know the failure when > just one test has been found as not passing. See attached. > Thanks. I'm setting up a container with Debian Wheezy, which looks old enough to test proposed framework changes, so I'll be able to test future patches against a real Perl 5.8. -- Craig Ringer http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
Re: [HACKERS] Move PinBuffer and UnpinBuffer to atomics
On Wed, Mar 2, 2016 at 10:39 AM, Andres Freundwrote: > Sounds like a ppc vs. x86 issue. The regression was on the former, right? Well, Regression what I reported last two time, out of that one was on X86 and other was on PPC. Copied from older Threads -- On PPC >> > > > >> ./pgbench -j$ -c$ -T300 -M prepared -S postgres >> > > > >> >> > > > >> ClientBasePatch >> > > > >> 11716916454 >> > > > >> 8108547105559 >> > > > >> 32241619262818 >> > > > >> 64206868233606 >> > > > >> 128137084217013 On X86 >> > > > >>Shared Buffer= 8GB >> > > > >>Scale Factor=300 >> > > > >>./pgbench -j$ -c$ -T300 -M prepared -S postgres >> > > > >>client basepatch >> > > > >>1 7057 5230 >> > > > >>2 10043 9573 >> > > > >>4 2014018188 And this latest result (no regression) is on X86 but on my local machine. I did not exactly saw what this new version of patch is doing different, so I will test this version in other machines also and see the results. -- Regards, Dilip Kumar EnterpriseDB: http://www.enterprisedb.com
Re: [HACKERS] TAP / recovery-test fs-level backups, psql enhancements etc
On Wed, Mar 2, 2016 at 2:18 PM, Alvaro Herrerawrote: > Tom Lane wrote: >> I wrote: >> > Can't use string ("Test::Builder") as a HASH ref while "strict refs" in >> > use at /usr/share/perl5/Test/Builder.pm line 1798. >> >> > The referenced line number is the end of the file, >> >> Oh, scratch that; I was looking at the wrong file. Actually, >> /usr/share/perl5/Test/Builder.pm has >> >> sub details { >> my $self = shift; >> return @{ $self->{Test_Results} }; >> } >> >> and line 1798 is the "return" statement in that. I still lack enough >> Perl-fu to decipher this, though. > > I think it's complaining about being called as a class method. Perhaps > this would work: > > + foreach my $detail (Test::More->builder->details()) Yes, that's the problem. Instead of using details(), summary() is enough actually. And it is enough to let caller know the failure when just one test has been found as not passing. See attached. -- Michael tap-tmp-failure.patch Description: binary/octet-stream -- 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] TAP / recovery-test fs-level backups, psql enhancements etc
Tom Lane wrote: > I wrote: > > Can't use string ("Test::Builder") as a HASH ref while "strict refs" in use > > at /usr/share/perl5/Test/Builder.pm line 1798. > > > The referenced line number is the end of the file, > > Oh, scratch that; I was looking at the wrong file. Actually, > /usr/share/perl5/Test/Builder.pm has > > sub details { > my $self = shift; > return @{ $self->{Test_Results} }; > } > > and line 1798 is the "return" statement in that. I still lack enough > Perl-fu to decipher this, though. I think it's complaining about being called as a class method. Perhaps this would work: + foreach my $detail (Test::More->builder->details()) -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Move PinBuffer and UnpinBuffer to atomics
On March 1, 2016 8:41:33 PM PST, Dilip Kumarwrote: >On Tue, Mar 1, 2016 at 10:19 AM, Dilip Kumar >wrote: > >> >> OK, I will test it, sometime in this week. >> > >I have tested this patch in my laptop, and there i did not see any >regression at 1 client > >Shared buffer 10GB, 5 mins run with pgbench, read-only test > >basepatch >run122187 24334 >run226288 27440 >run326306 27411 > > >May be in a day or 2 I will test it in the same machine where I >reported >the regression, and if regression is there I will check the instruction >using Call grind. Sounds like a ppc vs. x86 issue. The regression was on the former, right? Andres -- Sent from my Android device with K-9 Mail. Please excuse my brevity. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Recovery test failure for recovery_min_apply_delay on hamster
Hi all, I have enabled yesterday the recovery test suite on hamster, and we did not have to wait long before seeing the first failure on it, the machine being slow as hell so it is quite good at catching race conditions: http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=hamster=2016-03-01%2016%3A00%3A06 Honestly, I did runs on this machine of the test suite, but I didn't see it, so that's quite sporadic. Yesterday's run worked fine for example. In more details, the following problem showed up: ### Running SQL command on node "standby": SELECT count(*) FROM tab_int not ok 1 - check content with delay of 1s # Failed test 'check content with delay of 1s' # at t/005_replay_delay.pl line 39. # got: '20' # expected: '10' ### Running SQL command on node "master": SELECT pg_current_xlog_location(); ### Running SQL command on node "standby": SELECT count(*) FROM tab_int ok 2 - check content with delay of 2s This is a timing issue, caused by the use of recovery_min_apply_delay, the test doing the following: 1) Set up recovery_min_apply_delay to 2 seconds 2) Start the standby 3) Apply an INSERT on master, save pg_current_xlog_location from master 4) sleep 1s 5) query standby, and wait that WAL has not been applied yet. 6) Wait that required LSN from master has been applied 7) query again standby, and see that WAL has been applied. The problem is that visibly hamster is so slow that more than 2s have been spent between phases 3 and 5, meaning that the delay has already been reached, and WAL was applied. Here are a couple of ways to address this problem: 1) Remove the check before applying the delay 2) Increase recovery_min_apply_delay to a time that will allow even slow machines to see a difference. By experience with the other tests 30s would be enough. The sleep time needs to be increased as well, making the time taken for the test to run longer 3) Remove all together 005, because doing either 1) or 2) reduces the value of the test. I'd like 1) personally, I still see value in this test. Thoughts? -- 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] Relation extension scalability
On Tue, Mar 1, 2016 at 4:36 PM, Amit Kapilawrote: > One thing that is slightly unclear is that whether there is any overhead > due to buffer eviction especially when the buffer to be evicted is already > dirty and needs XLogFlush(). One reason why it might not hurt is that by > the time we tries to evict the buffer, corresponding WAL is already flushed > by WALWriter or other possibility could be that even if it is getting done > during buffer eviction, the impact for same is much lesser. Can we try to > measure the number of flush calls which happen during buffer eviction? > > Good Idea, I will do this test and post the results.. > > >> Proc Sleep test for Insert test when data don't fits in shared buffer and >> inserting big record of 1024 bytes, is currently running >> once I get the data will post the same. >> >> > Okay. However, I wonder if the performance data for the case when data > doesn't fit into shared buffer also shows similar trend, then it might be > worth to try by doing extend w.r.t load in system. I mean to say we can > batch the extension requests (as we have done in ProcArrayGroupClearXid) > and extend accordingly, if that works out then the benefit could be that we > don't need any configurable knob for the same. > 1. One option can be as you suggested like ProcArrayGroupClearXid, With some modification, because when we wait for the request and extend w.r.t that, may be again we face the Context Switch problem, So may be we can extend in some multiple of the Request. (But we need to take care whether to give block directly to requester or add it into FSM or do both i.e. give at-least one block to requester and put some multiple in FSM) 2. Other option can be that we analyse the current Load on the extend and then speed up or slow down the extending speed. Simple algorithm can look like this If (GotBlockFromFSM()) Success++ // We got the block from FSM Else Failure++// Did not get Block from FSM and need to extend by my self Now while extending - Speed up - If (failure - success > Threshold ) // Threshold can be one number assume 10. ExtendByBlock += failure- success; --> If many failure means load is high then Increase the ExtendByBlock Failure = success= 0 --> reset after this so that we can measure the latest trend. Slow down.. -- //Now its possible that demand of block is reduced but ExtendByBlock is still high.. So analyse statistic and slow down the extending pace.. If (success - failure > Threshold) { // Can not reduce it by big number because, may be more request are satisfying because this is correct amount, so gradually decrease the pace and re-analyse the statistics next time. ExtendByBlock --; Failure = success= 0 } Any Suggestions are Welcome... -- Regards, Dilip Kumar EnterpriseDB: http://www.enterprisedb.com
Re: [HACKERS] proposal: PL/Pythonu - function ereport
Hi 2016-03-01 18:48 GMT+01:00 Catalin Iacob: > On 3/1/16, Pavel Stehule wrote: > >> I though about it before and I prefer variant with possibility to enter > >> message as keyword parameter. > > That's also ok, but indeed with a check that it's not specified twice > which I see you already added. > > > I merged your patches without @3 (many thanks for it). Instead @3 I > > disallow double message specification (+regress test) > > Great, welcome. Ran the tests for plpython-enhanced-error-06 again on > 2.4 - 2.7 and 3.5 versions and they passed. > > I see the pfree you added isn't allowed on a NULL pointer but as far > as I see message is guaranteed not to be NULL as dgettext never > returns NULL. > > I'll mark this Ready for Committer. > Thank you very much Nice day Pavel
Re: [HACKERS] TAP / recovery-test fs-level backups, psql enhancements etc
Craig Ringerwrites: > Really, really this time, the version in git that actually works, not a > format-patch'd version before I made a last fix. Sigh. I can't even blame > lack of coffee... Hmm, still doesn't work for me: make check-world dies with Can't use string ("Test::Builder") as a HASH ref while "strict refs" in use at / usr/share/perl5/Test/Builder.pm line 1798. END failed--call queue aborted. # Looks like your test exited with 22 just after 14. The referenced line number is the end of the file, which is right in keeping with the TAP tests' infrastructure's habit of being utterly impenetrable when it's not working. My small amount of Perl-fu is not sufficient to debug this. perl-5.10.1-141.el6_7.1.x86_64 perl-Test-Harness-3.17-141.el6_7.1.x86_64 perl-Test-Simple-0.92-141.el6_7.1.x86_64 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] TAP / recovery-test fs-level backups, psql enhancements etc
On Wed, Mar 2, 2016 at 1:33 PM, Tom Lanewrote: > Craig Ringer writes: >> This upset buildfarm members running prehistoric Perl versions because >> is_passing was added after 5.8.8. > > Sir, RHEL6 is not prehistoric ... and this is failing on my server too. > I'm not sure when "is_passing" was added, but it was later than 5.10.1. Test::Builder is managing this variable, as documented here for people wondering: http://perldoc.perl.org/Test/Builder.html I am tracking the first version as being 5.12.0. {details} would prove to work, it is available in 5.8.8. -- 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: fix lock contention for HASHHDR.mutex
On Tue, Mar 1, 2016 at 8:13 PM, Aleksander Alekseev < a.aleks...@postgrespro.ru> wrote: > > Hello, Amit > > > I am not sure, if this is exactly what has been suggested by Robert, > > so it is not straightforward to see if his suggestion can allow us to > > use NUM_FREELISTS as 8 rather than 32. I think instead of trying to > > use FREELISTBUFF, why not do it as Robert has suggested and try with > > different values of NUM_FREELISTS? > > Well, what I did is in fact a bit more general solution then Robert > suggested. At first I just joined freeList and mutex arrays into one > array and tried different NUM_FREELISTS values (see FREELISTS column). > That was Robert's idea. Unfortunately it didn't give any considerable > speedup comparing with previous approach. > Okay, now I can understand the data better and I think your tests indicate that it is better to keep NUM_FREELISTS as 32. > > Then I tried to manually control sizeof every array item (see > different SIZE values). By making it larger we can put every array item > into a separate cache line. This approach helped a bit in terms of TPS > (before: +33.9%, after: +34.2%) but only if NUM_FREELISTS remains the > same (32). > > So answering your question - it turned out that we _can't_ reduce > NUM_FREELISTS this way. > > Also I don't believe that +0.3% TPS according to synthetic benchmark > that doesn't represent any possible workload worth it considering > additional code complexity. > I think data structure HASHHDR is looking simpler, however the code is looking more complex, especially with the addition of FREELISTBUFF for cacheline purpose. I think you might want to try with exactly what has been suggested and see if the code looks better that way, but if you are not convinced, then lets leave it to committer unless somebody else wants to try that suggestion. > > In which case, do you think entries can go negative? I think the > > nentries is incremented and decremented in the way as without patch, > > so I am not getting what can make it go negative. > > In this context we are discussing a quick and dirty fix "what would > happen if FREELIST_IDX would be implemented as getpid() % NUM_FREE_LIST > instead of hash % NUM_FREELIST". The code is: > > int freelist_idx = FREELIST_IDX(hctl, hashvalue); > > // ... > > switch (action) > { > > // ... > > case HASH_REMOVE: > if (currBucket != NULL) > { > if (IS_PARTITIONED(hctl)) > SpinLockAcquire(&(FREELIST(hctl, freelist_idx).mutex)); > > // Assert(FREELIST(hctl, freelist_idx).nentries > 0); > FREELIST(hctl, freelist_idx).nentries--; > > /* remove record from hash bucket's chain. */ > *prevBucketPtr = currBucket->link; > > // ... > > No matter what freelist was used when element was added to a hash table. > We always try to return free memory to the same freelist number getpid() > % FREELIST_ITEMS and decrease number of elements in a hash table using > corresponding nentries field. > Won't it always use the same freelist to remove and add the entry from freelist as for both cases it will calculate the freelist_idx in same way? With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
Re: [HACKERS] Move PinBuffer and UnpinBuffer to atomics
On Tue, Mar 1, 2016 at 10:19 AM, Dilip Kumarwrote: > > OK, I will test it, sometime in this week. > I have tested this patch in my laptop, and there i did not see any regression at 1 client Shared buffer 10GB, 5 mins run with pgbench, read-only test basepatch run122187 24334 run226288 27440 run326306 27411 May be in a day or 2 I will test it in the same machine where I reported the regression, and if regression is there I will check the instruction using Call grind. -- Regards, Dilip Kumar EnterpriseDB: http://www.enterprisedb.com
Re: [HACKERS] TAP / recovery-test fs-level backups, psql enhancements etc
Craig Ringerwrites: > This upset buildfarm members running prehistoric Perl versions because > is_passing was added after 5.8.8. Sir, RHEL6 is not prehistoric ... and this is failing on my server too. I'm not sure when "is_passing" was added, but it was later than 5.10.1. > Fix attached. Will check and apply. > I think I'm going to have to do an archaeology-grade Perl install, there's > just too much to keep an eye on manually. Ugh. > Why do we requite a 10yo Perl anyway? The point of running ancient versions in the buildfarm is so that we don't move the goalposts on software compatibility without knowing it. When we come across a good reason to desupport an old Perl or Tcl or whatever version, we'll do it; but having to add another ten lines or so of code doesn't seem like a good reason. 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] 2016-03 Commitfest Manager
On 02/03/16 17:01, Michael Paquier wrote: On Wed, Mar 2, 2016 at 11:25 AM, David Steelewrote: Agreed. I see you created the new CF so no reason to keep it open. OK. Done. May the force to manage all those patches be with you, manager. May the Source be with you Luke^H^H^H^HMichael! -- 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] 2016-03 Commitfest Manager
On Wed, Mar 2, 2016 at 11:25 AM, David Steelewrote: > Agreed. I see you created the new CF so no reason to keep it open. OK. Done. May the force to manage all those patches be with you, manager. -- 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] TAP / recovery-test fs-level backups, psql enhancements etc
On 2 March 2016 at 10:07, Craig Ringerwrote: > On 2 March 2016 at 05:46, Alvaro Herrera wrote: > > >> >> I think we should change the existing psql method to be what you propose >> as psql_expert. I don't see any advantage in keeping the old one. Many >> of the existing uses of psql should become what you call psql_check; but >> we should probably call that psql_ok() instead, and also have a >> psql_fails() for src/test/recovery/t/001_stream_rep.pl (and others to >> come). >> > > I agree and that's what I really wanted to do. I just didn't want to > produce a massive diff that renames the method across all of src/bin etc > too, since I thought that'd be harder to commit and might have backporting > consequences. > > If you think that's the way to go I'm very happy with that and will > proceed. > I'll make the change you suggested to make 'psql_expert' into 'psql' and change call sites to use it or psql_check as appropriate. I'll probably make it an immediately following patch in the series so it's easier to separate the bulk-rename from the functional changes, but it can be trivially squashed for commit. On reflection I want to keep the name as psql_check, rather than psql_ok. I'll insert another patch that changes src/bin to use psql_check where appropriate. The reason I used _check rather than psql_ok is partly that psql_check isn't a test. It doesn't run any Test::More checks, it die()s on failure because failure isn't expected but is incidental to the test that's using psql. I did it that way because I don't think the psql invocation should be a test in its self - then you'd have to pass a test name to every psql_ok invocation and you'd get a flood of meaningless micro-tests showing up that obscure the real thing being tested. It'd also be a PITA maintaining the number of tests in the tests => 'n' argument to Test::More. So I'm inclined to keep it as psql_check, to avoid confusion with the names 'ok' and 'fails' that Test::More uses. It's not actually a test. I don't think we need or should have a psql_ok wrapper, since with this change you can now just write: is($node->psql('db', 'SELECT syntaxerror;'), 3, 'psql exits with code 3 on syntax error'); which is clearer and more specific than: $node->psql_ok('db', 'SELECT syntaxerror;', test => 'psql exits on syntax error'); > > The reason I didn't do that is that the indenting in PostgresNode.pm is > already well out of whack and, TBH, I didn't want to rebase on top of a > perltidy'd version. I can bite the bullet and move the perltidy to the > start of the patch series then make sure each subsequent patch is tidy'd > but I'd want to be very sure you'd be OK to commit the perltidy of > PostgresNode.pm otherwise I'd have to rebase messily all over again... > This wasn't as bad as I thought. I pulled the tidy changes to the $self->psql stuff into that patch and rebased the rest to the start of the series so it only touches what's currently committed. I agree that's better. Updated tree pushed. I'll send a new patch series once I've done the psql_ok part. It's funny that as part of implementing timeline following in logical decoding and implementing failover slots I'm writing perl test framework improvements -- Craig Ringer http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
Re: [HACKERS] TAP / recovery-test fs-level backups, psql enhancements etc
On 2 March 2016 at 11:23, Craig Ringerwrote: > Really, this time. > Really, really this time, the version in git that actually works, not a format-patch'd version before I made a last fix. Sigh. I can't even blame lack of coffee... -- Craig Ringer http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services From a6aa0139cc8fea2fb93b7c3087d8878d8923c49a Mon Sep 17 00:00:00 2001 From: Craig Ringer Date: Wed, 2 Mar 2016 11:17:46 +0800 Subject: [PATCH] TAP: Fix Perl 5.8.8 support Commit 7132810c (Retain tempdirs for failed tests) used the is_passing method present only after Perl 5.10 in Test::More 0.89_01. Work around its absence. --- src/test/perl/TestLib.pm | 18 +- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/src/test/perl/TestLib.pm b/src/test/perl/TestLib.pm index 63a0e77..1900922 100644 --- a/src/test/perl/TestLib.pm +++ b/src/test/perl/TestLib.pm @@ -110,7 +110,23 @@ INIT END { # Preserve temporary directory for this test on failure - $File::Temp::KEEP_ALL = 1 unless Test::More->builder->is_passing; + # + # We should be able to use Test::More->builder->is_passing here, but + # we require Perl 5.8.8 support so we have to work around it. + # + $File::Temp::KEEP_ALL = 1 unless all_tests_passing(); +} + +# Workaround for is_passing being missing prior to Test::More 0.89_01 +# Credit http://stackoverflow.com/a/1506700/398670 +sub all_tests_passing +{ + my $FAILcount = 0; + foreach my $detail (Test::Builder->details()) + { + if (${%$detail}{ok} == 0) { $FAILcount++; } + } + return !$FAILcount; } # -- 2.1.0 -- 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] TAP / recovery-test fs-level backups, psql enhancements etc
On 2 March 2016 at 11:22, Craig Ringerwrote: > 2016-03-02 6:57 GMT+08:00 Alvaro Herrera : > >> Just pushed 0006. >> >> > This upset buildfarm members running prehistoric Perl versions because > is_passing was added after 5.8.8. > > Fix attached. > Really, this time. -- Craig Ringer http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services From bd913623092d92cc65d5b94b1b0c3fd5e66b8856 Mon Sep 17 00:00:00 2001 From: Craig Ringer Date: Wed, 2 Mar 2016 11:17:46 +0800 Subject: [PATCH] TAP: Fix Perl 5.8.8 support Commit 7132810c (Retain tempdirs for failed tests) used the is_passing method present only after Perl 5.10 in Test::More 0.89_01. Work around its absence. --- src/test/perl/TestLib.pm | 18 +- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/src/test/perl/TestLib.pm b/src/test/perl/TestLib.pm index 63a0e77..c069d43 100644 --- a/src/test/perl/TestLib.pm +++ b/src/test/perl/TestLib.pm @@ -110,7 +110,23 @@ INIT END { # Preserve temporary directory for this test on failure - $File::Temp::KEEP_ALL = 1 unless Test::More->builder->is_passing; + # + # We should be able to use Test::More->builder->is_passing here, but + # we require Perl 5.8.8 support so we have to work around it. + # + $File::Temp::KEEP_ALL = 1 unless all_tests_passing; +} + +# Workaround for is_passing being missing prior to Test::More 0.89_01 +# Credit http://stackoverflow.com/a/1506700/398670 +sub all_tests_passing +{ + my $FAILcount = 0; + foreach my $detail (Test::Builder->details()) + { + if (${%$detail}{ok} == 0) { $FAILcount++; } + } + return !$FAILcount; } # -- 2.1.0 -- 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] TAP / recovery-test fs-level backups, psql enhancements etc
2016-03-02 6:57 GMT+08:00 Alvaro Herrera: > Just pushed 0006. > > This upset buildfarm members running prehistoric Perl versions because is_passing was added after 5.8.8. Fix attached. I think I'm going to have to do an archaeology-grade Perl install, there's just too much to keep an eye on manually. Ugh. Why do we requite a 10yo Perl anyway? -- Craig Ringer http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
Re: [HACKERS] Freeze avoidance of very large table.
On Thu, Feb 18, 2016 at 3:45 AM, Masahiko Sawadawrote: > Attached updated 5 patches. > I would like to explain these patch shortly again here to make > reviewing more easier. > > We can divided these patches into 2 purposes. > > 1. Freeze map > 000_ patch adds additional frozen bit into visibility map, but doesn't > include the logic for improve freezing performance. > 001_ patch gets rid of page-conversion code from pg_upgrade. (This > patch doesn't related to this feature essentially, but is required by > 002_ patch.) > 002_ patch adds upgrading mechanism from 9.6- to 9.6+ and its regression test. > > 2. Improve freezing logic > 003_ patch changes the VACUUM to optimize scans based on freeze map > (i.g., 000_ patch), and its regression test. > 004_ patch enhances debug messages in src/backend/access/heap/visibilitymap.c > > Please review them. I have pushed 000 and part of 003, with substantial revisions to the 003 part and minor revisions to the 000 part. This gets the basic infrastructure in place, but the vacuum optimization and pg_upgrade fixes still need to be done. I discovered that make check-world failed with 000 applied, because the Assert()s added to visibilitymap_set were using | rather than & to test for a set bit. I fixed that. I revised the code in vacuumlazy.c that updates the new map bits rather heavily. I hope I didn't break anything; please have a look and see if you spot any problems. One big problem was that it's inadequate to judge whether a tuple needs freezing just by looking at xmin; xmax might need to be cleared, for example. I removed the pgstat stuff. I'm not sure we want that stuff in that form; it doesn't seem to fit with the rest of what's in that view, and it wasn't reliable in my testing. I did however throw together a little contrib module for testing, which I attach here. I'm not sure we want to commit this, and at the least someone would need to write documentation. But it's certainly handy for checking whether this works. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company pg_visibilitymap-v1.patch Description: application/download -- 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] Random inconsistencies in GiST support function declarations
On Mon, Jan 18, 2016 at 2:29 PM, Tom Lanewrote: > > Fixing the pg_proc entries in HEAD seems like no big deal, but some of > the errors are in contrib modules. If we wanted to be really clean > about that, we'd have to bump those modules' extension versions, which > is a pain in the rear. Since this has no user-visible impact AFAICS > (the support functions not being callable from SQL), I'm inclined to > just fix the incorrect declarations in-place. I think it's sufficient > if the contrib modules pass amvalidate checks in future, I don't feel > a need to make existing installations pass. > > Any objections? > > regards, tom lane > This work (9ff60273e35cad6e9) seems have broken pg_upgrade when tsearch2 is installed. On an empty 9.4 instance with nothing but tsearch2 installed, using HEAD's pg_upgrade gives this error: pg_restore: creating OPERATOR CLASS "public.gin_tsvector_ops" pg_restore: [archiver (db)] Error while PROCESSING TOC: pg_restore: [archiver (db)] Error from TOC entry 3037; 2616 19016 OPERATOR CLASS gist_tp_tsquery_ops jjanes pg_restore: [archiver (db)] could not execute query: ERROR: function gtsquery_consistent(internal, internal, integer, oid, internal) does not exist Command was: CREATE OPERATOR CLASS "gist_tp_tsquery_ops" FOR TYPE "pg_catalog"."tsquery" USING "gist" AS STORAGE bigint , OPE... Cheers, Jeff -- 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] 2016-03 Commitfest Manager
On 3/1/16 8:49 PM, Michael Paquier wrote: > On Wed, Mar 2, 2016 at 7:22 AM, Michael Paquier >wrote: >> On Wed, Mar 2, 2016 at 6:15 AM, David Steele wrote: >>> On 3/1/16 3:04 PM, Tom Lane wrote: David Steele writes: > I volunteered a while back to be the CFM and I haven't seen any other > volunteers or objections to my offer. > I am still ready, eager, and willing! I haven't heard any other volunteers either. You have the conn, sir. >>> >>> Aye aye! Once the 2016-03 CF has been closed I'll send out a kickoff >>> report. >> >> Magnus, should David have administration rights in the CF app? I >> believe that he cannot change the CF status now. We should switch to >> "In Progress". I can do that at least. > > I am planning to switch 2016-03 from "Open" to "Processing" in a > couple of hours, at which stage nobody will be able to register new > patches to it. I guess it's more than time. Agreed. I see you created the new CF so no reason to keep it open. -- -David da...@pgmasters.net signature.asc Description: OpenPGP digital signature
Re: [HACKERS] Equivalent of --enable-tap-tests in MSVC scripts
On Wed, Mar 2, 2016 at 12:40 AM, Andrew Dunstanwrote: > On 03/01/2016 08:00 AM, Michael Paquier wrote: >> As of now the MSVC scripts control if TAP tests are enabled or not >> using a boolean flag as $config->{tap_tests}. However, this flag is >> just taken into account in vcregress.pl, with the following issues: >> 1) config_default.pl does not list tap_tests, so it is unclear to >> users to enable them. People need to look into vcregress.pl as a start >> point. >> 2) GetFakeConfigure() does not translate $config->{tap_tests} into >> --enable-tap-tests, leading to pg_config not reporting it in >> CONFIGURE. This is inconsistent with what is done in ./configure. >> >> Attached is a patch to address those two issues. > > Good work. There seem to be some unrelated whitespace changes. Shouldn't > this just be two extra lines? pertidy is telling me the contrary. Is that bad to run it for such a change? I thought we cared about the format of the perl code, and the length of the variable name "tap_tests" has an impact on the indentation of the whole block per the rules in src/tools/pgindent/perltidyrc. (Actually I made a mistake in previous patch the portion for asserts should not be indented, not sure why it was, attached is an updated patch). -- Michael From 3fd1f0a883954173c8f22722ffbf82748f3b6748 Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Mon, 22 Feb 2016 14:29:45 + Subject: [PATCH] Fix handling of --enable-tap-tests in MSVC scripts MSVC build scripts use a boolean flag called tap_tests to track if TAP tests are supported or not but this variable is only referenced in vcregress.pl, causing the following problems: 1) config_default.pl does not list this parameter, users need to look directly at vcregress.pl to check how to enable TAP tests 2) GetFakeConfigure() does not set --enable-tap-tests, leading to pg_config not listing it in its variable CONFIGURE should users enable this set of tests. --- src/tools/msvc/Solution.pm | 1 + src/tools/msvc/config_default.pl | 25 + 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/src/tools/msvc/Solution.pm b/src/tools/msvc/Solution.pm index ac116b7..c5a43f9 100644 --- a/src/tools/msvc/Solution.pm +++ b/src/tools/msvc/Solution.pm @@ -643,6 +643,7 @@ sub GetFakeConfigure $cfg .= ' --enable-integer-datetimes' if ($self->{options}->{integer_datetimes}); $cfg .= ' --enable-nls' if ($self->{options}->{nls}); + $cfg .= ' --enable-tap-tests' if ($self->{options}->{tap_tests}); $cfg .= ' --with-ldap' if ($self->{options}->{ldap}); $cfg .= ' --without-zlib' unless ($self->{options}->{zlib}); $cfg .= ' --with-extra-version' if ($self->{options}->{extraver}); diff --git a/src/tools/msvc/config_default.pl b/src/tools/msvc/config_default.pl index b9f2ff4..66d0d8c 100644 --- a/src/tools/msvc/config_default.pl +++ b/src/tools/msvc/config_default.pl @@ -13,18 +13,19 @@ our $config = { # blocksize => 8, # --with-blocksize, 8kB by default # wal_blocksize => 8, # --with-wal-blocksize, 8kB by default # wal_segsize => 16, # --with-wal-segsize, 16MB by default - ldap => 1,# --with-ldap - extraver => undef,# --with-extra-version= - nls => undef,# --enable-nls= - tcl => undef,# --with-tls= - perl => undef,# --with-perl - python => undef,# --with-python= - openssl => undef,# --with-openssl= - uuid => undef,# --with-ossp-uuid - xml => undef,# --with-libxml= - xslt => undef,# --with-libxslt= - iconv=> undef,# (not in configure, path to iconv) - zlib => undef # --with-zlib= + ldap => 1,# --with-ldap + extraver => undef,# --with-extra-version= + nls => undef,# --enable-nls= + tap_tests => undef,# --enable-tap-tests + tcl => undef,# --with-tls= + perl => undef,# --with-perl + python=> undef,# --with-python= + openssl => undef,# --with-openssl= + uuid => undef,# --with-ossp-uuid + xml => undef,# --with-libxml= + xslt => undef,# --with-libxslt= + iconv => undef,# (not in configure, path to iconv) + zlib => undef # --with-zlib= }; 1; -- 2.7.2 -- 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] TAP / recovery-test fs-level backups, psql enhancements etc
On 2 March 2016 at 05:46, Alvaro Herrerawrote: > > I think we should change the existing psql method to be what you propose > as psql_expert. I don't see any advantage in keeping the old one. Many > of the existing uses of psql should become what you call psql_check; but > we should probably call that psql_ok() instead, and also have a > psql_fails() for src/test/recovery/t/001_stream_rep.pl (and others to > come). > I agree and that's what I really wanted to do. I just didn't want to produce a massive diff that renames the method across all of src/bin etc too, since I thought that'd be harder to commit and might have backporting consequences. If you think that's the way to go I'm very happy with that and will proceed. > > +=item $node->backup_fs_hot(backup_name) > > + > > +Create a backup with a filesystem level copy in $node->backup_dir, > > +including transaction logs. Archiving must be enabled as pg_start_backup > > +and pg_stop_backup are used. This is not checked or enforced. > > Perhaps "WAL archiving or streaming must be enabled ..." > Good point, will do. > I would rather have the patches be submitted with a reasonable > approximation at indentation rather than submit a separate indent patch. > The reason I didn't do that is that the indenting in PostgresNode.pm is already well out of whack and, TBH, I didn't want to rebase on top of a perltidy'd version. I can bite the bullet and move the perltidy to the start of the patch series then make sure each subsequent patch is tidy'd but I'd want to be very sure you'd be OK to commit the perltidy of PostgresNode.pm otherwise I'd have to rebase messily all over again... -- Craig Ringer http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
Re: [HACKERS] 2016-03 Commitfest Manager
On Wed, Mar 2, 2016 at 7:22 AM, Michael Paquierwrote: > On Wed, Mar 2, 2016 at 6:15 AM, David Steele wrote: >> On 3/1/16 3:04 PM, Tom Lane wrote: >>> David Steele writes: I volunteered a while back to be the CFM and I haven't seen any other volunteers or objections to my offer. >>> I am still ready, eager, and willing! >>> >>> I haven't heard any other volunteers either. You have the conn, sir. >> >> Aye aye! Once the 2016-03 CF has been closed I'll send out a kickoff >> report. > > Magnus, should David have administration rights in the CF app? I > believe that he cannot change the CF status now. We should switch to > "In Progress". I can do that at least. I am planning to switch 2016-03 from "Open" to "Processing" in a couple of hours, at which stage nobody will be able to register new patches to it. I guess it's more than time. -- 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] Addition of extra commit fest entry to park future patches
On Wed, Mar 2, 2016 at 5:46 AM, Magnus Haganderwrote: > Yes, it's trivial to rename. That's the only advantage of our ugly url > scheme which uses the surrogate key in the url instead of the actual name of > the CF :) 2016-09 has been created then: https://commitfest.postgresql.org/10/ People, feel free to park future patches 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] TAP / recovery-test fs-level backups, psql enhancements etc
On 2 March 2016 at 07:07, Alvaro Herrerawrote: > Craig Ringer wrote: > > > diff --git a/src/test/perl/TestLib.pm b/src/test/perl/TestLib.pm > > index 3d11cbb..8c13655 100644 > > --- a/src/test/perl/TestLib.pm > > +++ b/src/test/perl/TestLib.pm > > @@ -112,9 +112,11 @@ INIT > > # > > sub tempdir > > { > > + my ($prefix) = @_; > > + $prefix = "tmp_test" if (!$prefix); > > This should be "unless defined $prefix". Otherwise if you pass the > string literal "0" as prefix it will be ignored. > > Ha. I thought something was funny with !$prefix when splitting that out of Kyotaro Horiguchi's patch but couldn't put my finger on what. Will amend in the next push. I'll keep the committed 006 Retain tempdirs for failed tests in that series but amend it to show it's committed upstream. -- Craig Ringer http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
Re: [HACKERS] The plan for FDW-based sharding
Hi, On 03/01/2016 08:02 PM, Bruce Momjian wrote: On Tue, Mar 1, 2016 at 07:56:58PM +0100, Petr Jelinek wrote: Note that I am not saying that other discussed approaches are any better, I am saying that we should know approximately what we actually want and not just beat FDWs with a hammer and hope sharding will eventually emerge and call that the plan. I will say it again --- FDWs are the only sharding method I can think of that has a chance of being accepted into Postgres core. I don't quite see why that would be the case. Firstly, it assumes that FDW-based approach is going to work, but given the lack of prototype or even a technical analysis discussing the missing pieces, that's very difficult to judge. I find it a bit annoying that there are objections from people who implemented (or attempted to implement) sharding on PostgreSQL, yet no reasonable analysis of their arguments and how the FDW approach will address them. My my understanding is they deem FDWs a bad foundation for sharding because it was designed for a different purpose, but the abstractions are a bad fit for sharding (which assumes isolated nodes, certain form of execution etc.) It is a plan, and if it fails, it fails. If is succeeds, that's good. What more do you want me to say? I know of no other way to answer the questions you asked above. Well, wouldn't it be great if we could do the decision based on some facts and not mere belief that it'll help. That's exactly what Petr is talking about - the fear that we'll spend a few years working on sharding based on FDWs, only to find out that it does not work too well. That'd be a pretty bad outcome, wouldn't it? My other worry is that we'll eventually mess the FDW infrastructure, making it harder to use for the original purpose. Granted, most of the improvements proposed so far look sane and useful for FDWs in general, but sooner or later that ceases to be the case - there sill be changes needed merely for the sharding. Those will be tough decisions. While I disagree with Simon on various things, I absolutely understand why he was asking about a prototype, and some sort of analysis of what usecases we expect to support initially/later/never, and what pieces are missing to get the sharding working. IIRC at the FOSDEM Dev Meeting you've claimed you're essentially working on a prototype - once we have the missing FDW pieces, we'll know if it works. I disagree that - it's not a prototype if it takes several years to find the outcome. Also, in another branch of this thread you've said this (I don't want to sprinkle the thread with responses, so I'll just respond here): In a way, I don't see any need for an FDW sharding prototype because, as I said, we already know XC/XL work, so copying what they do doesn't help. What we need to know is if we can get near the XC/XL benchmarks with an acceptable addition of code, which is what I thought I already said. Perhaps this can be done with FDWs, or some other approach I have not heard of yet. I don't quite understand the reasoning presented here. The XC/XL are not based on FDWs at all, therefore the need for prototype of the FDW-based sharding is entirely independent to the fact that these solutions seem to work quite well. regards -- Tomas Vondra http://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] pl/tcl Unicode conversion doesn't actually work?
I'd always sort of assumed that the UTF_U2E() / UTF_E2U() macros in pltcl.c were enabled by default. I just realized that that isn't so: they're enabled by a test #if defined(UNICODE_CONVERSION) && HAVE_TCL_VERSION(8,1) UNICODE_CONVERSION is defined nowhere in our sources, and I can find no indication that the Tcl headers define it either. A quick test proves that indeed no encoding conversion happens when running pltcl. It looks like this has been broken since commit 034895125d648b86, which is rather distressing because it means pretty much nobody is using pltcl with any non-ASCII data. It also means that pltcl is a trivial way to break encoding validity inside any non-UTF8 database. If we're moving the minimum Tcl version to be 8.4, as I just proposed we should do in another thread, I think we should remove the above-quoted #if and just compile this code unconditionally. 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] TAP / recovery-test fs-level backups, psql enhancements etc
Craig Ringer wrote: > diff --git a/src/test/perl/TestLib.pm b/src/test/perl/TestLib.pm > index 3d11cbb..8c13655 100644 > --- a/src/test/perl/TestLib.pm > +++ b/src/test/perl/TestLib.pm > @@ -112,9 +112,11 @@ INIT > # > sub tempdir > { > + my ($prefix) = @_; > + $prefix = "tmp_test" if (!$prefix); This should be "unless defined $prefix". Otherwise if you pass the string literal "0" as prefix it will be ignored. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Convert pltcl from strings to objects
Victor Wagnerwrites: > On Mon, 22 Feb 2016 17:57:36 -0600 > Jim Nasby wrote: >> Is there any backwards compatibility risk to these changes? Could >> having that new info break someone's existing code? > I don't think so. ErrorCode and ErrorInfo mechanisms present in the Tcl > for ages. As far as I remember, it predates Tcl_Obj API. Unfortunately > I've lost my copy of 2-nd edition of "Practical Programming on Tcl/Tk" > and have only 4-th edition handy, so it's quite hard to dig out Tcl 7.6 > docs. I've got some dead-tree Tcl 7.3 documentation that describes those functions, so for sure we can rely on them if we're going to rely on Tcl objects. Speaking of which, I wonder if this isn't a good time to move the goalposts a little further in terms of which Tcl versions we support. Tcl 8.4 was released in 2002; does anybody really think that someone would try to use Postgres 9.6+ with a Tcl older than that? If we just said "8.4 is the minimum", we could get rid of a number of #if's in pltcl.c. I also note the comment in front of one of those #if's: /* * Hack to override Tcl's builtin Notifier subsystem. This prevents the * backend from becoming multithreaded, which breaks all sorts of things. * ... * We can only fix this with Tcl >= 8.4, when Tcl_SetNotifier() appeared. */ In other words, if you run PG with a pre-8.4 version of Tcl, you're at very serious risk of breakage. Also, AFAICT we have no buildfarm members testing anything older than 8.4, so it's pretty hypothetical whether it'd even still work at all. So I propose that we remove those #if's in favor of something like this near the top: #if !HAVE_TCL_VERSION(8,4) #error PostgreSQL only supports Tcl 8.4 or later. #endif If we don't do that, I'm at least going to put in a similar #error for Tcl 8.0; but I really think we ought to just say 8.4 is the minimum. 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] TAP / recovery-test fs-level backups, psql enhancements etc
Just pushed 0006. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] 2016-03 Commitfest Manager
On Wed, Mar 2, 2016 at 6:15 AM, David Steelewrote: > On 3/1/16 3:04 PM, Tom Lane wrote: >> David Steele writes: >>> I volunteered a while back to be the CFM and I haven't seen any other >>> volunteers or objections to my offer. >> >>> I am still ready, eager, and willing! >> >> I haven't heard any other volunteers either. You have the conn, sir. > > Aye aye! Once the 2016-03 CF has been closed I'll send out a kickoff > report. Magnus, should David have administration rights in the CF app? I believe that he cannot change the CF status now. We should switch to "In Progress". I can do that at least. -- 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] TAP / recovery-test fs-level backups, psql enhancements etc
Craig Ringer wrote: > Hi all > > I've been working with the new TAP tests for recovery and have a number of > enhancements I'd like to make to the tooling to make writing tests easier > and nicer. I think we should change the existing psql method to be what you propose as psql_expert. I don't see any advantage in keeping the old one. Many of the existing uses of psql should become what you call psql_check; but we should probably call that psql_ok() instead, and also have a psql_fails() for src/test/recovery/t/001_stream_rep.pl (and others to come). > +=item $node->backup_fs_hot(backup_name) > + > +Create a backup with a filesystem level copy in $node->backup_dir, > +including transaction logs. Archiving must be enabled as pg_start_backup > +and pg_stop_backup are used. This is not checked or enforced. Perhaps "WAL archiving or streaming must be enabled ..." I would rather have the patches be submitted with a reasonable approximation at indentation rather than submit a separate indent patch. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_dump / copy bugs with "big lines" ?
"Daniel Verite"writes: > I've tried adding another large field to see what happens if the whole row > exceeds 2GB, and data goes to the client rather than to a file. > My idea was to check if the client side was OK with that much data on > a single COPY row, but it turns out that the server is not OK anyway. BTW, is anyone checking the other side of this, ie "COPY IN" with equally wide rows? There doesn't seem to be a lot of value in supporting dump if you can't reload ... 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] [NOVICE] WHERE clause not used when index is used
Sorry to keep coming back to this, but I just realized that the next para in _bt_preprocess_keys' doco explains yet another way in which this patch is broken: * Note that one reason we need direction-sensitive required-key flags is * precisely that we may not be able to eliminate redundant keys. Suppose * we have "x > 4::int AND x > 10::bigint", and we are unable to determine * which key is more restrictive for lack of a suitable cross-type operator. * _bt_first will arbitrarily pick one of the keys to do the initial * positioning with. If it picks x > 4, then the x > 10 condition will fail * until we reach index entries > 10; but we can't stop the scan just because * x > 10 is failing. On the other hand, if we are scanning backwards, then * failure of either key is indeed enough to stop the scan. (In general, when * inequality keys are present, the initial-positioning code only promises to * position before the first possible match, not exactly at the first match, * for a forward scan; or after the last match for a backward scan.) This means that the patch's basic assumption, that _bt_first() leaves us positioned on a row that satisfies all the keys, is sometimes wrong. It may not be possible to demonstrate this bug using any standard opclasses, because we don't ship any incomplete cross-type operator families in core Postgres, but it's still wrong. Offhand the most practical solution seems to be to not mark keys with MATCH or NORECHECK or whatever you call it unless there is just a single inequality key for the column after preprocessing. That might add a bit more complication in bt_preprocess_keys, I fear, because it would be that much less like the REQFWD/REQBKWD case; but it would be a localized fix. I doubt we want to get into making _bt_first's API contract tighter. 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_dump / copy bugs with "big lines" ?
I wrote: > If splitting the table into 3 fields, each smaller than 512MB: > > postgres=# create table big2 as select > substring(binarycol from 1 for 300*1024*1024) as b1, > substring(binarycol from 1+300*1024*1024 for 300*1024*1024) as b2 , > substring(binarycol from 1+600*1024*1024 for 400*1024*1024) as b3 > from big; I've tried adding another large field to see what happens if the whole row exceeds 2GB, and data goes to the client rather than to a file. postgres=# alter table big2 add b4 bytea; postgres=# update big2 set b4=b1; So big2 has 4 bytea columns with 300+300+400+300 MB on a single row. Then I'm trying to \copy this from a 9.5.1 backend with patch applied, configured with --enable-debug, on Debian8 x86-64 with 64GB of RAM and all defaults in postgresql.conf My idea was to check if the client side was OK with that much data on a single COPY row, but it turns out that the server is not OK anyway. postgres=# \copy big2 to /dev/null lost synchronization with server: got message type "d", length -1568669676 The backend aborts with the following backtrace: Program terminated with signal 6, Aborted. #0 0x7f82ee68e165 in raise () from /lib/x86_64-linux-gnu/libc.so.6 #1 0x7f82ee6913e0 in abort () from /lib/x86_64-linux-gnu/libc.so.6 #2 0x7f82ee6c839b in ?? () from /lib/x86_64-linux-gnu/libc.so.6 #3 0x7f82ee6d1be6 in ?? () from /lib/x86_64-linux-gnu/libc.so.6 #4 0x7f82ee6d698c in free () from /lib/x86_64-linux-gnu/libc.so.6 #5 0x007b5a89 in AllocSetDelete (context=0x) at aset.c:643 #6 0x007b63e3 in MemoryContextDelete (context=0x1fa58c8) at mcxt.c:229 #7 0x0055fa25 in CopyTo (cstate=0x1fb1050) at copy.c:1967 #8 DoCopyTo (cstate=cstate@entry=0x1fb1050) at copy.c:1778 #9 0x00562ea9 in DoCopy (stmt=stmt@entry=0x1f796d0, queryString=queryString@entry=0x1f78c60 "COPY big2 TO STDOUT ", processed=processed@entry=0x7fff22103338) at copy.c:961 #10 0x006b4440 in standard_ProcessUtility (parsetree=0x1f796d0, queryString=0x1f78c60 "COPY big2 TO STDOUT ", context=, params=0x0, dest=0x1f79a30, completionTag=0x7fff22103680 "") at utility.c:541 #11 0x006b1937 in PortalRunUtility (portal=0x1f26680, utilityStmt=0x1f796d0, isTopLevel=1 '\001', dest=0x1f79a30, completionTag=0x7fff22103680 "") at pquery.c:1183 #12 0x006b2535 in PortalRunMulti (portal=portal@entry=0x1f26680, isTopLevel=isTopLevel@entry=1 '\001', dest=dest@entry=0x1f79a30, altdest=altdest@entry=0x1f79a30, completionTag=completionTag@entry=0x7fff22103680 "") at pquery.c:1314 #13 0x006b3022 in PortalRun (portal=portal@entry=0x1f26680, count=count@entry=9223372036854775807, isTopLevel=isTopLevel@entry=1 '\001', dest=dest@entry=0x1f79a30, altdest=altdest@entry=0x1f79a30, completionTag=completionTag@entry=0x7fff22103680 "") at pquery.c:812 #14 0x006b0393 in exec_simple_query ( query_string=0x1f78c60 "COPY big2 TO STDOUT ") at postgres.c:1104 #15 PostgresMain (argc=, argv=argv@entry=0x1f0d240, dbname=0x1f0d0f0 "postgres", username=) at postgres.c:4030 #16 0x0047072b in BackendRun (port=0x1f2d230) at postmaster.c:4239 #17 BackendStartup (port=0x1f2d230) at postmaster.c:3913 #18 ServerLoop () at postmaster.c:1684 #19 0x0065b8cd in PostmasterMain (argc=argc@entry=3, argv=argv@entry=0x1f0c330) at postmaster.c:1292 #20 0x00471161 in main (argc=3, argv=0x1f0c330) at main.c:223 The server log has this: STATEMENT: COPY big2 TO STDOUT *** glibc detected *** postgres: postgres postgres [local] COPY: double free or corruption (out): 0x7f8234929010 *** === Backtrace: = /lib/x86_64-linux-gnu/libc.so.6(+0x75be6)[0x7f82ee6d1be6] /lib/x86_64-linux-gnu/libc.so.6(cfree+0x6c)[0x7f82ee6d698c] postgres: postgres postgres [local] COPY[0x7b5a89] postgres: postgres postgres [local] COPY(MemoryContextDelete+0x43)[0x7b63e3] postgres: postgres postgres [local] COPY[0x55fa25] postgres: postgres postgres [local] COPY(DoCopy+0x479)[0x562ea9] postgres: postgres postgres [local] COPY(standard_ProcessUtility+0x590)[0x6b4440] postgres: postgres postgres [local] COPY[0x6b1937] postgres: postgres postgres [local] COPY[0x6b2535] postgres: postgres postgres [local] COPY(PortalRun+0x202)[0x6b3022] postgres: postgres postgres [local] COPY(PostgresMain+0x1493)[0x6b0393] postgres: postgres postgres [local] COPY[0x47072b] postgres: postgres postgres [local] COPY(PostmasterMain+0xe7d)[0x65b8cd] postgres: postgres postgres [local] COPY(main+0x3e1)[0x471161] /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xfd)[0x7f82ee67aead] postgres: postgres postgres [local] COPY[0x4711c9] I will try other tests without bytea exported in text format but with several huge text columns. Best regards, -- Daniel Vérité PostgreSQL-powered mailer: http://www.manitou-mail.org Twitter: @DanielVerite -- Sent via pgsql-hackers mailing list
Re: [HACKERS] Publish autovacuum informations
On 3/1/16 3:02 PM, Julien Rouhaud wrote: You mean for database wide vacuum? I mean manual vacuum. Some hooks and stats would apply only to autovac obviously (and it'd be nice to get visibility into the scheduling decisions both daemons are making). But as much as possible things should be done in vacuum.c/lazyvacuum.c so it works for manual vacuums as well. -- Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX Experts in Analytics, Data Architecture and PostgreSQL Data in Trouble? Get it in Treble! http://BlueTreble.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] Improve error handling in pltcl
On 2/29/16 10:01 PM, Tom Lane wrote: Jim Nasbywrites: On 2/28/16 5:50 PM, Jim Nasby wrote: Per discussion in [1], this patch improves error reporting in pltcl. I forgot to mention that this work is sponsored by Flight Aware (http://flightaware.com). Huh ... I use that site. There's PG and pltcl code behind it? Cool! Heh, I didn't realize you were a TCL fan. They've been heavy PG users from the start. Eventually PG had trouble keeping up with the in-flight tracking so they created Speed Tables [1]. And Karl (one of the founders) is a well known TCL contributor[2]. When it comes to sheer geek factor though, I think the multilateration[3] stuff they're doing with their ADS-B network is a really cool data application. It's basically a form of "reverse GPS" for tracking aircraft. [1] https://github.com/flightaware/speedtables [2] http://wiki.tcl.tk/83 [3] http://flightaware.com/adsb/mlat/ -- Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX Experts in Analytics, Data Architecture and PostgreSQL Data in Trouble? Get it in Treble! http://BlueTreble.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] [NOVICE] WHERE clause not used when index is used
I wrote: > I believe the way to fix this would be to stop regarding SK_BT_MATCHED > as state, and instead treat it as a scankey property identified during > _bt_preprocess_keys, analogously to SK_BT_REQFWD/SK_BT_REQBKWD --- and, > like those, you'd need two flags not one since the properties will be > determined independently of knowing which direction you'll be going in. BTW, the analogy to SK_BT_REQFWD/SK_BT_REQBKWD exposes another way in which the patch leaves money on the table: if the leading key is "=" then MATCHED behavior can't apply to it, but it might apply to a later key. I'm imagining a specification like this (in the comments for _bt_preprocess_keys, after the para starting "The output keys are marked with flags SK_BT_REQFWD and/or SK_BT_REQBKWD ..."): * Another property of the first attribute without an "=" key is that it may * not be necessary to recheck its value at each index entry as we scan * through the index. Again considering "x = 1 AND y < 4 AND z < 5", once we * have positioned to an entry satisfying those keys, it is unnecessary to * recheck "y < 4" as we scan forward, at least so long as the index's y * value is not NULL. Every later row with x=1 must have y>=4; though we * can't make any similar statement about z. Similarly, a key like "y > 4" * need not be rechecked in a backwards scan. We mark appropriate keys with * flags SK_BT_NORECHECK_FWD or SK_BT_NORECHECK_BKWD to indicate that _bt_next * can skip checking those keys (at non-null index entries) when scanning in * the indicated direction. I'm also wondering whether it'd be worth taking more care about the handling of index entries containing some null columns. Right now, the presence of any nulls disables the MATCH improvement, but it would still apply if the null(s) are in lower-order columns. I'm not sure if that case comes up often enough to justify checking the flag bit twice per iteration, but it might. 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] 2016-03 Commitfest Manager
On 3/1/16 3:04 PM, Tom Lane wrote: > David Steelewrites: >> I volunteered a while back to be the CFM and I haven't seen any other >> volunteers or objections to my offer. > >> I am still ready, eager, and willing! > > I haven't heard any other volunteers either. You have the conn, sir. Aye aye! Once the 2016-03 CF has been closed I'll send out a kickoff report. -- -David da...@pgmasters.net signature.asc Description: OpenPGP digital signature
Re: [HACKERS] psql completion for ids in multibyte string
On Wed, Mar 2, 2016 at 7:54 AM, Robert Haaswrote: > On Mon, Feb 29, 2016 at 6:32 PM, Thomas Munro > wrote: >> On Fri, Feb 26, 2016 at 6:34 PM, Kyotaro HORIGUCHI >> wrote: >>> Hello, this is the second patch plitted out. This allows >>> multibyte names to be completed in psql. >>> >>> At Fri, 06 Nov 2015 11:47:17 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI >>> wrote in >>> <20151106.114717.170526268.horiguchi.kyot...@lab.ntt.co.jp> At Thu, 5 Nov 2015 18:32:59 +0900, Amit Langote wrote in <563b224b.3020...@lab.ntt.co.jp> > On 2015/11/05 18:10, Kyotaro HORIGUCHI wrote: > > Hello. I don't know whether this is a bug fix or improvement, > > Would it be 50-50? :-) Yeah, honestly saying, I doubt that this is valuable but feel uneasy to see some of the candidates vanish as completon proceeds for no persuasive reason. >> >> +1 from me, it's entirely reasonable to want to name database objects >> in any human language and use auto-completion. It's not working today >> as you showed. >> >>> The current version of tab-completion failed to complete names >>> with multibyte characters. >>> >>> =# create table いろは (あ int); >>> =# create table いこい (あ int); >>> =# drop table >>> "いろは" hint_plan. pg_toast. >>> "いこい" information_schema. pg_toast_temp_1. >>> ALL IN TABLESPACEpg_catalog. public. >>> dbms_stats. pg_temp_1. >>> postgres=# alter table "い >>> =# drop table "い >>> =# drop table "い /* No candidate offered */ >>> >>> This is because _complet_from_query counts the string length in >>> bytes, instead of characters. With this patch the candidates will >>> appear. >>> >>> =# drop table "い >>> "いこい" "いろは" >>> postgres=# drpo table "い >> >> The patch looks correct to me: it counts characters rather than bytes, >> which is the right thing to do because the value is passed to substr() >> which also works in characters rather than bytes. I tested with >> "éclair", and without the patch, tab completion doesn't work if you >> press tab after 'é'. With the patch it does. > > OK, but I am a little concerned about this code: > > /* Find something that matches */ > if (result && PQresultStatus(result) == PGRES_TUPLES_OK) > { > const char *item; > > while (list_index < PQntuples(result) && >(item = PQgetvalue(result, list_index++, 0))) > if (pg_strncasecmp(text, item, string_length) == 0) > return pg_strdup(item); > } > > Every other use of string_length in this function is using it as the > argument to the SQL substring function, which cares about characters, > not bytes. But this use seems to care about bytes, not characters. > > Am I wrong? Ugh, and the other problem is that string_length is always 0 there if state isn't 0 (in master it is static so that the value is reused for subsequent calls, but this patch made it automatic). I think we should leave string_length as it is and use a new variable for character-based length, as in the attached. -- Thomas Munro http://www.enterprisedb.com multibyte-tab-complete.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Publish autovacuum informations
On 01/03/2016 20:29, Jim Nasby wrote: > On 3/1/16 8:37 AM, Julien Rouhaud wrote: >>> > >>> >We understood (IMHO is an interesting idea) but as Michael said >>> hooks is >>> >for a general purpose. So can you demonstrate other use cases for this >>> >new hooks? >>> > >> I can think of several usage. First, since the hook will always be >> called, an extension will see all the activity a worker is doing when >> exposing private structure will always be some kind of sampling. Then, > > I think that's pretty key. If you wanted to create an extension that > logs vacuums (which would be great, since current state of the art is > logs + pgBadger), you'd want to gather your data about what the vacuum > did as the vacuum was ending. > Indeed these information are missing. I guess that'd be possible by adding (or moving) a hook in lazy_vacuum_rel() that provide access to part or all of the LVRelStats and rusage informations. > I can certainly see cases where you don't care about that and just want > what's in shared memory, but that would only be useful for monitoring > what's happening real-time, not for knowing what final results are. > > BTW, I think as much of this as possible should also work for regular > vacuums. > You mean for database wide vacuum? >> you can have other information that wouldn't be available just by >> exposing private structure. For instance knowing a VACUUM isn't >> performed by the worker (either because another worker is already >> working on it or because it isn't needed anymore). IIRC there was a >> discussion about concurrency issue in this case. We can also know if the >> maintenance was cancelled due to lock not obtained fast enough. >> Finally, as long as the hooks aren't use, they don't have any overhead. >> I agree that all this is for monitoring purpose. >> >> I'm not sure what are the fancy things that Michael had in mind with >> exposing the private structure. Michael, was it something like having >> the ability to change some of these data through an extension? -- Julien Rouhaud http://dalibo.com - http://dalibo.org -- 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] Addition of extra commit fest entry to park future patches
On Tue, Mar 1, 2016 at 3:40 PM, Tom Lanewrote: > Magnus Hagander writes: > > On Tue, Mar 1, 2016 at 10:12 AM, Alvaro Herrera < > alvhe...@2ndquadrant.com> > > wrote: > >> Magnus Hagander wrote: > >>> Yeah, we can do that. I'd suggest we either name it based on the > current > >>> tentative date for CF1 (september), or name it specificaly "9.7-first" > or > >>> something like that rather than just plain "future", to make it more > >>> clear. > > >> +1 to both names suggested by Magnus. > > > We do need to pick one of them :) > > Anybody else with preferences? > > 2016-09 would be in keeping with all previous CF names. 9.7-first sounds > like it'd be more future-proof in case we change the schedule, but I'm not > sure about that either ... what if we decide over the summer that parallel > query is so cool that we should rename 9.6 to 10.0? > > On balance I'd go with 2016-09, but I'm not going to argue very hard. > > BTW, is there an ability to rename a CF once it's in the app? Seems like > that would reduce the stakes here. > > Yes, it's trivial to rename. That's the only advantage of our ugly url scheme which uses the surrogate key in the url instead of the actual name of the CF :) -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/
Re: [HACKERS] Addition of extra commit fest entry to park future patches
Magnus Haganderwrites: > On Tue, Mar 1, 2016 at 10:12 AM, Alvaro Herrera > wrote: >> Magnus Hagander wrote: >>> Yeah, we can do that. I'd suggest we either name it based on the current >>> tentative date for CF1 (september), or name it specificaly "9.7-first" or >>> something like that rather than just plain "future", to make it more >>> clear. >> +1 to both names suggested by Magnus. > We do need to pick one of them :) > Anybody else with preferences? 2016-09 would be in keeping with all previous CF names. 9.7-first sounds like it'd be more future-proof in case we change the schedule, but I'm not sure about that either ... what if we decide over the summer that parallel query is so cool that we should rename 9.6 to 10.0? On balance I'd go with 2016-09, but I'm not going to argue very hard. BTW, is there an ability to rename a CF once it's in the app? Seems like that would reduce the stakes here. 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] Addition of extra commit fest entry to park future patches
On 3/1/16 3:35 PM, Kevin Grittner wrote: > On Tue, Mar 1, 2016 at 2:28 PM, Magnus Haganderwrote: >> On Tue, Mar 1, 2016 at 10:12 AM, Alvaro Herrera >> wrote: >>> Magnus Hagander wrote: > I'd suggest we either name it based on the current tentative date for CF1 (september), or name it specificaly "9.7-first" or something like that rather than just plain "future", to make it more clear. >>> >>> +1 to both names suggested by Magnus. >> >> We do need to pick one of them :) >> >> Anybody else with preferences? > > I would prefer to see a consistent namimg pattern (i.e., 2016-09) > and rename it if we reschedule. I'm fine with that - it does help set expectations. -- -David da...@pgmasters.net signature.asc Description: OpenPGP digital signature
Re: [HACKERS] Addition of extra commit fest entry to park future patches
On 3/1/16 3:28 PM, Magnus Hagander wrote: > On Tue, Mar 1, 2016 at 10:12 AM, Alvaro Herrera > Magnus Hagander wrote: > > > Yeah, we can do that. I'd suggest we either name it based on the current > > tentative date for CF1 (september), or name it specificaly "9.7-first" > or > > something like that rather than just plain "future", to make it more > clear. > > +1 to both names suggested by Magnus. > > > > We do need to pick one of them :) I'm good with 9.7-first. I presume it can be renamed later to fit the standard scheme? -- -David da...@pgmasters.net signature.asc Description: OpenPGP digital signature
Re: [HACKERS] Addition of extra commit fest entry to park future patches
On Tue, Mar 1, 2016 at 2:28 PM, Magnus Haganderwrote: > On Tue, Mar 1, 2016 at 10:12 AM, Alvaro Herrera > wrote: >> Magnus Hagander wrote: >>> I'd suggest we either name it based on the current tentative >>> date for CF1 (september), or name it specificaly "9.7-first" or >>> something like that rather than just plain "future", to make it >>> more clear. >> >> +1 to both names suggested by Magnus. > > We do need to pick one of them :) > > Anybody else with preferences? I would prefer to see a consistent namimg pattern (i.e., 2016-09) and rename it if we reschedule. -- Kevin Grittner EDB: 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] Addition of extra commit fest entry to park future patches
On Tue, Mar 1, 2016 at 10:12 AM, Alvaro Herrerawrote: > Magnus Hagander wrote: > > On Tue, Mar 1, 2016 at 8:09 AM, Michael Paquier < > michael.paqu...@gmail.com> > > wrote: > > > > > Hi all, > > > > > > I guess that commit fest 2016-03 is going to begin soon, at which > > > point nobody will be able to add new patches because > > > 1) already closed CF don't accept them. > > > 2) A CF currently running neither. > > > I propose to create an extra CF, called "Future" or similar where > > > people will be able to park the patches submitted for the 9.7 cycle. > > > This will be renamed later on as the first CF of 9.7 once the > > > development schedule for 9.7 is decided. > > > > Yeah, we can do that. I'd suggest we either name it based on the current > > tentative date for CF1 (september), or name it specificaly "9.7-first" or > > something like that rather than just plain "future", to make it more > clear. > > +1 to both names suggested by Magnus. > We do need to pick one of them :) Anybody else with preferences? -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/
Re: [HACKERS] 2016-03 Commitfest Manager
David Steelewrites: > I volunteered a while back to be the CFM and I haven't seen any other > volunteers or objections to my offer. > I am still ready, eager, and willing! I haven't heard any other volunteers either. You have the conn, sir. 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] [NOVICE] WHERE clause not used when index is used
I wrote: > Petr Jelinekwrites: >> I can only get the issue when the sort order of the individual keys does >> not correlate and the operator sorts according to the first column and >> there are duplicate values for the first column. > Yeah, I think the combination of ASC and DESC columns may be necessary to > break things. It needs closer analysis. Okay, I've now studied the patch more carefully and understood that it wasn't quite so naive as to not consider multi-column cases at all. It does work for simple scalar multi-column cases, because the test on SK_BT_MATCHED is placed so that it only skips checking the first column's condition, not the conditions on lower-order columns. Rather, the problem is failure to consider ROW() comparisons, which do not necessarily have the same simple behavior as scalar comparisons. They sort of accidentally work, *if* the lower-order columns in the ROW() comparison match the index's lower-order columns as to position and index direction. Tobias' example fails because the second column in the ROW() isn't sorted the same, and you could also make it fail with a 3-or-more-column index in which the ROW() comparison tested, say, just the first and third columns. What I think we should do is move the SK_BT_MATCHED flag down to the first sub-key of the ROW() comparison, which is the one that does behave the same as in scalar cases. The fact that it doesn't fail for the case where the lower-order columns of the ROW() do match the index shows that this still leaves something on the table: within a ROW() comparison, you could set SK_BT_MATCHED on more than just the first sub-key, *if* the subsequent columns match the index. This makes me think that the analysis should be getting done in or near _bt_mark_scankey_required, not in the rather random place it is now, because _bt_mark_scankey_required already understands about sub-keys matching the index or not. However ... there is an even larger problem, which is that the patch thinks it can use skey->sk_flags for what's effectively transient state storage --- not merely transient, but scan-direction-dependent. This means that a cursor that reads forwards for awhile and then turns around and reads backwards will get the wrong answer, even without anything as complicated as multiple key columns. It's a bit harder to exhibit such a bug than you might guess, because of btree's habit of prefetching an index page at a time; you have to make the scan cross an index page boundary before you turn around and back it up. Bearing that in mind, I was soon able to devise a failure case in the regression database: regression=# set enable_seqscan TO 0; SET regression=# set enable_bitmapscan TO 0; SET regression=# begin; BEGIN regression=# declare c cursor for select unique1,unique2 from tenk1 where unique1 > 9500; DECLARE CURSOR regression=# fetch 20 from c; unique1 | unique2 -+- 9501 |5916 9502 |1812 9503 |1144 9504 |9202 9505 |4300 9506 |5551 9507 | 847 9508 |4093 9509 |9418 9510 |1862 9511 | 848 9512 |4823 9513 |1125 9514 |9276 9515 |1160 9516 |5177 9517 |3600 9518 |9677 9519 |5518 9520 |1429 (20 rows) regression=# fetch backward 30 from c; unique1 | unique2 -+- 9519 |5518 9518 |9677 9517 |3600 9516 |5177 9515 |1160 9514 |9276 9513 |1125 9512 |4823 9511 | 848 9510 |1862 9509 |9418 9508 |4093 9507 | 847 9506 |5551 9505 |4300 9504 |9202 9503 |1144 9502 |1812 9501 |5916 9500 |3676 9499 | 927 9498 |2807 9497 |6558 9496 |1857 9495 |1974 9494 | 560 9493 |3531 9492 |9875 9491 |7237 9490 |8928 (30 rows) Ooops. I believe the way to fix this would be to stop regarding SK_BT_MATCHED as state, and instead treat it as a scankey property identified during _bt_preprocess_keys, analogously to SK_BT_REQFWD/SK_BT_REQBKWD --- and, like those, you'd need two flags not one since the properties will be determined independently of knowing which direction you'll be going in. In any event, I am now of the opinion that this patch needs to be reverted outright and returned to the authors for redesign. There are too many things wrong with it and too little reason to think that we have to have it in 9.5. 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] 2016-03 Commitfest Manager
I volunteered a while back to be the CFM and I haven't seen any other volunteers or objections to my offer. I am still ready, eager, and willing! -- -David da...@pgmasters.net signature.asc Description: OpenPGP digital signature
Re: [HACKERS] PROPOSAL: Fast temporary tables
As far as I know we are trying to kill two birds with one stone: 1. Reduce overhead of accessing temporary tables 2. Make it possible to create temporary tables on replica. Replicas with hot-standby are widely used for running read-only OLAP queries. But such queries usually stores intermediate results in temporary tables. Unfortunately creating temporary table at read-only replica is impossible now. So some customers do the following tricks: them create pool of file FDWs at master and then use them at replicas. But IMHO it is ugly and inefficient hack. Ideally we should be able to create temporary tables at replica, not affecting system catalog. But there are a lot of problems: where it should be stores, how to assign XIDs to the ruples inserted in temporary table,... Unfortunately, looks like there is no simple solution of the problem. The 100% solution is multimaster (which we are currently developing), but it is completely different story... On 03/01/2016 10:17 PM, Jim Nasby wrote: On 3/1/16 10:05 AM, Atri Sharma wrote: Fair point, that means inventing a whole new OID generation structure.. Generation is just the tip of the iceberg. You still need the equivalent to foreign keys (ie: pg_depend). While you would never have a permanent object depend on a temp object, the reverse certainly needs to be supported. If I were attempting to solve this at a SQL level, I'd be thinking about using table inheritance such that the permanent objects are stored in a permanent parent. New backends would create UNLOGGED children off of that parent. There would be a pid column that was always NULL in the parent, but populated in children. That means children could use their own local form of an OID. When a backend terminates you'd just truncate all it's tables. Actually translating that into relcache and everything else would be a serious amount of work. -- Konstantin Knizhnik Postgres Professional: http://www.postgrespro.com The Russian Postgres 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] Publish autovacuum informations
On 3/1/16 8:37 AM, Julien Rouhaud wrote: > >We understood (IMHO is an interesting idea) but as Michael said hooks is >for a general purpose. So can you demonstrate other use cases for this >new hooks? > I can think of several usage. First, since the hook will always be called, an extension will see all the activity a worker is doing when exposing private structure will always be some kind of sampling. Then, I think that's pretty key. If you wanted to create an extension that logs vacuums (which would be great, since current state of the art is logs + pgBadger), you'd want to gather your data about what the vacuum did as the vacuum was ending. I can certainly see cases where you don't care about that and just want what's in shared memory, but that would only be useful for monitoring what's happening real-time, not for knowing what final results are. BTW, I think as much of this as possible should also work for regular vacuums. you can have other information that wouldn't be available just by exposing private structure. For instance knowing a VACUUM isn't performed by the worker (either because another worker is already working on it or because it isn't needed anymore). IIRC there was a discussion about concurrency issue in this case. We can also know if the maintenance was cancelled due to lock not obtained fast enough. Finally, as long as the hooks aren't use, they don't have any overhead. I agree that all this is for monitoring purpose. I'm not sure what are the fancy things that Michael had in mind with exposing the private structure. Michael, was it something like having the ability to change some of these data through an extension? -- Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX Experts in Analytics, Data Architecture and PostgreSQL Data in Trouble? Get it in Treble! http://BlueTreble.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] The plan for FDW-based sharding
On 03/01/2016 09:19 PM, Petr Jelinek wrote: Since this thread heavily discusses the XTM, I have question about the XTM as proposed because one thing is very unclear to me - what happens when user changes the XTM plugin on the server? I didn't see any xid handover API which makes me wonder if changes of a plugin (or for example failure to load previously used plugin due to admin error) will send server to similar situation as xid wraparound. Transaction manager is very "intimate" part of DBMS and certainly bugs and problems in custom TM implementation can break the server. So if you are providing custom TM implementation, you should take full responsibility on system integrity. XTM API itself doesn't enforce any XID handling policy. As far as we do not want to change tuple header format, XID is still 32-bit integer. In case of pg_dtm, global transactions at all nodes are assigned the same XID by arbiter. Arbiter is handling XID wraparound. In pg_tsdtm each node maintains its own XIDs, actually pg_tsdtm doesn't change way of assigning CIDs by Postgres. So wraparound in this case is handled in standard way. Instead of assigning own global XIDs, pg_tsdtm provides mapping between local XIDs and global CSNs. Visibility checking rules looks on CSNs, not on XIDs. In both cases if system is for some reasons restarted and DTM plugin failed to be loaded, you can still access database locally. No data can be lost. -- Konstantin Knizhnik Postgres Professional: http://www.postgrespro.com The Russian Postgres 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] PROPOSAL: Fast temporary tables
On 3/1/16 10:05 AM, Atri Sharma wrote: Fair point, that means inventing a whole new OID generation structure.. Generation is just the tip of the iceberg. You still need the equivalent to foreign keys (ie: pg_depend). While you would never have a permanent object depend on a temp object, the reverse certainly needs to be supported. If I were attempting to solve this at a SQL level, I'd be thinking about using table inheritance such that the permanent objects are stored in a permanent parent. New backends would create UNLOGGED children off of that parent. There would be a pid column that was always NULL in the parent, but populated in children. That means children could use their own local form of an OID. When a backend terminates you'd just truncate all it's tables. Actually translating that into relcache and everything else would be a serious amount of work. -- Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX Experts in Analytics, Data Architecture and PostgreSQL Data in Trouble? Get it in Treble! http://BlueTreble.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: Commitfest Bug (was: [HACKERS] Re: Reusing abbreviated keys during second pass of ordered [set] aggregates)
On Tue, Mar 1, 2016 at 7:27 AM, Tom Lanewrote: > +1 for not moving such patches to the new CF until the author does > something --- at which point they'd change to "Needs Review" state. > But we should not change them into that state without author input. > And I don't see the value of having them in a new CF until the > author does something. To be clear: My position was always that it's good that the author has to do *something* to get their patch into the next CF. It's bad that this change in state can easily be missed, though. I've now been on both sides of this, as a patch author and patch reviewer. If the patch was left as "Waiting on Author", as my review of Alexander's patch was, then it ought to not change to "Needs Review" silently. That makes absolutely no sense. -- 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
Re: [HACKERS] The plan for FDW-based sharding
On Tue, Mar 1, 2016 at 02:02:44PM -0500, Bruce wrote: > On Tue, Mar 1, 2016 at 07:56:58PM +0100, Petr Jelinek wrote: > > Note that I am not saying that other discussed approaches are any > > better, I am saying that we should know approximately what we > > actually want and not just beat FDWs with a hammer and hope sharding > > will eventually emerge and call that the plan. > > I will say it again --- FDWs are the only sharding method I can think of > that has a chance of being accepted into Postgres core. It is a plan, > and if it fails, it fails. If is succeeds, that's good. What more do > you want me to say? I know of no other way to answer the questions you > asked above. I guess all I can say is that if FDWs existed when Postgres XC/XL were being developed, that they likely would have been used or at least considered. I think we are basically making that attempt now. -- Bruce Momjianhttp://momjian.us EnterpriseDB http://enterprisedb.com + As you are, so once was I. As I am, so you will be. + + Roman grave inscription + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS][REVIEW]: Password identifiers, protocol aging and SCRAM protocol
On 1 March 2016 at 06:34, Michael Paquierwrote: > On Mon, Feb 29, 2016 at 8:43 PM, Valery Popov > wrote: > > vpopov@vpopov-Ubuntu:~/Projects/pwdtest/postgresql$ git branch > > Thanks for the input! > > > 0001-Add-facility-to-store-multiple-password-verifiers.patch:2547: > trailing > > whitespace. > > warning: 1 line adds whitespace errors. > > 0003-Add-pg_auth_verifiers_sanitize.patch:87: indent with spaces. > > if (!superuser()) > > warning: 1 line adds whitespace errors. > > Argh, yes. Those two ones have slipped though my successive rebases I > think. Will fix in my tree, I don't think that it is worth sending > again the whole series just for that though. > -- > Michael > > > -- > Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-hackers > > Hi, Michael Few questions about the documentation. config.sgml:1200 > > >Specifies a comma-separated list of supported password formats by >the server. Supported formats are currently plain and >md5. > > > >When a password is specified in or >, this parameter determines if the >password specified is authorized to be stored or not, returning >an error message to caller if it is not. > > > >The default is plain,md5,scram, meaning that MD5-encrypted >passwords, plain passwords, and SCRAM-encrypted passwords are accepted. > > The default value contains "scram". Shouldn't be here also: >Specifies a comma-separated list of supported password formats by >the server. Supported formats are currently plain, >md5 and scram. Or I missed something? And one more: config.sgml:1284 > >db_user_namespace causes the client's and >server's user name representation to differ. >Authentication checks are always done with the server's user name >so authentication methods must be configured for the >server's user name, not the client's. Because >md5 uses the user name as salt on both the >client and server, md5 cannot be used with >db_user_namespace. > Looks like the same (pls, correct me if I'm wrong) is applicable for "scram" as I see from the code below. Shouldn't be "scram" mentioned here also? Here's the code: > diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c > index 28f9fb5..df0cc1d 100644 > --- a/src/backend/libpq/hba.c > +++ b/src/backend/libpq/hba.c > @@ -1184,6 +1184,19 @@ parse_hba_line(List *line, int line_num, char *raw_line) > } > parsedline->auth_method = uaMD5; > } >+ else if (strcmp(token->string, "scram") == 0) >+ { >+ if (Db_user_namespace) >+ { >+ ereport(LOG, >+ (errcode(ERRCODE_CONFIG_FILE_ERROR), >+ errmsg("SCRAM authentication is not supported when \"db_user_namespace\" is enabled"), >+ errcontext("line %d of configuration file \"%s\"", >+ line_num, HbaFileName))); >+ return NULL; >+ } >+ parsedline->auth_method = uaSASL; >+ } > else if (strcmp(token->string, "pam") == 0) > #ifdef USE_PAM > parsedline->auth_method = uaPAM;
Re: [HACKERS] The plan for FDW-based sharding
On Tue, Mar 1, 2016 at 07:56:58PM +0100, Petr Jelinek wrote: > Note that I am not saying that other discussed approaches are any > better, I am saying that we should know approximately what we > actually want and not just beat FDWs with a hammer and hope sharding > will eventually emerge and call that the plan. I will say it again --- FDWs are the only sharding method I can think of that has a chance of being accepted into Postgres core. It is a plan, and if it fails, it fails. If is succeeds, that's good. What more do you want me to say? I know of no other way to answer the questions you asked above. -- Bruce Momjianhttp://momjian.us EnterpriseDB http://enterprisedb.com + As you are, so once was I. As I am, so you will be. + + Roman grave inscription + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Reduce lock levels others reloptions in ALTER TABLE
On Mon, Feb 29, 2016 at 12:58 PM, Fabrízio de Royes Mellowrote: > Some time ago we added [1] the infrastructure to allow different lock levels > for relation options. > > So per discussion [2] the attached patch reduce lock levels down to > ShareUpdateExclusiveLock for: > - fastupdate > - fillfactor > - gin_pending_list_limit > - seq_page_cost > - random_page_cost > - n_distinct > - n_distinct_inherited > - buffering I am of the opinion that there needs to be comments or a README or some kind of documentation somewhere indicating the rationale for the lock levels we choose here. Just changing it without analyzing why a certain level is sufficient for safety and no higher than necessary seems poor. And the reasoning should be documented for future readers. -- 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] The plan for FDW-based sharding
On 27/02/16 04:54, Robert Haas wrote: On Fri, Feb 26, 2016 at 10:56 PM, Konstantin Knizhnikwrote: We do not have formal prove that proposed XTM is "general enough" to handle all possible transaction manager implementations. But there are two general ways of dealing with isolation: snapshot based and CSN based. I don't believe that for a minute. For example, consider this article: https://en.wikipedia.org/wiki/Global_serializability I think the neutrality of that article is *very* debatable, but it certainly contradicts the idea that snapshots and CSNs are the only methods of achieving global serializability. Or consider this lecture: http://hssl.cs.jhu.edu/~randal/416/lectures.old/ln5.2.pdf That's a great introduction to the problem we're trying to solve here, but again, snapshots are not mentioned, and CSNs certainly aren't mentioned. This write-up goes further, explaining three different methods for ensuring global serializability, none of which mention snapshots or CSNs: http://heaven.eee.metu.edu.tr/~vision/LectureNotes/EE442/Ee442ch7.html Actually, I think the second approach is basically a snapshot/CSN-type approach, but it doesn't use that terminology and the connection to what you are proposing is very unclear. I think you're approaching this problem from a viewpoint that is entirely too focused on the code that exists in PostgreSQL today. Lots of people have done lots of academic research on how to solve this problem, and you can't possibly say that CSNs and snapshots are the only solution to this problem unless you haven't read any of those papers. The articles above aren't exceptional in mentioning neither of the approaches that you are advocating - they are typical of the literature in this area. How can it be that the only solutions to this problem are ones that are totally different from the approaches that university professors who spend time doing research on concurrency have spent time exploring? I think we need to back up here and examine our underlying design assumptions. The goal here shouldn't necessarily be to replace PostgreSQL's current transaction management with a distributed version of the same thing. We might want to do that, but I think the goal is or should be to provide ACID semantics in a multi-node environment, and specifically the I in ACID: transaction isolation. Making the existing transaction manager into something that can be spread across multiple nodes is one way of accomplishing that. Maybe the best one. Certainly one that's been experimented within Postgres-XC. But it is often the case that an algorithm that works tolerably well on a single machine starts performing extremely badly in a distributed environment, because the latency of communicating between multiple systems is vastly higher than the latency of communicating between CPUs or cores on the same system. So I don't think we should be assuming that's the way forward. I have similar problem with the FDW approach though. It seems to me like because we have something that solves access to external tables somebody decided that it should be used as base for the whole sharding solution but there is no real concept of how it will look like together, no ideas what it will be usable for and not even simple prototype that would prove that the idea is sound (although again, I am not clear on what the actual idea is beyond "we will use FDWs"). Don't get me wrong, I agree that the current FDW enhancements are useful, I am just worried about them being presented as future of sharding in Postgres when nobody has sketched how the future might look like. And once we get to more interesting parts like consistency, distributed query planning, p2p connections (and I am really concerned about these as FDWs abstract some knowledge that coordinator and or data nodes might need to do these well), etc we might very well find ourselves painted in the corner and have to start from beginning, while if we had some idea on how the whole thing might look like we could identify this early and not postpone built-in sharding by several years just because somebody said we will use FDWs and that's what we worked on in those years. Note that I am not saying that other discussed approaches are any better, I am saying that we should know approximately what we actually want and not just beat FDWs with a hammer and hope sharding will eventually emerge and call that the plan. -- Petr Jelinek http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] psql completion for ids in multibyte string
On Mon, Feb 29, 2016 at 6:32 PM, Thomas Munrowrote: > On Fri, Feb 26, 2016 at 6:34 PM, Kyotaro HORIGUCHI > wrote: >> Hello, this is the second patch plitted out. This allows >> multibyte names to be completed in psql. >> >> At Fri, 06 Nov 2015 11:47:17 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI >> wrote in >> <20151106.114717.170526268.horiguchi.kyot...@lab.ntt.co.jp> >>> At Thu, 5 Nov 2015 18:32:59 +0900, Amit Langote >>> wrote in <563b224b.3020...@lab.ntt.co.jp> >>> > On 2015/11/05 18:10, Kyotaro HORIGUCHI wrote: >>> > > Hello. I don't know whether this is a bug fix or improvement, >>> > >>> > Would it be 50-50? :-) >>> >>> Yeah, honestly saying, I doubt that this is valuable but feel >>> uneasy to see some of the candidates vanish as completon proceeds >>> for no persuasive reason. > > +1 from me, it's entirely reasonable to want to name database objects > in any human language and use auto-completion. It's not working today > as you showed. > >> The current version of tab-completion failed to complete names >> with multibyte characters. >> >> =# create table いろは (あ int); >> =# create table いこい (あ int); >> =# drop table >> "いろは" hint_plan. pg_toast. >> "いこい" information_schema. pg_toast_temp_1. >> ALL IN TABLESPACEpg_catalog. public. >> dbms_stats. pg_temp_1. >> postgres=# alter table "い >> =# drop table "い >> =# drop table "い /* No candidate offered */ >> >> This is because _complet_from_query counts the string length in >> bytes, instead of characters. With this patch the candidates will >> appear. >> >> =# drop table "い >> "いこい" "いろは" >> postgres=# drpo table "い > > The patch looks correct to me: it counts characters rather than bytes, > which is the right thing to do because the value is passed to substr() > which also works in characters rather than bytes. I tested with > "éclair", and without the patch, tab completion doesn't work if you > press tab after 'é'. With the patch it does. OK, but I am a little concerned about this code: /* Find something that matches */ if (result && PQresultStatus(result) == PGRES_TUPLES_OK) { const char *item; while (list_index < PQntuples(result) && (item = PQgetvalue(result, list_index++, 0))) if (pg_strncasecmp(text, item, string_length) == 0) return pg_strdup(item); } Every other use of string_length in this function is using it as the argument to the SQL substring function, which cares about characters, not bytes. But this use seems to care about bytes, not characters. Am I wrong? -- 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] [NOVICE] WHERE clause not used when index is used
I wrote: > I'm not sure if the costing change is a bug or not --- the non-bitmap scan > does seem to be cheaper in reality, but not by a couple orders of > magnitude as the planner now thinks. Ah, scratch that, I wasn't looking closely enough. The 9.4 plan is an IndexScan whereas 9.5+ uses IndexOnlyScan, which accounts for the cost differential. The reason it changed is the remove_unused_subquery_outputs patch (55d5b3c08279b487), which allows the subquery to know that it doesn't actually have to return all columns of tenk1, so that an index-only scan is legal. 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] Fixing wrong comment on PQmblen and PQdsplen.
On Fri, Feb 26, 2016 at 12:33 AM, Kyotaro HORIGUCHIwrote: > I divided the last patch into one typo-fix patch and one > improvement patch. This is the former one. Committed. -- 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] [NOVICE] WHERE clause not used when index is used
Petr Jelinekwrites: > On 01/03/16 18:37, Tom Lane wrote: >> However, I'm not sure that's 100% of the issue, because in playing around >> with this I was having a harder time reproducing the failure outside of >> Tobias' example than I expected. There may be more than one bug, or there >> may be other changes that sometimes mask the problem. > I can only get the issue when the sort order of the individual keys does > not correlate and the operator sorts according to the first column and > there are duplicate values for the first column. Yeah, I think the combination of ASC and DESC columns may be necessary to break things. It needs closer analysis. There is another behavorial difference between 9.4 and 9.5, which is that the planner's costing of scans of this sort seems to have changed. I can reproduce the problem now in the regression database: regression=# select count(*) from (select * from tenk1 where (thousand,tenthous) < (9,5000) order by thousand desc, tenthous asc) ss; count --- 95-- correct answer (1 row) regression=# create index on tenk1(thousand desc,tenthous asc); CREATE INDEX regression=# select count(*) from (select * from tenk1 where (thousand,tenthous) < (9,5000) order by thousand desc, tenthous asc) ss; count --- 100-- WRONG (1 row) What was confusing me is that the plan's changed: HEAD gives Aggregate (cost=7.29..7.29 rows=1 width=0) -> Index Only Scan using tenk1_thousand_tenthous_idx on tenk1 (cost=0.29..6.04 rows=100 width=8) Index Cond: (ROW(thousand, tenthous) < ROW(9, 5000)) whereas 9.4 prefers Aggregate (cost=232.50..232.51 rows=1 width=0) -> Sort (cost=231.00..231.25 rows=100 width=244) Sort Key: tenk1.thousand, tenk1.tenthous -> Bitmap Heap Scan on tenk1 (cost=5.06..227.67 rows=100 width=244) Recheck Cond: (ROW(thousand, tenthous) < ROW(9, 5000)) -> Bitmap Index Scan on tenk1_thousand_tenthous_idx (cost=0.00..5.04 rows=100 width=0) Index Cond: (ROW(thousand, tenthous) < ROW(9, 5000)) However you can force 9.4 to do it the same as HEAD by setting enable_sort to zero: Aggregate (cost=359.27..359.28 rows=1 width=0) -> Index Scan using tenk1_thousand_tenthous_idx on tenk1 (cost=0.29..358.02 rows=100 width=244) Index Cond: (ROW(thousand, tenthous) < ROW(9, 5000)) But 9.4 correctly answers "95" with either plan, and 9.5 gives the wrong answer with either plan, so the plan change is not the cause of the bug. I'm not sure if the costing change is a bug or not --- the non-bitmap scan does seem to be cheaper in reality, but not by a couple orders of magnitude as the planner now thinks. 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] RFC: replace pg_stat_activity.waiting with something more descriptive
On Mon, Feb 29, 2016 at 8:01 AM, Amit Kapilawrote: > On Thu, Feb 25, 2016 at 2:54 AM, Peter Eisentraut wrote: >> >> Could you enhance the documentation about the difference between "wait >> event type name" and "wait event name" (examples?)? >> > > I am planning to add possible values for each of the wait event type and > wait event and will add few examples as well. Let me know if you want to > see something else with respect to documentation? That's pretty much what I had in mind. I imagine that most of the list of wait events will be a list of the individual LWLocks, which I suppose then will each need a brief description. -- 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] A trivial fix on extensiblenode
On Mon, Feb 29, 2016 at 4:07 AM, Kouhei Kaigaiwrote: > RegisterExtensibleNodeMethods() initializes its hash table > with keysize=NAMEDATALEN, instead of EXTNODENAME_MAX_LEN. > > The attached patch fixes it. Oops. Thanks, committed. -- 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] The plan for FDW-based sharding
On 01/03/16 18:18, Konstantin Knizhnik wrote: On 01.03.2016 19:03, Robert Haas wrote: On Tue, Mar 1, 2016 at 10:37 AM, Bruce Momjianwrote: On Tue, Mar 1, 2016 at 10:19:45AM -0500, Robert Haas wrote: Two reasons: 1. There is no ideal implementation of DTM which will fit all possible needs and be efficient for all clusters. Hmm, what is the reasoning behind that statement? I mean, it is certainly true that there are some places where we have decided that one-size-fits-all is not the right approach. Indexing, for example. Uh, is that even true of indexing? While the plug-in nature of indexing allows for easier development and testing, does anyone create plug-in indexing that isn't shipped by us? I thought WAL support was something that prevented external indexing solutions from working. True. There is an API, though, and having pluggable WAL support seems desirable too. At the same time, I don't think we know of anyone maintaining a non-core index AM ... and there are probably good reasons for that. We end up revising the index AM API pretty regularly every time somebody wants to do something new, so it's not really a stable API that extensions can just tap into. I suspect that a transaction manager API would end up similarly situated. IMHO non-stable API is better than lack of API. Just because it makes it possible to implement features in modular way. And refactoring of API is not so difficult thing... Since this thread heavily discusses the XTM, I have question about the XTM as proposed because one thing is very unclear to me - what happens when user changes the XTM plugin on the server? I didn't see any xid handover API which makes me wonder if changes of a plugin (or for example failure to load previously used plugin due to admin error) will send server to similar situation as xid wraparound. -- Petr Jelinek http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] extend pgbench expressions with functions
On Wed, Feb 17, 2016 at 3:22 AM, Fabien COELHOwrote: > Indeed. My gcc 4.8.4 with --Wall does not show the warning, too bad. > > Attached is the fixed patch for the array method. Committed with a few tweaks, including running pgindent over some of it. -- 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] [NOVICE] WHERE clause not used when index is used
On 01/03/16 18:37, Tom Lane wrote: Jeff Janeswrites: Bisects down to: 606c0123d627b37d5ac3f7c2c97cd715dde7842f is the first bad commit commit 606c0123d627b37d5ac3f7c2c97cd715dde7842f Author: Simon Riggs Date: Tue Nov 18 10:24:55 2014 + Reduce btree scan overhead for < and > strategies On looking at this, the problem seems pretty obvious: that commit simply fails to consider multi-column keys at all. (For that matter, it also fails to consider zero-column keys.) I think the logic might be all right if a test on so->numberOfKeys == 1 were added; I don't see how the optimization could work in multi-column cases. It seems that way to me as well. However, I'm not sure that's 100% of the issue, because in playing around with this I was having a harder time reproducing the failure outside of Tobias' example than I expected. There may be more than one bug, or there may be other changes that sometimes mask the problem. I can only get the issue when the sort order of the individual keys does not correlate and the operator sorts according to the first column and there are duplicate values for the first column. This makes sense to me as we switch to SK_BT_MATCHED as soon as we hit first match, but because there isn't correlation on the second column we get false positives because while we are only scanning part of the btree where first column matches, it does not necessary has to be true for second column. I don't know our btree code to sufficient detail to be sure I didn't miss anything though. Quick fix to me seems what you said above, although it seems like we could make it work even for multicolumn indexes if we checked that the ordering of everything is correct. But I would refrain from adding the complexity as part of a fix. -- Petr Jelinek http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] proposal: PL/Pythonu - function ereport
On 3/1/16, Pavel Stehulewrote: >> I though about it before and I prefer variant with possibility to enter >> message as keyword parameter. That's also ok, but indeed with a check that it's not specified twice which I see you already added. > I merged your patches without @3 (many thanks for it). Instead @3 I > disallow double message specification (+regress test) Great, welcome. Ran the tests for plpython-enhanced-error-06 again on 2.4 - 2.7 and 3.5 versions and they passed. I see the pfree you added isn't allowed on a NULL pointer but as far as I see message is guaranteed not to be NULL as dgettext never returns NULL. I'll mark this Ready for Committer. -- 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] [NOVICE] WHERE clause not used when index is used
Jeff Janeswrites: > Bisects down to: > 606c0123d627b37d5ac3f7c2c97cd715dde7842f is the first bad commit > commit 606c0123d627b37d5ac3f7c2c97cd715dde7842f > Author: Simon Riggs > Date: Tue Nov 18 10:24:55 2014 + > Reduce btree scan overhead for < and > strategies On looking at this, the problem seems pretty obvious: that commit simply fails to consider multi-column keys at all. (For that matter, it also fails to consider zero-column keys.) I think the logic might be all right if a test on so->numberOfKeys == 1 were added; I don't see how the optimization could work in multi-column cases. However, I'm not sure that's 100% of the issue, because in playing around with this I was having a harder time reproducing the failure outside of Tobias' example than I expected. There may be more than one bug, or there may be other changes that sometimes mask the problem. 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] [NOVICE] WHERE clause not used when index is used
* Tom Lane (t...@sss.pgh.pa.us) wrote: > [ trimming -novice from the cc list ] > > Jeff Janeswrites: > > On Tue, Mar 1, 2016 at 7:40 AM, Tom Lane wrote: > >> (Problem is reproducible in 9.5 and HEAD, but not 9.4.) > > > Bisects down to: > > > 606c0123d627b37d5ac3f7c2c97cd715dde7842f is the first bad commit > > commit 606c0123d627b37d5ac3f7c2c97cd715dde7842f > > Author: Simon Riggs > > Date: Tue Nov 18 10:24:55 2014 + > > > Reduce btree scan overhead for < and > strategies > > Hmm ... just from the commit message, this seems much more likely to be > the cause than does the buffer locking patch Stephen fingered. Stephen, > how'd you identify 2ed5b87f as being the problem? Badly. :) I didn't expect it to be something that far back and was just going backwards through reverting/testing patches that looked like candidates, but forgot to create the index when I tested the revert of that, which made it look like reverting it 'fixed' it. Apologies for the noise. Thanks! Stephen signature.asc Description: Digital signature
Re: [HACKERS] Confusing with commit time usage in logical decoding
On 01/03/16 18:18, Andres Freund wrote: Hi, On 2016-03-01 18:09:28 +0100, Petr Jelinek wrote: On 01/03/16 17:57, Alvaro Herrera wrote: Artur Zakirov wrote: Hello, Andres You have introduced a large replication progress tracking infrastructure last year. And there is a problem described at the link in the quote below. Attached patch fix this issue. Is this patch correct? I will be grateful if it is and if it will be committed. AFAICS this is clearly a bug introduced in 5aa235042: /* replay actions of all transaction + subtransactions in order */ ReorderBufferCommit(ctx->reorder, xid, buf->origptr, buf->endptr, - parsed->xact_time); + commit_time, origin_id, origin_lsn); } Well yeah but the commit_time is set few lines above as Artur pointed out, I think the proposed fix is correct one. I'd rather just initialize commit_time to parsed->xact_time. This indeed is clearly a bug. I do wonder if anybody has a good idea about how to add regression tests for this? It's rather annoying that we have to suppress timestamps in the test_decoding tests, because they're obviously not reproducible... The test for commit timestamps checks that the timestamps are within reasonable time frame (for example, bigger than value of a timestamp column in the table since that's assigned before commit obviously) , it's not perfect but similar approach should catch issues like this one. -- Petr Jelinek http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [NOVICE] WHERE clause not used when index is used
[ trimming -novice from the cc list ] Jeff Janeswrites: > On Tue, Mar 1, 2016 at 7:40 AM, Tom Lane wrote: >> (Problem is reproducible in 9.5 and HEAD, but not 9.4.) > Bisects down to: > 606c0123d627b37d5ac3f7c2c97cd715dde7842f is the first bad commit > commit 606c0123d627b37d5ac3f7c2c97cd715dde7842f > Author: Simon Riggs > Date: Tue Nov 18 10:24:55 2014 + > Reduce btree scan overhead for < and > strategies Hmm ... just from the commit message, this seems much more likely to be the cause than does the buffer locking patch Stephen fingered. Stephen, how'd you identify 2ed5b87f as being the problem? 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] Confusing with commit time usage in logical decoding
Andres Freund wrote: > I'd rather just initialize commit_time to parsed->xact_time. That also works. Probably also change its declaration to actually be TimestampTz ... > This indeed is clearly a bug. I do wonder if anybody has a good idea > about how to add regression tests for this? It's rather annoying that > we have to suppress timestamps in the test_decoding tests, because > they're obviously not reproducible... Maybe commit two transactions with a 1s sleep in between, and verify that the difference between the two timestamps is >= 1s and <= now()? (I don't know the test_decoding test suite) -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Small PATCH: check of 2 Perl modules
On Tue, Feb 16, 2016 at 11:48 AM, Victor Wagnerwrote: > On Tue, 16 Feb 2016 12:23:56 -0300 > Alvaro Herrera wrote: > >> ... but I agree with the point upthread that this should wait to see >> what happens with the CMake stuff, since this is not a newly >> introduced problem. > > I doubt, that CMake stuff would be ready for 9.6. There is just one > commitfest left, and it would be quite a radical change. > > And even if would appear in the next release, it cannot be easily > backpatched to 9.5. So we'll probably live with autoconf until the > end-of-life of 9.5 series at least. Yeah, I'm rather doubtful about anything happening with cmake any time soon. -- 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] [NOVICE] WHERE clause not used when index is used
On Tue, Mar 1, 2016 at 7:40 AM, Tom Lanewrote: > Tobias Florek writes: >> When creating an index to use for an ORDER BY clause, a simple query >> starts to return more results than expected. See the following detailed >> log. > > Ugh. That is *badly* broken. I thought maybe it had something to do with > the "abbreviated keys" work, but the same thing happens if you change the > numeric column to integer, so I'm not very sure where to look. Who's > touched btree key comparison logic lately? > > (Problem is reproducible in 9.5 and HEAD, but not 9.4.) Bisects down to: 606c0123d627b37d5ac3f7c2c97cd715dde7842f is the first bad commit commit 606c0123d627b37d5ac3f7c2c97cd715dde7842f Author: Simon Riggs Date: Tue Nov 18 10:24:55 2014 + Reduce btree scan overhead for < and > strategies For <, <=, > and >= strategies, mark the first scan key as already matched if scanning in an appropriate direction. If index tuple contains no nulls we can skip the first re-check for each tuple. Author: Rajeev Rastogi Reviewer: Haribabu Kommi Rework of the code and comments by Simon Riggs It is not a part of the code-base I'm familiar with, so it is unlikely I can find the bug. Cheers, Jeff -- 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] [NOVICE] WHERE clause not used when index is used
* Tom Lane (t...@sss.pgh.pa.us) wrote: > Tobias Florekwrites: > > When creating an index to use for an ORDER BY clause, a simple query > > starts to return more results than expected. See the following detailed > > log. > > Ugh. That is *badly* broken. I thought maybe it had something to do with > the "abbreviated keys" work, but the same thing happens if you change the > numeric column to integer, so I'm not very sure where to look. Who's > touched btree key comparison logic lately? > > (Problem is reproducible in 9.5 and HEAD, but not 9.4.) Looks to have been introduced in 2ed5b87f. Reverting that gets us back to results which look correct. > > Create enough test data for planer to use an index (if exists) for the > > condition. > > > CREATE TABLE "index_cond_test" AS > > SELECT > > (10 + random() * 10)::int AS "final_score", > > round((10 + random() * 10)::numeric, 5) "time_taken" > > FROM generate_series(1, 1) s; > > > > Run control query without an index (will be less than 1 rows). Pay > > attention to tuples of (20,a) with a > 11. > > > SELECT * > > FROM "index_cond_test" > > WHERE (final_score, time_taken) < (20, 11) > > ORDER BY final_score DESC, time_taken ASC; > > > > Or wrapped in count(*), to make it even more obvious > > > SELECT count(*) FROM ( SELECT * > >FROM "index_cond_test" > >WHERE (final_score, time_taken) < (20, 11) > >ORDER BY final_score DESC, time_taken ASC) q; > > > Create the index > > > CREATE INDEX "index_cond_test_ranking" ON "index_cond_test" USING btree > > (final_score DESC, time_taken ASC); > > > Run test query (will return all 1 rows) > > > SELECT * > > FROM "index_cond_test" > > WHERE (final_score, time_taken) < (20, 11) > > ORDER BY final_score DESC, time_taken ASC; > > > or wrapped > > > SELECT count(*) FROM ( SELECT * > >FROM "index_cond_test" > >WHERE (final_score, time_taken) < (20, 11) > >ORDER BY final_score DESC, time_taken ASC) q; Thanks! Stephen signature.asc Description: Digital signature
Re: [HACKERS] GetExistingLocalJoinPath() vs. the docs
On Tue, Feb 16, 2016 at 7:53 AM, Ashutosh Bapatwrote: > PFA patch fixing those things. I think that you need to take a little broader look at this section. At the top, it says "To use any of these functions, you need to include the header file foreign/foreign.h in your source file", but this function is defined in foreign/fdwapi.h. It's not clear to me whether we should consider moving the prototype, or just document that this function is someplace else. The other functions prototyped in fdwapi.h aren't documented at all, except for IsImportableForeignTable, which is mentioned in passing. Further down, the section says "Some object types have name-based lookup functions in addition to the OID-based ones:" and you propose to put the documentation for this function after that. But this comment doesn't actually describe this particular function. Actually, this function just doesn't seem to fit into this section at all. It's really quite different from the others listed there. How about something like the attached instead? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company gejlp-rmh-doc.patch Description: application/download -- 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] The plan for FDW-based sharding
On 01.03.2016 19:03, Robert Haas wrote: On Tue, Mar 1, 2016 at 10:37 AM, Bruce Momjianwrote: On Tue, Mar 1, 2016 at 10:19:45AM -0500, Robert Haas wrote: Two reasons: 1. There is no ideal implementation of DTM which will fit all possible needs and be efficient for all clusters. Hmm, what is the reasoning behind that statement? I mean, it is certainly true that there are some places where we have decided that one-size-fits-all is not the right approach. Indexing, for example. Uh, is that even true of indexing? While the plug-in nature of indexing allows for easier development and testing, does anyone create plug-in indexing that isn't shipped by us? I thought WAL support was something that prevented external indexing solutions from working. True. There is an API, though, and having pluggable WAL support seems desirable too. At the same time, I don't think we know of anyone maintaining a non-core index AM ... and there are probably good reasons for that. We end up revising the index AM API pretty regularly every time somebody wants to do something new, so it's not really a stable API that extensions can just tap into. I suspect that a transaction manager API would end up similarly situated. IMHO non-stable API is better than lack of API. Just because it makes it possible to implement features in modular way. And refactoring of API is not so difficult thing... -- Konstantin Knizhnik Postgres Professional: http://www.postgrespro.com The Russian Postgres 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] Confusing with commit time usage in logical decoding
Hi, On 2016-03-01 18:09:28 +0100, Petr Jelinek wrote: > On 01/03/16 17:57, Alvaro Herrera wrote: > >Artur Zakirov wrote: > >>Hello, Andres > >> > >>You have introduced a large replication progress tracking infrastructure > >>last year. And there is a problem described at the link in the quote below. > >> > >>Attached patch fix this issue. Is this patch correct? I will be grateful if > >>it is and if it will be committed. > > > >AFAICS this is clearly a bug introduced in 5aa235042: > > > > /* replay actions of all transaction + subtransactions in order */ > > ReorderBufferCommit(ctx->reorder, xid, buf->origptr, buf->endptr, > >- parsed->xact_time); > >+ commit_time, origin_id, origin_lsn); > > } > > > > Well yeah but the commit_time is set few lines above as Artur pointed out, I > think the proposed fix is correct one. I'd rather just initialize commit_time to parsed->xact_time. This indeed is clearly a bug. I do wonder if anybody has a good idea about how to add regression tests for this? It's rather annoying that we have to suppress timestamps in the test_decoding tests, because they're obviously not reproducible... 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] Confusing with commit time usage in logical decoding
Petr Jelinek wrote: > On 01/03/16 17:57, Alvaro Herrera wrote: > >Artur Zakirov wrote: > >>Hello, Andres > >> > >>You have introduced a large replication progress tracking infrastructure > >>last year. And there is a problem described at the link in the quote below. > >> > >>Attached patch fix this issue. Is this patch correct? I will be grateful if > >>it is and if it will be committed. > > > >AFAICS this is clearly a bug introduced in 5aa235042: > > > > /* replay actions of all transaction + subtransactions in order */ > > ReorderBufferCommit(ctx->reorder, xid, buf->origptr, buf->endptr, > >- parsed->xact_time); > >+ commit_time, origin_id, origin_lsn); > > } > > > > Well yeah but the commit_time is set few lines above as Artur pointed out, I > think the proposed fix is correct one. Err, yes, that's exactly what I am saying. Sorry for being unclear. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] The plan for FDW-based sharding
Thank you very much for you comments. On 01.03.2016 18:19, Robert Haas wrote: On Sat, Feb 27, 2016 at 2:29 AM, Konstantin Knizhnikwrote: How do you prevent clock skew from causing serialization anomalies? If node receives message from "feature" it just needs to wait until this future arrive. Practically we just "adjust" system time in this case, moving it forward (certainly system time is not actually changed, we just set correction value which need to be added to system time). This approach was discussed in the article: http://research.microsoft.com/en-us/people/samehe/clocksi.srds2013.pdf I hope, in this article algorithm is explained much better than I can do here. Hmm, the approach in that article is very interesting, but it sounds different than what you are describing - they do not, AFAICT, have anything like a "correction value" In the article them used anotion "wait": if T.SnapshotTime>GetClockTime() then wait until T.SnapshotTime
Re: [HACKERS]WIP: Covering + unique indexes.
29.02.2016 18:17, Anastasia Lubennikova: 25.02.2016 21:39, Jeff Janes: As promised, here's the new version of the patch "including_columns_4.0". I fixed all issues except some points mentioned below. Thanks for the update patch. I get a compiler warning: genam.c: In function 'BuildIndexValueDescription': genam.c:259: warning: unused variable 'tupdesc' Thank you for the notice, I'll fix it in the next update. Also, I can't create a primary key INCLUDING columns directly: jjanes=# create table foobar (a int, b int, c int); jjanes=# alter table foobar add constraint foobar_pkey primary key (a,b) including (c); ERROR: syntax error at or near "including" But I can get there using a circuitous route: jjanes=# create unique index on foobar (a,b) including (c); jjanes=# alter table foobar add constraint foobar_pkey primary key using index foobar_a_b_c_idx; The description of the table's index knows to include the including column: jjanes=# \d foobar Table "public.foobar" Column | Type | Modifiers +-+--- a | integer | not null b | integer | not null c | integer | Indexes: "foobar_pkey" PRIMARY KEY, btree (a, b) INCLUDING (c) Since the machinery appears to all be in place to have primary keys with INCLUDING columns, it would be nice if the syntax for adding primary keys allowed one to implement them directly. Is this something or future expansion, or could it be added at the same time as the main patch? Good point. At quick glance, this looks easy to implement it. The only problem is that there are too many places in code which must be updated. I'll try to do it, and if there would be difficulties, it's fine with me to delay this feature for the future work. I found one more thing to do. Pgdump does not handle included columns now. I will fix it in the next version of the patch. As promised, fixed patch is in attachments. It allows to perform following statements: create table utbl (a int, b box); alter table utbl add unique (a) including(b); create table ptbl (a int, b box); alter table ptbl add primary key (a) including(b); And now they can be dumped/restored successfully. I used following settings pg_dump --verbose -Fc postgres -f pg.dump pg_restore -d newdb pg.dump It is not the final version, because it breaks pg_dump for previous versions. I need some help from hackers here. pgdump. line 5466 if (fout->remoteVersion >= 90400) What does 'remoteVersion' mean? And what is the right way to change it? Or it changes between releases? I guess that 90400 is for 9.4 and 80200 is for 8.2 but is it really so? That is totally new to me. BTW, While we are on the subject, maybe it's worth to replace these magic numbers with some set of macro? P.S. I'll update documentation for ALTER TABLE in the next patch. -- Anastasia Lubennikova Postgres Professional: http://www.postgrespro.com The Russian Postgres 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] Confusing with commit time usage in logical decoding
Artur Zakirov wrote: > Hello, Andres > > You have introduced a large replication progress tracking infrastructure > last year. And there is a problem described at the link in the quote below. > > Attached patch fix this issue. Is this patch correct? I will be grateful if > it is and if it will be committed. AFAICS this is clearly a bug introduced in 5aa235042: /* replay actions of all transaction + subtransactions in order */ ReorderBufferCommit(ctx->reorder, xid, buf->origptr, buf->endptr, - parsed->xact_time); + commit_time, origin_id, origin_lsn); } -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_dump / copy bugs with "big lines" ?
"Daniel Verite"writes: > Alvaro Herrera wrote: >> If others can try this patch to ensure it enables pg_dump to work on >> their databases, it would be great. > It doesn't seem to help if one field exceeds 1Gb, for instance when > inflated by a bin->hex translation. It's not going to be possible to fix that without enormously invasive changes (affecting individual datatype I/O functions, for example). And in general, fields approaching that size are going to give you problems in all kinds of ways, not only COPY. I think we should be satisfied if we can make COPY deal with the sum of a line's fields exceeding 1GB. 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_dump / copy bugs with "big lines" ?
Alvaro Herrera wrote: > If others can try this patch to ensure it enables pg_dump to work on > their databases, it would be great. It doesn't seem to help if one field exceeds 1Gb, for instance when inflated by a bin->hex translation. postgres=# create table big as select pg_read_binary_file('data') as binarycol; postgres=# select octet_length(binarycol) from big; octet_length -- 107370 postgres=# copy big to '/var/tmp/big.copy'; ERROR: XX000: invalid memory alloc request size 214743 LOCATION: palloc, mcxt.c:903 Same problem with pg_dump. OTOH, it improves the case where the cumulative size of field contents for a row exceeds 1 Gb, but not any single field exceeds that size. If splitting the table into 3 fields, each smaller than 512MB: postgres=# create table big2 as select substring(binarycol from 1 for 300*1024*1024) as b1, substring(binarycol from 1+300*1024*1024 for 300*1024*1024) as b2 , substring(binarycol from 1+600*1024*1024 for 400*1024*1024) as b3 from big; postgres=# copy big2 to '/var/tmp/big.copy'; COPY 1 then that works, producing a single line of 2097152012 chars in the output file. By contrast, it fails with an unpatched 9.5: postgres=# copy big2 to '/var/tmp/big.copy'; ERROR: 54000: out of memory DETAIL: Cannot enlarge string buffer containing 629145605 bytes by 629145602 more bytes. LOCATION: enlargeStringInfo, stringinfo.c:260 If setting bytea_output to 'escape', it also fails with the patch applied, as it tries to allocate 4x the binary field size, and it exceeds 1GB again. postgres=# set bytea_output =escape; SET postgres=# copy big2 to '/var/tmp/big.copy'; ERROR: invalid memory alloc request size 1258291201 LOCATION: palloc, mcxt.c:821 1258291201 = 300*1024*1024*4+1 Also, the COPY of both tables work fine if using (FORMAT BINARY), on both the patched and unpatched server. Best regards, -- Daniel Vérité PostgreSQL-powered mailer: http://www.manitou-mail.org Twitter: @DanielVerite -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers