Re: [HACKERS] WIP: Make timestamptz_out less slow.

2015-07-28 Thread Andres Freund
On 2015-07-28 10:59:15 +1200, David Rowley wrote:
> It won't be quite as fast as what you've written, but I think it will be
> much neater and more likely to be used in other places if we invent a
> function like pg_ltoa() which returns a pointer to the new end of string.
> 
> Also if we're specifying padding with zeros then we can skip the reverse
> part that's in pg_ltoa(), (normally needed since the numeric string is
> build in reverse)
> 
> The code could then be written as:
> 
> str = pg_int2str_pad(str, year, 4);
> *str++ = '-';
> str = pg_int2str_pad(str, tm->tm_mon, 2);
> *str++ = '-';
> str = pg_int2str_pad(str, tm->tm_mday, 2);
> 
> etc
> 
> I've used this method before and found it to be about 10 times faster than
> snprintf(), but I was reversing the string, so quite likely it be more than
> 10 times.

Yes, that might be worthwhile to try. Certainly would look less
ugly. Willing to give it a try?

> I'm interested to see how much you're really gaining by manually unrolling
> the part that builds the fractional part of the second.

It seems to help a fair amount, but this really was more a quick POC
than something serious. My theory why it helps is that each loop
iteration is independent of the previous one and thus can take full
advantage of the pipeline.

Regards,

Andres


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


[HACKERS] Typo in a comment in set_foreignscan_references

2015-07-28 Thread Amit Langote
Attached fixes a minor typo:

s/custom/foreign/g

Thanks,
Amit
diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c
index ea185d4..ee8710d 100644
--- a/src/backend/optimizer/plan/setrefs.c
+++ b/src/backend/optimizer/plan/setrefs.c
@@ -1099,7 +1099,7 @@ set_foreignscan_references(PlannerInfo *root,
 
 	if (fscan->fdw_scan_tlist != NIL || fscan->scan.scanrelid == 0)
 	{
-		/* Adjust tlist, qual, fdw_exprs to reference custom scan tuple */
+		/* Adjust tlist, qual, fdw_exprs to reference foreign scan tuple */
 		indexed_tlist *itlist = build_tlist_index(fscan->fdw_scan_tlist);
 
 		fscan->scan.plan.targetlist = (List *)

-- 
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] [DESIGN] ParallelAppend

2015-07-28 Thread David Rowley
On 27 July 2015 at 21:09, Kyotaro HORIGUCHI  wrote:

> Hello, can I ask some questions?
>
> I suppose we can take this as the analog of ParalleSeqScan.  I
> can see not so distinction between Append(ParalleSeqScan) and
> ParallelAppend(SeqScan). What difference is there between them?
>
> If other nodes will have the same functionality as you mention at
> the last of this proposal, it might be better that some part of
> this feature is implemented as a part of existing executor
> itself, but not as a deidicated additional node, just as my
> asynchronous fdw execution patch patially does. (Although it
> lacks planner part and bg worker launching..) If that is the
> case, it might be better that ExecProcNode is modified so that it
> supports both in-process and inter-bgworker cases by the single
> API.
>
> What do you think about this?
>

I have to say that I really like the thought of us having parallel enabled
stuff in Postgres, but I also have to say that I don't think inventing all
these special parallel node types is a good idea. If we think about
everything that we can parallelise...

Perhaps sort, hash join, seqscan, hash, bitmap heap scan, nested loop.
I don't want to debate that, but perhaps there's more, perhaps less.
Are we really going to duplicate all of the code and add in the parallel
stuff as new node types?

My other concern here is that I seldom hear people talk about the planner's
architectural lack of ability to make a good choice about how many parallel
workers to choose. Surely to properly calculate costs you need to know the
exact number of parallel workers that will be available at execution time,
but you need to know this at planning time!? I can't see how this works,
apart from just being very conservative about parallel workers, which I
think is really bad, as many databases have busy times in the day, and also
quiet times, generally quiet time is when large batch stuff gets done, and
that's the time that parallel stuff is likely most useful. Remember queries
are not always planned just before they're executed. We could have a
PREPAREd query, or we could have better plan caching in the future, or if
we build some intelligence into the planner to choose a good number of
workers based on the current server load, then what's to say that the
server will be under this load at exec time? If we plan during a quiet
time, and exec in a busy time all hell may break loose.

I really do think that existing nodes should just be initialized in a
parallel mode, and each node type can have a function to state if it
supports parallelism or not.

I'd really like to hear more opinions in the ideas I discussed here:

http://www.postgresql.org/message-id/CAApHDvp2STf0=pqfpq+e7wa4qdympfm5qu_ytupe7r0jlnh...@mail.gmail.com


This design makes use of the Funnel node that Amit has already made and
allows more than 1 node to be executed in parallel at once.

It appears that parallel enabling the executor node by node is
fundamentally locked into just 1 node being executed in parallel, then
perhaps a Funnel node gathering up the parallel worker buffers and
streaming those back in serial mode. I believe by design, this does not
permit a whole plan branch from executing in parallel and I really feel
like doing things this way is going to be very hard to undo and improve
later. I might be too stupid to figure it out, but how would parallel hash
join work if it can't gather tuples from the inner and outer nodes in
parallel?

Sorry for the rant, but I just feel like we're painting ourselves into a
corner by parallel enabling the executor node by node.
Apologies if I've completely misunderstood things.

Regards

David Rowley

--
 David Rowley   http://www.2ndQuadrant.com/

 PostgreSQL Development, 24x7 Support, Training & Services


Re: [HACKERS] A little RLS oversight?

2015-07-28 Thread Dean Rasheed
On 28 July 2015 at 03:19, Joe Conway  wrote:
> -BEGIN PGP SIGNED MESSAGE-
> Hash: SHA1
>
> On 07/27/2015 03:05 PM, Stephen Frost wrote:
>> AFK at the moment, but my thinking was that we should avoid having
>> the error message change based on what a GUC is set to. I agree
>> that there should be comments which explain that.
>

Except it's already dependant on the GUC if it's set to FORCE.

> I changed back to using GetUserId() for the call to check_enable_rls()
> at those three call sites, and added to the comments to explain why.
>

I'm not entirely convinced about this. The more I think about it, the
more I think that if we know the user has BYPASSRLS, and they've set
row_security to OFF, then they ought to get the more detailed error
message, as they would if there was no RLS. That error detail is
highly useful, and we know the user has been granted privilege by a
superuser, and that they have direct access to the underlying table in
this context, so we're not leaking any info that they cannot directly
SELECT anyway.

> While looking at ri_ReportViolation() I spotted what I believe to be a
> bug in the current logic -- namely, has_perm is initialized to true,
> and when check_enable_rls() returns RLS_ENABLED we never reset
> has_perm to false, and thus leak info even though the comments claim
> we don't. I fixed that here, but someone please take a look and
> confirm I am reading that correctly.
>

Ah yes, well spotted. That looks correct to me.

Regards,
Dean


-- 
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] [DESIGN] ParallelAppend

2015-07-28 Thread Ashutosh Bapat
On Tue, Jul 28, 2015 at 12:59 PM, David Rowley  wrote:

>
> On 27 July 2015 at 21:09, Kyotaro HORIGUCHI <
> horiguchi.kyot...@lab.ntt.co.jp> wrote:
>
>> Hello, can I ask some questions?
>>
>> I suppose we can take this as the analog of ParalleSeqScan.  I
>> can see not so distinction between Append(ParalleSeqScan) and
>> ParallelAppend(SeqScan). What difference is there between them?
>>
>> If other nodes will have the same functionality as you mention at
>> the last of this proposal, it might be better that some part of
>> this feature is implemented as a part of existing executor
>> itself, but not as a deidicated additional node, just as my
>> asynchronous fdw execution patch patially does. (Although it
>> lacks planner part and bg worker launching..) If that is the
>> case, it might be better that ExecProcNode is modified so that it
>> supports both in-process and inter-bgworker cases by the single
>> API.
>>
>> What do you think about this?
>>
>
> I have to say that I really like the thought of us having parallel enabled
> stuff in Postgres, but I also have to say that I don't think inventing all
> these special parallel node types is a good idea. If we think about
> everything that we can parallelise...
>
> Perhaps sort, hash join, seqscan, hash, bitmap heap scan, nested loop.
> I don't want to debate that, but perhaps there's more, perhaps less.
> Are we really going to duplicate all of the code and add in the parallel
> stuff as new node types?
>
> My other concern here is that I seldom hear people talk about the
> planner's architectural lack of ability to make a good choice about how
> many parallel workers to choose. Surely to properly calculate costs you
> need to know the exact number of parallel workers that will be available at
> execution time, but you need to know this at planning time!? I can't see
> how this works, apart from just being very conservative about parallel
> workers, which I think is really bad, as many databases have busy times in
> the day, and also quiet times, generally quiet time is when large batch
> stuff gets done, and that's the time that parallel stuff is likely most
> useful. Remember queries are not always planned just before they're
> executed. We could have a PREPAREd query, or we could have better plan
> caching in the future, or if we build some intelligence into the planner to
> choose a good number of workers based on the current server load, then
> what's to say that the server will be under this load at exec time? If we
> plan during a quiet time, and exec in a busy time all hell may break loose.
>
> I really do think that existing nodes should just be initialized in a
> parallel mode, and each node type can have a function to state if it
> supports parallelism or not.
>
> I'd really like to hear more opinions in the ideas I discussed here:
>
>
> http://www.postgresql.org/message-id/CAApHDvp2STf0=pqfpq+e7wa4qdympfm5qu_ytupe7r0jlnh...@mail.gmail.com
>
>
> This design makes use of the Funnel node that Amit has already made and
> allows more than 1 node to be executed in parallel at once.
>
> It appears that parallel enabling the executor node by node is
> fundamentally locked into just 1 node being executed in parallel, then
> perhaps a Funnel node gathering up the parallel worker buffers and
> streaming those back in serial mode. I believe by design, this does not
> permit a whole plan branch from executing in parallel and I really feel
> like doing things this way is going to be very hard to undo and improve
> later. I might be too stupid to figure it out, but how would parallel hash
> join work if it can't gather tuples from the inner and outer nodes in
> parallel?
>
> Sorry for the rant, but I just feel like we're painting ourselves into a
> corner by parallel enabling the executor node by node.
> Apologies if I've completely misunderstood things.
>
>
+1, well articulated.
-- 
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company


Re: [HACKERS] proposal: multiple psql option -c

2015-07-28 Thread Marc Mamin

>
>
>2015-07-28 5:24 GMT+02:00 Pavel Stehule :
>
>2015-07-27 21:57 GMT+02:00 Andrew Dunstan :
>
>On 07/27/2015 02:53 PM, Pavel Stehule wrote:
>
>I am trying to run parallel execution
>
>psql -At -c "select datname from pg_database" postgres | xargs -n 
> 1 -P 3 psql -c "select current_database()"
>
>
>
>I don't think it's going to be a hugely important feature, but I don't 
> see a problem with creating a new option (-C seems fine) which would have the 
> same effect as if the arguments were contatenated into a file which is then 
> used with -f. IIRC -c has some special characteristics which means it's 
> probably best not to try to extend it for this feature.
>
>
>ok, I'll try to write patch.
>
>
>I have a question. Can be -C option multiple?


hello,
Have you thought of how to support -1 along with -C ?

> handle the input as with -f
 that is, -1 -C would be equivalent to -c

and
psql -1 -C "sql_1; sql_2;" -C "sql_3; sql_4;"

=> ?

BEGIN;
sql_1;
sql_2;
END;

BEGIN;
sql_3;
sql_4;
END;

thoughts ?

The same logic could be added to -f
although I see less advantages as with adding -C

psql -1 -f "file1, file2" -f "file3, file4"

regards,
Marc Mamin


Re: [HACKERS] REVOKE [ADMIN OPTION FOR] ROLE

2015-07-28 Thread Egor Rogov

On 27.07.2015 22:09, Stephen Frost wrote:

* Egor Rogov (e.ro...@postgrespro.ru) wrote:

On Thu, Jul 23, 2015 at 8:30 AM, Egor Rogov  wrote:

So, the question: is it a documentation bug (as it seems to me), code bug,
or I missed something?

Your analysis looks right to me, but I don't know whether the code or
the documentation should be changed.  This claim was added by Tom Lane
in 2005 in commit 58d214e51fe50b10b4439da6ec263d54c155afbf.  It might
be worth checking whether the claim was true at that time and later
became false, or whether it was never true to begin with.


As far as I can see, modern revoke syntax for revoking membership in
a role (along with "admin option") was introduced in commit 7762619
(by Tom Lane, 2005). Code for handling this command didn't pay
attention for "restrict/cascade" keywords then, as it does not now.
Before that, another syntax was in use: alter group groupname drop
user username [, ...]. It did not include notion of "cascade" at
all.
I guess that "revoke role_name from role_name" inherited
"[cascade|restrict]" section from general revoke command but never
actually used it. And I see no point in changing this, because role
membership is somewhat more static than privileges.
So I would propose the attached fix for documentation.

Have you looked at the SQL spec at all for this..?  That's what we
really should be looking at to determine if this is a documentation
issue or a code issue.

I'll take a look in a day or two after I've caught up on other things,
if no one beats me to it.

Well, I looked into a draft of SQL:2003. It basically says that 
"cascade" for  must behave the same way as for 
. That is, from standard's point of view we 
have a code issue.


Still I doubt about usefulness of this behavior. Do we really need it in 
PostgreSQL?


Thanks,
Egor Rogov
Postgres Professional: http://www.postgrespro.com
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


[HACKERS] pg_rewind tap test unstable

2015-07-28 Thread Christoph Berg
Hi,

for something between 10% and 20% of the devel builds for apt.postgresql.org
(which happen every 6h if there's a git change, so it happens every few days),
I'm seeing this:

make[2]: Entering directory 
'/tmp/buildd/postgresql-9.6-9.6~~devel~20150728.0405/build/src/bin/pg_rewind'
rm -rf 
/tmp/buildd/postgresql-9.6-9.6~~devel~20150728.0405/build/../src/bin/pg_rewind/tmp_check/log
cd 
/tmp/buildd/postgresql-9.6-9.6~~devel~20150728.0405/build/../src/bin/pg_rewind 
&& 
TESTDIR='/tmp/buildd/postgresql-9.6-9.6~~devel~20150728.0405/build/src/bin/pg_rewind'
 
PATH="/tmp/buildd/postgresql-9.6-9.6~~devel~20150728.0405/build/tmp_install/usr/lib/postgresql/9.6/bin:$PATH"
 
LD_LIBRARY_PATH="/tmp/buildd/postgresql-9.6-9.6~~devel~20150728.0405/build/tmp_install/usr/lib/x86_64-linux-gnu"
 PGPORT='65432' 
top_builddir='/tmp/buildd/postgresql-9.6-9.6~~devel~20150728.0405/build/src/bin/pg_rewind/../../..'
 prove -I 
/tmp/buildd/postgresql-9.6-9.6~~devel~20150728.0405/build/../src/test/perl/ 
--verbose t/*.pl

#   Failed test 'tail-copy: query result matches'
#   at RewindTest.pm line 131.
#  got: '1
# '
# expected: '10001
# '
# Looks like you failed 1 test of 8.
t/001_basic.pl ... 
1..8
ok 1 - pg_rewind local
ok 2 - table content: query result matches
ok 3 - truncation: query result matches
not ok 4 - tail-copy: query result matches
ok 5 - pg_rewind remote
ok 6 - table content: query result matches
ok 7 - truncation: query result matches
ok 8 - tail-copy: query result matches
Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/8 subtests 

I don't have the older logs available, but from memory, the subtest
failing and the two numbers mentioned are always the same.

Christoph
-- 
c...@df7cb.de | http://www.df7cb.de/


-- 
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] Autonomous Transaction is back

2015-07-28 Thread Joel Jacobson
On Tue, Jul 28, 2015 at 12:56 AM, Josh Berkus  wrote:

> Ah, ok.  The goal of the project is that the writer of X() *cannot*
> prevent Y() from writing its data (B1) and committing it.
>

> One of the primary use cases for ATX is audit triggers.  If a function
> writer could override ATX and prevent the audit triggers from
> committing, then that use case would be violated.
>
> Can you explain what use case you have where simply telling the staff
> "if you use ATX without clearing it, you'll be fired" is not sufficient?
>  Possibly there's something we failed to account for in the unconference
> discussion.
>

I fully understand and agree why you want to prevent X() from letting Y()
commit, if the use-case is e.g. auditing.

I'll try to explain where I'm coming from by providing a bit of background
and context.

One of the greatest strengths with writing an entire application using only
sql and plpgsql functions,
is you don't have to worry about side effects when calling functions, since
you are always in total control to
rollback all your writes at your stack-depth and deeper down the stack, and
the caller can likewise be certain
that if the function it called threw an exception, all of its work is
rollbacked.

However, if e.g. making use of plperlu functions, it's possible those
functions might have written files to disk
or made network connections to the outside world,
i.e. it's possible they have caused side-effects that naturally cannot be
rollbacked by postgres.

Currently, in our codebase at Trustly, we have already quite a few plperlu
functions.
In each one of them, we need to think carefully about side-effects,
it's usually fine since most of them are immutable.

But if we had to worry about all plpgsql functions being able to write
things without consent from the caller,
then we would have a completely different situation with increased
complexity and risk for failures.

>"if you use ATX without clearing it, you'll be fired"
We trust each other so we don't have that kind of problem.
But still, even when trusting your own and others code, I would say quite
often you make use of a function
for which there are small details you have forgotten about or never knew in
the first place,
and if that little detail would be something written in an ATX, that could
be a problem
if you the caller wouldn't want whatever was written to be written.

Don't get me wrong, ATX is something I would absoltely love, since then you
could
for instance in function doing password validation, update the
FailedLoginAttempts column
in an ATX and then still raise an exception to rollback the operation and
return an error to the client.

However, the need for ATXs is at least for us a special need you won't need
in most functions,
and since the risk and complexity increases with it, I would prefer if it
can be enaled/disabled
by default globally and explicitly enabled/disabled per function.

If the global default is "disabled" or if it's "disabled" for a specific
function, like for X() in your example,
and if it's enabled for Y(), then X() tries to call Y() you should get an
error even before Y() is executed.

That way we can still do auditing, since X() couldn't execute Y() since it
was declared as AT,
and that would be the same thing as if X() wouldn't have the line of code
in it that executs Y(),
something which X() is in power of as if X() calls Y() or not, is
ultimately X()'s decision.

If we declare entire functions as AT, then we only have to check before
executing the function
if AT is allowed or not in the context, determined by the global default or
if the caller function is defined AT or NOT AT.

Use cases:

1. X[AT] -> Y[AT]
OK, since caller X() is declared AT i.e. allows AT in itself and in callees.

2. X[AT] -> Y[NOT AT]
OK, since caller X() is declared AT i.e. allows AT in itself and in callees,
and since Y() is NOT AT, i.e. not making use of AT and not allowing it in
callees
that is not in violation with anything.

3: X[NOT AT] -> Y[AT]
Invalid, since caller X() is declared NOT AT, i.e. disallows AT in itself
and in callees,
and since Y() is declared AT it cannot be executed since it's declared AT.

4: X[NOT AT] -> Y[NOT AT]
OK, since caller X() is declared NOT AT, i.e. disallows AT in itself and in
callees,
and since Y() is also declared NOT AT, it can be executed.


Re: [HACKERS] pg_rewind tap test unstable

2015-07-28 Thread Michael Paquier
On Tue, Jul 28, 2015 at 5:57 PM, Christoph Berg  wrote:
> for something between 10% and 20% of the devel builds for apt.postgresql.org
> (which happen every 6h if there's a git change, so it happens every few days),
> I'm seeing this:
> Dubious, test returned 1 (wstat 256, 0x100)
> Failed 1/8 subtests
>
> I don't have the older logs available, but from memory, the subtest
> failing and the two numbers mentioned are always the same.

There should be some output logs in src/bin/pg_rewind/tmp_check/log/*?
Could you attach them here if you have them? That would be helpful to
understand what is happening.
-- 
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] Autonomous Transaction is back

2015-07-28 Thread Craig Ringer
On 23 July 2015 at 13:31, Rajeev rastogi  wrote:

> 1.The autonomous transaction treated as a completely different
> transaction from the master transaction.

Personally I think that's a lot more useful than having the inner tx
able to see the outer tx's uncommitted changes.

> 2.It should be allowed to deadlock with master transaction. We
> need to work-out a solution to avoid deadlock.

The deadlock case in autonomous tx's is a bit different.

Assuming you don't intend to allow interleaving, where you can switch
between transactions at will rather than just at begin/commit, the
only way a deadlock can happen is when the outer tx holds a lock that
the inner tx tries to acquire.

That should be practical to special-case by maintaining a list of
parent transaction (virtual?) transaction IDs. Attempts to wait on a
lock held by any of those should fail immediately. There's no point
waiting for the deadlock detector since the outer tx can never
progress and commit/rollback to release locks, and it might not be
able to see the parent/child relationship from outside the backend
doing the nested tx anyway.

There's no need to check the parent list until we actually try to wait
on a lock, though I don't know whether it's practical to delay until
then.

-- 
 Craig Ringer   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] proposal: multiple psql option -c

2015-07-28 Thread Pavel Stehule
2015-07-28 10:43 GMT+02:00 Marc Mamin :

>
> >
> >
> >2015-07-28 5:24 GMT+02:00 Pavel Stehule :
> >
> >2015-07-27 21:57 GMT+02:00 Andrew Dunstan :
> >
> >On 07/27/2015 02:53 PM, Pavel Stehule wrote:
> >
> >I am trying to run parallel execution
> >
> >psql -At -c "select datname from pg_database" postgres |
> xargs -n 1 -P 3 psql -c "select current_database()"
> >
> >
> >
> >I don't think it's going to be a hugely important feature, but I
> don't see a problem with creating a new option (-C seems fine) which would
> have the same effect as if the arguments were contatenated into a file
> which is then used with -f. IIRC -c has some special characteristics which
> means it's probably best not to try to extend it for this feature.
> >
> >
> >ok, I'll try to write patch.
> >
> >
> >I have a question. Can be -C option multiple?
>
>
> hello,
> Have you thought of how to support -1 along with -C ?
>
> > handle the input as with -f
>  that is, -1 -C would be equivalent to -c
>
> and
> psql -1 -C "sql_1; sql_2;" -C "sql_3; sql_4;"
>
> => ?
>
> BEGIN;
> sql_1;
> sql_2;
> END;
>
> BEGIN;
> sql_3;
> sql_4;
> END;
>
> thoughts ?
>

"-1" option is global -, I expected so following steps are more natural

BEGIN
  sql_1;
  sql_2;
  sql_3;
  sql_4;
END;

Regards

pavel




>
> The same logic could be added to -f
> although I see less advantages as with adding -C
>
> psql -1 -f "file1, file2" -f "file3, file4"
>
> regards,
> Marc Mamin
>


Re: [HACKERS] proposal: multiple psql option -c

2015-07-28 Thread Craig Ringer
On 17 July 2015 at 03:42, Pavel Stehule  wrote:
> Hi
>
> can we support multiple "-c" option?
>
> Why? Because some statements like VACUUM cannot be used together with any
> other statements with single -c option. The current solution is using echo
> and pipe op, but it is a complication in some complex scripts - higher
> complication when you run psql via multiple sudo statement.
>
> Example:
>
> psql -c "select pg_stat_reset()" -c "vacuum full analyze" dbname

I don't see the point. Taking your 'sudo' issue into account, just:

sudo -u postgres psql <'__END__'
select pg_stat_reset();
vacuum full analyze;
__END__


or

echo -e 'select pg_stat_reset()\n vacuum full analyze;' | sudo -u postgres psql

or, of course, just run two commands.

There are plenty of existing ways to do this. Personally I find -c
awkward due to the need to worry about shell quoting and tend to
prefer a quoted here-document lots of the time anyway.

-- 
 Craig Ringer   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [DESIGN] ParallelAppend

2015-07-28 Thread Kouhei Kaigai
> On Tue, Jul 28, 2015 at 7:59 AM, Kouhei Kaigai  wrote:
> >
> > > -Original Message-
> > > From: pgsql-hackers-ow...@postgresql.org
> > > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Kouhei Kaigai
> > > Sent: Monday, July 27, 2015 11:07 PM
> > > To: Amit Kapila
> > > >
> > > > Is there a real need to have new node like ParallelAppendPath?
> > > > Can't we have Funnel node beneath AppendNode and then each
> > > > worker will be responsible to have SeqScan on each inherited child
> > > > relation.  Something like
> > > >
> > > > Append
> > > >---> Funnel
> > > >   --> SeqScan rel1
> > > >   --> SeqScan rel2
> > > >
> > > If Funnel can handle both of horizontal and vertical parallelism,
> > > it is a great simplification. I never stick a new node.
> > >
> > > Once Funnel get a capability to have multiple child nodes, probably,
> > > Append node above will have gone. I expect set_append_rel_pathlist()
> > > add two paths based on Append and Funnel, then planner will choose
> > > the cheaper one according to its cost.
> > >
> > In the latest v16 patch, Funnel is declared as follows:
> >
> >   typedef struct Funnel
> >   {
> >   Scanscan;
> >   int num_workers;
> >   } Funnel;
> >
> > If we try to add Append capability here, I expects the structure will
> > be adjusted as follows, for example:
> >
> >   typedef struct Funnel
> >   {
> >   Scanscan;
> >   List   *funnel_plans;
> >   List   *funnel_num_workers;
> >   } Funnel;
> >
> > As literal, funnel_plans saves underlying Plan nodes instead of the
> > lefttree. Also, funnel_num_workers saves number of expected workers
> > to be assigned on individual child plans.
> >
> 
> or shall we have a node like above and name it as FunnelAppend or
> AppenFunnel?
>
It is better to have smaller number of node types which are capable to
kick background workers because of simplification of path construction.

Let's assume the case below. When planner considers a path to append
child scans on rel1, rel2 and rel3 but the cheapest path of rel2 is
Funnel+PartialSeqScan, we cannot put Funnel here unless we don't pull
up Funnel of rel2, can we?

  (Append? or Funnel)
   --> SeqScan on rel1
   --> Funnel
--> PartialSeqScan on rel2
   --> IndexScan on rel3

If we pull Funnel here, I think the plan shall be as follows:
  Funnel
   --> SeqScan on rel1
   --> PartialSeqScan on rel2
   --> IndexScan on rel3

If all we have to pay attention is Funnel node, it makes the code
around path construction and pull-up logic much simpler, rather than
multiple node types can kick background workers.

> > Even though create_parallelscan_paths() in v16 set num_workers not
> > larger than parallel_seqscan_degree, total number of the concurrent
> > background workers may exceed this configuration if more than two
> > PartialSeqScan nodes are underlying.
> > It is a different configuration from max_worker_processes, so it is
> > not a matter as long as we have another restriction.
> > However, how do we control the cap of number of worker processes per
> > "appendable" Funnel node? For example, if a parent table has 200
> > child tables but max_worker_processes are configured to 50.
> > It is obviously impossible to launch all the background workers
> > simultaneously. One idea I have is to suspend launch of some plans
> > until earlier ones are completed.
> >
> 
> Okay, but I think in that idea you need to re-launch the workers again for
> new set of relation scan's which could turn out to be costly, how about
> designing some way where workers after completing their assigned work
> check for new set of task/'s (which in this case would be to scan a new) and
> then execute the same.  I think in this way we can achieve dynamic allocation
> of work and achieve maximum parallelism with available set of workers.
> We have achieved this in ParallelSeqScan by scanning at block level, once
> a worker finishes a block, it checks for new block to scan.
>
Is it possible to put multiple PlannedStmt on TOC, isn't it?
If background worker picks up an uncompleted PlannedStmt first
(based on round-robin likely?), it may achieve the maximum
parallelism. Yep, it seems to me a good idea which I want to try.
If (num of worker) > (num of sub-plans), some of sub-plans can
have multiple workers from the beginning, then, other workers
also help to execute heavy plans later.
It may be better to put PlannedStmt in order of total_cost to
bias multi-workers execution from the beginning.

TODO: Even if a heavy query occupied most of available worker slots,
another session wants to use parallel execution later but during
execution of the primary query. We may need to have a 'scoreboard'
on shared memory to know how many workers are potentially needed
and how much ones are overused by somebody. If someone overconsumed
background workers, it should exit first, rather than picking up
the next PlannedStmt.

> > > We will need to pay attention

Re: [HACKERS] pg_dump -Fd and compression level

2015-07-28 Thread Andrew Dunstan


On 07/27/2015 03:52 AM, Marc Mamin wrote:

As per attached patch.

Comments?

It seems that the first test on the compression in pg_backup_tar.c is now 
obsolete.
It didn't make much sense anyway.



211 if (AH->compression < 0 || AH->compression > 9)
212 AH->compression = Z_DEFAULT_COMPRESSION;
213
214 /* Don't compress into tar files unless asked to do so */
215 if (AH->compression == Z_DEFAULT_COMPRESSION)
216 AH->compression = 0;
217
218 /*
219  * We don't support compression because reading the files back 
is not
220  * possible since gzdopen uses buffered IO which totally screws 
file
221  * positioning.
222  */
223 if (AH->compression != 0)
224 exit_horribly(modulename,
225  "compression is not supported by tar archive 
format\n");
226 }



In fact, the first two tests look unnecessary. Neither condition should
be possible now.


Hello,

Isn't the second test still required if you call pg_dump -Ft without setting 
-Z0 explicitly ?
(=> AH->compression == Z_DEFAULT_COMPRESSION)




No. Z_DEFAULT_COMPRESSION is only set for directory and custom archive 
types. See pg_dump.c at lines 578-592.



cheers

andrew



--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] proposal: multiple psql option -c

2015-07-28 Thread Marc Mamin

>
>
>2015-07-28 10:43 GMT+02:00 Marc Mamin :
>
>
>>
>>
>>2015-07-28 5:24 GMT+02:00 Pavel Stehule :
>>
>>2015-07-27 21:57 GMT+02:00 Andrew Dunstan :
>>
>>On 07/27/2015 02:53 PM, Pavel Stehule wrote:
>>
>>I am trying to run parallel execution
>>
>>psql -At -c "select datname from pg_database" postgres | 
> xargs -n 1 -P 3 psql -c "select current_database()"
>>
>>
>>
>>I don't think it's going to be a hugely important feature, but I 
> don't see a problem with creating a new option (-C seems fine) which would 
> have the same effect as if the arguments were contatenated into a file which 
> is then used with -f. IIRC -c has some special characteristics which means 
> it's probably best not to try to extend it for this feature.
>>
>>
>>ok, I'll try to write patch.
>>
>>
>>I have a question. Can be -C option multiple?
>
>
>hello,
>Have you thought of how to support -1 along with -C ?
>
>> handle the input as with -f
> that is, -1 -C would be equivalent to -c
>
>and
>psql -1 -C "sql_1; sql_2;" -C "sql_3; sql_4;"
>
>=> ?
>
>BEGIN;
>sql_1;
>sql_2;
>END;
>
>BEGIN;
>sql_3;
>sql_4;
>END;
>
>thoughts ?
>
>
>"-1" option is global -, I expected so following steps are more natural
>
>BEGIN
>  sql_1;
>  sql_2;
>  sql_3;
>  sql_4;
>END;

This is then exactly the same as -c.
If introducing multiple -C to better manage transaction handling,
why not enrich this new feature with the abilities to define batches of 
transactions ?

Marc


Re: [HACKERS] [DESIGN] ParallelAppend

2015-07-28 Thread Kouhei Kaigai
> KaiGai-san,
> 
> On 2015-07-27 PM 11:07, Kouhei Kaigai wrote:
> >
> >   Append
> >--> Funnel
> > --> PartialSeqScan on rel1 (num_workers = 4)
> >--> Funnel
> > --> PartialSeqScan on rel2 (num_workers = 8)
> >--> SeqScan on rel3
> >
> >  shall be rewritten to
> >   Funnel
> > --> PartialSeqScan on rel1 (num_workers = 4)
> > --> PartialSeqScan on rel2 (num_workers = 8)
> > --> SeqScan on rel3(num_workers = 1)
> >
> 
> In the rewritten plan, are respective scans (PartialSeq or Seq) on rel1,
> rel2 and rel3 asynchronous w.r.t each other? Or does each one wait for the
> earlier one to finish? I would think the answer is no because then it
> would not be different from the former case, right? Because the original
> premise seems that (partitions) rel1, rel2, rel3 may be on different
> volumes so parallelism across volumes seems like a goal of parallelizing
> Append.
> 
> From my understanding of parallel seqscan patch, each worker's
> PartialSeqScan asks for a block to scan using a shared parallel heap scan
> descriptor that effectively keeps track of division of work among
> PartialSeqScans in terms of blocks. What if we invent a PartialAppend
> which each worker would run in case of a parallelized Append. It would use
> some kind of shared descriptor to pick a relation (Append member) to scan.
> The shared structure could be the list of subplans including the mutex for
> concurrency. It doesn't sound as effective as proposed
> ParallelHeapScanDescData does for PartialSeqScan but any more granular
> might be complicated. For example, consider (current_relation,
> current_block) pair. If there are more workers than subplans/partitions,
> then multiple workers might start working on the same relation after a
> round-robin assignment of relations (but of course, a later worker would
> start scanning from a later block in the same relation). I imagine that
> might help with parallelism across volumes if that's the case.
>
I initially thought ParallelAppend kicks fixed number of background workers
towards sub-plans, according to the estimated cost on the planning stage.
However, I'm now inclined that background worker picks up an uncompleted
PlannedStmt first. (For more details, please see the reply to Amit Kapila)
It looks like less less-grained worker's job distribution.
Once number of workers gets larger than number of volumes / partitions,
it means more than two workers begin to assign same PartialSeqScan, thus
it takes fine-grained job distribution using shared parallel heap scan.

> MergeAppend
> parallelization might involve a bit more complication but may be feasible
> with a PartialMergeAppend with slightly different kind of coordination
> among workers. What do you think of such an approach?
>
Do we need to have something special in ParallelMergeAppend?
If individual child nodes are designed to return sorted results,
what we have to do seems to me same.

Thanks,
--
NEC Business Creation Division / PG-Strom Project
KaiGai Kohei 


-- 
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: multiple psql option -c

2015-07-28 Thread Andrew Dunstan


On 07/28/2015 04:43 AM, Marc Mamin wrote:


>
>
>2015-07-28 5:24 GMT+02:00 Pavel Stehule :
>
>2015-07-27 21:57 GMT+02:00 Andrew Dunstan :
>
>On 07/27/2015 02:53 PM, Pavel Stehule wrote:
>
>I am trying to run parallel execution
>
>psql -At -c "select datname from pg_database" postgres | 
xargs -n 1 -P 3 psql -c "select current_database()"

>
>
>
>I don't think it's going to be a hugely important feature, 
but I don't see a problem with creating a new option (-C seems fine) 
which would have the same effect as if the arguments were contatenated 
into a file which is then used with -f. IIRC -c has some special 
characteristics which means it's probably best not to try to extend it 
for this feature.

>
>
>ok, I'll try to write patch.
>
>
>I have a question. Can be -C option multiple?


hello,
Have you thought of how to support -1 along with -C ?

> handle the input as with -f
 that is, -1 -C would be equivalent to -c

and
psql -1 -C "sql_1; sql_2;" -C "sql_3; sql_4;"

=> ?

BEGIN;
sql_1;
sql_2;
END;

BEGIN;
sql_3;
sql_4;
END;

thoughts ?

The same logic could be added to -f
although I see less advantages as with adding -C

psql -1 -f "file1, file2" -f "file3, file4"




This is way too complex and baroque. -1 should be global. Multiple -C 
options should be concatenated. -f should not be touched.


cheers

andrew


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] proposal: multiple psql option -c

2015-07-28 Thread Andrew Dunstan


On 07/28/2015 12:08 AM, Pavel Stehule wrote:



2015-07-28 5:24 GMT+02:00 Pavel Stehule >:




2015-07-27 21:57 GMT+02:00 Andrew Dunstan mailto:and...@dunslane.net>>:


On 07/27/2015 02:53 PM, Pavel Stehule wrote:





I am trying to run parallel execution

psql -At -c "select datname from pg_database" postgres |
xargs -n 1 -P 3 psql -c "select current_database()"




I don't think it's going to be a hugely important feature, but
I don't see a problem with creating a new option (-C seems
fine) which would have the same effect as if the arguments
were contatenated into a file which is then used with -f. IIRC
-c has some special characteristics which means it's probably
best not to try to extend it for this feature.


ok, I'll try to write patch.


I have a question. Can be -C option multiple?

The SQL is without problem, but what about \x command?

postgres=# \dt \dn select 10;
No relations found.
List of schemas
┌──┬───┐
│ Name │ Owner │
╞══╪═══╡
└──┴───┘
(0 rows)

\dn: extra argument "10;" ignored



I don't understand the question.

You should include one sql or psql command per -C option, ISTM. e.g.

psql -C '\dt' -C '\dn' -C 'select 10;'


Isn't that what we're talking about with this whole proposal?

cheers

andrew




--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] MultiXact member wraparound protections are now enabled

2015-07-28 Thread Robert Haas
On Sat, Jul 25, 2015 at 4:11 AM, Simon Riggs  wrote:
> On 22 July 2015 at 21:45, Robert Haas  wrote:
>> But it seemed to me that this could be rather confusing.  I thought it
>> would be better to be explicit about whether the protections are
>> enabled in all cases.  That way, (1) if you see the message saying
>> they are enabled, they are enabled; (2) if you see the message saying
>> they are disabled, they are disabled; and (3) if you see neither
>> message, your version does not have those protections.
>
> (3) would imply that we can't ever remove the message, in case people think
> they are unprotected.
>
> If we display (1) and then we find a further bug, where does that leave us?
> Do we put a second "really, really fixed" message?
>
> AIUI this refers to a bug fix, its not like we've invented some anti-virus
> mode to actively prevent or even scan for further error. I'm not sure why we
> need a message to say a bug fix has been applied; that is what the release
> notes are for.
>
> If something is disabled, we should say so, but otherwise silence means
> safety and success.

Well, I think that we can eventually downgrade or remove the message
once (1) we've actually fixed all of the known multixact bugs and (2)
a couple of years have gone by and most people are in the clear.  But
right now, we've still got significant bugs unfixed.

https://wiki.postgresql.org/wiki/MultiXact_Bugs

Therefore, in my opinion, anything that might make it harder to debug
problems with the MultiXact system is premature at this point.  The
detective work that it took to figure out the chain of events that led
to the problem fixed in 068cfadf9e2190bdd50a30d19efc7c9f0b825b5e was
difficult; I wanted to make sure that future debugging would be
easier, not harder.  I still think that's the right decision, but I
recognize that not everyone agrees.

-- 
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] [DESIGN] ParallelAppend

2015-07-28 Thread Kouhei Kaigai
> On 27 July 2015 at 21:09, Kyotaro HORIGUCHI 
> wrote:
> 
> 
>   Hello, can I ask some questions?
> 
>   I suppose we can take this as the analog of ParalleSeqScan.  I
>   can see not so distinction between Append(ParalleSeqScan) and
>   ParallelAppend(SeqScan). What difference is there between them?
> 
>   If other nodes will have the same functionality as you mention at
>   the last of this proposal, it might be better that some part of
>   this feature is implemented as a part of existing executor
>   itself, but not as a deidicated additional node, just as my
>   asynchronous fdw execution patch patially does. (Although it
>   lacks planner part and bg worker launching..) If that is the
>   case, it might be better that ExecProcNode is modified so that it
>   supports both in-process and inter-bgworker cases by the single
>   API.
> 
>   What do you think about this?
> 
> 
> 
> I have to say that I really like the thought of us having parallel enabled 
> stuff
> in Postgres, but I also have to say that I don't think inventing all these 
> special
> parallel node types is a good idea. If we think about everything that we can
> parallelise...
> 
> Perhaps sort, hash join, seqscan, hash, bitmap heap scan, nested loop. I 
> don't
> want to debate that, but perhaps there's more, perhaps less.
> Are we really going to duplicate all of the code and add in the parallel stuff
> as new node types?
>
> My other concern here is that I seldom hear people talk about the planner's
> architectural lack of ability to make a good choice about how many parallel 
> workers
> to choose. Surely to properly calculate costs you need to know the exact 
> number
> of parallel workers that will be available at execution time, but you need to
> know this at planning time!? I can't see how this works, apart from just being
> very conservative about parallel workers, which I think is really bad, as many
> databases have busy times in the day, and also quiet times, generally quiet 
> time
> is when large batch stuff gets done, and that's the time that parallel stuff 
> is
> likely most useful. Remember queries are not always planned just before 
> they're
> executed. We could have a PREPAREd query, or we could have better plan caching
> in the future, or if we build some intelligence into the planner to choose a 
> good
> number of workers based on the current server load, then what's to say that 
> the
> server will be under this load at exec time? If we plan during a quiet time, 
> and
> exec in a busy time all hell may break loose.
>
Even though it is not easy to estimate available workers at planning time,
it might be possible to define a "target" number of workers to run.
If Funnel cannot get enough number of workers less than target, my preference
is to tell other workers (via scoreboard?) not to pick up next PlannedStmt and
exit when another Funnel cannot launch enough number of workers.

> I really do think that existing nodes should just be initialized in a parallel
> mode, and each node type can have a function to state if it supports 
> parallelism
> or not.
> 
> I'd really like to hear more opinions in the ideas I discussed here:
> 
> http://www.postgresql.org/message-id/CAApHDvp2STf0=pQfpq+e7WA4QdYmpFM5qu_YtU
> pe7r0jlnh...@mail.gmail.com
> 
> This design makes use of the Funnel node that Amit has already made and allows
> more than 1 node to be executed in parallel at once.
> 
> It appears that parallel enabling the executor node by node is fundamentally 
> locked
> into just 1 node being executed in parallel, then perhaps a Funnel node 
> gathering
> up the parallel worker buffers and streaming those back in serial mode. I 
> believe
> by design, this does not permit a whole plan branch from executing in parallel
> and I really feel like doing things this way is going to be very hard to undo
> and improve later. I might be too stupid to figure it out, but how would 
> parallel
> hash join work if it can't gather tuples from the inner and outer nodes in 
> parallel?
>
Hash-Join and Nest-Loop should not have PartialSeqScan in the inner-side, but
outer side can be PartialSeqScan under the Funnel node.
In case of Hash-Join, SeqScan of inner-side loads any tuples (*1) to hash-table
once, then records come from outer-side shall be combined with the hash-table.
Even though inner-side is read redundantly, advantage of parallel join will win
as long as inner-side is enough small; This assumption is right on usual pair of
master tables (small) and fact table (big).


(*1) Our colleague is now working on this feature. It enables to drop 
unnecessary
rows under the partitioned tables. So, we may not need to have entire hash table
for each background workers.
http://www.postgresql.org/message-id/9a28c8860f777e439aa12e8aea7694f8010f6...@bpxm15gp.gisp.nec.co.jp

Thanks,
--
NEC Business Creation Division / PG-Strom Project
KaiGai Kohei 

-- 
Sent via pgsql-hackers mailing l

Re: [HACKERS] Planner debug views

2015-07-28 Thread Alvaro Herrera
Qingqing Zhou wrote:

> Attached is a draft patch implementing the idea. To play with it, you
> shall create the follow two foreign tables:
> CREATE EXTENSION file_fdw;
> CREATE SERVER pglog FOREIGN DATA WRAPPER file_fdw;
> create foreign table pg_planner_rels(rel text, content text)server
> pglog options(filename '/data/debug_planner_relopt.csv',
> format 'csv');
> create foreign table pg_planner_paths(rel text, path text, replacedby
> text, reason int, startupcost float, totalcost float, cheapest text,
> innerp text, outerp text, content text) server pglog options(filename
> '/data/debug_planner_paths.csv', format 'csv');

I think this is a pretty neat idea, but I'm not sure this user interface
is a good one.  Why not have a new option for EXPLAIN, so you would call
"EXPLAIN (planner_stuff=on)" and it returns this as a resultset?  This
idea of creating random CSV files seems odd and inconvenient in the long
run.  For instance it fails if you have two sessions doing it
simultaneously; you could tack the process ID at the end of the file
name to prevent that problem, but then the foreign table breaks each
time.

-- 
Á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] LWLock deadlock and gdb advice

2015-07-28 Thread Andres Freund
Hi,

On 2015-07-19 11:49:14 -0700, Jeff Janes wrote:
> After applying this patch to commit fdf28853ae6a397497b79f, it has survived
> testing long enough to convince that this fixes the problem.

What was the actual workload breaking with the bug? I ran a small
variety and I couldn't reproduce it yet. I'm not saying there's no bug,
I just would like to be able to test my version of the fixes...

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] pgbench stats per script & other stuff

2015-07-28 Thread Fabien COELHO


v6 is just a rebase after a bug fix by Andres Freund.

Also a small question: The patch currently displays pgbench scripts 
starting numbering at 0. Probably a little too geek... should start at 1?


--
Fabien.diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml
index 2517a3a..99670d4 100644
--- a/doc/src/sgml/ref/pgbench.sgml
+++ b/doc/src/sgml/ref/pgbench.sgml
@@ -261,6 +261,23 @@ pgbench  options  dbname
 benchmarking arguments:
 
 
+ 
+  -b scriptname[@weight]
+  --builtin scriptname[@weight]
+  
+   
+Add the specified builtin script to the list of executed scripts.
+An optional integer weight after @ allows to adjust the
+probability of drawing the test.
+Available builtin scripts are: tpcb-like,
+simple-update and select-only.
+The provided scriptname needs only to be a prefix
+of the builtin name, hence simp would be enough to select
+simple-update.
+   
+  
+ 
+
 
  
   -c clients
@@ -307,14 +324,15 @@ pgbench  options  dbname
  
 
  
-  -f filename
-  --file=filename
+  -f filename[@weight]
+  --file=filename[@weight]
   

-Read transaction script from filename.
+Add a transaction script read from filename to
+the list of executed scripts.
+An optional integer weight after @ allows to adjust the
+probability of drawing the test.
 See below for details.
--N, -S, and -f
-are mutually exclusive.

   
  
@@ -404,10 +422,7 @@ pgbench  options  dbname
   --skip-some-updates
   

-Do not update pgbench_tellers and
-pgbench_branches.
-This will avoid update contention on these tables, but
-it makes the test case even less like TPC-B.
+Shorthand for -b simple-update@1.

   
  
@@ -499,9 +514,9 @@ pgbench  options  dbname
 Report the specified scale factor in pgbench's
 output.  With the built-in tests, this is not necessary; the
 correct scale factor will be detected by counting the number of
-rows in the pgbench_branches table.  However, when testing
-custom benchmarks (-f option), the scale factor
-will be reported as 1 unless this option is used.
+rows in the pgbench_branches table.
+However, when testing only custom benchmarks (-f option),
+the scale factor will be reported as 1 unless this option is used.

   
  
@@ -511,7 +526,7 @@ pgbench  options  dbname
   --select-only
   

-Perform select-only transactions instead of TPC-B-like test.
+Shorthand for -b select-only@1.

   
  
@@ -567,6 +582,16 @@ pgbench  options  dbname
  
 
  
+  --per-script-stats
+  
+   
+Report some statistics per script run by pgbench.
+   
+  
+ 
+
+
+ 
   --sampling-rate=rate
   

@@ -661,7 +686,20 @@ pgbench  options  dbname
   What is the Transaction Actually Performed in pgbench?
 
   
-   The default transaction script issues seven commands per transaction:
+   Pgbench executes test scripts chosen randomly from a specified list.
+   They include built-in scripts with -b and
+   user-provided custom scripts with -f.
+   Each script may be given a relative weight specified after a
+   @ so as to change its drawing probability.
+   The default weight is 1.
+ 
+
+  
+   The default builtin transaction script (also invoked with -b tpcb-like)
+   issues seven commands per transaction over randomly chosen aid,
+   tid, bid and balance.
+   The scenario is inspired by the TPC-B benchmark, but is not actually TPC-B,
+   hence the name.
   
 
   
@@ -675,9 +713,15 @@ pgbench  options  dbname
   
 
   
-   If you specify -N, steps 4 and 5 aren't included in the
-   transaction.  If you specify -S, only the SELECT is
-   issued.
+   If you select the simple-update builtin (also -N),
+   steps 4 and 5 aren't included in the transaction.
+   This will avoid update contention on these tables, but
+   it makes the test case even less like TPC-B.
+  
+
+  
+   If you select the select-only builtin (also -S),
+   only the SELECT is issued.
   
  
 
@@ -689,10 +733,7 @@ pgbench  options  dbname
benchmark scenarios by replacing the default transaction script
(described above) with a transaction script read from a file
(-f option).  In this case a transaction
-   counts as one execution of a script file.  You can even specify
-   multiple scripts (multiple -f options), in which
-   case a random one of the scripts is chosen each time a client session
-   starts a new transaction.
+   counts as one execution of a script file.
   
 
   
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index 3b8a80f..98a88f9 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -162,11 +162,10 @@ b

Re: [HACKERS] Improving log capture of TAP tests with IPC::Run

2015-07-28 Thread Andrew Dunstan


On 07/09/2015 06:29 AM, Heikki Linnakangas wrote:

On 07/09/2015 04:50 AM, Michael Paquier wrote:


Except that this patch looks good to me. Thanks for the black magic on
stdout/stderr handling.


Thanks, fixed the parenthesis and committed. The missing --debug is a 
separate issue.






What was the reason for not backpatching this? I have a fix for the 
execrable treatment of vpath builds, but it only applies to the tip 
branch because it relies on this.


cheers

andrew



--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] WIP: Make timestamptz_out less slow.

2015-07-28 Thread David Rowley
On 28 July 2015 at 19:10, Andres Freund  wrote:

> On 2015-07-28 10:59:15 +1200, David Rowley wrote:
> > It won't be quite as fast as what you've written, but I think it will be
> > much neater and more likely to be used in other places if we invent a
> > function like pg_ltoa() which returns a pointer to the new end of string.
> >
> > Also if we're specifying padding with zeros then we can skip the reverse
> > part that's in pg_ltoa(), (normally needed since the numeric string is
> > build in reverse)
> >
> > The code could then be written as:
> >
> > str = pg_int2str_pad(str, year, 4);
> > *str++ = '-';
> > str = pg_int2str_pad(str, tm->tm_mon, 2);
> > *str++ = '-';
> > str = pg_int2str_pad(str, tm->tm_mday, 2);
> >
> > etc
> >
> > I've used this method before and found it to be about 10 times faster
> than
> > snprintf(), but I was reversing the string, so quite likely it be more
> than
> > 10 times.
>
> Yes, that might be worthwhile to try. Certainly would look less
> ugly. Willing to give it a try?
>
>
I had a quick try at this and ended up just writing a small test program to
see what's faster.

Please excuse the mess of the file, I just hacked it together as quickly as
I could with the sole intention of just to get an idea of which is faster
and by how much.

For me the output is as follows:

timestamp_out() = 2015-07-29 02:24:33.34 in 3.506000
timestamp_out_old() = 2015-07-29 02:24:33.034 in 64.518000
timestamp_out_af() = 2015-07-29 02:24:33.034 in 2.981000

timestamp_out_old is master's version, the timestamp_out_af() is yours, and
timestamp_out() is my one. times are in seconds to perform 100 million
calls.

So it appears your version is a bit faster than mine, but we're both about
20 times faster than the current one.
Also mine needs fixed up as the fractional part is not padded the same as
yours, but I doubt that'll affect the performance by much.

My view: It's probably not worth going quite as far as you've gone for a
handful of nanoseconds per call, but perhaps something along the lines of
mine can be fixed up.

Have you thought about what to do when HAVE_INT64_TIMESTAMP is not defined?

Regards

David Rowley

--
 David Rowley   http://www.2ndQuadrant.com/

 PostgreSQL Development, 24x7 Support, Training & Services
#include 
#include 

struct pg_tm
{
	int			tm_sec;
	int			tm_min;
	int			tm_hour;
	int			tm_mday;
	int			tm_mon;			/* origin 0, not 1 */
	int			tm_year;		/* relative to 1900 */
	int			tm_wday;
	int			tm_yday;
	int			tm_isdst;
	long int	tm_gmtoff;
	const char *tm_zone;
};


char *
pg_uint2str_padding(char *str, unsigned int value, unsigned int padding)
{
	char	   *start = str;
	char	   *end = &str[padding];
	//Assert(padding > 0);
	
	*end = '\0';
	
	while (padding--)
	{
		str[padding] = value % 10 + '0';
		value /= 10;
	}
	
	return end;
}

char *
pg_uint2str(char *str, unsigned int value)
{
	char *start = str;
	char *end;
	/* Compute the result string backwards. */
	do
	{
		int		remainder;
		int		oldval = value;

		value /= 10;
		remainder = oldval - value * 10;
		*str++ = '0' + remainder;
	} while (value != 0);


	/* Add trailing NUL byte, and back up 'str' to the last character. */
	*str-- = '\0';
	end = str;
	
	/* Reverse string. */
	while (start < str)
	{
		char		swap = *start;

		*start++ = *str;
		*str-- = swap;
	}
	return end;
}


char *
timestamp_out_af(char *buffer, struct pg_tm *tm, unsigned int fsec)
{
	char *str = buffer;

	*str++ = (tm->tm_year / 1000) + '0';
	*str++ = (tm->tm_year / 100) % 10 + '0';
	*str++ = (tm->tm_year / 10) % 10 + '0';
	*str++ = tm->tm_year % 10 + '0';
	*str++ = '-';
	*str++ = (tm->tm_mon / 10) + '0';
	*str++ = tm->tm_mon % 10 + '0';
	*str++ = '-';
	*str++ = (tm->tm_mday / 10) + '0';
	*str++ = tm->tm_mday % 10 + '0';
	*str++ = ' ';
	*str++ = (tm->tm_hour / 10) + '0';
	*str++ = tm->tm_hour % 10 + '0';
	*str++ = ':';
	*str++ = (tm->tm_min / 10) + '0';
	*str++ = tm->tm_min % 10 + '0';
	*str++ = ':';
	*str++ = (tm->tm_sec / 10) + '0';
	*str++ = tm->tm_sec % 10 + '0';

	/*
	 * Yes, this is darned ugly and would look nicer in a loop,
	 * but some versions of gcc can't convert the divisions into
	 * more efficient instructions unless manually unrolled.
	 */
	if (fsec != 0)
	{
		int fseca = abs(fsec);

		*str++ = '.';

		if (fseca % 100 != 0)
		{
			*str++ = (fseca / 10) + '0';

			if (fseca % 10 != 0)
			{
*str++ = ((fseca / 1) % 10) + '0';

if (fseca % 1 != 0)
{
	*str++ = ((fseca / 1000) % 10) + '0';

	if (fseca % 1000 != 0)
	{
		*str++ = ((fseca / 100) % 10) + '0';

		if (fseca % 100 != 0)
		{
			*str++ = ((fseca / 10) % 10) + '0';

			if (fseca % 10 != 0)
			{
*str++ = (fseca % 10) + '0';
			}
		}
	}
}
			}
		}
	}
	
	return buffer;
}

char *
timestamp_out(char *buffer, struct pg_tm *tm, unsigned int fsec)
{
	char *str = buffer;
	
	str = pg_uint2str_padding(str, tm->tm_year, 4);
	*str++ = '-';
	str = pg_uint2str_paddi

Re: [HACKERS] WIP: Make timestamptz_out less slow.

2015-07-28 Thread Andres Freund
On 2015-07-29 03:10:41 +1200, David Rowley wrote:
> timestamp_out() = 2015-07-29 02:24:33.34 in 3.506000
> timestamp_out_old() = 2015-07-29 02:24:33.034 in 64.518000
> timestamp_out_af() = 2015-07-29 02:24:33.034 in 2.981000
>
> timestamp_out_old is master's version, the timestamp_out_af() is yours, and
> timestamp_out() is my one. times are in seconds to perform 100 million
> calls.

That looks good.

> So it appears your version is a bit faster than mine, but we're both about
> 20 times faster than the current one.
> Also mine needs fixed up as the fractional part is not padded the same as
> yours, but I doubt that'll affect the performance by much.

Worthwhile to finish that bit and try ;)

> My view: It's probably not worth going quite as far as you've gone for a
> handful of nanoseconds per call, but perhaps something along the lines of
> mine can be fixed up.

Yes, I agreee that your's is probably going to be fast enough.

> Have you thought about what to do when HAVE_INT64_TIMESTAMP is not defined?

I don't think it's actually important. The only difference vs float
timestamps is that in the latter case we set fsecs to zero BC.

Unless we want to slow down the common case it seems not unlikely that
we're going to end up with a separate slow path anyway. E.g. neither
your version nor mine handles 5 digit years (which is why I fell back to
the slow path in that case in my patch).

Regards,

Andres


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Sharing aggregate states between different aggregate functions

2015-07-28 Thread Heikki Linnakangas

On 07/28/2015 04:14 AM, David Rowley wrote:

On 27 July 2015 at 20:11, Heikki Linnakangas  wrote:


On 07/27/2015 08:34 AM, David Rowley wrote:


In this function I also wasn't quite sure if it was with comparing both
non-NULL INITCOND's here. I believe my code comments may slightly
contradict what the code actually does, as the comments talk about them
having to match, but the code just bails if any are non-NULL. The reason I
didn't check them was because it seems inevitable that some duplicate work
needs to be done when setting up the INITCOND. Perhaps it's worth it?


It would be nice to handle non-NULL initconds. I think you'll have to
check that the input function isn't volatile. Or perhaps just call the
input function, and check that the resulting Datum is byte-per-byte
identical, although that might be awkward to do with the current code
structure.


I've not done anything with this.
I'd not thought of an input function being volatile before, but I guess
it's possible, which makes me a bit scared that we could be treading on
ground we shouldn't be. I know it's more of an output function thing than
an input function thing, but a GUC like extra_float_digits could cause
problems here.


Yeah, a volatile input function seems highly unlikely, but who knows. 
BTW, we're also not checking if the transition or final functions are 
volatile. But that was the same before this patch too.


It sure would be nice to support the built-in float aggregates, so I 
took a stab at this. I heavily restructured the code again, so that 
there are now two separate steps. First, we check for any identical 
Aggrefs that could be shared. If that fails, we proceed to the 
permission checks, look up the transition function and build the initial 
datum. And then we call another function that tries to find an existing, 
compatible per-trans structure. I think this actually looks better than 
before, and checking for identical init values is now easy. This does 
lose one optimization: if there are two aggregates with identical 
transition functions and final functions, they are not merged into a 
single per-Agg struct. They still share the same per-Trans struct, 
though, and I think that's enough.


How does the attached patch look to you? The comments still need some 
cleanup, in particular, the explanations of the different scenarios 
don't belong where they are anymore.


BTW, the permission checks were not correct before. You cannot skip the 
check on the transition function when you're sharing the per-trans 
state. We check that the aggregate's owner has permission to execute the 
transition function, and the previous aggregate whose state value we're 
sharing might have different owner.



Hmm. I think it should be "AggStatePerTransData" then, to keep the same
pattern as AggStatePerAggData and AggStatePerGroupData.


Sounds good. I've renamed it to that in the attached delta patch.


Thanks!

- Heikki



sharing_aggstate-heikki-2.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] Improving log capture of TAP tests with IPC::Run

2015-07-28 Thread Heikki Linnakangas

On 07/28/2015 05:49 PM, Andrew Dunstan wrote:


On 07/09/2015 06:29 AM, Heikki Linnakangas wrote:

On 07/09/2015 04:50 AM, Michael Paquier wrote:


Except that this patch looks good to me. Thanks for the black magic on
stdout/stderr handling.


Thanks, fixed the parenthesis and committed. The missing --debug is a
separate issue.


What was the reason for not backpatching this? I have a fix for the
execrable treatment of vpath builds, but it only applies to the tip
branch because it relies on this.


No reason, other than the general only-backpatch-bug-fixes rule. Several 
people requested it, and it seems reasonable to me too, but it fell off 
my radar. Feel free to backpatch...


- Heikki


--
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: multiple psql option -c

2015-07-28 Thread Pavel Stehule
2015-07-28 15:16 GMT+02:00 Andrew Dunstan :

>
> On 07/28/2015 12:08 AM, Pavel Stehule wrote:
>
>>
>>
>> 2015-07-28 5:24 GMT+02:00 Pavel Stehule > pavel.steh...@gmail.com>>:
>>
>>
>>
>> 2015-07-27 21:57 GMT+02:00 Andrew Dunstan > >:
>>
>>
>> On 07/27/2015 02:53 PM, Pavel Stehule wrote:
>>
>>
>>
>>
>>
>> I am trying to run parallel execution
>>
>> psql -At -c "select datname from pg_database" postgres |
>> xargs -n 1 -P 3 psql -c "select current_database()"
>>
>>
>>
>>
>> I don't think it's going to be a hugely important feature, but
>> I don't see a problem with creating a new option (-C seems
>> fine) which would have the same effect as if the arguments
>> were contatenated into a file which is then used with -f. IIRC
>> -c has some special characteristics which means it's probably
>> best not to try to extend it for this feature.
>>
>>
>> ok, I'll try to write patch.
>>
>>
>> I have a question. Can be -C option multiple?
>>
>> The SQL is without problem, but what about \x command?
>>
>> postgres=# \dt \dn select 10;
>> No relations found.
>> List of schemas
>> ┌──┬───┐
>> │ Name │ Owner │
>> ╞══╪═══╡
>> └──┴───┘
>> (0 rows)
>>
>> \dn: extra argument "10;" ignored
>>
>
>
> I don't understand the question.
>
> You should include one sql or psql command per -C option, ISTM. e.g.
>
> psql -C '\dt' -C '\dn' -C 'select 10;'
>
>
> Isn't that what we're talking about with this whole proposal?
>


I am searching some agreement, how to solve a current "-c" limits. I am
100% for >>> psql -C '\dt' -C '\dn' -C 'select 10;' <<<

Regards

Pavel


>
> cheers
>
> andrew
>
>
>


Re: [HACKERS] LWLock deadlock and gdb advice

2015-07-28 Thread Jeff Janes
On Tue, Jul 28, 2015 at 7:06 AM, Andres Freund  wrote:

> Hi,
>
> On 2015-07-19 11:49:14 -0700, Jeff Janes wrote:
> > After applying this patch to commit fdf28853ae6a397497b79f, it has
> survived
> > testing long enough to convince that this fixes the problem.
>
> What was the actual workload breaking with the bug? I ran a small
> variety and I couldn't reproduce it yet. I'm not saying there's no bug,
> I just would like to be able to test my version of the fixes...
>

It was the torn-page fault-injection code here:

https://drive.google.com/open?id=0Bzqrh1SO9FcEfkxFb05uQnJ2cWg0MEpmOXlhbFdyNEItNmpuek1zU2gySGF3Vk1oYXNNLUE

It is not a minimal set, I don't know if all parts of this are necessary to
rerproduce it.  The whole crash-recovery cycling might not even be
important.

Compiled with:

./configure --enable-debug --with-libxml --with-perl --with-python
--with-ldap --with-openssl --with-gssapi
--prefix=/home/jjanes/pgsql/torn_bisect/

(Also with or without --enable-cassert).

I just ran "sh do.sh >& do.out" and eventually it stopped producing output,
and I find everything hung up.

Cheers,

Jeff


Re: [HACKERS] more RLS oversights

2015-07-28 Thread Joe Conway
On 07/03/2015 10:03 AM, Noah Misch wrote:
> (4) When DefineQueryRewrite() is about to convert a table to a view, it checks
> the table for features unavailable to views.  For example, it rejects tables
> having triggers.  It omits to reject tables having relrowsecurity or a
> pg_policy record.  Test case:

Please see the attached patch for this issue. Comments?

Thanks,

Joe

diff --git a/src/backend/commands/policy.c b/src/backend/commands/policy.c
index 17b48d4..aa858fa 100644
*** a/src/backend/commands/policy.c
--- b/src/backend/commands/policy.c
*** get_relation_policy_oid(Oid relid, const
*** 1002,1004 
--- 1002,1033 
  
  	return policy_oid;
  }
+ 
+ /*
+  * relation_has_policies - Determine if relation has any policies
+  */
+ bool
+ relation_has_policies(Relation rel)
+ {
+ 	Relation	catalog;
+ 	ScanKeyData skey;
+ 	SysScanDesc sscan;
+ 	HeapTuple	policy_tuple;
+ 	bool		ret = false;
+ 
+ 	catalog = heap_open(PolicyRelationId, AccessShareLock);
+ 	ScanKeyInit(&skey,
+ Anum_pg_policy_polrelid,
+ BTEqualStrategyNumber, F_OIDEQ,
+ ObjectIdGetDatum(RelationGetRelid(rel)));
+ 	sscan = systable_beginscan(catalog, PolicyPolrelidPolnameIndexId, true,
+ 			   NULL, 1, &skey);
+ 	policy_tuple = systable_getnext(sscan);
+ 	if (HeapTupleIsValid(policy_tuple))
+ 		ret = true;
+ 
+ 	systable_endscan(sscan);
+ 	heap_close(catalog, AccessShareLock);
+ 
+ 	return ret;
+ }
diff --git a/src/backend/rewrite/rewriteDefine.c b/src/backend/rewrite/rewriteDefine.c
index a88d73e..39c83a6 100644
*** a/src/backend/rewrite/rewriteDefine.c
--- b/src/backend/rewrite/rewriteDefine.c
***
*** 27,32 
--- 27,33 
  #include "catalog/objectaccess.h"
  #include "catalog/pg_rewrite.h"
  #include "catalog/storage.h"
+ #include "commands/policy.h"
  #include "miscadmin.h"
  #include "nodes/nodeFuncs.h"
  #include "parser/parse_utilcmd.h"
*** DefineQueryRewrite(char *rulename,
*** 410,420 
  		 *
  		 * If so, check that the relation is empty because the storage for the
  		 * relation is going to be deleted.  Also insist that the rel not have
! 		 * any triggers, indexes, or child tables.  (Note: these tests are too
! 		 * strict, because they will reject relations that once had such but
! 		 * don't anymore.  But we don't really care, because this whole
! 		 * business of converting relations to views is just a kluge to allow
! 		 * dump/reload of views that participate in circular dependencies.)
  		 */
  		if (event_relation->rd_rel->relkind != RELKIND_VIEW &&
  			event_relation->rd_rel->relkind != RELKIND_MATVIEW)
--- 411,422 
  		 *
  		 * If so, check that the relation is empty because the storage for the
  		 * relation is going to be deleted.  Also insist that the rel not have
! 		 * any triggers, indexes, child tables, policies, or RLS enabled.
! 		 * (Note: these tests are too strict, because they will reject
! 		 * relations that once had such but don't anymore.  But we don't
! 		 * really care, because this whole business of converting relations
! 		 * to views is just a kluge to allow dump/reload of views that
! 		 * participate in circular dependencies.)
  		 */
  		if (event_relation->rd_rel->relkind != RELKIND_VIEW &&
  			event_relation->rd_rel->relkind != RELKIND_MATVIEW)
*** DefineQueryRewrite(char *rulename,
*** 451,456 
--- 453,470 
  		 errmsg("could not convert table \"%s\" to a view because it has child tables",
  RelationGetRelationName(event_relation;
  
+ 			if (event_relation->rd_rel->relrowsecurity)
+ ereport(ERROR,
+ 		(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+ 		 errmsg("could not convert table \"%s\" to a view because it has row security enabled",
+ RelationGetRelationName(event_relation;
+ 
+ 			if (relation_has_policies(event_relation))
+ ereport(ERROR,
+ 		(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+ 		 errmsg("could not convert table \"%s\" to a view because it has row security policies",
+ RelationGetRelationName(event_relation;
+ 
  			RelisBecomingView = true;
  		}
  	}
diff --git a/src/include/commands/policy.h b/src/include/commands/policy.h
index ac322e0..be00043 100644
*** a/src/include/commands/policy.h
--- b/src/include/commands/policy.h
*** extern Oid get_relation_policy_oid(Oid r
*** 31,35 
--- 31,36 
  
  extern ObjectAddress rename_policy(RenameStmt *stmt);
  
+ extern bool relation_has_policies(Relation rel);
  
  #endif   /* POLICY_H */
diff --git a/src/test/regress/expected/rowsecurity.out b/src/test/regress/expected/rowsecurity.out
index 72361e8..2a8db6d 100644
*** a/src/test/regress/expected/rowsecurity.out
--- b/src/test/regress/expected/rowsecurity.out
*** SELECT * FROM coll_t;
*** 2908,2913 
--- 2908,2936 
  
  ROLLBACK;
  --
+ -- Converting table to view
+ --
+ BEGIN;
+ SET ROW_SECURITY = FORCE;
+ CREATE TABLE t (c int);
+ CREATE POLICY p ON t USING (c % 2 = 1);
+ 

Re: [HACKERS] Sharing aggregate states between different aggregate functions

2015-07-28 Thread Tom Lane
Heikki Linnakangas  writes:
> On 07/28/2015 04:14 AM, David Rowley wrote:
>> I'd not thought of an input function being volatile before, but I guess
>> it's possible, which makes me a bit scared that we could be treading on
>> ground we shouldn't be. I know it's more of an output function thing than
>> an input function thing, but a GUC like extra_float_digits could cause
>> problems here.

GUC dependence is considered to make a function stable not volatile.
(I realize you can probably break that if you try hard enough, but
then you get to keep both pieces.)

> Yeah, a volatile input function seems highly unlikely, but who knows. 

We have a project policy against volatile I/O functions.  One reason why
is that it would break the assumption that record_in/record_out can be
marked stable.  I think there are other reasons too.

> BTW, we're also not checking if the transition or final functions are 
> volatile. But that was the same before this patch too.

Up to now it hasn't mattered.  Possibly this patch should refuse to
combine states across volatile transition functions?

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] proposal: multiple psql option -c

2015-07-28 Thread Andrew Dunstan


On 07/28/2015 11:52 AM, Pavel Stehule wrote:



2015-07-28 15:16 GMT+02:00 Andrew Dunstan >:



On 07/28/2015 12:08 AM, Pavel Stehule wrote:



2015-07-28 5:24 GMT+02:00 Pavel Stehule
mailto:pavel.steh...@gmail.com>
>>:



2015-07-27 21:57 GMT+02:00 Andrew Dunstan
mailto:and...@dunslane.net>
>>:


On 07/27/2015 02:53 PM, Pavel Stehule wrote:





I am trying to run parallel execution

psql -At -c "select datname from pg_database"
postgres |
xargs -n 1 -P 3 psql -c "select current_database()"




I don't think it's going to be a hugely important
feature, but
I don't see a problem with creating a new option (-C seems
fine) which would have the same effect as if the arguments
were contatenated into a file which is then used with
-f. IIRC
-c has some special characteristics which means it's
probably
best not to try to extend it for this feature.


ok, I'll try to write patch.


I have a question. Can be -C option multiple?

The SQL is without problem, but what about \x command?

postgres=# \dt \dn select 10;
No relations found.
List of schemas
┌──┬───┐
│ Name │ Owner │
╞══╪═══╡
└──┴───┘
(0 rows)

\dn: extra argument "10;" ignored



I don't understand the question.

You should include one sql or psql command per -C option, ISTM. e.g.

psql -C '\dt' -C '\dn' -C 'select 10;'


Isn't that what we're talking about with this whole proposal?



I am searching some agreement, how to solve a current "-c" limits. I 
am 100% for >>> psql -C '\dt' -C '\dn' -C 'select 10;' <<<





I think you're probably best off leaving -c alone. If there are issues 
to be solved for -c they should be handled separately from the feature 
we agree on.


cheers

andrew





--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Re: Removing SSL renegotiation (Was: Should we back-patch SSL renegotiation fixes?)

2015-07-28 Thread Andres Freund
Hi,

Attached are:

a) a slightly evolved version of Michael's patch disabling renegotiation
   by default that I'm planning to apply to 9.4 - 9.0

b) a patch removing renegotiation entirely from master and 9.5

Unless somebody protests soon I'm going to push something like that
after having dinner.

I am wondering whether b) ought to remove Port->count, but I'm currently
leaning to leaving it in place for now; perhaps adding a comment in the
struct.  I'm actually thinking we very well might want to add something
like it to all backends, but more importantly it'd make the diff larger
with mostly unrelated changes.

Regards,

Andres


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Re: Removing SSL renegotiation (Was: Should we back-patch SSL renegotiation fixes?)

2015-07-28 Thread Andres Freund
On 2015-07-28 18:59:02 +0200, Andres Freund wrote:
> Attached are:
> 
> a) a slightly evolved version of Michael's patch disabling renegotiation
>by default that I'm planning to apply to 9.4 - 9.0
> 
> b) a patch removing renegotiation entirely from master and 9.5
> 
> Unless somebody protests soon I'm going to push something like that
> after having dinner.
> 
> I am wondering whether b) ought to remove Port->count, but I'm currently
> leaning to leaving it in place for now; perhaps adding a comment in the
> struct.  I'm actually thinking we very well might want to add something
> like it to all backends, but more importantly it'd make the diff larger
> with mostly unrelated changes.

And really attached.
>From 768dd6560e262d7597d0efb37043a96e1594508e Mon Sep 17 00:00:00 2001
From: Andres Freund 
Date: Tue, 28 Jul 2015 18:41:32 +0200
Subject: [PATCH] Disable ssl renegotiation by default.

While postgres' use of SSL renegotiation is a good idea in theory, it
turned out to not work well in practice. The specification and openssl's
implementation of it have lead to several security issues. Postgres' use
of renegotiation also had its share of bugs.

Additionally OpenSSL has a bunch of bugs around renegotiation, reported
and open for years, that regularly lead to connections breaking with
obscure error messages. We tried increasingly complex workarounds around
these bugs, but we didn't find anything complete.

Since these connection breakages often lead to hard to debug problems,
e.g. spuriously failing base backups and significant latency spikes when
synchronous replication is used, we have decided to change the default
setting for ssl renegotiation to 0 (disabled) in the released
backbranches and remove it entirely in 9.5 and master..

Author: Michael Paquier, with changes by me
Discussion: 20150624144148.gq4...@alap3.anarazel.de
Backpatch: 9.0-9.4; 9.5 and master get a different patch
---
 doc/src/sgml/config.sgml  | 10 +-
 src/backend/utils/misc/guc.c  |  2 +-
 src/backend/utils/misc/postgresql.conf.sample |  2 +-
 3 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index c669f75..871b04a 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -1040,7 +1040,7 @@ include_dir 'conf.d'
 cryptanalysis when large amounts of traffic can be examined, but it
 also carries a large performance penalty. The sum of sent and received
 traffic is used to check the limit. If this parameter is set to 0,
-renegotiation is disabled. The default is 512MB.
+renegotiation is disabled. The default is 0.


 
@@ -1052,6 +1052,14 @@ include_dir 'conf.d'
  disabled.
 

+
+   
+
+ Due to bugs in OpenSSL enabling ssl renegotiation, by
+ configuring a non-zero ssl_renegotiation_limit, is likely
+ to lead to problems like long-lived connections breaking.
+
+   
   
  
 
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 6ad0892..396c68b 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -2457,7 +2457,7 @@ static struct config_int ConfigureNamesInt[] =
 			GUC_UNIT_KB,
 		},
 		&ssl_renegotiation_limit,
-		512 * 1024, 0, MAX_KILOBYTES,
+		0, 0, MAX_KILOBYTES,
 		NULL, NULL, NULL
 	},
 
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index 8dfd485..3845d57 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -83,7 +83,7 @@
 	# (change requires restart)
 #ssl_prefer_server_ciphers = on		# (change requires restart)
 #ssl_ecdh_curve = 'prime256v1'		# (change requires restart)
-#ssl_renegotiation_limit = 512MB	# amount of data between renegotiations
+#ssl_renegotiation_limit = 0		# amount of data between renegotiations
 #ssl_cert_file = 'server.crt'		# (change requires restart)
 #ssl_key_file = 'server.key'		# (change requires restart)
 #ssl_ca_file = ''			# (change requires restart)
-- 
2.4.0.rc2.1.g3d6bc9a

>From 0d488c5f1d7eba48aa19894339c69488094878a1 Mon Sep 17 00:00:00 2001
From: Andres Freund 
Date: Tue, 28 Jul 2015 17:59:55 +0200
Subject: [PATCH] Remove ssl renegotiation support.

While postgres' use of SSL renegotiation is a good idea in theory, it
turned out to not work well in practice. The specification and openssl's
implementation of it have lead to several security issues. Postgres' use
of renegotiation also had its share of bugs.

Additionally OpenSSL has a bunch of bugs around renegotiation, reported
and open for years, that regularly lead to connections breaking with
obscure error messages. We tried increasingly complex workarounds around
these bugs, but we didn't find anything complete.

Since these connection breakages often lead to hard to debug problems,
e.g. spuriously failin

Re: [HACKERS] Sharing aggregate states between different aggregate functions

2015-07-28 Thread Heikki Linnakangas

On 07/28/2015 07:18 PM, Tom Lane wrote:

Heikki Linnakangas  writes:

On 07/28/2015 04:14 AM, David Rowley wrote:
Yeah, a volatile input function seems highly unlikely, but who knows.


We have a project policy against volatile I/O functions.  One reason why
is that it would break the assumption that record_in/record_out can be
marked stable.  I think there are other reasons too.


Ok. In the latest patch I'm not relying that anyway, so it doesn't 
matter, but good to know.



BTW, we're also not checking if the transition or final functions are
volatile. But that was the same before this patch too.


Up to now it hasn't mattered.


Yes, it has. We combine identical aggregates even without this patch. 
For example:


SELECT sum(x), sum(x) FROM foo

Sum(x) gets calculated only once. If its transition function or final 
function was volatile, that could produce two different results if we 
ran the aggregate twice.


No-one's complained so far, and I can't think of a use case for a 
volatile transition or final function, so maybe it's not worth worrying 
about. Then again, checking for the volatility of those functions would 
be easy too.


- Heikki



--
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] Sharing aggregate states between different aggregate functions

2015-07-28 Thread Tom Lane
Heikki Linnakangas  writes:
> On 07/28/2015 07:18 PM, Tom Lane wrote:
>> Heikki Linnakangas  writes:
>>> BTW, we're also not checking if the transition or final functions are
>>> volatile. But that was the same before this patch too.

>> Up to now it hasn't mattered.

> Yes, it has. We combine identical aggregates even without this patch. 

Ah, right, how'd I forget about that?

> No-one's complained so far, and I can't think of a use case for a 
> volatile transition or final function, so maybe it's not worth worrying 
> about. Then again, checking for the volatility of those functions would 
> be easy too.

Given the lack of complaints, I tend to agree that it's not the province
of this patch to make a change in that policy.

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] Shouldn't we document "don't use a mountpoint as $PGDATA"?

2015-07-28 Thread Tom Lane
I had a discussion with some folks at Red Hat about this:
https://bugzilla.redhat.com/show_bug.cgi?id=1247477

I had the idea that we had documented somewhere that the data directory
should not be a filesystem mount point, but I sure can't find it now.
Any objections to adding some text about this to section 17.2, near the
caveats about NFS?

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] Sequence Access Method WIP

2015-07-28 Thread Heikki Linnakangas
So, we have this patch in the commitfest again. Let's see where we are, 
and try to find a consensus on what needs to be done before this can be 
committed.


On 06/17/2015 06:51 PM, Petr Jelinek wrote:

On 2015-06-15 11:32, Vik Fearing wrote:

I've been looking at these patches a bit and here are some comments:


Thanks for looking at this.


+1, thanks Vik.


As mentioned upthread, this patch isn't a seamless replacement for
what's already there because of the amdata field.  I wasn't part of the
conversation of FOSDEM unfortunately, and there's not enough information
in this thread to know why this solution is preferred over each seqam
having its own table type with all the columns it needs.  I see that
Heikki is waffling a bit between the two, and I have a fairly strong
opinion that amdata should be split into separate columns.  The patch
already destroys and recreates what it needs when changing access method
via ALTER SEQUENCE, so I don't really see what the problem is.


FOSDEM was just about agreeing that amdata is simpler after we discussed
it with Heikki. Nothing too important you missed there I guess.

I can try to summarize what are the differences:
- amdata is somewhat simpler in terms of code for both init, alter and
DDL, since with custom columns you have to specify them somehow and deal
with them in catalog, also ALTER SEQUENCE USING means that there are
going to be colums marked as deleted which produces needless waste, etc
- amdata make it easier to change the storage model as the tuple
descriptor is same for all sequences
- the separate columns are much nicer from user point of view
- my opinion is that separate columns also more nicely separate state
from options and I think that if we move to separate storage model,
there can be state table per AM which solves the tuple descriptor issue
- there is probably some slight performance benefit to amdata but I
don't think it's big enough to be important

I personally have slight preference to separate columns design, but I am
ok with both ways honestly.


Regarding the amdata issue, I'm also leaning towards set of columns. 
I've felt that way all along, but not very strongly, so I relented at 
some point when Andres felt strongly that a single column would be 
better. But the more I think about it, the more I feel that separate 
columns really would be better. As evidence, I offer this recent thread:


Tom said 
(http://www.postgresql.org/message-id/8739.1436893...@sss.pgh.pa.us):

I really don't see what's wrong with "SELECT last_value FROM sequence",
especially since that has worked in every Postgres version since 6.x.
Anyone slightly worried about backwards compatibility wouldn't use
an equivalent function even if we did add one.


If we went with the single amdata column, that would break. Or we'd need 
to leave last_value as a separate column anyway, and leave it unused for 
sequence AMs where it's not applicable. But that's a bit ugly too.


Jim Nasby said in the same thread:

FWIW, I think it'd be better to have a pg_sequences view that's the
equivalent of SELECT * FROM  for every sequence in the
database. That would let you get whatever info you needed.


Creating such a view would be difficult if all the sequences have a 
different set of columns. But when you think about it, it's not really 
any better with a single amdata column. You can't easily access the data 
in the amdata column that way either.


Anyway, that's my opinion. Several others have weighed in to support 
separate columns, too, so I think that is the consensus. Separate 
columns it is.



There is no \d command for sequence access methods.  Without querying
pg_seqam directly, how does one discover what's available?


Good point.


Well, you can query pg_seqam. I don't think this deserves a \d command.


On the whole, I think this is a pretty good patchset.  Aside from the
design decision of whether amdata is a single opaque column or a set of
columns, there are only a few things that need to be changed before it's
ready for committer, and those things are mostly documentation.


Unfortunately the amdata being opaque vs set of columns is the main
issue here.


There was discussion on another thread on how the current sequence AM 
API is modeled after the indexam API, at 
http://www.postgresql.org/message-id/3896.1437059...@sss.pgh.pa.us. Will 
need to do something about that too.


Petr, is this enough feedback on this patch for this commitfest, or are 
there some other issues you want to discuss before I mark this as returned?


- Heikki



--
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] Shouldn't we document "don't use a mountpoint as $PGDATA"?

2015-07-28 Thread Josh Berkus
On 07/28/2015 11:01 AM, Tom Lane wrote:
> I had a discussion with some folks at Red Hat about this:
> https://bugzilla.redhat.com/show_bug.cgi?id=1247477
> 
> I had the idea that we had documented somewhere that the data directory
> should not be a filesystem mount point, but I sure can't find it now.
> Any objections to adding some text about this to section 17.2, near the
> caveats about NFS?

Nope.  This is one of those things which seems obvious, but isn't to
newcomers who aren't former sysadmins.

-- 
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.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] A little RLS oversight?

2015-07-28 Thread Joe Conway
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 07/28/2015 12:32 AM, Dean Rasheed wrote:
>> On 07/27/2015 03:05 PM, Stephen Frost wrote:
>>> AFK at the moment, but my thinking was that we should avoid
>>> having the error message change based on what a GUC is set to.
>>> I agree that there should be comments which explain that.
> 
> Except it's already dependent on the GUC if it's set to FORCE.

Dean is correct. See test case below...

>> I changed back to using GetUserId() for the call to
>> check_enable_rls() at those three call sites, and added to the
>> comments to explain why.
>> 
> 
> I'm not entirely convinced about this. The more I think about it,
> the more I think that if we know the user has BYPASSRLS, and
> they've set row_security to OFF, then they ought to get the more
> detailed error message, as they would if there was no RLS. That
> error detail is highly useful, and we know the user has been
> granted privilege by a superuser, and that they have direct access
> to the underlying table in this context, so we're not leaking any
> info that they cannot directly SELECT anyway.

Agreed -- this patch version goes back to using InvalidOid at those
three call sites and the behavior is what I now believe to be correct.
Here is a test case to illustrate:

8<
BEGIN;
CREATE ROLE alice;
CREATE ROLE bob WITH BYPASSRLS;

SET SESSION AUTHORIZATION alice;
CREATE TABLE t1 (id int primary key, f1 text);
INSERT INTO t1 VALUES(1,'a');
CREATE TABLE t2 (id int primary key, f1 text, t1_id int REFERENCES
t1(id));
GRANT ALL ON t2 TO bob;
ALTER TABLE t2 ENABLE ROW LEVEL SECURITY;
CREATE POLICY P ON t2 TO alice, bob USING (true);

SET SESSION AUTHORIZATION bob;
INSERT INTO t2 VALUES(1,'a',1); -- should succeed

SAVEPOINT q;
INSERT INTO t2 VALUES(2,'b',2); -- should fail, no details
ROLLBACK TO q;

SET row_security = OFF;
SAVEPOINT q;
INSERT INTO t2 VALUES(2,'b',2); -- should fail, full details
ROLLBACK TO q;

SET SESSION AUTHORIZATION alice;
SAVEPOINT q;
INSERT INTO t2 VALUES(2,'b',2); -- should fail, full details
ROLLBACK TO q;

SET row_security = FORCE;
SAVEPOINT q;
INSERT INTO t2 VALUES(2,'b',2); -- should fail, no details
ROLLBACK TO q;

ROLLBACK;
8<

I'm going to commit the attached in the next few hours unless someone
has serious objections. We can always revisit the specific behavior of
those messages separately if we change our minds...

Joe

-BEGIN PGP SIGNATURE-
Version: GnuPG v2.0.22 (GNU/Linux)

iQIcBAEBAgAGBQJVt8dQAAoJEDfy90M199hl1ncQAI/XUZ3VSoW0Vegf09Y2DqlJ
f8lGnwSf+djSXgKVrUsKQsuLn7c+Ac9fqoRUQJMuCcOvnu7auljzaMjuMjrXhOIC
hhiP8QyYUoEMF+5Sggh/A532rFXRbI1R/g9eu8TTT9vJGkITVMGAucizSY+fPiBg
gH3JyuCVaIZbvlVv0OkqPXPiP1VR/7bDTBcIbv56XHOk1AavNlUMW5BWWR0/9Mt8
VMh9ri3eQ5beKxrDhAZ+39ddlQzk9yJsN5pd/Pu0zPNxwBcvTNra0ZZGv7PoPwUF
7F98A1bL/NWFDdFuOI2E61/a5lA70t5HV4UTPPugQr82NhBS9JdpxgqA8W7B9+P9
4TKqmmYIvOcxM+TtglIlyr+JBwfJERw8j3+IcnM3mjLnkyflNbX2kbOF0B5Ghpt/
EzrVIJi/Pl3ctm+9r/oQYiwo/6Qsy8hco9QLCY4GVhBEE93Wr8P6NVlcjzyocMRs
FBjgvxeL/1wL8g3Q8ZDsAVOu9Ld0OCGEkA27XRS3sXbZfHroeTNW5aUqvKIzFkKB
gsr09pIVtdd7ysEdxxHZpELaU8H2rcA5O8b380HauIi41GaDc5E0XLXJSu6dIWCP
x/Em3qTpt74YgZiqsbs3a21Ak5n8fBdTMyXhmPQbXctllALI3Kj7bbyqoeGywpxi
PKhGDzgw+M7OQzfWS7UF
=e5Qo
-END PGP SIGNATURE-
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 98d1497..fd82ea4 100644
*** a/doc/src/sgml/func.sgml
--- b/doc/src/sgml/func.sgml
*** SET search_path TO schemaboolean
 does current user have privilege for role

+   
+row_security_active(table)
+
+boolean
+does current user have row level security active for table
+   
   
  
 
*** SET search_path TO schema
  pg_has_role
 
+
+ row_security_active
+
  
 
  has_table_privilege checks whether a user
*** SELECT has_function_privilege('joeuser',
*** 15462,15467 
--- 15471,15483 
  are immediately available without doing SET ROLE.
 
  
+
+ row_security_active checks whether row level
+ security is active for the specified table in the context of the
+ current_user and environment. The table can
+ be specified by name or by OID.
+
+ 

  shows functions that
 determine whether a certain object is visible in the
diff --git a/src/backend/access/index/genam.c b/src/backend/access/index/genam.c
index 1043362..aa5b28c 100644
*** a/src/backend/access/index/genam.c
--- b/src/backend/access/index/genam.c
*** BuildIndexValueDescription(Relation inde
*** 204,210 
  	Assert(indexrelid == idxrec->indexrelid);
  
  	/* RLS check- if RLS is enabled then we don't return anything. */
! 	if (check_enable_rls(indrelid, GetUserId(), true) == RLS_ENABLED)
  	{
  		ReleaseSysCache(ht_idx);
  		return NULL;
--- 204,210 
  	Assert(indexrelid == idxrec->indexrelid);
  
  	/* RLS check- if RLS is enabled then we don't return anything. */
! 	if (check_enable_rls(indrelid, In

Re: [HACKERS] Shouldn't we document "don't use a mountpoint as $PGDATA"?

2015-07-28 Thread Andrew Dunstan


On 07/28/2015 02:01 PM, Tom Lane wrote:

I had a discussion with some folks at Red Hat about this:
https://bugzilla.redhat.com/show_bug.cgi?id=1247477

I had the idea that we had documented somewhere that the data directory
should not be a filesystem mount point, but I sure can't find it now.
Any objections to adding some text about this to section 17.2, near the
caveats about NFS?




Please do. I came across a client who should have known better doing 
this just a week or two ago.


cheers

andrew


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Planner debug views

2015-07-28 Thread Qingqing Zhou
On Mon, Jul 27, 2015 at 8:20 PM, Alvaro Herrera
 wrote:
>
> I think this is a pretty neat idea, but I'm not sure this user interface
> is a good one.  Why not have a new option for EXPLAIN, so you would call
> "EXPLAIN (planner_stuff=on)" and it returns this as a resultset?

Thank you for the feedback.

Yeah, I agree piggy back on EXPLAIN sounds a better interface. A good
thing about GUC is that it is global, so deep in planner we can see it.
For example, in add_path(), where we add tracking of discarded paths. If
we do EXPLAIN, we may either have to borrow another global variable, or
add a flag on several planner data structures to make sure the flag can
penetrate deep.

Another thing is that with a GUC, we can mark it internal (PGC_INTERNAL),
which compatibility maintenance might be relaxed, especially for this
advanced option.

> This idea of creating random CSV files seems odd and inconvenient in the
> long run.  For instance it fails if you have two sessions doing it
> simultaneously; you could tack the process ID at the end of the file
> name to prevent that problem, but then the foreign table breaks each
> time.

The reason to use CSV file is a kinda of balance. We do have other
options, like pass data to pgstat, or persist in some shared memory/heap,
but they all have their own issues. Any suggestion here?

The file name is not random, it is fixed so we can create foreign table
once and use it afterwards - I actually want to push them into
system_views.sql. The file is opened with O_APPEND, the same way as log
files, so concurrent writes are serialized. Read could be problematic
though as no atomic guarantee between read/write. This is however a
general issue of file_fdw, as the file is out of control of the core. We
shall expect query returning format errors with concurrent read/write, and
retry shall resolve the issue.

Thanks,
Qingqing


-- 
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 little RLS oversight?

2015-07-28 Thread Stephen Frost
* Dean Rasheed (dean.a.rash...@gmail.com) wrote:
> On 28 July 2015 at 03:19, Joe Conway  wrote:
> > On 07/27/2015 03:05 PM, Stephen Frost wrote:
> >> AFK at the moment, but my thinking was that we should avoid having
> >> the error message change based on what a GUC is set to. I agree
> >> that there should be comments which explain that.
> 
> Except it's already dependant on the GUC if it's set to FORCE.

Yeah, you can get it to change if you own the table and set the GUC to
FORCE.

> > I changed back to using GetUserId() for the call to check_enable_rls()
> > at those three call sites, and added to the comments to explain why.
> 
> I'm not entirely convinced about this. The more I think about it, the
> more I think that if we know the user has BYPASSRLS, and they've set
> row_security to OFF, then they ought to get the more detailed error
> message, as they would if there was no RLS. That error detail is
> highly useful, and we know the user has been granted privilege by a
> superuser, and that they have direct access to the underlying table in
> this context, so we're not leaking any info that they cannot directly
> SELECT anyway.

I agree that the error detail is useful.  Perhaps it's alright that
we'll end up with something different if you've set row security to OFF
and you have BYPASSRLS.

> > While looking at ri_ReportViolation() I spotted what I believe to be a
> > bug in the current logic -- namely, has_perm is initialized to true,
> > and when check_enable_rls() returns RLS_ENABLED we never reset
> > has_perm to false, and thus leak info even though the comments claim
> > we don't. I fixed that here, but someone please take a look and
> > confirm I am reading that correctly.
> 
> Ah yes, well spotted. That looks correct to me.

Ugh, yes, agreed.  The back-branches are fine, but master and 9.5 need
that fix.

Thanks!

Stephen


signature.asc
Description: Digital signature


[HACKERS] TODO: replica information functions

2015-07-28 Thread Josh Berkus
Hackers,

Since merging recovery.conf with postgresql.conf is apparently off the
table indefinitely, we could really use some additional information
functions which work on the replica.  Here's my list of what I need for
failover automation:

pg_standby_is_streaming()
returns true if the standby is configured for streaming and
is currently connected with the master.
returns false if the connection to the master is broken,
of if there is no primary_conninfo

pg_standby_conninfo()
returns connection string to master.  Superuser-only for
previously discussed reasons

pg_recovery_config(config_item TEXT)
returns the specified configuration item from recovery.conf
superuser-only?

Does this make sense?  Is there other information we need?

-- 
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.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] TODO: replica information functions

2015-07-28 Thread Andres Freund
On 2015-07-28 11:35:52 -0700, Josh Berkus wrote:
> Since merging recovery.conf with postgresql.conf is apparently off the
> table indefinitely

Off the table as in "somebody needs to actually work on it instead of
just talking about it".



-- 
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] Failing assertions in indxpath.c, placeholder.c and brin_minmax.c

2015-07-28 Thread Alvaro Herrera
Tom Lane wrote:

> Bottom line is that somebody failed to consider the possibility of a
> null comparison value reaching the BRIN index lookup machinery.
> The code stanza that's failing supposes that only IS NULL or IS NOT NULL
> tests could have SK_ISNULL set, but that's just wrong.

I think the easiest way to solve this is to consider that all indexable
operators are strict, and have the function return false in that case.
The attached patch implements that.  (In a quick check, the only
non-strict operator in the regression database is <%(point,widget),
which seems okay to ignore given that the type itself is only part of
pg_regress.  I wonder what would happen if the regression tests defined
an index using that operator.)

What btree actually does is precompute a "qual_ok" property at
scan-restart time, which seems pretty clever (maybe too much).  I think
something like _bt_preprocess_keys should probably be applied to BRIN
scans someday.

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
diff --git a/src/backend/access/brin/brin_inclusion.c b/src/backend/access/brin/brin_inclusion.c
index 803b07f..926487e 100644
--- a/src/backend/access/brin/brin_inclusion.c
+++ b/src/backend/access/brin/brin_inclusion.c
@@ -276,8 +276,14 @@ brin_inclusion_consistent(PG_FUNCTION_ARGS)
 		 * For IS NOT NULL, we can only skip ranges that are known to have
 		 * only nulls.
 		 */
-		Assert(key->sk_flags & SK_SEARCHNOTNULL);
-		PG_RETURN_BOOL(!column->bv_allnulls);
+		if (key->sk_flags & SK_SEARCHNOTNULL)
+			PG_RETURN_BOOL(!column->bv_allnulls);
+
+		/*
+		 * Neither IS NULL nor IS NOT NULL was used; assume all indexable
+		 * operators are strict and return false.
+		 */
+		PG_RETURN_BOOL(false);
 	}
 
 	/* If it is all nulls, it cannot possibly be consistent. */
diff --git a/src/backend/access/brin/brin_minmax.c b/src/backend/access/brin/brin_minmax.c
index 7cd9888..2cc6e41 100644
--- a/src/backend/access/brin/brin_minmax.c
+++ b/src/backend/access/brin/brin_minmax.c
@@ -174,8 +174,14 @@ brin_minmax_consistent(PG_FUNCTION_ARGS)
 		 * For IS NOT NULL, we can only skip ranges that are known to have
 		 * only nulls.
 		 */
-		Assert(key->sk_flags & SK_SEARCHNOTNULL);
-		PG_RETURN_BOOL(!column->bv_allnulls);
+		if (key->sk_flags & SK_SEARCHNOTNULL)
+			PG_RETURN_BOOL(!column->bv_allnulls);
+
+		/*
+		 * Neither IS NULL nor IS NOT NULL was used; assume all indexable
+		 * operators are strict and return false.
+		 */
+		PG_RETURN_BOOL(false);
 	}
 
 	/* if the range is all empty, it cannot possibly be consistent */

-- 
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] more RLS oversights

2015-07-28 Thread Stephen Frost
* Joe Conway (joe.con...@crunchydata.com) wrote:
> On 07/03/2015 10:03 AM, Noah Misch wrote:
> > (4) When DefineQueryRewrite() is about to convert a table to a view, it 
> > checks
> > the table for features unavailable to views.  For example, it rejects tables
> > having triggers.  It omits to reject tables having relrowsecurity or a
> > pg_policy record.  Test case:
> 
> Please see the attached patch for this issue. Comments?

Looks good to me.

Thanks!

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] Sequence Access Method WIP

2015-07-28 Thread Petr Jelinek

On 2015-07-28 20:11, Heikki Linnakangas wrote:


Petr, is this enough feedback on this patch for this commitfest, or are
there some other issues you want to discuss before I mark this as returned?



You can mark it as returned, I didn't have much time to actually do much 
useful work on this in the current CF.


--
 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] group locking: incomplete patch, just for discussion

2015-07-28 Thread Robert Haas
On Wed, Nov 5, 2014 at 9:26 PM, Robert Haas  wrote:
> On Sun, Nov 2, 2014 at 7:31 AM, Simon Riggs  wrote:
>> The procgloballist stuff should be the subject of a separate patch
>> which I agree with.
>
> Yes, I think that's probably a net improvement in robustness quite
> apart from what we decide to do about any of the rest of this.  I've
> attached it here as revise-procglobal-tracking.patch and will commit
> that bit if nobody objects.

In reviewing this thread I realized that I never got around to
committing this bit.  And it still seems like a good idea, so I've
done that now.

-- 
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] security labels on databases are bad for dump & restore

2015-07-28 Thread Robert Haas
On Sun, Jul 26, 2015 at 11:43 PM, Craig Ringer  wrote:
> On 20 July 2015 at 01:18, Noah Misch  wrote:
>> On Wed, Jul 15, 2015 at 11:08:53AM +0200, Andres Freund wrote:
>>> On 2015-07-15 12:04:40 +0300, Alvaro Herrera wrote:
>>> > Andres Freund wrote:
>>> > > One thing worth mentioning is that arguably the problem is caused by the
>>> > > fact that we're here emitting database level information in pg_dump,
>>> > > normally only done for dumpall.
>>
>> Consistency with existing practice would indeed have pg_dump ignore
>> pg_shseclabel and have pg_dumpall reproduce its entries.
>
> Existing practice is pretty broken though, and not necessarily a good guide.
>
> COMMENT ON DATABASE and SECURITY LABEL FOR DATABASE are dumped by
> pg_dump, but always refer to the database's name at the time it was
> dumped, so restoring it can break.
>
> GRANTs on databases are ignored and not dumped by pg_dump or by
> pg_dumpall --globals-only. The only way to dump them seems to be to
> use pg_dumpall, which nobody uses in the real world.
>
> I'd be strongly in favour of teaching GRANT, SECURITY LABEL, COMMENT
> ON DATABASE, etc to recognise CURRENT_DATABASE as a keyword. Then
> dumping them in pg_dump --create, and in pg_dump -Fc .
>
> In practice I see zero real use of pg_dumpall without --globals-only,
> and almost everyone does pg_dump -Fc . I'd like to see that method
> case actually preserve the whole state of the system and do the right
> thing sensibly.
>
> A pg_restore option to skip database-level settings could be useful,
> but I think by default they should be restored.

Yes, I think we should make restoring the database's properties the
job of pg_dump and remove it completely from pg_dumpall, unless we can
find a case where that's really going to break things.

-- 
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] security labels on databases are bad for dump & restore

2015-07-28 Thread Stephen Frost
* Robert Haas (robertmh...@gmail.com) wrote:
> On Sun, Jul 26, 2015 at 11:43 PM, Craig Ringer  wrote:
> > On 20 July 2015 at 01:18, Noah Misch  wrote:
> >> On Wed, Jul 15, 2015 at 11:08:53AM +0200, Andres Freund wrote:
> >>> On 2015-07-15 12:04:40 +0300, Alvaro Herrera wrote:
> >>> > Andres Freund wrote:
> >>> > > One thing worth mentioning is that arguably the problem is caused by 
> >>> > > the
> >>> > > fact that we're here emitting database level information in pg_dump,
> >>> > > normally only done for dumpall.
> >>
> >> Consistency with existing practice would indeed have pg_dump ignore
> >> pg_shseclabel and have pg_dumpall reproduce its entries.
> >
> > Existing practice is pretty broken though, and not necessarily a good guide.
> >
> > COMMENT ON DATABASE and SECURITY LABEL FOR DATABASE are dumped by
> > pg_dump, but always refer to the database's name at the time it was
> > dumped, so restoring it can break.
> >
> > GRANTs on databases are ignored and not dumped by pg_dump or by
> > pg_dumpall --globals-only. The only way to dump them seems to be to
> > use pg_dumpall, which nobody uses in the real world.
> >
> > I'd be strongly in favour of teaching GRANT, SECURITY LABEL, COMMENT
> > ON DATABASE, etc to recognise CURRENT_DATABASE as a keyword. Then
> > dumping them in pg_dump --create, and in pg_dump -Fc .
> >
> > In practice I see zero real use of pg_dumpall without --globals-only,
> > and almost everyone does pg_dump -Fc . I'd like to see that method
> > case actually preserve the whole state of the system and do the right
> > thing sensibly.
> >
> > A pg_restore option to skip database-level settings could be useful,
> > but I think by default they should be restored.
> 
> Yes, I think we should make restoring the database's properties the
> job of pg_dump and remove it completely from pg_dumpall, unless we can
> find a case where that's really going to break things.

I believe that means, as discussed, that we'll need to support
"CURRENT_DATABASE" or similar for all database properties, but that
seems like a wholly good thing to do anyway, provided we can do so
without causing problems.

In other words, I agree.

Thanks!

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] security labels on databases are bad for dump & restore

2015-07-28 Thread Andres Freund
On 2015-07-28 14:58:26 -0400, Robert Haas wrote:
> Yes, I think we should make restoring the database's properties the
> job of pg_dump and remove it completely from pg_dumpall, unless we can
> find a case where that's really going to break things.

CREATE DATABASE blarg;
SECURITY LABEL ON blarg IS 'noaccess';
ALTER DATABASE blarg SET default_tablespace = space_with_storage;
pg_restore
-> SECURITY LABEL ON blarg IS 'allow_access';
-> ALTER DATABASE blarg SET default_tablespace = space_without_storage;

That's probably not sufficient reasons not to go that way, but I do
think there's a bunch more issues like that.


At the very least all these need to be emitted as ALTER DATABASE
current_database ... et al. Otherwise it's impossible to rename
databases, which definitely would not be ok.

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] security labels on databases are bad for dump & restore

2015-07-28 Thread Robert Haas
On Tue, Jul 28, 2015 at 3:03 PM, Andres Freund  wrote:
> On 2015-07-28 14:58:26 -0400, Robert Haas wrote:
>> Yes, I think we should make restoring the database's properties the
>> job of pg_dump and remove it completely from pg_dumpall, unless we can
>> find a case where that's really going to break things.
>
> CREATE DATABASE blarg;
> SECURITY LABEL ON blarg IS 'noaccess';
> ALTER DATABASE blarg SET default_tablespace = space_with_storage;
> pg_restore
> -> SECURITY LABEL ON blarg IS 'allow_access';
> -> ALTER DATABASE blarg SET default_tablespace = space_without_storage;
>
> That's probably not sufficient reasons not to go that way, but I do
> think there's a bunch more issues like that.

Could you use some complete sentences to describe what the actual
issue is?  I can't make heads or tails of what you wrote there.

> At the very least all these need to be emitted as ALTER DATABASE
> current_database ... et al. Otherwise it's impossible to rename
> databases, which definitely would not be ok.

Yep, I think that's the plan.

-- 
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] Planner debug views

2015-07-28 Thread Alvaro Herrera
Qingqing Zhou wrote:
> On Mon, Jul 27, 2015 at 8:20 PM, Alvaro Herrera
>  wrote:
> >
> > I think this is a pretty neat idea, but I'm not sure this user interface
> > is a good one.  Why not have a new option for EXPLAIN, so you would call
> > "EXPLAIN (planner_stuff=on)" and it returns this as a resultset?
> 
> Thank you for the feedback.
> 
> Yeah, I agree piggy back on EXPLAIN sounds a better interface. A good
> thing about GUC is that it is global, so deep in planner we can see it.

Um, okay, I gather that GUC is convenient to use for this purpose.  I
don't see it as a good choice; I think a bare separate global variable
at the C level is more appropriate.

> > This idea of creating random CSV files seems odd and inconvenient in the
> > long run.  For instance it fails if you have two sessions doing it
> > simultaneously; you could tack the process ID at the end of the file
> > name to prevent that problem, but then the foreign table breaks each
> > time.
> 
> The reason to use CSV file is a kinda of balance. We do have other
> options, like pass data to pgstat, or persist in some shared memory/heap,
> but they all have their own issues. Any suggestion here?

I would have a tuplestore, and the planner code would push tuples to it.
After the planning is done, EXPLAIN can read and return tuples from the
store to the user.

> The file name is not random, it is fixed so we can create foreign table
> once and use it afterwards - I actually want to push them into
> system_views.sql.

Got that.  That seems fragile and not very convenient; I don't think
forcing retries until no concurrent writers were using the same file is
convenient at all.  When you need this facility the most, which is
during slow planner runs, it is more likely that somebody else will
overwrite your file.

-- 
Á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] security labels on databases are bad for dump & restore

2015-07-28 Thread Andres Freund
On 2015-07-28 15:05:01 -0400, Robert Haas wrote:
> On Tue, Jul 28, 2015 at 3:03 PM, Andres Freund  wrote:
> > On 2015-07-28 14:58:26 -0400, Robert Haas wrote:
> >> Yes, I think we should make restoring the database's properties the
> >> job of pg_dump and remove it completely from pg_dumpall, unless we can
> >> find a case where that's really going to break things.
> >
> > CREATE DATABASE blarg;
> > SECURITY LABEL ON blarg IS 'noaccess';
> > ALTER DATABASE blarg SET default_tablespace = space_with_storage;
> > pg_restore
> > -> SECURITY LABEL ON blarg IS 'allow_access';
> > -> ALTER DATABASE blarg SET default_tablespace = space_without_storage;
> >
> > That's probably not sufficient reasons not to go that way, but I do
> > think there's a bunch more issues like that.
> 
> Could you use some complete sentences to describe what the actual
> issue is?  I can't make heads or tails of what you wrote there.

DBA creates a database and sets some properties (security labels, gucs,
acls) on it. Then goes on to restore a backup. Unfortunately that backup
might, or might not, overwrite the properties he configured depending on
whether the restored database already contains them and from which
version the backup originates.

Greetings,

Andres Freund


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] security labels on databases are bad for dump & restore

2015-07-28 Thread Robert Haas
On Tue, Jul 28, 2015 at 3:10 PM, Andres Freund  wrote:
> On 2015-07-28 15:05:01 -0400, Robert Haas wrote:
>> On Tue, Jul 28, 2015 at 3:03 PM, Andres Freund  wrote:
>> > On 2015-07-28 14:58:26 -0400, Robert Haas wrote:
>> >> Yes, I think we should make restoring the database's properties the
>> >> job of pg_dump and remove it completely from pg_dumpall, unless we can
>> >> find a case where that's really going to break things.
>> >
>> > CREATE DATABASE blarg;
>> > SECURITY LABEL ON blarg IS 'noaccess';
>> > ALTER DATABASE blarg SET default_tablespace = space_with_storage;
>> > pg_restore
>> > -> SECURITY LABEL ON blarg IS 'allow_access';
>> > -> ALTER DATABASE blarg SET default_tablespace = space_without_storage;
>> >
>> > That's probably not sufficient reasons not to go that way, but I do
>> > think there's a bunch more issues like that.
>>
>> Could you use some complete sentences to describe what the actual
>> issue is?  I can't make heads or tails of what you wrote there.
>
> DBA creates a database and sets some properties (security labels, gucs,
> acls) on it. Then goes on to restore a backup. Unfortunately that backup
> might, or might not, overwrite the properties he configured depending on
> whether the restored database already contains them and from which
> version the backup originates.

Well, I think that's just a potential incompatibility between 9.6 and
previous versions, and a relatively minor one at that.  We can't and
don't guarantee that a dump taken using the 9.3 version of pg_dump
will restore correctly on any server version except 9.3.  It might
work OK on a newer or older version, but then again it might not.

-- 
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] security labels on databases are bad for dump & restore

2015-07-28 Thread Andres Freund
On 2015-07-28 15:14:11 -0400, Robert Haas wrote:
> On Tue, Jul 28, 2015 at 3:10 PM, Andres Freund  wrote:
> > DBA creates a database and sets some properties (security labels, gucs,
> > acls) on it. Then goes on to restore a backup. Unfortunately that backup
> > might, or might not, overwrite the properties he configured depending on
> > whether the restored database already contains them and from which
> > version the backup originates.
> 
> Well, I think that's just a potential incompatibility between 9.6 and
> previous versions, and a relatively minor one at that.  We can't and
> don't guarantee that a dump taken using the 9.3 version of pg_dump
> will restore correctly on any server version except 9.3.  It might
> work OK on a newer or older version, but then again it might not.

Even within a single major version it'll be a bit confusing that one
time a restore yielded the desired result (previously set property
survives) and the next restore it doesn't, because now the backup does
contain the property.


-- 
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] security labels on databases are bad for dump & restore

2015-07-28 Thread Josh Berkus
On 07/28/2015 11:58 AM, Robert Haas wrote:
> I'd be strongly in favour of teaching GRANT, SECURITY LABEL, COMMENT
>> ON DATABASE, etc to recognise CURRENT_DATABASE as a keyword. Then
>> dumping them in pg_dump --create, and in pg_dump -Fc .
>>
>> In practice I see zero real use of pg_dumpall without --globals-only,
>> and almost everyone does pg_dump -Fc . I'd like to see that method
>> case actually preserve the whole state of the system and do the right
>> thing sensibly.
>>
>> A pg_restore option to skip database-level settings could be useful,
>> but I think by default they should be restored.

+1

Let's get rid of pg_dumpall -g.

-- 
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.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] security labels on databases are bad for dump & restore

2015-07-28 Thread Stephen Frost
* Andres Freund (and...@anarazel.de) wrote:
> On 2015-07-28 15:14:11 -0400, Robert Haas wrote:
> > On Tue, Jul 28, 2015 at 3:10 PM, Andres Freund  wrote:
> > > DBA creates a database and sets some properties (security labels, gucs,
> > > acls) on it. Then goes on to restore a backup. Unfortunately that backup
> > > might, or might not, overwrite the properties he configured depending on
> > > whether the restored database already contains them and from which
> > > version the backup originates.
> > 
> > Well, I think that's just a potential incompatibility between 9.6 and
> > previous versions, and a relatively minor one at that.  We can't and
> > don't guarantee that a dump taken using the 9.3 version of pg_dump
> > will restore correctly on any server version except 9.3.  It might
> > work OK on a newer or older version, but then again it might not.
> 
> Even within a single major version it'll be a bit confusing that one
> time a restore yielded the desired result (previously set property
> survives) and the next restore it doesn't, because now the backup does
> contain the property.

I'm not sure that I agree with this at all- you might create one SSL
certificate after you install PG and then you use one of the various
utilities to restore a prior cluster and, blam, you get a different
certificate because that's what was in the backup.

I might see having an option to enable/disable restoring the database
level properies which exist inside a backup as that may be useful
flexibility, but I don't believe this concern should stop us from
including the database properties in the database backup.

Thanks!

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] security labels on databases are bad for dump & restore

2015-07-28 Thread Alvaro Herrera
Josh Berkus wrote:
> On 07/28/2015 11:58 AM, Robert Haas wrote:
> > I'd be strongly in favour of teaching GRANT, SECURITY LABEL, COMMENT
> >> ON DATABASE, etc to recognise CURRENT_DATABASE as a keyword. Then
> >> dumping them in pg_dump --create, and in pg_dump -Fc .
> >>
> >> In practice I see zero real use of pg_dumpall without --globals-only,
> >> and almost everyone does pg_dump -Fc . I'd like to see that method
> >> case actually preserve the whole state of the system and do the right
> >> thing sensibly.
> >>
> >> A pg_restore option to skip database-level settings could be useful,
> >> but I think by default they should be restored.
> 
> +1
> 
> Let's get rid of pg_dumpall -g.

Quite the opposite, I think --- let's get rid of pg_dumpall EXCEPT when
invoked as pg_dumpall -g.

-- 
Á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] security labels on databases are bad for dump & restore

2015-07-28 Thread Robert Haas
On Tue, Jul 28, 2015 at 3:16 PM, Andres Freund  wrote:
> On 2015-07-28 15:14:11 -0400, Robert Haas wrote:
>> On Tue, Jul 28, 2015 at 3:10 PM, Andres Freund  wrote:
>> > DBA creates a database and sets some properties (security labels, gucs,
>> > acls) on it. Then goes on to restore a backup. Unfortunately that backup
>> > might, or might not, overwrite the properties he configured depending on
>> > whether the restored database already contains them and from which
>> > version the backup originates.
>>
>> Well, I think that's just a potential incompatibility between 9.6 and
>> previous versions, and a relatively minor one at that.  We can't and
>> don't guarantee that a dump taken using the 9.3 version of pg_dump
>> will restore correctly on any server version except 9.3.  It might
>> work OK on a newer or older version, but then again it might not.
>
> Even within a single major version it'll be a bit confusing that one
> time a restore yielded the desired result (previously set property
> survives) and the next restore it doesn't, because now the backup does
> contain the property.

How would that happen?  We're not gonna back-patch this.

-- 
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] RFC: replace pg_stat_activity.waiting with something more descriptive

2015-07-28 Thread Heikki Linnakangas

On 07/27/2015 01:20 PM, Ildus Kurbangaliev wrote:

Hello.
In the attached patch I've made a refactoring for tranches.
The prefix for them was extended,  and I've did a split of LWLockAssign
to two
functions (one with tranche and second for user defined LWLocks).


This needs some work in order to be maintainable:

* The patch requires that the LWLOCK_INDIVIDUAL_NAMES array is kept in 
sync with the list of individual locks in lwlock.h. Sooner or later 
someone will add an LWLock and forget to update the names-array. That 
needs to be made less error-prone, so that the names are maintained in 
the same place as the #defines. Perhaps something like rmgrlist.h.


* The "base" tranches are a bit funny. They all have the same 
array_base, pointing to MainLWLockArray. If there are e.g. 5 clog buffer 
locks, I would expect the T_NAME() to return "ClogBufferLocks" for all 
of them, and T_ID() to return numbers between 0-4. But in reality, 
T_ID() will return something like 55-59.


Instead of passing a tranche-id to LWLockAssign(), I think it would be 
more clear to have a new function to allocate a contiguous block of 
lwlocks as a new tranche. It could then set the base correctly.


* Instead of having LWLOCK_INDIVIDUAL_NAMES to name "individual" locks, 
how about just giving each one of them a separate tranche?


* User manual needs to be updated to explain the new column in 
pg_stat_activity.


- Heikki



--
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] TODO: replica information functions

2015-07-28 Thread Evgeniy Shishkin

> On 28 Jul 2015, at 21:35, Josh Berkus  wrote:
> 
> Hackers,
> 
> Since merging recovery.conf with postgresql.conf is apparently off the
> table indefinitely, we could really use some additional information
> functions which work on the replica.  Here's my list of what I need for
> failover automation:
> 
> pg_standby_is_streaming()
>   returns true if the standby is configured for streaming and
>   is currently connected with the master.
>   returns false if the connection to the master is broken,
>   of if there is no primary_conninfo
> 

I believe we should have some function to tell if standby is configured for 
streaming
and another function to tell if everything is okay.


> pg_standby_conninfo()
>   returns connection string to master.  Superuser-only for
>   previously discussed reasons
> 
> pg_recovery_config(config_item TEXT)
>   returns the specified configuration item from recovery.conf
>   superuser-only?
> 
> Does this make sense?  Is there other information we need?
> 
> -- 
> Josh Berkus
> PostgreSQL Experts Inc.
> http://pgexperts.com
> 
> 
> -- 
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers



-- 
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] security labels on databases are bad for dump & restore

2015-07-28 Thread Andres Freund
On 2015-07-28 15:27:51 -0400, Robert Haas wrote:
> On Tue, Jul 28, 2015 at 3:16 PM, Andres Freund  wrote:
> > On 2015-07-28 15:14:11 -0400, Robert Haas wrote:
> >> On Tue, Jul 28, 2015 at 3:10 PM, Andres Freund  wrote:
> >> > DBA creates a database and sets some properties (security labels, gucs,
> >> > acls) on it. Then goes on to restore a backup. Unfortunately that backup
> >> > might, or might not, overwrite the properties he configured depending on
> >> > whether the restored database already contains them and from which
> >> > version the backup originates.
> >>
> >> Well, I think that's just a potential incompatibility between 9.6 and
> >> previous versions, and a relatively minor one at that.  We can't and
> >> don't guarantee that a dump taken using the 9.3 version of pg_dump
> >> will restore correctly on any server version except 9.3.  It might
> >> work OK on a newer or older version, but then again it might not.
> >
> > Even within a single major version it'll be a bit confusing that one
> > time a restore yielded the desired result (previously set property
> > survives) and the next restore it doesn't, because now the backup does
> > contain the property.
>
> How would that happen?  We're not gonna back-patch this.

Hm?  Let me try again: If the admin does a ALTER DATABASE ... SET guc =
... *before* restoring a backup and the backup does contain a setting
for the same guc, but with a different value it'll overwrite the
previous explicit action by the DBA without any warning.  If the backup
does *not* contain that guc the previous action survives.  That's
confusing, because you're more likely to be in the 'the backup does not
contain the guc' situation when testing where it thus will work.


-- 
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] security labels on databases are bad for dump & restore

2015-07-28 Thread Robert Haas
On Tue, Jul 28, 2015 at 3:33 PM, Andres Freund  wrote:
> On 2015-07-28 15:27:51 -0400, Robert Haas wrote:
>> On Tue, Jul 28, 2015 at 3:16 PM, Andres Freund  wrote:
>> > On 2015-07-28 15:14:11 -0400, Robert Haas wrote:
>> >> On Tue, Jul 28, 2015 at 3:10 PM, Andres Freund  wrote:
>> >> > DBA creates a database and sets some properties (security labels, gucs,
>> >> > acls) on it. Then goes on to restore a backup. Unfortunately that backup
>> >> > might, or might not, overwrite the properties he configured depending on
>> >> > whether the restored database already contains them and from which
>> >> > version the backup originates.
>> >>
>> >> Well, I think that's just a potential incompatibility between 9.6 and
>> >> previous versions, and a relatively minor one at that.  We can't and
>> >> don't guarantee that a dump taken using the 9.3 version of pg_dump
>> >> will restore correctly on any server version except 9.3.  It might
>> >> work OK on a newer or older version, but then again it might not.
>> >
>> > Even within a single major version it'll be a bit confusing that one
>> > time a restore yielded the desired result (previously set property
>> > survives) and the next restore it doesn't, because now the backup does
>> > contain the property.
>>
>> How would that happen?  We're not gonna back-patch this.
>
> Hm?  Let me try again: If the admin does a ALTER DATABASE ... SET guc =
> ... *before* restoring a backup and the backup does contain a setting
> for the same guc, but with a different value it'll overwrite the
> previous explicit action by the DBA without any warning.  If the backup
> does *not* contain that guc the previous action survives.  That's
> confusing, because you're more likely to be in the 'the backup does not
> contain the guc' situation when testing where it thus will work.

True.  But I don't think modifying a database before restoring into it
is terribly supported.  Even pg_dump --clean, which is supposed to do
this sort of thing, doesn't seem to work terribly reliably.  We could
try to fix this by having a command like ALTER DATABASE ... RESET ALL
that we issue before restoring the settings, but I'm afraid that will
take us into all sorts of unreasonable scenarios that are better just
labeled as "don't do that".

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


[HACKERS] Remaining 'needs review' patchs in July commitfest

2015-07-28 Thread Heikki Linnakangas
21 patches remain in Needs Review state, in the July commitfest. Some of 
them have a reviewer signed up. I have highlighted some of them below 
that worry me the most. What are we going to do about these? For each of 
them, I'd like the authors to have some idea on what they need to do to 
get the patch into committable state (or if the whole approach is going 
to be rejected), but I don't know what that advise should be.



pgbench - allow backslash-continuations in custom scripts


Everyone wants the feature, using multi-line SELECTs in pgbench scripts, 
but we don't seem to be reaching a consensus on how it should work. I 
think we'll need to integrate the lexer, but it would be nice to still 
support multi-statements as well, with some syntax.



multivariate statistics


This has been a long discussion. Are we getting close to a committable 
state?



COPY RAW


No consensus on whether to add this to the server's COPY command, or as 
a new psql backslash option.



Unique Joins


Kyotaro HORIGUCHI commented on this back in March, but no-one's reviewed 
the latest version. It's a fairly big patch. I guess I'll review it if 
no-one else does, but it's going to take me some time to really dig into 
it...



checkpoint continuous flushing


This does a big memory allocation at checkpoint, which Tom vehemently 
objects to. I don't much like it either, although I would be OK with a 
more moderately-sized allocation. It's not clear on what criteria this 
should be accepted or rejected. What workloads need to be tested?



plpgsql raise statement with context


Impasse. Everyone wants this feature in some form, but no consensus on 
whether to do this client-side or server-side.



Configurable location for extension .control files


Do we want this? In its current form? I feel that this isn't ready as it 
is, but I'm not sure what to suggest instead.



dblink: add polymorphic result functions


Seems pretty ugly to me to add a dummy argument to functions, just so 
that you can specify the result type. The problem it's trying to solve 
is real, though. Should we take it as it is, or wait for some cleaner 
approach?



Improving test coverage of extensions with pg_dump


Do we want to have this in src/test/modules or src/bin/pg_dump/t?


Asynchronous execution on postgres-fdw


If we're going to have some sort of general-purpose Parallel node 
support soon, should we just forget about this? Or is it still useful? 
This adds a fair amount of new infrastructure, for a fairly niche feature..


- Heikki

--

- Heikki


--
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] Remaining 'needs review' patchs in July commitfest

2015-07-28 Thread Josh Berkus
On 07/28/2015 12:51 PM, Heikki Linnakangas wrote:
> Everyone wants the feature, using multi-line SELECTs in pgbench scripts,
> but we don't seem to be reaching a consensus on how it should work. I
> think we'll need to integrate the lexer, but it would be nice to still
> support multi-statements as well, with some syntax.

Seems like a perfect candidate for "returned with feedback".  We'll
straighten it out by the next commitfest.

Also note that this has actually spiralled out into 2 or 3 separate
features.


-- 
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.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] Remaining 'needs review' patchs in July commitfest

2015-07-28 Thread Andres Freund
On 2015-07-28 22:51:55 +0300, Heikki Linnakangas wrote:
> >checkpoint continuous flushing
> 
> This does a big memory allocation at checkpoint, which Tom vehemently
> objects to.

Uh. Didn't he just object to failing in that case? IIRC he even
indicated tentative assent, a year or so back, with my idea of just
pre-allocating all the required memory in shared memory, so that we
don't need to allocate anything at some point.

I've not read the last version of the patch, but in my old version the
allocation wasn't actually that large in comparison to the size of
shared buffers itself.


> I don't much like it either, although I would be OK with a more
> moderately-sized allocation.

That'll disallow some mighty fine optimizations like e.g. being able to
do the fsyncs of files early, directly after we wrote out all their
buffer, thereby reducing how much dirty data (written out by backend)
that needs to be flushed to disk.


Greetings,

Andres Freund


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Planner debug views

2015-07-28 Thread Tom Lane
Alvaro Herrera  writes:
> Qingqing Zhou wrote:
>> The file name is not random, it is fixed so we can create foreign table
>> once and use it afterwards - I actually want to push them into
>> system_views.sql.

> Got that.  That seems fragile and not very convenient; I don't think
> forcing retries until no concurrent writers were using the same file is
> convenient at all.  When you need this facility the most, which is
> during slow planner runs, it is more likely that somebody else will
> overwrite your file.

FWIW, I would be very much against anything that requires going through
the filesystem for this.  That will create security/privilege issues that
we should not want to introduce, quite aside from the usability problems
Alvaro points out.

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] Remaining 'needs review' patchs in July commitfest

2015-07-28 Thread Alvaro Herrera
Heikki Linnakangas wrote:
> 21 patches remain in Needs Review state, in the July commitfest. Some of
> them have a reviewer signed up. I have highlighted some of them below that
> worry me the most. What are we going to do about these? For each of them,
> I'd like the authors to have some idea on what they need to do to get the
> patch into committable state (or if the whole approach is going to be
> rejected), but I don't know what that advise should be.
> 
> >pgbench - allow backslash-continuations in custom scripts
> 
> Everyone wants the feature, using multi-line SELECTs in pgbench scripts, but
> we don't seem to be reaching a consensus on how it should work. I think
> we'll need to integrate the lexer, but it would be nice to still support
> multi-statements as well, with some syntax.

Excuse me -- what's a multi-statement?  I thought this effort was all
about multi-line single statements only.  I think the proposed patch to
integrate psql's lexer in pgbench is a reasonable step forward, but we
need it to use our standard technique of symlinking the .l file in place
via some additional lines in pgbench's Makefile rather than copying it.

> >dblink: add polymorphic result functions
> 
> Seems pretty ugly to me to add a dummy argument to functions, just so that
> you can specify the result type. The problem it's trying to solve is real,
> though. Should we take it as it is, or wait for some cleaner approach?

Put like that, it does sound quite ugly.  I take it we have no better
alternative proposed?

> >Improving test coverage of extensions with pg_dump
> 
> Do we want to have this in src/test/modules or src/bin/pg_dump/t?

Are we testing pg_dump here, or are we testing extensions?  If the
former, src/bin/pg_dump/t seems best.

> >Asynchronous execution on postgres-fdw
> 
> If we're going to have some sort of general-purpose Parallel node support
> soon, should we just forget about this? Or is it still useful? This adds a
> fair amount of new infrastructure, for a fairly niche feature..

-0.1 on this one.

-- 
Á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] Re: Removing SSL renegotiation (Was: Should we back-patch SSL renegotiation fixes?)

2015-07-28 Thread Andres Freund
On 2015-07-28 18:59:02 +0200, Andres Freund wrote:
> Unless somebody protests soon I'm going to push something like that
> after having dinner.

Done.


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Re: Removing SSL renegotiation (Was: Should we back-patch SSL renegotiation fixes?)

2015-07-28 Thread Alvaro Herrera
Andres Freund wrote:
> On 2015-07-28 18:59:02 +0200, Andres Freund wrote:
> > Unless somebody protests soon I'm going to push something like that
> > after having dinner.
> 
> Done.

Yay!

-- 
Á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] Remaining 'needs review' patchs in July commitfest

2015-07-28 Thread Heikki Linnakangas

On 07/28/2015 11:01 PM, Alvaro Herrera wrote:

Heikki Linnakangas wrote:

pgbench - allow backslash-continuations in custom scripts


Everyone wants the feature, using multi-line SELECTs in pgbench scripts, but
we don't seem to be reaching a consensus on how it should work. I think
we'll need to integrate the lexer, but it would be nice to still support
multi-statements as well, with some syntax.


Excuse me -- what's a multi-statement?


Sending "SELECT 1; SELECT 2;" to the server in one Query message. I.e. 
PQexec(conn, "SELECT 1; SELECT 2;").


If you put that on a single line in pgbench today, it will send it as 
one message to the server. If we start using a lexer, and split that 
into two, that's a change in behaviour. Not the end of the world, but it 
would still be nice to be able to use multi-statements in pgbench, too.



dblink: add polymorphic result functions


Seems pretty ugly to me to add a dummy argument to functions, just so that
you can specify the result type. The problem it's trying to solve is real,
though. Should we take it as it is, or wait for some cleaner approach?


Put like that, it does sound quite ugly.  I take it we have no better
alternative proposed?


Joe Conway suggested a more generic approach here: 
http://www.postgresql.org/message-id/559a9643.9070...@joeconway.com. I'm 
not sure why that was not pursued. It certainly seems better to me.



Improving test coverage of extensions with pg_dump


Do we want to have this in src/test/modules or src/bin/pg_dump/t?


Are we testing pg_dump here, or are we testing extensions?  If the
former, src/bin/pg_dump/t seems best.


I think that's the crux of the disagreement :-).

- Heikki



--
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 little RLS oversight?

2015-07-28 Thread Joe Conway
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 07/28/2015 11:17 AM, Joe Conway wrote:
> I'm going to commit the attached in the next few hours unless
> someone has serious objections. We can always revisit the specific
> behavior of those messages separately if we change our minds...

Pushed to master and 9.5

Joe
-BEGIN PGP SIGNATURE-
Version: GnuPG v2.0.22 (GNU/Linux)

iQIcBAEBAgAGBQJVt+U0AAoJEDfy90M199hlPb8P/0cgoaY4YWIcGc2HsezMAvfn
Fx2NqTpN7i1HCkWZcVv85TqfY4lura9wkakHOf6WFPSfBPMKGwBjDOPGwDc/iR42
RchChOXFK8Up12iqEZJMQ8PJQoaN3ZWg2QRrIpxPlQkhxsOKDDR9ZehczYSRrK5P
Nx/PE+cCHvI48zRQ0b31uPTeYMtUoCu/7qPQeSk3es/K5x3GBGls8aV/drw3CBRK
nrq7wu84jCrENoRhJSYKnntpjtBF43/SSojnJd0uOL6dytdEztxfyugEDnGni46k
vdEJNewKPIqZR5V8Qa4UGF4YKzxH8IgyWpYOuHTeNTEnNY+3/D+jmJoIwNw/P7gn
t+MMj6m4oRvWFY/J6sRZJ7bsFwj9KexQuu2rGDesnpWbJE5B7IricyUdtDrgia7p
qCS5wEnoG9uTrisPNi+oMZ1+IdsZ0FOHjfq6pNUUGDlmdR/iN4A8iNrAxIto5mHy
mB1Fx37LsMxeknDNQvsepOVa573goGlLwEDb5slxDySEGdXx6vm2pQo+Yy+L5s1g
dsFM7aVhCzsPjHZLKM9hEb84cjor7YzjuKlkQKfLkr7e6v6N2mtk/PTGYR51+li5
/8eZUxNF9K9tVe8dtP+RGtQ7ZgwmzOXcR34v6JYXOJz+X0DBT9c50A3986KMMS2X
1FrY7SUZlzMxRExmPXGH
=0YFQ
-END PGP SIGNATURE-


-- 
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] Remaining 'needs review' patchs in July commitfest

2015-07-28 Thread Simon Riggs
On 28 July 2015 at 20:51, Heikki Linnakangas  wrote:

>
>  multivariate statistics
>>
>
> This has been a long discussion. Are we getting close to a committable
> state?
>

This is important, but big.


>  COPY RAW
>>
>
> No consensus on whether to add this to the server's COPY command, or as a
> new psql backslash option.
>

I think there is something salvageable there.


I've added my name as committer to a few things, but won't be able to work
on them until at least next week when I've finished 9.5 stuff. Happy to
step back if anyone else wants to claim those.

-- 
Simon Riggshttp://www.2ndQuadrant.com/

PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


Re: [HACKERS] Remaining 'needs review' patchs in July commitfest

2015-07-28 Thread Heikki Linnakangas

On 07/28/2015 11:30 PM, Simon Riggs wrote:

I've added my name as committer to a few things, but won't be able to work
on them until at least next week when I've finished 9.5 stuff. Happy to
step back if anyone else wants to claim those.


Thanks, every little helps!

- Heikki



--
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] Remaining 'needs review' patchs in July commitfest

2015-07-28 Thread Tom Lane
Andres Freund  writes:
> On 2015-07-28 22:51:55 +0300, Heikki Linnakangas wrote:
>>> checkpoint continuous flushing

>> This does a big memory allocation at checkpoint, which Tom vehemently
>> objects to.

> Uh. Didn't he just object to failing in that case?

Right.  If it can fall back to "stupid" mode when there's not spare
memory, I'd be ok with that.

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] Remaining 'needs review' patchs in July commitfest

2015-07-28 Thread Joe Conway
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 07/28/2015 01:22 PM, Heikki Linnakangas wrote:
 dblink: add polymorphic result functions
>>> 
>>> Seems pretty ugly to me to add a dummy argument to functions,
>>> just so that you can specify the result type. The problem it's
>>> trying to solve is real, though. Should we take it as it is, or
>>> wait for some cleaner approach?
>> 
>> Put like that, it does sound quite ugly.  I take it we have no
>> better alternative proposed?
> 
> Joe Conway suggested a more generic approach here: 
> http://www.postgresql.org/message-id/559a9643.9070...@joeconway.com.
> I'm not sure why that was not pursued. It certainly seems better to
> me.

I really think the second approach in that email is the right way to
go. Is there any benefit we get from the dblink specific hack that we
don't get via the cast?

I guess it is just be a matter of someone getting around to making it
happen. I can put it on my list of things to do, but probably will not
get to looking at it for at least a few more weeks.

- -- 
Joe Conway
-BEGIN PGP SIGNATURE-
Version: GnuPG v2.0.22 (GNU/Linux)

iQIcBAEBAgAGBQJVt+xpAAoJEDfy90M199hlIM8P/iRq3nZo6Q/u14D9Vqvd8qZ1
cPaBgGoHvCCuM3rS/z0DobBMjKll0oEerjXeuBKTjWBwilPrdHMZahueFZ1896Mh
atcj7kz8O3nBj27sBAHI1tXCTPRkjOIpt6nNu8BEsG8mUmX8wC2odfyGFZIKzw2j
dtLGsczSMwrXnE1yTzgCYWVl0ej8y/CtAmL6uEQOIA5XDHmYe6EqqPd3rcNBNg+I
endHrAXGm/VVsKGUJfisWItVbPJy4VBx1y1iXfApxePceHjW6/U0Ivds5Qu+K3wd
0pwsBmqlHNBGzwril1jBpm9gecIYY6916Hbru7Fafae4bbFHRTDw0ExURybrlhGo
MjOhWmfpsCutGmBktgWxqiOY9UDnG1+aXREKIv7bIavPhBYbBRkvqT7qrAKureex
y2XZz86tMV2bUZxRyp6tFo8aL5hheV7UoaPKGlOxfgUNFv3U2uRUopVPH6uR1Z80
+JQoC2hDadI3wQ6/2egL62pQR3l/Ts8zF0sCFEGAZVrFklEneHsUS7PAbqIfY+TI
Ffc5q8Vm0QplWXr5vvxL1JKepXGgKSidMEnylAwIxKAnPtXyosPe1sZ+ocA+3gbP
Uj/bElWemX8b8YF/F1mecHqoLAvJ+3NKUdwS805VmdCLO1nxictKEOHAJFeQomif
OQquxdkcGtA9dmCt8JJ9
=SaUZ
-END PGP SIGNATURE-


-- 
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] Planner debug views

2015-07-28 Thread Qingqing Zhou
On Tue, Jul 28, 2015 at 12:08 PM, Alvaro Herrera
 wrote:
> I would have a tuplestore, and the planner code would push tuples to it.
> After the planning is done, EXPLAIN can read and return tuples from the
> store to the user.
>

Not sure if I got it: so EXPLAIN will return tuples to libpq client. But
how do we store these returned tuples (RelOptInfo, Path etc) so we can
throw queries against them later?

Something like this:
INSERT INTO my_space SELECT (EXPLAIN SELECT ...); -- won't get parsed

Regards,
Qingqing


-- 
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: Make timestamptz_out less slow.

2015-07-28 Thread David Rowley
On 29 July 2015 at 03:25, Andres Freund  wrote:

> On 2015-07-29 03:10:41 +1200, David Rowley wrote:
> > Have you thought about what to do when HAVE_INT64_TIMESTAMP is not
> defined?
>
> I don't think it's actually important. The only difference vs float
> timestamps is that in the latter case we set fsecs to zero BC.
>
>
 I was also thinking that the % 10 won't work when fsec_t is double.

typedef double fsec_t

Regards

David Rowley

--
 David Rowley   http://www.2ndQuadrant.com/

 PostgreSQL Development, 24x7 Support, Training & Services


Re: [HACKERS] Planner debug views

2015-07-28 Thread Tom Lane
Qingqing Zhou  writes:
> Not sure if I got it: so EXPLAIN will return tuples to libpq client. But
> how do we store these returned tuples (RelOptInfo, Path etc) so we can
> throw queries against them later?

> Something like this:
> INSERT INTO my_space SELECT (EXPLAIN SELECT ...); -- won't get parsed

You can do something like that in plpgsql, for example

declare t text;

for t in EXPLAIN SELECT ...
loop
   insert into whatever values(t);
end loop;

There's an example of doing this sort of thing in the brin.sql regression
test, where it's used to verify that we're getting the plan type we
expect.

I don't feel a strong need to invent additional forms of that wheel.

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] Buildfarm failure from overly noisy warning message

2015-07-28 Thread Tom Lane
Kevin Grittner  writes:
> Tom Lane  wrote:
>> Kevin Grittner  writes:
>>> I think a LOG entry when an autovacuum process is actually canceled
>>> has value just in case it is happening on a particular table so
>>> frequently that the table starts to bloat.  I see no reason to log
>>> anything if there is an intention to cancel an autovacuum process
>>> but it actually completes before we can do so.

>> Hm.  By that logic, I'm not sure if we need anything to be logged here,
>> because the autovacuum process will log something about having received
>> a query cancel signal.

> That seems sufficient to me for normal cases.

Rather than remove the "sending signal" elog entirely, I reduced it to
DEBUG1; that will avoid log chatter for normal cases but the info can
still be obtained at need.

>> If we're in the business of minimizing log chatter, I'd suggest that
>> we remove the entirely-routine "sending cancel" log message, and only
>> log something in the uncommon case where the kill() fails (but, per
>> original point, reduce that entry to LOG or so; or else print something
>> only for not-ESRCH cases).

> +1 for only printing for the non-ESRCH cases.

Left that one as a WARNING, but it doesn't print for ESRCH.

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] Remaining 'needs review' patchs in July commitfest

2015-07-28 Thread Tomas Vondra

Hi,

On 07/28/2015 09:51 PM, Heikki Linnakangas wrote:



multivariate statistics


This has been a long discussion. Are we getting close to a committable
state?


Certainly not - the discussion may seem long, but it only deals with 
some aspects of the patch so far. There was very little discussion of 
low-level implementation details (e.g. how exactly the histograms are 
built, data structures etc.) - we haven't got that far yet.



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


Re: [HACKERS] Re: Removing SSL renegotiation (Was: Should we back-patch SSL renegotiation fixes?)

2015-07-28 Thread Michael Paquier
On Wed, Jul 29, 2015 at 2:00 AM, Andres Freund  wrote:
> On 2015-07-28 18:59:02 +0200, Andres Freund wrote:
>> Attached are:
>>
>> a) a slightly evolved version of Michael's patch disabling renegotiation
>>by default that I'm planning to apply to 9.4 - 9.0
>>
>> b) a patch removing renegotiation entirely from master and 9.5

Note: there was already upthread a patch for that:
http://www.postgresql.org/message-id/cab7npqqnjidixr5qnj86qnm++skpytedtnlf_vnpmvtu5xo...@mail.gmail.com
But it doesn't matter much. Thanks for the final push.
-- 
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] Remaining 'needs review' patchs in July commitfest

2015-07-28 Thread Michael Paquier
On Wed, Jul 29, 2015 at 5:22 AM, Heikki Linnakangas wrote:
> On 07/28/2015 11:01 PM, Alvaro Herrera wrote:
 Improving test coverage of extensions with pg_dump
>>>
>>>
>>> Do we want to have this in src/test/modules or src/bin/pg_dump/t?
>>
>>
>> Are we testing pg_dump here, or are we testing extensions?  If the
>> former, src/bin/pg_dump/t seems best.

All the tests are using pg_dump, but it is testing dumpable tables in
an extension. At this point I am not sure which one is better honestly
X/. Putting it in pg_dump/t will require two lines in the make target
prove_check such as modules in this path are installed as well, and
the argument against having it in src/test/modules is that it would
bloat it in the future if we do the same for all binaries,
particularly if we have multiple modules for each one.

> I think that's the crux of the disagreement :-).

Yep.
-- 
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] Failing assertions in indxpath.c, placeholder.c and brin_minmax.c

2015-07-28 Thread Tom Lane
Alvaro Herrera  writes:
> Tom Lane wrote:
>> Bottom line is that somebody failed to consider the possibility of a
>> null comparison value reaching the BRIN index lookup machinery.
>> The code stanza that's failing supposes that only IS NULL or IS NOT NULL
>> tests could have SK_ISNULL set, but that's just wrong.

> I think the easiest way to solve this is to consider that all indexable
> operators are strict, and have the function return false in that case.
> The attached patch implements that.

This looks fine to me as a localized fix.  I was wondering whether we
could short-circuit the index lookup further upstream, but I take it from
your comment about _bt_preprocess_keys that BRIN has no convenient place
for that today.  (Even if it did, I'd still vote for making this change,
for safety's sake.)

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] more RLS oversights

2015-07-28 Thread Joe Conway
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 07/27/2015 05:34 PM, Joe Conway wrote:
> On 07/27/2015 01:13 PM, Alvaro Herrera wrote:
>> Hmm, these are not ACL objects, so conceptually it seems cleaner
>> to use a different symbol for this.  I think the catalog state
>> and the error messages would be a bit confusing otherwise.
> 
> Ok -- done
> 
>> Isn't this leaking the previously allocated array?  Not sure
>> it's all that critical, but still.  (I don't think you really
>> need to call palloc at all here.)
> 
> Agreed -- I think the attached is a bit cleaner.
> 
> Any other comments or concerns?

Pushed to HEAD and 9.5

Joe


-BEGIN PGP SIGNATURE-
Version: GnuPG v2.0.22 (GNU/Linux)

iQIcBAEBAgAGBQJVuAp9AAoJEDfy90M199hlokgP/2RVYjPiwM0EcE9pAdGP3Uqa
qkCqj5cPq/5SdLH553h/Xd6056qwchoMJYQo9RZzzVR4wS4HPyeZ00e33RrJZbtm
MGvjat1NJWEyl03y1AwAy9c9yHCRJa5vQlKtm0v2xWDG3xXqwejz/SK2ijw2IWVW
W1/7OnObbquHa7a+IsO1ndXoI0jECzOFDXY6YzFVJuPAf3asOHT44lJbHkfUq3Kv
k5eK14Xrb3kcYqhBQg+eG50lr1aRDj8yDlYQuoGmw/dg/X0TyAo2v+AOaFrN8rXD
igsYaMDafsXY55izkiRuNfcYZAnHC7hNVt+ffV+wLycCTiaEEflp+BCxLTYmh53T
xDB39Lr0Sz7AP77l0M9hbMr3Ao3GA1KFOc9OfRN11eSo5Y6uDtpMeOIFBAke6hxl
DanWPc/YmXzacga99xzOQglDZkDWTohWsEDwniRJmi7UC0Z/gwZ2P60OnwE1lqbd
eOmUu0JZCwklInWoDo9XmWdfp9+OrviGNm0vhQbplhEm/LC9PqBB/DOy44QjSv84
jfY/iMPn8uvGqWiQ/65za1O/1QsRukgp5PVnj7TyNojSskSuAYOF5BcFVIdB7krj
ZKHChreUMVw1nH8py4HkdPOXTHmAItV9/9T2c/UUuJWAECiLy+tIY/if+Tzi+Zn6
nRm99YM401PAsRKLyn0m
=gYmH
-END PGP SIGNATURE-


-- 
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] Remaining 'needs review' patchs in July commitfest

2015-07-28 Thread Tom Lane
Michael Paquier  writes:
> On Wed, Jul 29, 2015 at 5:22 AM, Heikki Linnakangas wrote:
>> On 07/28/2015 11:01 PM, Alvaro Herrera wrote:
>>> Do we want to have this in src/test/modules or src/bin/pg_dump/t?

>> Are we testing pg_dump here, or are we testing extensions?  If the
>> former, src/bin/pg_dump/t seems best.

> All the tests are using pg_dump, but it is testing dumpable tables in
> an extension. At this point I am not sure which one is better honestly
> X/.

ISTM we're testing pg_dump, but I don't especially want to clutter
the pg_dump directory with the code for a dummy extension, so I'd
vote for putting the extension code under src/test/modules.
(Or you could hide it inside src/bin/pg_dump/t/, but that seems
pretty weird too.)

In the end though, this is the sort of thing we generally leave to the
discretion of the responsible committer.  I won't whinge too hard
about whatever the decision is ...

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] more RLS oversights

2015-07-28 Thread Joe Conway
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 07/28/2015 11:50 AM, Stephen Frost wrote:
> * Joe Conway (joe.con...@crunchydata.com) wrote:
>> On 07/03/2015 10:03 AM, Noah Misch wrote:
>>> (4) When DefineQueryRewrite() is about to convert a table to a
>>> view, it checks the table for features unavailable to views.
>>> For example, it rejects tables having triggers.  It omits to
>>> reject tables having relrowsecurity or a pg_policy record.
>>> Test case:
>> 
>> Please see the attached patch for this issue. Comments?
> 
> Looks good to me.

Thanks -- pushed to HEAD and 9.5

Joe


-BEGIN PGP SIGNATURE-
Version: GnuPG v2.0.22 (GNU/Linux)

iQIcBAEBAgAGBQJVuA+UAAoJEDfy90M199hlrtYQAJGVucRtarWTQoP39of8IYXE
OoAi+q6CKEJWEJdVHmnnCwMc6y0lx0+Qugpb5SAVqMG/0oYvZ/bnvvN5OHMxujCI
bw7A/eJ62BILAt07Oczs1gtt+k4PT1001c4jFjgSC6AV51cZ9UM+d0b4C2YhKcm7
B4tycmC/yGxa5UFT6vdOhneILzBeOMLxgMXKouoD7Gnf4UKh1KUWxxWKU9q0Kl7d
mvRJke1wj/+WfquO09g33+1jHCsnbV4fcQ1q8RQbFPItpqcp4alsuuIYzLiPRbbs
VtZYurFLpjKdJg8OuHw9IYNbDjPZoEuv5eV16aSjOJxnngcXROxwXOwZMVaCuQMq
1D5FoUunJqsqlxVKggJJLfBt9uM/gafjmnRHGBZftfX/A3ZM6c6YewA68Nxo+rTE
xtgA+n1lzWsahje0n2KSFUNRSCqdWcnLQ/HtnVcNjGdFdCxeUDQ5kUAE1hFCMCXe
5eqAvohQQ25RiurZ1rI1IfUoeAPRDp3nvgcMMM7FQMmv6oKNBLr2RivVz0ZE17vd
Htz0y0cPx8mJgEHKMJrV/yF9odECTxevBzkO5rASLCLnGHEYp8WZfqWO/s/HoujS
KU99lfzOfnyBIyl2zIGSmkmCvUIqaP1cUP5xMHIedNhjDRy/Rt6IxwA9qEtgUokI
sC6BWHpxd19RAh5NLXCK
=NFOR
-END PGP SIGNATURE-


-- 
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] TODO: replica information functions

2015-07-28 Thread Euler Taveira

On 28-07-2015 15:35, Josh Berkus wrote:

pg_standby_is_streaming()
returns true if the standby is configured for streaming and
is currently connected with the master.
returns false if the connection to the master is broken,
of if there is no primary_conninfo


+1.


pg_standby_conninfo()
returns connection string to master.  Superuser-only for
previously discussed reasons


-1. It could be retrieved using the proposal below.


pg_recovery_config(config_item TEXT)
returns the specified configuration item from recovery.conf
superuser-only?


pg_recovery_config(OUT name text, OUT setting text) SETOF record

or

pg_recovery_config(OUT name text, OUT setting text, IN all bool) SETOF 
record


This function covers pg_standby_conninfo().


--
   Euler Taveira   Timbira - http://www.timbira.com.br/
   PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento


--
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_basebackup and replication slots

2015-07-28 Thread Peter Eisentraut
On 7/22/15 12:43 AM, Michael Paquier wrote:
> OK, thanks for the updated versions. Those ones look good to me.

Committed, thanks.

> Now, do we plan to do something about the creation of a slot. I
> imagine that it would be useful if we could have --create-slot to
> create a slot when beginning a base backup and if-not-exists to
> control the error flow. There is already CreateReplicationSlot for
> this purpose in streamutils.c so most of the work is already done.
> Also, when a base backup fails for a reason or another, we should try
> to drop the slot in disconnect_and_exit() if it has been created by
> pg_basebackup. if if-not-exists is true and the slot already existed
> when beginning, we had better not dropping it perhaps...

I think it would be worthwhile to work on that, but in my mind it's a
separate feature, and I don't have any use for it, so I'm not going to
rush into it.




-- 
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] Planner debug views

2015-07-28 Thread Qingqing Zhou
On Tue, Jul 28, 2015 at 2:43 PM, Tom Lane  wrote:
>
> You can do something like that in plpgsql, for example
>
> declare t text;
>
> for t in EXPLAIN SELECT ...
> loop
>insert into whatever values(t);
> end loop;
>

I see - this is cool.

There are still something bothering me: EXPLAIN is a mixed output with
original text, rows for RelOptInfo, rows for Paths and possible others
added later. So we have to use 't as text' to receive each line. To do the
insertion, we have to further decompose each text line into fields, and
then do the insertion - seems quite involved with plpgsql programming. So
to simplify user's task, we may end up introduce some function to do this,
like this:

/* EXPLAIN target query and dump records to target tables */
select pg_dump_searchspace('target_table_for_rel',
   'target_table_for_paths', 'select ... /* target query */');

Is this something we want?

Regards,
Qingqing


-- 
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] Reload SSL certificates on SIGHUP

2015-07-28 Thread Peter Eisentraut
On 7/21/15 8:52 PM, Andreas Karlsson wrote:
> It is not enough to just add a hook to the GUCs since I would guess most
> users would expect the certificate to be reloaded if just the file has
> been replaced and no GUC was changed. To support this we would need to
> also check the mtimes of the SSL files, would that complexity really be
> worth it?

Actually, I misread your patch.  I thought you only wanted to reload the
SSL files when the GUC settings change, but of course we also want to
reload them when the files are changed.

I don't have a problem with rebuilding the SSL context on every reload
cycle.  We already do a lot of extra reloading every time, so a bit more
shouldn't hurt.  But I'm not so sure whether we should do that in the
SIGHUP handler.  I don't know how we got into the situation of doing all
the file reloads directly in the handler, but at least we can control
that code.  Making a bunch of calls into an external library is a
different thing, though.  Can we find a way to do this differently?


-- 
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] Buildfarm TAP testing is useless as currently implemented

2015-07-28 Thread Andrew Dunstan


On 07/27/2015 12:15 PM, Andrew Dunstan wrote:


On 07/27/2015 10:06 AM, Tom Lane wrote:

I challenge anybody to figure out what happened here:
http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=hornet&dt=2015-07-27%2010%3A25%3A17 


or here:
http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=hamster&dt=2015-07-04%2016%3A00%3A23 


or here:
http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=crake&dt=2015-07-07%2016%3A35%3A06 



With no visibility of pg_ctl's output, and no copy of the postmaster 
log,

there is no chance of debugging intermittent failures like this one.
This isn't entirely the buildfarm's fault --- AFAICS, prove-based 
testing

has inadequate error reporting by design.  If "not ok" isn't enough
information for you, tough beans.  (It might help if the farm script
captured the postmaster log after a failure, but that would do nothing
for prove's unwillingness to pass through client-side messages.)

I think we should disable TAP testing in the buildfarm until there is
some credible form of error reporting for it.  I've grown tired of
looking into buildfarm failure reports only to meet a dead end.
Aside from the wasted investigation time, which admittedly isn't huge,
there's an opportunity cost in that subsequent test steps didn't get 
run.




Well, it does create a lot of files that we don't pick up. An example 
list is show below, and I am attaching their contents in a single 
gzipped attachment. However, these are in the wrong location. This was 
a vpath build and yet these tmp_check directories are all created in 
the source tree. Let's fix that and then I'll set about having the 
buildfarm collect them. That should get us further down the track.





The situation should now be substantially improved. This buildfarm 
change 
 
uses today's core changes to pick up log files. See 
 
for an example.


cheers

andrew


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Buildfarm TAP testing is useless as currently implemented

2015-07-28 Thread Tom Lane
Andrew Dunstan  writes:
>> On 07/27/2015 10:06 AM, Tom Lane wrote:
>>> I think we should disable TAP testing in the buildfarm until there is
>>> some credible form of error reporting for it.

> The situation should now be substantially improved.

Hm, I was just thinking we weren't there yet, because:
http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=axolotl&dt=2015-07-28%2023%3A03%3A39

> This buildfarm change 
> 
>  
> uses today's core changes to pick up log files.

Ah, so we need a new buildfarm script release?

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] Planner debug views

2015-07-28 Thread Tom Lane
Qingqing Zhou  writes:
> There are still something bothering me: EXPLAIN is a mixed output with
> original text, rows for RelOptInfo, rows for Paths and possible others
> added later. So we have to use 't as text' to receive each line. To do the
> insertion, we have to further decompose each text line into fields, and
> then do the insertion - seems quite involved with plpgsql programming.

Well, that's only true if we don't expend some effort to make it better.

I could imagine, for instance, that if you specify the EXPLAIN option that
turns on this additional output, the output is no longer just a single
"text" column but is multiple columns.  Perhaps one column could be a
key indicating what the other column(s) contain.

Another point is that we decided a long time ago that EXPLAIN's plain-text
output format is not intended to be machine-parsable, and so objecting to
a design on the grounds that it makes machine parsing harder is pretty
wrongheaded.  I'd think there is plenty of room for dropping in additional
output data in the non-text output formats.  That needs some careful
document-schema design effort, for sure, but it doesn't seem like it would
be particularly hard.

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] Reload SSL certificates on SIGHUP

2015-07-28 Thread Tom Lane
Peter Eisentraut  writes:
> I don't have a problem with rebuilding the SSL context on every reload
> cycle.  We already do a lot of extra reloading every time, so a bit more
> shouldn't hurt.  But I'm not so sure whether we should do that in the
> SIGHUP handler.  I don't know how we got into the situation of doing all
> the file reloads directly in the handler, but at least we can control
> that code.  Making a bunch of calls into an external library is a
> different thing, though.  Can we find a way to do this differently?

Do we have an idea how expensive it is to load that data?

A brute-force answer is to not have the postmaster load it at all,
but to have new backends do so (if needed) during their connection
acceptance/authentication phase.  I'm not sure how much that would
add to the SSL connection startup time though.  It would also mean
that problems with the SSL config files would only be reported during
subsequent connection starts, not at SIGHUP time, and indeed that
SIGHUP is more or less meaningless for SSL file changes: the instant
you change a file, it's live for later connections.  On the plus side,
it would make Windows and Unix behavior closer, since (I suppose)
we're reloading that stuff anyway in EXEC_BACKEND builds.

I'm not entirely sure your concern is valid, though.  We have always had
the principle that almost everything of interest in the postmaster happens
in signal handler functions.  We could possibly change things so that
reloading config files is done in the "main loop" of ServerLoop, but
if we did, it would have to execute with all signals blocked, which seems
like just about as much of a risk for third-party code as executing that
code in a signal handler is.

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] Buildfarm TAP testing is useless as currently implemented

2015-07-28 Thread Andrew Dunstan


On 07/28/2015 08:58 PM, Tom Lane wrote:

Andrew Dunstan  writes:

On 07/27/2015 10:06 AM, Tom Lane wrote:

I think we should disable TAP testing in the buildfarm until there is
some credible form of error reporting for it.

The situation should now be substantially improved.

Hm, I was just thinking we weren't there yet, because:
http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=axolotl&dt=2015-07-28%2023%3A03%3A39


This buildfarm change

uses today's core changes to pick up log files.

Ah, so we need a new buildfarm script release?





Yeah. I'll push one in the next couple of days.

cheers

andrew


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


  1   2   >