Re: [HACKERS] query_planner() API change

2013-08-04 Thread Atri Sharma
> While we could complicate query_planner()'s API even more to add some
> understanding of unnecessary resjunk items, I think this is probably
> the straw that breaks the camel's back for the current approach here.
> There is already a comment like this in query_planner():
>
>  * This introduces some undesirable coupling between this code and
>  * grouping_planner, but the alternatives seem even uglier; we couldn't
>  * pass back completed paths without making these decisions here.

I agree with the idea,but am trying to understand why adding
understanding of resjunk columns is a bad idea. Just for understanding
purpose, could you please elaborate a bit on it?

Regards,

Atri




-- 
Regards,

Atri
l'apprenant


-- 
Sent 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: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: Proposal for Allow postgresql.conf values to be changed via SQL [review])

2013-08-04 Thread Tom Lane
Amit Kapila  writes:
> On Saturday, August 03, 2013 12:53 AM Tom Lane wrote:
>> Yeah, this approach is a nonstarter because there's no reason to assume
>> that a postmaster started with default parameters will start
>> successfully,
>> or will be connectable-to if it does start.  Maybe there's another
>> postmaster hogging the default port, for instance.

> Okay, but user will always have option to start server with different value
> of parameter (pg_ctl -o "-p 5434").

You're assuming that the user starts the postmaster manually.  On most
modern installations there's a few layers of scripting in there, which
might not be that easy to hack to add some command-line parameters,
even assuming that the DBA has sufficient wits about him to think of
this solution.  (When your postmaster unexpectedly fails to restart
at four in the morning, having to think of such an approach isn't what
you want to be doing.)

My point here is just that we should keep the parameter values in plain
text files, so that one possible solution is reverting a bogus change with
vi/emacs/your-weapon-of-choice.  If we "improve" matters so that the only
possible way to fix the parameter setting is via a running postmaster,
we've narrowed the number of escape routes that a frantic DBA will have.
And what would we have bought for that?  Not much.

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] Bottlenecks with large number of relation segment files

2013-08-04 Thread Amit Langote
Hello,

I am looking the effect of having large number of relation files under
$PGDATA/base/ (for example, in cases where I choose lower segment size
using --with-segsize). Consider a case where I am working with a large
database with large relations, for example a database similar in size
to what "pgbench -i -s 3500" would be.

May the routines in fd.c become bottleneck with a large number of
concurrent connections to above database, say something like "pgbench
-j 8 -c 128"? Is there any other place I should be paying attention
to?

-- 
Amit Langote


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


Re: [HACKERS] query_planner() API change

2013-08-04 Thread Etsuro Fujita
> Robert Haas  writes:
> > On Sun, Aug 4, 2013 at 6:20 PM, Tom Lane  wrote:
> >> I think it's time to bite the bullet and *not* pass back completed paths.
> >> What's looking more attractive now is to just pass back the top-level
> >> RelOptInfo ("final_rel" in query_planner()).
> 
> > I tend to think this is a pretty good plan.
> 
> I looked around a little more and noted that this would complicate the
> special-case handling of an empty join tree (viz, "SELECT 2+2").  Right now
> query_planner() just has to make the appropriate Result path and it's done.
> We'd have to create a dummy RelOptInfo representing an empty set of relations,
> which is a bit weird but probably not too unreasonable when all's said and
done.

I think this is reasonable.

Thanks,

Best regards,
Etsuro Fujita



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


Re: [HACKERS] Re: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: Proposal for Allow postgresql.conf values to be changed via SQL [review])

2013-08-04 Thread Amit Kapila
On Saturday, August 03, 2013 12:53 AM Tom Lane wrote:
> Stephen Frost  writes:
> > * Josh Berkus (j...@agliodbs.com) wrote:
> >> A much simpler solution to the issue Stephen proposes is to have a
> way
> >> to start up the server with all settings from ALTER SYSTEM SET
> disabled,
> >> just like some software allows you to start it up in "safe mode".
> 
> > See above for why I'm not thrilled wih this approach, unless it was
> set
> > up to happen automatically, but you couldn't simply ignore *all* the
> > ALTER SYSTEM SET parameters because then you might not be able to
> > connect in due to some ALTER SYSTEM SET parameter being necessary for
> > remote connectivity or authentication.
> 
> Yeah, this approach is a nonstarter because there's no reason to assume
> that a postmaster started with default parameters will start
> successfully,
> or will be connectable-to if it does start.  Maybe there's another
> postmaster hogging the default port, for instance.

Okay, but user will always have option to start server with different value
of parameter (pg_ctl -o "-p 5434").

Now as a summarization we have below ways to move forward:

1. Provide a way for user to start server if not able to start due to
in-appropriate value of unsafe parameter
   a. already user has an option that he can mention value of any particular
parameter with which sever can start
   b. keep one backup copy of parameters, so that user can option to start
with that copy, else if that also doesn't work he
  can use point 'a'.

2. Don't allow unsafe parameters to be modified by ALTER SYSTEM
   a. List of un-safe parameters
   b. mechanism so that ALTER SYSTEM throws error for non-modifiable
parameters
   c. user can view non-modifiable parameters (may be in pg_settings)
   d. some way such that if user wants to take risk of server not getting
started, he should allow to modify such parameters.
  may be server is started with some specific option. This can reduce
the fear Josh had regarding this command to be not of much use.

I think if we choose Option-2, then one of the initial difficulty will be to
get an agreement on list of un-safe parameters.
I believe even if we want to go with Option-2, then in first cut the work
should be minimized. 

With Regards,
Amit Kapila.



-- 
Sent 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 for reserved connections for replication users

2013-08-04 Thread Andres Freund
On 2013-08-02 08:16:15 -0400, Robert Haas wrote:
> On Tue, Jul 30, 2013 at 3:10 AM, Gibheer  wrote:
> > here is an update off my patch based on the discussion with Marko
> > Tiikkaja and Andres Freund.
> >
> > Marko and I had the idea of introducing reserved connections based on
> > roles as it would create a way to garantuee specific roles to connect
> > when other roles use up all connections for whatever reason. But
> > Andreas said, that it would make connecting take much too long.
> >
> > So to just fix the issue at hand, we decided that adding
> > max_wal_senders to the pool of reserved connections is better. With
> > that, we are sure that streaming replication can connect to the master.
> >
> > So instead of creating a new configuration option I added
> > max_wal_senders to the reserved connections and changed the check for
> > new connections.
> >
> > The test.pl is a small script to test, if the patch does what it should.
> 
> Hmm.  It seems like this match is making MaxConnections no longer mean
> the maximum number of connections, but rather the maximum number of
> non-replication connections.  I don't think I support that
> definitional change, and I'm kinda surprised if this is sufficient to
> implement it anyway (e.g. see InitProcGlobal()).

I don't think the implementation is correct, but why don't you like the
definitional change? The set of things you can do from replication
connections are completely different from a normal connection. So using
separate "pools" for them seems to make sense.
That they end up allocating similar internal data seems to be an
implementation detail to me.

Greetings,

Andres Freund

-- 
 Andres Freund 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] Patch for reserved connections for replication users

2013-08-04 Thread Gibheer
On Fri, 2 Aug 2013 08:16:15 -0400
Robert Haas  wrote:

> On Tue, Jul 30, 2013 at 3:10 AM, Gibheer 
> wrote:
> > here is an update off my patch based on the discussion with Marko
> > Tiikkaja and Andres Freund.
> >
> > Marko and I had the idea of introducing reserved connections based
> > on roles as it would create a way to garantuee specific roles to
> > connect when other roles use up all connections for whatever
> > reason. But Andreas said, that it would make connecting take much
> > too long.
> >
> > So to just fix the issue at hand, we decided that adding
> > max_wal_senders to the pool of reserved connections is better. With
> > that, we are sure that streaming replication can connect to the
> > master.
> >
> > So instead of creating a new configuration option I added
> > max_wal_senders to the reserved connections and changed the check
> > for new connections.
> >
> > The test.pl is a small script to test, if the patch does what it
> > should.
> 
> Hmm.  It seems like this match is making MaxConnections no longer mean
> the maximum number of connections, but rather the maximum number of
> non-replication connections.  I don't think I support that
> definitional change, and I'm kinda surprised if this is sufficient to
> implement it anyway (e.g. see InitProcGlobal()).
> 

You are right, that can't be correct. The slots I added with
max_wal_sender would end up as background worker slots. I have to check
my tests again.

In my first patch I just copied the part to limit the connections based
on superuser reserved connections + replication reserved connections.
That did not change the definition of max_connections and made
superuser connections higher in priority than replication connections.
Is that the better approach?

Thank you for your input.

Stefan


-- 
Sent 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 for removng unused targets

2013-08-04 Thread Etsuro Fujita
> From: Tom Lane [mailto:t...@sss.pgh.pa.us]

> Having said all that, there is one situation where this type of approach might
> still be useful even after such a fix, and that's KNNGist-style
> queries:
> 
> select a,b,c from t order by col <-> constant limit 10;
> 
> In a KNNGist search, there's no provision for the index AM to return the
actual
> value of the ORDER BY expression, and in fact it's theoretically possible that
> that value is never even explicitly computed inside the index AM.  So we
couldn't
> suppress the useless evaluation of <-> by dint of requiring the physical scan
> to return that value as a Var.
> 
> Reading between the lines of the original submission at
> ,
> I gather that it's the KNNGist-style case that worries you, so maybe it's
worth
> applying this type of patch anyway.  I'd want to rejigger it to be aware of
> the cost implications though, at least for grouping_planner's choices.

+1 for improving  KNNGist-style queries.

Sorry for the late response.

Thanks,

Best regards,
Etsuro Fujita



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


Re: [HACKERS] [9.4 CF 1]Commitfest ... over!

2013-08-04 Thread Amit Kapila
On Saturday, August 03, 2013 5:17 AM Josh Berkus wrote:
> Folks,
> 
> The first CF for the 9.4 development cycle is officially over.
> 
> In all, 49 patches were committed, 47 were returned with feedback, 6
> were rejected outright, and 6 were punted to CF2.  We're 17 days over
> the CF deadline at this point, but that's unsurprising considering that
> this CF included a record number of patches -- 108 patches at peak,
> compared with 59 for last year's CF1, and 101 for even CF4.  So this
> was, measured strictly by patch count, the biggest CF ever (of course,
> that's not the only measure).
> 
> See my blog at
> http://www.databasesoup.com/2013/08/94-commitfest-1-wrap-up.html for
> some additional details.
> 
> Given that we can expect to be dealing with more patches per CF in the
> future, I'd like some feedback about what things would make the CF
> process more efficient.  For that matter, for the first time we tried
> enforcing some of the "rules" of CFs this time, and I'd like to hear if
> people think that helped.

First of all Thank you very much for running a wonderful Commit  Fest.
The best part according to me is that all patches got fair review 
comments/suggestions to improve.


With Regards,
Amit Kapila.



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


Re: [HACKERS] PostgreSQL and ASLR on Linux

2013-08-04 Thread Tom Lane
Robert Haas  writes:
> On Wed, Jul 31, 2013 at 4:35 PM, Robert Lerche (rlerche)
>  wrote:
>> Hi.  Has anyone had experience building PostgreSQL to support Address Space
>> Layout Randomization (ASLR)?  I recently took a brute-force approach
>> (compiling everything with -fPIC and specifying -pie on all executables).
>> This worked, but a (very superficial) performance test indicated a high cost
>> (around 50%, much more than I expected).  This was on 64-bit Linux x86.

> AFAIK you've got it backwards: ASLR is something that happens
> automatically, unless you take steps to suppress it, at least on MacOS
> X.  I not long ago built with EXEC_BACKEND on that platform and found
> that it broke stuff until I disabled ASLR.

I believe pretty much everybody applies ASLR to stack and data areas by
default, and to code in shared libraries (which have to be built with
-fPIC anyway).  But you don't get ASLR on a program executable's code area
unless it's built with PIE, at least not on Linux.  Also, there are some
other defenses against code-stomp attacks, like RELRO and "-z now" (early
linkage binding so the GOT can be marked readonly), which tend to be
included as well when people speak of hardened binaries.

Back when I was still wearing a red fedora, I experimented a bit with
hardening Red Hat's PG build, but I didn't get to the question of whether
it hurt performance because it flat out did not work in 32-bit builds:
https://bugzilla.redhat.com/show_bug.cgi?id=947022
Those problems wouldn't apply to a 64-bit build though.

A 50% cost sounds pretty astonishing for x86_64 --- there's no way that
people would be clamoring for enabling these methods by default (which
they are) if that were the typical penalty.  The Fedora packaging
guidelines claim the only significant performance cost is loss of prelink
support, which should theoretically only affect process startup time.
If you were doing very short-term performance tests, maybe that's what
you were seeing?

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] PostgreSQL and ASLR on Linux

2013-08-04 Thread Andres Freund
On 2013-08-04 21:07:02 -0400, Robert Haas wrote:
> On Sun, Aug 4, 2013 at 8:54 PM, Andres Freund  wrote:
> >> AFAIK you've got it backwards: ASLR is something that happens
> >> automatically, unless you take steps to suppress it, at least on MacOS
> >> X.  I not long ago built with EXEC_BACKEND on that platform and found
> >> that it broke stuff until I disabled ASLR.
> >
> > ALSR for code can only happen if code is built as position independent
> > code, otherwise addresses are hardcoded. That is - in modern unixoid
> > systems - nearly always the case for shared libraries et al, but not
> > necessarily for plain binaries or statically linked code. The above
> > referenced -fPIC and -pie make the code/executable position independent.
> 
> Ah, for code, yeah, I suppose that would be true.  In the case I
> mentioned though, though, it definitely seemed that other things were
> moving around each time through, particularly the stack.

Oh, yes. Those just don't require PIE executables, so you can see the
problem independently and to my knowledge their price is far lower.

I personally think that that property/requirement of EXEC_BACKEND is
going to come from behind and bite us hard...

Greetings,

Andres Freund

-- 
 Andres Freund 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] PostgreSQL and ASLR on Linux

2013-08-04 Thread Robert Haas
On Sun, Aug 4, 2013 at 8:54 PM, Andres Freund  wrote:
>> AFAIK you've got it backwards: ASLR is something that happens
>> automatically, unless you take steps to suppress it, at least on MacOS
>> X.  I not long ago built with EXEC_BACKEND on that platform and found
>> that it broke stuff until I disabled ASLR.
>
> ALSR for code can only happen if code is built as position independent
> code, otherwise addresses are hardcoded. That is - in modern unixoid
> systems - nearly always the case for shared libraries et al, but not
> necessarily for plain binaries or statically linked code. The above
> referenced -fPIC and -pie make the code/executable position independent.

Ah, for code, yeah, I suppose that would be true.  In the case I
mentioned though, though, it definitely seemed that other things were
moving around each time through, particularly the stack.

-- 
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] query_planner() API change

2013-08-04 Thread Tom Lane
Robert Haas  writes:
> On Sun, Aug 4, 2013 at 6:20 PM, Tom Lane  wrote:
>> I think it's time to bite the bullet and *not* pass back completed paths.
>> What's looking more attractive now is to just pass back the top-level
>> RelOptInfo ("final_rel" in query_planner()).

> I tend to think this is a pretty good plan.

I looked around a little more and noted that this would complicate the
special-case handling of an empty join tree (viz, "SELECT 2+2").  Right
now query_planner() just has to make the appropriate Result path and it's
done.  We'd have to create a dummy RelOptInfo representing an empty set
of relations, which is a bit weird but probably not too unreasonable
when all's said and done.

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] PostgreSQL and ASLR on Linux

2013-08-04 Thread Andres Freund
On 2013-08-04 20:33:50 -0400, Robert Haas wrote:
> On Wed, Jul 31, 2013 at 4:35 PM, Robert Lerche (rlerche)
>  wrote:
> > Hi.  Has anyone had experience building PostgreSQL to support Address Space
> > Layout Randomization (ASLR)?  I recently took a brute-force approach
> > (compiling everything with -fPIC and specifying -pie on all executables).
> > This worked, but a (very superficial) performance test indicated a high cost
> > (around 50%, much more than I expected).  This was on 64-bit Linux
> > x86.

What benchmark did you run? Did you run a profile?

I am not really surprised that compiling the backend itself as position
independent code has a high price. There's lots of switch/jump tables in
pg that are called in hot paths. Adding math to those will have a price.

> > Google turns up some references to the Ubuntu distribution of version 8.3
> > being built this way but nothing much more interesting.
> >
> > I’d appreciate any information or help anyone can give me on this.  Thanks.
> 
> AFAIK you've got it backwards: ASLR is something that happens
> automatically, unless you take steps to suppress it, at least on MacOS
> X.  I not long ago built with EXEC_BACKEND on that platform and found
> that it broke stuff until I disabled ASLR.

ALSR for code can only happen if code is built as position independent
code, otherwise addresses are hardcoded. That is - in modern unixoid
systems - nearly always the case for shared libraries et al, but not
necessarily for plain binaries or statically linked code. The above
referenced -fPIC and -pie make the code/executable position independent.

Greetings,

Andres Freund

-- 
 Andres Freund 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] PostgreSQL and ASLR on Linux

2013-08-04 Thread Robert Haas
On Wed, Jul 31, 2013 at 4:35 PM, Robert Lerche (rlerche)
 wrote:
> Hi.  Has anyone had experience building PostgreSQL to support Address Space
> Layout Randomization (ASLR)?  I recently took a brute-force approach
> (compiling everything with -fPIC and specifying -pie on all executables).
> This worked, but a (very superficial) performance test indicated a high cost
> (around 50%, much more than I expected).  This was on 64-bit Linux x86.
>
> Google turns up some references to the Ubuntu distribution of version 8.3
> being built this way but nothing much more interesting.
>
> I’d appreciate any information or help anyone can give me on this.  Thanks.

AFAIK you've got it backwards: ASLR is something that happens
automatically, unless you take steps to suppress it, at least on MacOS
X.  I not long ago built with EXEC_BACKEND on that platform and found
that it broke stuff until I disabled ASLR.

-- 
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] Review: UNNEST (and other functions) WITH ORDINALITY

2013-08-04 Thread Robert Haas
On Mon, Jul 29, 2013 at 1:42 AM, Andrew Gierth
 wrote:
> I propose the following patch (which goes on top of the current
> ordinality one) to implement the suggested grammar changes.
>
> I think this is the cleanest way, and I've tested that it both
> passes regression and allows constructs like WITH time AS (...)
> to work.

This looks like really nice work.

-- 
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] query_planner() API change

2013-08-04 Thread Robert Haas
On Sun, Aug 4, 2013 at 6:20 PM, Tom Lane  wrote:
> I've been looking at what it would take to do proper cost estimation
> for the recently-discussed patch to suppress calculation of unnecessary
> ORDER BY expressions.  It turns out that knowledge of that would have
> to propagate into query_planner(), because the place where we do the cost
> comparison between unsorted and presorted paths is in there (planmain.c
> lines 390ff in HEAD).  As it stands, query_planner() will actually refuse
> to return the presorted path to grouping_planner() at all if it thinks
> it's a loser cost-wise, meaning grouping_planner() would have no
> opportunity to override the decision.  So there's no way to fix this
> without some API change for query_planner().
>
> While we could complicate query_planner()'s API even more to add some
> understanding of unnecessary resjunk items, I think this is probably
> the straw that breaks the camel's back for the current approach here.
> There is already a comment like this in query_planner():
>
>  * This introduces some undesirable coupling between this code and
>  * grouping_planner, but the alternatives seem even uglier; we couldn't
>  * pass back completed paths without making these decisions here.
>
> I think it's time to bite the bullet and *not* pass back completed paths.
> What's looking more attractive now is to just pass back the top-level
> RelOptInfo ("final_rel" in query_planner()).  We could remove all three
> output parameters of query_planner(), and essentially just move lines
> 265-420 (pretty much everything after the make_one_rel() call) into
> planner.c.  Since that code is almost all about grouping-related choices,
> this seems like it'll be a net improvement modularity-wise, even though
> it'll make grouping_planner() even bigger.  We could probably ameliorate
> the latter problem by putting the calculation of num_groups and adjustment
> of tuple_fraction into a subroutine.
>
> Objections, better ideas?

I tend to think this is a pretty good 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


[HACKERS] query_planner() API change

2013-08-04 Thread Tom Lane
I've been looking at what it would take to do proper cost estimation
for the recently-discussed patch to suppress calculation of unnecessary
ORDER BY expressions.  It turns out that knowledge of that would have
to propagate into query_planner(), because the place where we do the cost
comparison between unsorted and presorted paths is in there (planmain.c
lines 390ff in HEAD).  As it stands, query_planner() will actually refuse
to return the presorted path to grouping_planner() at all if it thinks
it's a loser cost-wise, meaning grouping_planner() would have no
opportunity to override the decision.  So there's no way to fix this
without some API change for query_planner().

While we could complicate query_planner()'s API even more to add some
understanding of unnecessary resjunk items, I think this is probably
the straw that breaks the camel's back for the current approach here.
There is already a comment like this in query_planner():

 * This introduces some undesirable coupling between this code and
 * grouping_planner, but the alternatives seem even uglier; we couldn't
 * pass back completed paths without making these decisions here.

I think it's time to bite the bullet and *not* pass back completed paths.
What's looking more attractive now is to just pass back the top-level
RelOptInfo ("final_rel" in query_planner()).  We could remove all three
output parameters of query_planner(), and essentially just move lines
265-420 (pretty much everything after the make_one_rel() call) into
planner.c.  Since that code is almost all about grouping-related choices,
this seems like it'll be a net improvement modularity-wise, even though
it'll make grouping_planner() even bigger.  We could probably ameliorate
the latter problem by putting the calculation of num_groups and adjustment
of tuple_fraction into a subroutine.

Objections, better ideas?

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: ALTER SYSTEM SET command to change postgresql.conf parameters (RE: [HACKERS] Proposal for Allow postgresql.conf values to be changed via SQL [review])

2013-08-04 Thread Dimitri Fontaine
Alvaro Herrera  writes:
> I remind you that event triggers are not fired for global objects
> such as databases and roles.  Do you intend to lift that restriction?

That's not on my TODO list for 9.4. My understanding about implementing
that is:

  - we agree that it would be nice to have,
  - it requires a separate *shared* catalog for event triggers.

What I'm yet unsure about is that there's a consensus that the use cases
are worthy of a new shared catalog in the system. Also I didn't look how
hard it is to actually provide for it.

Regards,
-- 
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support


-- 
Sent 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_restore: show object schema name in verbose output

2013-08-04 Thread Ian Lawrence Barwick
2013/8/4 Ian Lawrence Barwick :
> 2013/8/4 Erik Rijkers :
>> On Sun, August 4, 2013 04:51, Ian Lawrence Barwick wrote:
>>> I just noticed that pg_restore executing in "verbose" mode displays the
>>> name of the object being restored, but not its schema.
>>>
>>
>> Good idea.  We have many schemata with tables of the same name and
>> reporting the schema name certainly improves the readability and error
>> trackdown during restore.
>>
>> I notice 2 things:
>>
>>
>> 1.  pg_restore now outputs reports about COMMENT like this:
>> pg_restore: creating COMMENT restore_verbose_test.TABLE t
>> pg_restore: creating COMMENT restore_verbose_test.COLUMN t.c
>> pg_restore: creating COMMENT restore_verbose_test.COLUMN t.i
>>
>> I assume the .TABLE and .COLUMN here is a bug; it should just be:
>>
>> pg_restore: creating COMMENT restore_verbose_test t
>>
>> as it used to be.

Actually the current output would be:

  pg_restore: creating COMMENT TABLE t

Anyway this is a bit trickier than I originally thought, but I understand
the inner workings of pg_restore et al better now anyway :)

>> 2.  Several of the lines that are output by pg_restore now mention
>> the schema, but not the "processing" line:
>>
>> pg_restore: processing data for table "t"
>>
>> Could it be added there too?

That looks quite straightforward.

> Thanks for the feedback and test case. I'll submit a revised patch.

The attached patch should work somewhat better, but methinks I'll need
to work on it a bit more. Also, for the sake of consistency it would
be useful to show the schema (where appropriate) in the owner/privileges
setting output, e.g.:

  pg_restore: setting owner and privileges for TABLE t


Regards

Ian Barwick


pg-restore-verbose-output-schema-2013-08-04.patch
Description: Binary data

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


Re: [HACKERS] Extension Templates S03E11

2013-08-04 Thread Dimitri Fontaine
Thom Brown  writes:
> Could you please resubmit this without using SnapshotNow as it's no longer
> supported?

Sure, sorry that I missed that, please find v12 attached.

Regards,
-- 
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support



templates.v12.patch.gz
Description: Binary data

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


Re: [HACKERS] pg_restore: show object schema name in verbose output

2013-08-04 Thread Ian Lawrence Barwick
2013/8/4 Erik Rijkers :
> On Sun, August 4, 2013 04:51, Ian Lawrence Barwick wrote:
>> I just noticed that pg_restore executing in "verbose" mode displays the
>> name of the object being restored, but not its schema.
>>
>
> Good idea.  We have many schemata with tables of the same name and
> reporting the schema name certainly improves the readability and error
> trackdown during restore.
>
> I notice 2 things:
>
>
> 1.  pg_restore now outputs reports about COMMENT like this:
> pg_restore: creating COMMENT restore_verbose_test.TABLE t
> pg_restore: creating COMMENT restore_verbose_test.COLUMN t.c
> pg_restore: creating COMMENT restore_verbose_test.COLUMN t.i
>
> I assume the .TABLE and .COLUMN here is a bug; it should just be:
>
> pg_restore: creating COMMENT restore_verbose_test t
>
> as it used to be.
>
>
> 2.  Several of the lines that are output by pg_restore now mention
> the schema, but not the "processing" line:
>
> pg_restore: processing data for table "t"
>
> Could it be added there too?

Thanks for the feedback and test case. I'll submit a revised patch.

Regards

Ian Barwick


-- 
Sent 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_restore: show object schema name in verbose output

2013-08-04 Thread Erik Rijkers
On Sun, August 4, 2013 04:51, Ian Lawrence Barwick wrote:
> I just noticed that pg_restore executing in "verbose" mode displays the
> name of the object being restored, but not its schema.
>

Good idea.  We have many schemata with tables of the same name and
reporting the schema name certainly improves the readability and error
trackdown during restore.

I notice 2 things:


1.  pg_restore now outputs reports about COMMENT like this:
pg_restore: creating COMMENT restore_verbose_test.TABLE t
pg_restore: creating COMMENT restore_verbose_test.COLUMN t.c
pg_restore: creating COMMENT restore_verbose_test.COLUMN t.i

I assume the .TABLE and .COLUMN here is a bug; it should just be:

pg_restore: creating COMMENT restore_verbose_test t

as it used to be.


2.  Several of the lines that are output by pg_restore now mention
the schema, but not the "processing" line:

pg_restore: processing data for table "t"

Could it be added there too?


Thanks,

Erik Rijkers



#!/bin/sh

schema=restore_verbose_test
 table=t
 t=$schema.$table

rm -rf $schema.dump
echo

echo "
drop schema if exists $schema cascade;
create schema $schema;
drop table if exists $t;
create table $t(c text, i serial primary key);
comment on table $t is 'table $t is a table';
comment on column $t.c is 'column c is a column text in table $t';
comment on column $t.i is 'column i is a column serial (pk) in table $t';
insert into $t select chr(i) from generate_series(65,70) f(i);
" | psql -X

echo "table $t limit 5; \\d+ $t" | psql -X

time pg_dump -v -F d -n $schema -f $schema.dump testdb

echo "drop schema if exists $schema cascade;" | psql -X

echo '\dn '$schema | psql -X

pg_restore -v -d testdb $schema.dump

echo '\dn '$schema | psql -X
echo "table $t limit 5; \\d+ $t" | psql -X




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