Re: [HACKERS] TAP / recovery-test fs-level backups, psql enhancements etc

2016-03-01 Thread Tom Lane
I wrote:
> Michael Paquier  writes:
>> 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

2016-03-01 Thread Craig Ringer
On 1 March 2016 at 05:30, Petr Jelinek  wrote:

>
> 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

2016-03-01 Thread Michael Paquier
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

2016-03-01 Thread David Fetter
On Wed, Mar 02, 2016 at 03:34:39PM +0900, Michael Paquier wrote:
> On Wed, Mar 2, 2016 at 2:53 PM, David Fetter  wrote:
> > 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

2016-03-01 Thread Michael Paquier
On Tue, Mar 1, 2016 at 10:21 PM, Dmitriy Sarafannikov
 wrote:
> 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

2016-03-01 Thread Michael Paquier
On Wed, Mar 2, 2016 at 2:53 PM, David Fetter  wrote:
> 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

2016-03-01 Thread Michael Paquier
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

2016-03-01 Thread Ashutosh Bapat
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

2016-03-01 Thread Tom Lane
Michael Paquier  writes:
> 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

2016-03-01 Thread David Fetter
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.

2016-03-01 Thread Michael Paquier
On Wed, Mar 2, 2016 at 2:10 AM, Anastasia Lubennikova
 wrote:
> 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

2016-03-01 Thread Craig Ringer
On 2 March 2016 at 13:22, Michael Paquier  wrote:

> 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

2016-03-01 Thread Dilip Kumar
On Wed, Mar 2, 2016 at 10:39 AM, Andres Freund  wrote:

> 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

2016-03-01 Thread Michael Paquier
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.
-- 
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

2016-03-01 Thread Alvaro Herrera
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

2016-03-01 Thread Andres Freund


On March 1, 2016 8:41:33 PM PST, Dilip Kumar  wrote:
>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

2016-03-01 Thread Michael Paquier
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

2016-03-01 Thread Dilip Kumar
On Tue, Mar 1, 2016 at 4:36 PM, Amit Kapila  wrote:

> 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

2016-03-01 Thread Pavel Stehule
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

2016-03-01 Thread Tom Lane
Craig Ringer  writes:
> 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

2016-03-01 Thread Michael Paquier
On Wed, Mar 2, 2016 at 1:33 PM, Tom Lane  wrote:
> 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

2016-03-01 Thread Amit Kapila
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

2016-03-01 Thread Dilip Kumar
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.

-- 
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com


Re: [HACKERS] TAP / recovery-test fs-level backups, psql enhancements etc

2016-03-01 Thread Tom Lane
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.

> 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

2016-03-01 Thread Gavin Flower

On 02/03/16 17:01, Michael Paquier wrote:

On Wed, Mar 2, 2016 at 11:25 AM, David Steele  wrote:

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

2016-03-01 Thread Michael Paquier
On Wed, Mar 2, 2016 at 11:25 AM, David Steele  wrote:
> 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

2016-03-01 Thread Craig Ringer
On 2 March 2016 at 10:07, Craig Ringer  wrote:

> 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

2016-03-01 Thread Craig Ringer
On 2 March 2016 at 11:23, Craig Ringer  wrote:


> 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

2016-03-01 Thread Craig Ringer
On 2 March 2016 at 11:22, Craig Ringer  wrote:

> 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-01 Thread Craig Ringer
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.

2016-03-01 Thread Robert Haas
On Thu, Feb 18, 2016 at 3:45 AM, Masahiko Sawada  wrote:
> 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

2016-03-01 Thread Jeff Janes
On Mon, Jan 18, 2016 at 2:29 PM, Tom Lane  wrote:
>
> 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

2016-03-01 Thread David Steele
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

2016-03-01 Thread Michael Paquier
On Wed, Mar 2, 2016 at 12:40 AM, Andrew Dunstan  wrote:
> 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

2016-03-01 Thread Craig Ringer
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.


> > +=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

2016-03-01 Thread Michael Paquier
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.
-- 
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

2016-03-01 Thread Michael Paquier
On Wed, Mar 2, 2016 at 5:46 AM, Magnus Hagander  wrote:
> 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

2016-03-01 Thread Craig Ringer
On 2 March 2016 at 07:07, Alvaro Herrera  wrote:

> 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

2016-03-01 Thread Tomas Vondra

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?

2016-03-01 Thread Tom Lane
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

2016-03-01 Thread Alvaro Herrera
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

2016-03-01 Thread Tom Lane
Victor Wagner  writes:
> 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

2016-03-01 Thread Alvaro Herrera
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

2016-03-01 Thread Michael Paquier
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.
-- 
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

2016-03-01 Thread Alvaro Herrera
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" ?

2016-03-01 Thread Tom Lane
"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

2016-03-01 Thread Tom Lane
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" ?

2016-03-01 Thread Daniel Verite
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

2016-03-01 Thread Jim Nasby

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

2016-03-01 Thread Jim Nasby

On 2/29/16 10:01 PM, Tom Lane wrote:

Jim Nasby  writes:

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

2016-03-01 Thread Tom Lane
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

2016-03-01 Thread David Steele
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.

-- 
-David
da...@pgmasters.net



signature.asc
Description: OpenPGP digital signature


Re: [HACKERS] psql completion for ids in multibyte string

2016-03-01 Thread Thomas Munro
On Wed, Mar 2, 2016 at 7:54 AM, Robert Haas  wrote:
> 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

2016-03-01 Thread Julien Rouhaud
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

2016-03-01 Thread Magnus Hagander
On Tue, Mar 1, 2016 at 3:40 PM, Tom Lane  wrote:

> 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

2016-03-01 Thread Tom Lane
Magnus Hagander  writes:
> 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

2016-03-01 Thread David Steele
On 3/1/16 3:35 PM, Kevin Grittner wrote:
> On Tue, Mar 1, 2016 at 2:28 PM, Magnus Hagander  wrote:
>> 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

2016-03-01 Thread David Steele
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

2016-03-01 Thread Kevin Grittner
On Tue, Mar 1, 2016 at 2:28 PM, Magnus Hagander  wrote:
> 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

2016-03-01 Thread Magnus Hagander
On Tue, Mar 1, 2016 at 10:12 AM, Alvaro Herrera 
wrote:

> 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

2016-03-01 Thread Tom Lane
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.

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

2016-03-01 Thread Tom Lane
I wrote:
> Petr Jelinek  writes:
>> 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

2016-03-01 Thread David Steele
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

2016-03-01 Thread Konstantin Knizhnik

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

2016-03-01 Thread Jim Nasby

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

2016-03-01 Thread Konstantin Knizhnik

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

2016-03-01 Thread Jim Nasby

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)

2016-03-01 Thread Peter Geoghegan
On Tue, Mar 1, 2016 at 7:27 AM, Tom Lane  wrote:
> +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

2016-03-01 Thread Bruce Momjian
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 Momjian  http://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

2016-03-01 Thread Dmitry Dolgov
On 1 March 2016 at 06:34, Michael Paquier  wrote:

> 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

2016-03-01 Thread Bruce Momjian
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 Momjian  http://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

2016-03-01 Thread Robert Haas
On Mon, Feb 29, 2016 at 12:58 PM, Fabrízio de Royes Mello
 wrote:
> 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

2016-03-01 Thread Petr Jelinek

On 27/02/16 04:54, Robert Haas wrote:

On Fri, Feb 26, 2016 at 10:56 PM, Konstantin Knizhnik
 wrote:

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

2016-03-01 Thread Robert Haas
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?

-- 
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

2016-03-01 Thread Tom Lane
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.

2016-03-01 Thread Robert Haas
On Fri, Feb 26, 2016 at 12:33 AM, Kyotaro HORIGUCHI
 wrote:
> 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

2016-03-01 Thread Tom Lane
Petr Jelinek  writes:
> 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

2016-03-01 Thread Robert Haas
On Mon, Feb 29, 2016 at 8:01 AM, Amit Kapila  wrote:
> 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

2016-03-01 Thread Robert Haas
On Mon, Feb 29, 2016 at 4:07 AM, Kouhei Kaigai  wrote:
> 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

2016-03-01 Thread Petr Jelinek

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 Momjian  wrote:

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

2016-03-01 Thread Robert Haas
On Wed, Feb 17, 2016 at 3:22 AM, Fabien COELHO  wrote:
> 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

2016-03-01 Thread Petr Jelinek

On 01/03/16 18:37, Tom Lane wrote:

Jeff Janes  writes:

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

2016-03-01 Thread 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.


-- 
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

2016-03-01 Thread Tom Lane
Jeff Janes  writes:
> 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

2016-03-01 Thread Stephen Frost
* Tom Lane (t...@sss.pgh.pa.us) wrote:
> [ trimming -novice from the cc list ]
> 
> Jeff Janes  writes:
> > 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

2016-03-01 Thread Petr Jelinek



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

2016-03-01 Thread Tom Lane
[ trimming -novice from the cc list ]

Jeff Janes  writes:
> 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

2016-03-01 Thread Alvaro Herrera
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

2016-03-01 Thread Robert Haas
On Tue, Feb 16, 2016 at 11:48 AM, Victor Wagner  wrote:
> 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

2016-03-01 Thread Jeff Janes
On Tue, Mar 1, 2016 at 7:40 AM, Tom Lane  wrote:
> 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

2016-03-01 Thread Stephen Frost
* Tom Lane (t...@sss.pgh.pa.us) wrote:
> 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.)

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

2016-03-01 Thread Robert Haas
On Tue, Feb 16, 2016 at 7:53 AM, Ashutosh Bapat
 wrote:
> 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

2016-03-01 Thread Konstantin Knizhnik



On 01.03.2016 19:03, Robert Haas wrote:

On Tue, Mar 1, 2016 at 10:37 AM, Bruce Momjian  wrote:

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

2016-03-01 Thread Andres Freund
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

2016-03-01 Thread Alvaro Herrera
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

2016-03-01 Thread Konstantin Knizhnik

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 Knizhnik
 wrote:

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.

2016-03-01 Thread Anastasia Lubennikova


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

2016-03-01 Thread Alvaro Herrera
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" ?

2016-03-01 Thread Tom Lane
"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" ?

2016-03-01 Thread Daniel Verite
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


  1   2   >