[HACKERS] CTAS not honoring NOT NULL, DEFAULT modifiers

2010-04-19 Thread Nikhil Sontakke
Guess no-one got to read this email. I sent it to pgsql-patches
without realizing that it is a dead-list. Shouldn't we atleast bounce
emails back to senders if they send an email to pgsql-patches?

Regards,
NIkhils


-- Forwarded message --
From: Nikhil Sontakke nikhil.sonta...@enterprisedb.com
Date: Fri, Apr 2, 2010 at 6:07 PM
Subject: CTAS not honoring NOT NULL, DEFAULT modifiers
To: pgsql-patc...@postgresql.org


Hi,

I saw this behavior with PG head:

postgres=# create table x(x int default 8 not null);
CREATE TABLE
postgres=# create table x1 as select * from x;
SELECT 0
postgres=# \d x
          Table public.x
 Column |  Type   |     Modifiers
+-+
 x      | integer | not null default 8

postgres=# \d x1
     Table public.x1
 Column |  Type   | Modifiers
+-+---
 x      | integer |

Note that column x for table x1 did not get the not null modifier. It
also did not get the default values.

Was wondering what are the standards for CTAS. Oracle seems to honor
the NOT NULL modifier. This might be a bug if we do not honor
modifiers in CTAS.

Regards,
Nikhils

--
http://www.enterprisedb.com



-- 
http://www.enterprisedb.com

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


[HACKERS] row estimation off the mark when generate_series calls are involved

2010-04-19 Thread Nikhil Sontakke
Another email which went into the wilderness when I sent it to pgsql-patches.

Regards,
Nikhils


-- Forwarded message --
From: Nikhil Sontakke nikhil.sonta...@enterprisedb.com
Date: Fri, Apr 16, 2010 at 6:50 PM
Subject: row estimation off the mark when generate_series calls are involved
To: pgsql-patc...@postgresql.org


Hi,

I observed the following behavior on PG head:

postgres=# create table x(x int);
CREATE TABLE
postgres=# explain verbose insert into public.x values (generate_series(1,10));

 Insert  (cost=0.00..0.01 rows=1 width=0)

postgres=# explain verbose insert into public.x values
(generate_series(1,1000));

 Insert  (cost=0.00..0.01 rows=1 width=0)

So even though generate_series has a prorows value of 1000 (why did we
pick this value, just a guesstimate I guess?), its effects are not
shown in the plan at all. I think the place where we set the
targetlist of the result_plan to sub_tlist, immediately after that we
should update the plan_rows estimate by walking this latest
targetlist. I did that and now we seem to get proper row estimates.

Comments?

Regards,
Nikhils
--
http://www.enterprisedb.com



-- 
http://www.enterprisedb.com
Index: src/backend/optimizer/plan/planner.c
===
RCS file: /repositories/postgreshome/cvs/pgsql/src/backend/optimizer/plan/planner.c,v
retrieving revision 1.267
diff -c -r1.267 planner.c
*** src/backend/optimizer/plan/planner.c	30 Mar 2010 21:58:10 -	1.267
--- src/backend/optimizer/plan/planner.c	16 Apr 2010 13:46:35 -
***
*** 1241,1246 
--- 1241,1253 
  	 * the desired tlist.
  	 */
  	result_plan-targetlist = sub_tlist;
+ 
+ 	/*
+ 	 * Account for changes in plan row estimates because of
+ 	 * this tlist addition
+ 	 */
+ 	result_plan-plan_rows += clamp_row_est(
+ expression_returns_set_rows((Node *)result_plan-targetlist));
  }
  
  /*

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


[HACKERS] Re: [COMMITTERS] pgsql: Tune GetSnapshotData() during Hot Standby by avoiding loop

2010-04-19 Thread Simon Riggs
On Mon, 2010-04-19 at 10:36 +0300, Heikki Linnakangas wrote:
 Simon Riggs wrote:
  Log Message:
  ---
  Tune GetSnapshotData() during Hot Standby by avoiding loop
  through normal backends. Makes code clearer also, since we
  avoid various Assert()s. Performance of snapshots taken
  during recovery no longer depends upon number of read-only
  backends.
 
 I think there's a little race condition there.
 snapshot-takenDuringRecovery is set before acquiring ProcArrayLock, so
 it's possible that by the time we acquire the lock, we're no longer in
 recovery. So this can happen:
 
 1. Backend A starts to take a snapshot, while we're still in recovery.
 takenDuringRecovery is assigned true.
 2. Recovery ends, and a normal transaction X begins in backend B.
 3. A skips scanning ProcArray because takenDuringRecovery is true.
 
 The snapshot doesn't include X, so any changes done in that transaction
 will not be visible to the snapshot while the transaction is still
 running, but will be after it commits.
 
 Of course, it's highly improbable for 2. to happen, but it's there.

The order of events is as you say, though I don't see the problem. The
new xids will be beyond xmax and would be filtered out even if we did
scan the procs, so they will be treated as running, which they are. Xmax
will not have advanced since that relies on completed transactions, not
started ones.

-- 
 Simon Riggs   www.2ndQuadrant.com


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


[HACKERS] Re: [COMMITTERS] pgsql: Tune GetSnapshotData() during Hot Standby by avoiding loop

2010-04-19 Thread Heikki Linnakangas
Simon Riggs wrote:
 On Mon, 2010-04-19 at 10:36 +0300, Heikki Linnakangas wrote:
 Simon Riggs wrote:
 Log Message:
 ---
 Tune GetSnapshotData() during Hot Standby by avoiding loop
 through normal backends. Makes code clearer also, since we
 avoid various Assert()s. Performance of snapshots taken
 during recovery no longer depends upon number of read-only
 backends.
 I think there's a little race condition there.
 snapshot-takenDuringRecovery is set before acquiring ProcArrayLock, so
 it's possible that by the time we acquire the lock, we're no longer in
 recovery. So this can happen:

 1. Backend A starts to take a snapshot, while we're still in recovery.
 takenDuringRecovery is assigned true.
 2. Recovery ends, and a normal transaction X begins in backend B.
 3. A skips scanning ProcArray because takenDuringRecovery is true.

 The snapshot doesn't include X, so any changes done in that transaction
 will not be visible to the snapshot while the transaction is still
 running, but will be after it commits.

 Of course, it's highly improbable for 2. to happen, but it's there.
 
 The order of events is as you say, though I don't see the problem. The
 new xids will be beyond xmax and would be filtered out even if we did
 scan the procs, so they will be treated as running, which they are. Xmax
 will not have advanced since that relies on completed transactions, not
 started ones.

Good point. However, it is theoretically possible that yet another
transaction starts and commits in that same window, bumping xmax.

-- 
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com

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


[HACKERS] Re: [COMMITTERS] pgsql: Tune GetSnapshotData() during Hot Standby by avoiding loop

2010-04-19 Thread Simon Riggs
On Mon, 2010-04-19 at 11:37 +0300, Heikki Linnakangas wrote:
 Simon Riggs wrote:
  On Mon, 2010-04-19 at 10:36 +0300, Heikki Linnakangas wrote:
  Simon Riggs wrote:
  Log Message:
  ---
  Tune GetSnapshotData() during Hot Standby by avoiding loop
  through normal backends. Makes code clearer also, since we
  avoid various Assert()s. Performance of snapshots taken
  during recovery no longer depends upon number of read-only
  backends.
  I think there's a little race condition there.
  snapshot-takenDuringRecovery is set before acquiring ProcArrayLock, so
  it's possible that by the time we acquire the lock, we're no longer in
  recovery. So this can happen:
 
  1. Backend A starts to take a snapshot, while we're still in recovery.
  takenDuringRecovery is assigned true.
  2. Recovery ends, and a normal transaction X begins in backend B.
  3. A skips scanning ProcArray because takenDuringRecovery is true.
 
  The snapshot doesn't include X, so any changes done in that transaction
  will not be visible to the snapshot while the transaction is still
  running, but will be after it commits.
 
  Of course, it's highly improbable for 2. to happen, but it's there.
  
  The order of events is as you say, though I don't see the problem. The
  new xids will be beyond xmax and would be filtered out even if we did
  scan the procs, so they will be treated as running, which they are. Xmax
  will not have advanced since that relies on completed transactions, not
  started ones.
 
 Good point. However, it is theoretically possible that yet another
 transaction starts and commits in that same window, bumping xmax.

If the snapshotting backend (#1) froze at the exact point between one
CPU instruction and the next, during which time recovery ends, two new
sessions connect, two new write transactions start (#2 and #3), they
begin to write data and so assign xids, then write WAL, then the #3
writes commit record into WAL, flushes disk (maybe), updates clog and
then is granted procarraylock in exclusive mode before snapshotting
backend attempts to do so, while #2 keeps running. 

Yes, it does seem theoretically possible, at that one exact point in
time, never to be repeated.

It doesn't seem to be something we should place highly on the list of
events we need protection from, does it?

-- 
 Simon Riggs   www.2ndQuadrant.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] CTAS not honoring NOT NULL, DEFAULT modifiers

2010-04-19 Thread Magnus Hagander
On Mon, Apr 19, 2010 at 08:32, Nikhil Sontakke
nikhil.sonta...@enterprisedb.com wrote:
 Hi,

 I saw this behavior with PG head:

 postgres=# create table x(x int default 8 not null);
 CREATE TABLE
 postgres=# create table x1 as select * from x;
 SELECT 0
 postgres=# \d x
           Table public.x
  Column |  Type   |     Modifiers
 +-+
  x      | integer | not null default 8

 postgres=# \d x1
      Table public.x1
  Column |  Type   | Modifiers
 +-+---
  x      | integer |

 Note that column x for table x1 did not get the not null modifier. It
 also did not get the default values.

 Was wondering what are the standards for CTAS. Oracle seems to honor
 the NOT NULL modifier. This might be a bug if we do not honor
 modifiers in CTAS.

Given that CREATE TABLE AS creates a table based on the result of a
query, it seems pretty logical that constraints wouldn't be copied
over - they're part of the table, they're not visible in a query
result.

The documentation pretty clearly says you should use CREATE TABLE LIKE
if you want to copy the constraints over, if you look at the CREATE
TABLE manpage (not on the CREATE TABLE AS though - perhaps a note
should be added there?)

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.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] master in standby mode croaks

2010-04-19 Thread Fujii Masao
On Mon, Apr 19, 2010 at 11:04 AM, Robert Haas robertmh...@gmail.com wrote:
 First of all, I wonder why the latter two need to affect the decision of
 whether additional information is written to WAL for HS. How about just
 removing XLogIsNeeded() condition from XLogStandbyInfoActive()?

 Bad idea, I think.  If XLogIsNeeded() is returning false and
 XLogStandbyInfoActive() is returning true, the resulting WAL will
 still be unusable for HS, at least AIUI.

Probably No. Such a WAL will be usable for HS unless an unlogged
operation (e.g., CLUSTER, CREATE TABLE AS SELECT, etc) happens.
I think that the occurrence of an unlogged operation rather than
XLogIsNeeded() itself must be checked in the standby, it's already
been being checked. So just removing XLogIsNeeded() condition makes
sense to me.

Regards,

-- 
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

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


Re: [HACKERS] walreceiver is uninterruptible on win32

2010-04-19 Thread Fujii Masao
On Fri, Apr 16, 2010 at 4:03 PM, Fujii Masao masao.fu...@gmail.com wrote:
 On Thu, Apr 15, 2010 at 11:26 PM, Robert Haas robertmh...@gmail.com wrote:
 I have to admit to finding this confusing.  According to the comments:

 +               /*
 +                * Don't emulate the PQexec()'s behavior of returning the 
 last
 +                * result when there are many, since walreceiver never sends 
 a
 +                * query returning multiple results.
 +                */

 ...but it looks like the code actually is implementing some sort of
 loop-that-returns-the-last result.

 Yeah, it's not a very accurate description. And I found another problem:
 libpqrcv_PQexec() ends as soon as an error result arrives even if its
 state has not been PGASYNC_IDLE yet.

 So I changed libpqrcv_PQexec() so that it emulates the PQexec()'s behavior
 except the concatenation of error messages. How about the attached patch?

BTW, even if you apply the patch, you would not be able to interrupt the
walreceiver by using smart shutdown because of the bug reported in another
thread. Please be careful about that when you test the patch.
http://archives.postgresql.org/pgsql-hackers/2010-04/msg00592.php

Regards,

-- 
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

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


Re: [HACKERS] Re: [COMMITTERS] pgsql: Tune GetSnapshotData() during Hot Standby by avoiding loop

2010-04-19 Thread Robert Haas
On Mon, Apr 19, 2010 at 5:05 AM, Simon Riggs si...@2ndquadrant.com wrote:
 It doesn't seem to be something we should place highly on the list of
 events we need protection from, does it?

Since when do we not protect against race-conditions just because
they're low likelihood?

...Robert

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


Re: [HACKERS] [GENERAL] trouble with to_char('L')

2010-04-19 Thread Magnus Hagander
On Mon, Apr 19, 2010 at 03:59, Takahiro Itagaki
itagaki.takah...@oss.ntt.co.jp wrote:

 Magnus Hagander mag...@hagander.net wrote:

 But I'm unsure how that would work. We're talking about the output of
 localeconv(), right? I don't see a version of localeconv() that does
 wide chars anywhere. (You can't just set LC_CTYPE and use the regular
 function - Windows has a separate set of functions for dealing with
 UTF16).

 Yeah, msvcrt doesn't have wlocaleconv :-( . Since localeconv() returns
 characters in the encoding specified in LC_TYPE, we need to hande the
 issue with codes something like:

    1. setlocale(LC_CTYPE, lc_monetary)
    2. setlocale(LC_MONETARY, lc_monetary)
    3. lc = localeconv()
    4. pg_do_encoding_conversion(lc-xxx,
          FROM pg_get_encoding_from_locale(lc_monetary),
          TO GetDatabaseEncoding())
    5. Revert LC_CTYPE and LC_MONETARY.


 Another idea is to use GetLocaleInfoW() [1], that is win32 native locale
 functions, instead of the libc one. It returns locale characters in wide
 chars, so we can safely convert them as UTF16-UTF8-db. But it requires
 an additional branch in our locale codes only for Windows.

If we can go UTF16-db directly, it might be a good idea. If we're
going via UTF8 anyway, I doubt it's going to be worth it.

Let's work off what we have now to start with at least. Bruce, can you
comment on that thing about the extra parameter? And UTF8?

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.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] CTAS not honoring NOT NULL, DEFAULT modifiers

2010-04-19 Thread Nikhil Sontakke
 Was wondering what are the standards for CTAS. Oracle seems to honor
 the NOT NULL modifier. This might be a bug if we do not honor
 modifiers in CTAS.

 Given that CREATE TABLE AS creates a table based on the result of a
 query, it seems pretty logical that constraints wouldn't be copied
 over - they're part of the table, they're not visible in a query
 result.


Yeah agreed, it is just a SELECT query afterall.

 The documentation pretty clearly says you should use CREATE TABLE LIKE
 if you want to copy the constraints over, if you look at the CREATE
 TABLE manpage (not on the CREATE TABLE AS though - perhaps a note
 should be added there?)


I think the semantics should be pretty ok as is. But I saw another DB
honoring the NOT NULL modifiers and hence the wonder if there is
something about this in the standards.

Regards,
Nikhils
-- 
http://www.enterprisedb.com

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


Re: [HACKERS] CTAS not honoring NOT NULL, DEFAULT modifiers

2010-04-19 Thread Tom Lane
Nikhil Sontakke nikhil.sonta...@enterprisedb.com writes:
 I think the semantics should be pretty ok as is. But I saw another DB
 honoring the NOT NULL modifiers and hence the wonder if there is
 something about this in the standards.

Actually, SQL:2008 does say that if an output column of the SELECT is
known not nullable, then the created table should have the NOT NULL
property for that column.  We don't implement anything about known not
nullable, though, so I'd view this as a small part of an unimplemented
SQL feature.  The usefulness seems rather debatable anyway.

regards, tom lane

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


Re: [HACKERS] walreceiver is uninterruptible on win32

2010-04-19 Thread Magnus Hagander
On Fri, Apr 16, 2010 at 09:03, Fujii Masao masao.fu...@gmail.com wrote:
 On Thu, Apr 15, 2010 at 11:26 PM, Robert Haas robertmh...@gmail.com wrote:
 I have to admit to finding this confusing.  According to the comments:

 +               /*
 +                * Don't emulate the PQexec()'s behavior of returning the 
 last
 +                * result when there are many, since walreceiver never sends 
 a
 +                * query returning multiple results.
 +                */

 ...but it looks like the code actually is implementing some sort of
 loop-that-returns-the-last result.

 Yeah, it's not a very accurate description. And I found another problem:
 libpqrcv_PQexec() ends as soon as an error result arrives even if its
 state has not been PGASYNC_IDLE yet.

 So I changed libpqrcv_PQexec() so that it emulates the PQexec()'s behavior
 except the concatenation of error messages. How about the attached patch?

Applied with only minor stylistic changes. Thanks!

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.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] shared_buffers documentation

2010-04-19 Thread Kevin Grittner
Robert Haas robertmh...@gmail.com wrote:
 
 2. Reading the section on checkpoint_segments reminds me, again,
 that the current value seems extremely conservative on modern
 hardware.  It's quite easy to hit this when doing large bulk data
 loads or even a big ol' CTAS.  I think we should consider raising
 this for 9.1.
 
Perhaps, but be aware the current default benchmarked better
than a larger setting in bulk loads.
 
http://archives.postgresql.org/pgsql-hackers/2009-06/msg01382.php
 
The apparent reason is that when there were fewer of them the WAL
files were re-used before the RAID controller flushed them from BBU
cache, causing an overall reduction in disk writes.  I have little
doubt that *without* a good BBU cached controller a larger setting
is better, but it's not universally true that bigger is better on
this one.
 
-Kevin

-- 
Sent 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: [COMMITTERS] pgsql: Tune GetSnapshotData() during Hot Standby by avoiding loop

2010-04-19 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 On Mon, Apr 19, 2010 at 5:05 AM, Simon Riggs si...@2ndquadrant.com wrote:
 It doesn't seem to be something we should place highly on the list of
 events we need protection from, does it?

 Since when do we not protect against race-conditions just because
 they're low likelihood?

Murphy's law says that the probability of any race condition happening
in the field is orders of magnitude higher than you think.  This has
been proven true many times ...

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] shared_buffers documentation

2010-04-19 Thread Robert Haas
On Mon, Apr 19, 2010 at 10:21 AM, Kevin Grittner
kevin.gritt...@wicourts.gov wrote:
 Robert Haas robertmh...@gmail.com wrote:

 2. Reading the section on checkpoint_segments reminds me, again,
 that the current value seems extremely conservative on modern
 hardware.  It's quite easy to hit this when doing large bulk data
 loads or even a big ol' CTAS.  I think we should consider raising
 this for 9.1.

 Perhaps, but be aware the current default benchmarked better
 than a larger setting in bulk loads.

 http://archives.postgresql.org/pgsql-hackers/2009-06/msg01382.php

 The apparent reason is that when there were fewer of them the WAL
 files were re-used before the RAID controller flushed them from BBU
 cache, causing an overall reduction in disk writes.  I have little
 doubt that *without* a good BBU cached controller a larger setting
 is better, but it's not universally true that bigger is better on
 this one.

I don't actually know what's best.  I'm just concerned that we have a
default in postgresql.conf and a tuning guide that says don't do
that.  Maybe the tuning guide needs to be more nuanced, or maybe
postgresql.conf needs to be changed, but it makes no sense to have
them saying contradictory things.

...Robert

-- 
Sent 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: [COMMITTERS] pgsql: Tune GetSnapshotData() during Hot Standby by avoiding loop

2010-04-19 Thread Simon Riggs
On Mon, 2010-04-19 at 10:24 -0400, Tom Lane wrote:
 Robert Haas robertmh...@gmail.com writes:
  On Mon, Apr 19, 2010 at 5:05 AM, Simon Riggs si...@2ndquadrant.com wrote:
  It doesn't seem to be something we should place highly on the list of
  events we need protection from, does it?
 
  Since when do we not protect against race-conditions just because
  they're low likelihood?
 
 Murphy's law says that the probability of any race condition happening
 in the field is orders of magnitude higher than you think.  This has
 been proven true many times ...

Choices are

1. Check RecoveryInProgress() once outside of lock, plus wild rumour of
Murphy

2. Check RecoveryInProgress() before and after holding lock

3. Check RecoveryInProgress() while holding lock

All of which perform better than

4. Revert patch

-- 
 Simon Riggs   www.2ndQuadrant.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] Re: [COMMITTERS] pgsql: Tune GetSnapshotData() during Hot Standby by avoiding loop

2010-04-19 Thread Heikki Linnakangas
Simon Riggs wrote:
 On Mon, 2010-04-19 at 10:24 -0400, Tom Lane wrote:
 Robert Haas robertmh...@gmail.com writes:
 On Mon, Apr 19, 2010 at 5:05 AM, Simon Riggs si...@2ndquadrant.com wrote:
 It doesn't seem to be something we should place highly on the list of
 events we need protection from, does it?
 Since when do we not protect against race-conditions just because
 they're low likelihood?
 Murphy's law says that the probability of any race condition happening
 in the field is orders of magnitude higher than you think.  This has
 been proven true many times ...

Right. And some future code changes elsewhere could make it more likely,
by the time we've forgotten all about this.

 Choices are
 
 1. Check RecoveryInProgress() once outside of lock, plus wild rumour of
 Murphy
 
 2. Check RecoveryInProgress() before and after holding lock
 
 3. Check RecoveryInProgress() while holding lock

4. Check RecoveryInProgress() once outside of lock, and scan the
ProcArray anyway, just in case. That's what we did before this patch.
Document that takenDuringRecovery == true means that the snapshot was
most likely taken during recovery, but there is some race conditions
where takenDuringRecovery is true even though the snapshot was taken
just after recovery finished. AFAICS all of the other current uses of
takenDuringRecovery work fine with that.

-- 
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com

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


Re: [HACKERS] Re: [COMMITTERS] pgsql: Tune GetSnapshotData() during Hot Standby by avoiding loop

2010-04-19 Thread Simon Riggs
On Mon, 2010-04-19 at 17:44 +0300, Heikki Linnakangas wrote:

  Choices are
  
  1. Check RecoveryInProgress() once outside of lock, plus wild rumour of
  Murphy
  
  2. Check RecoveryInProgress() before and after holding lock
  
  3. Check RecoveryInProgress() while holding lock
 
 4. Check RecoveryInProgress() once outside of lock, and scan the
 ProcArray anyway, just in case. That's what we did before this patch.
 Document that takenDuringRecovery == true means that the snapshot was
 most likely taken during recovery, but there is some race conditions
 where takenDuringRecovery is true even though the snapshot was taken
 just after recovery finished. AFAICS all of the other current uses of
 takenDuringRecovery work fine with that.

Checking RecoveryInProgress() is much cheaper than scanning the whole
ProcArray, so (4) is definitely worse than 1-3.

-- 
 Simon Riggs   www.2ndQuadrant.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] Thread safety and libxml2

2010-04-19 Thread Robert Haas
On Thu, Feb 18, 2010 at 8:41 PM, Bruce Momjian br...@momjian.us wrote:
 Peter Eisentraut wrote:
 On ons, 2009-12-30 at 12:55 -0500, Greg Smith wrote:
  Basically, configure failed on their OpenBSD system because thread
  safety is on but the libxml2 wasn't compiled with threaded support:
  http://xmlsoft.org/threads.html
 
  Disabling either feature (no --with-libxml or --disable-thread-safety)
  gives a working build.

 This could perhaps be fixed by excluding libxml when running the thread
 test.  The thread result is only used in the client libraries and libxml
 is only used in the backend, so those two shouldn't meet each other in
 practice.

 The attached patch removes -lxml2 from the link line of the thread
 test program.  Comments?  Can anyone test this fixes the OpenBSD
 problem?

Can someone take the time to test this whether this patch fixes the
problem?  This is on the list of open items for PG 9.0, but
considering that there's been a proposed patch available for almost
two months and no responses to this thread, it may be time to conclude
that nobody cares very much - in which case we can either remove this
item or relocate it to the TODO list.

...Robert

-- 
Sent 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: [COMMITTERS] pgsql: Tune GetSnapshotData() during Hot Standby by avoiding loop

2010-04-19 Thread Tom Lane
Simon Riggs si...@2ndquadrant.com writes:
 On Mon, 2010-04-19 at 17:44 +0300, Heikki Linnakangas wrote:
 Choices are
 1. Check RecoveryInProgress() once outside of lock, plus wild rumour of
 Murphy
 2. Check RecoveryInProgress() before and after holding lock
 3. Check RecoveryInProgress() while holding lock
 
 4. Check RecoveryInProgress() once outside of lock, and scan the
 ProcArray anyway, just in case. That's what we did before this patch.
 Document that takenDuringRecovery == true means that the snapshot was
 most likely taken during recovery, but there is some race conditions
 where takenDuringRecovery is true even though the snapshot was taken
 just after recovery finished. AFAICS all of the other current uses of
 takenDuringRecovery work fine with that.

 Checking RecoveryInProgress() is much cheaper than scanning the whole
 ProcArray, so (4) is definitely worse than 1-3.

If the lock we're talking about is an LWLock, #3 is okay.  If it's a
spinlock, not so 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


Re: [HACKERS] Standalone backends run StartupXLOG in an incorrect environment

2010-04-19 Thread Robert Haas
On Mon, Mar 22, 2010 at 9:32 PM, Bruce Momjian br...@momjian.us wrote:
 Tom Lane wrote:
 Bruce Momjian br...@momjian.us writes:
  [This is an open item for 9.0, hence the response to an apparently old
  hackers thread]

  Thanks for the reply;  9.0 open item removed.

 I think you misread his reply.  Please put that back.

 OK,  I re-read it and still don't understand, but I don't have to.

I re-read it too and I don't understand either.  This is LISTED as an
open item for 9.0, but it is apparently not a new regression, so I
think we should move it to the Todo list instead.  This problem was
discovered six months ago, is not a new regression, and there is
apparently no movement toward a fix, so it doesn't make sense to me
that we should hold up either 9.0 beta or 9.0 final on account of it.
Or am I confused?

Robert

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


[HACKERS] perltidy

2010-04-19 Thread Magnus Hagander
The MSVC scripts currently have a perltidy coding style so we run
perltidy with a specific set of arguments to format the code. (This is
in the README). Kind of like pgindent.

Should we be doing this on all the perlscripts we use?

And if we do, we should obviously use the same one everywhere -
probably just use the one we have for the msvc stuff today. Anything
in particular about that one that people hate?

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.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] Re: [BUGS] BUG #4887: inclusion operator (@) on tsqeries behaves not conforming to documentation

2010-04-19 Thread Robert Haas
On Wed, Feb 24, 2010 at 2:11 PM, Bruce Momjian br...@momjian.us wrote:
 Oleg, Teodor, would you look at this?  I see the same behavior in 9.0.

As there has been no movement on this I think we should punt this from:

http://wiki.postgresql.org/wiki/PostgreSQL_9.0_Open_Items

to

http://wiki.postgresql.org/wiki/Todo

...Robert

 Alexey Bashtanov wrote:

 The following bug has been logged online:

 Bug reference:      4887
 Logged by:          Alexey Bashtanov
 Email address:      bashta...@imap.cc
 PostgreSQL version: 8.3.1, 8.3.7
 Operating system:   Linux 2.6.20 FC5 i686, Linux 2.6.27 FC10 i686
 Description:        inclusion operator (@) on tsqeries behaves not
 conforming to documentation
 Details:

 Hello!

 '!a|b'::tsquery @ 'a|b'::tsquery evaluates to false, but should evalueate
 to true
 (http://www.postgresql.org/docs/8.3/interactive/functions-textsearch.html
 says The tsquery containment operators consider only the lexemes listed in
 the two queries, ignoring the combining operators.)

 I think negation operator is treated as a lexeme. So please correct the
 behaviour of operators or rewrite this line of docs.

 Thanks,
  Alexey

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

 --
  Bruce Momjian  br...@momjian.us        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com
  PG East:  http://www.enterprisedb.com/community/nav-pg-east-2010.do
  + If your life is a hard drive, Christ can be your backup. +

 --
 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] Standalone backends run StartupXLOG in an incorrect environment

2010-04-19 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 On Mon, Mar 22, 2010 at 9:32 PM, Bruce Momjian br...@momjian.us wrote:
 OK,  I re-read it and still don't understand, but I don't have to.

 I re-read it too and I don't understand either.

The point is that a standalone backend will fail to execute recovery
correctly:
http://archives.postgresql.org/pgsql-hackers/2009-09/msg01297.php
This is bad enough now but seems likely to be an even bigger foot-gun
in an HS/SR world.

 This is LISTED as an
 open item for 9.0, but it is apparently not a new regression, so I
 think we should move it to the Todo list instead.  This problem was
 discovered six months ago, is not a new regression, and there is
 apparently no movement toward a fix, so it doesn't make sense to me
 that we should hold up either 9.0 beta or 9.0 final on account of it.

If you think we're at the point where this item is the main thing
standing between us and beta, I'll go do something about it.  I've
been waiting for the HS code to settle before trying to design a
solution...

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

2010-04-19 Thread Andrew Dunstan



Magnus Hagander wrote:

The MSVC scripts currently have a perltidy coding style so we run
perltidy with a specific set of arguments to format the code. (This is
in the README). Kind of like pgindent.

Should we be doing this on all the perlscripts we use?

And if we do, we should obviously use the same one everywhere -
probably just use the one we have for the msvc stuff today. Anything
in particular about that one that people hate?

  


It's been debated in the past. My personal strong conviction (probably 
fuelled in part by my severe dislike of KR style indentation) is that 
we should have a single style for block structured languages across the 
project.


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] Standalone backends run StartupXLOG in an incorrect environment

2010-04-19 Thread Simon Riggs
On Mon, 2010-04-19 at 11:19 -0400, Tom Lane wrote:

 If you think we're at the point where this item is the main thing
 standing between us and beta, I'll go do something about it.  I've
 been waiting for the HS code to settle before trying to design a
 solution...

I'm not hugely interested in supporting HS in standalone backends. If it
works, that's nice. If it doesn't then I suggest we don't hold up beta
for it.

-- 
 Simon Riggs   www.2ndQuadrant.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] Standalone backends run StartupXLOG in an incorrect environment

2010-04-19 Thread Robert Haas
On Mon, Apr 19, 2010 at 11:19 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 Robert Haas robertmh...@gmail.com writes:
 On Mon, Mar 22, 2010 at 9:32 PM, Bruce Momjian br...@momjian.us wrote:
 OK,  I re-read it and still don't understand, but I don't have to.

 I re-read it too and I don't understand either.

 The point is that a standalone backend will fail to execute recovery
 correctly:
 http://archives.postgresql.org/pgsql-hackers/2009-09/msg01297.php
 This is bad enough now but seems likely to be an even bigger foot-gun
 in an HS/SR world.

OK.

 This is LISTED as an
 open item for 9.0, but it is apparently not a new regression, so I
 think we should move it to the Todo list instead.  This problem was
 discovered six months ago, is not a new regression, and there is
 apparently no movement toward a fix, so it doesn't make sense to me
 that we should hold up either 9.0 beta or 9.0 final on account of it.

 If you think we're at the point where this item is the main thing
 standing between us and beta, I'll go do something about it.  I've
 been waiting for the HS code to settle before trying to design a
 solution...

I'm not sure if this is the main thing, but I think it's probably in the top 5.

At present there are 8 items (not counting documentation issues) listed at:

http://wiki.postgresql.org/wiki/PostgreSQL_9.0_Open_Items

...not all of which seem likely to get fixed, and probably 1-3
additional patches that are floating around out there without having
formally gotten added to the list.  I think it's realistic to think
that we could be within 10 commits of beta.

...Robert

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


Re: [HACKERS] perltidy

2010-04-19 Thread Peter Eisentraut
On mån, 2010-04-19 at 17:02 +0200, Magnus Hagander wrote:
 The MSVC scripts currently have a perltidy coding style so we run
 perltidy with a specific set of arguments to format the code. (This is
 in the README). Kind of like pgindent.
 
 Should we be doing this on all the perlscripts we use?
 
 And if we do, we should obviously use the same one everywhere -
 probably just use the one we have for the msvc stuff today. Anything
 in particular about that one that people hate?

I tried it on create_help.pl and couldn't find a good combination of
options that I liked.  It either adds too much whitespace or removes too
much, or both.  Maybe that can be fined tuned.  I wouldn't want to use
the -bl option; it's not a typical Perl style.


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


Re: [HACKERS] perltidy

2010-04-19 Thread Magnus Hagander
On Mon, Apr 19, 2010 at 18:24, Peter Eisentraut pete...@gmx.net wrote:
 On mån, 2010-04-19 at 17:02 +0200, Magnus Hagander wrote:
 The MSVC scripts currently have a perltidy coding style so we run
 perltidy with a specific set of arguments to format the code. (This is
 in the README). Kind of like pgindent.

 Should we be doing this on all the perlscripts we use?

 And if we do, we should obviously use the same one everywhere -
 probably just use the one we have for the msvc stuff today. Anything
 in particular about that one that people hate?

 I tried it on create_help.pl and couldn't find a good combination of
 options that I liked.  It either adds too much whitespace or removes too
 much, or both.  Maybe that can be fined tuned.  I wouldn't want to use
 the -bl option; it's not a typical Perl style.

I doubt we're ever going to find a style taht everybody likes. Heck,
everybody certainly don't like our C style. We just need to decide on
one that's good enough and then go with it.

I don't recall exactly why -bl was added to the msvc style, but
probably to make it look more like our C code ;)

I don't care too much exactly *what* we do, but I think we should have
a common style...


-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.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] idle in txn query cancellation

2010-04-19 Thread Andres Freund
Hi Simon,

On Sunday 14 March 2010 20:12:00 Simon Riggs wrote:
 On Sun, 2010-03-14 at 19:50 +0100, Andres Freund wrote:
  On Sunday 14 February 2010 06:29:45 Simon Riggs wrote:
   On Sat, 2010-02-13 at 22:37 +0100, Andres Freund wrote:
I know it is late in the cycle
   
   No problem here. Thanks for your diligence. Will review.
  
  Got a chance to look at it?
 
 I need to spend my time on ensuring we can avoid the cancellation
 altogether, so I apologise for not reviewing. That's not a comment on
 your work or the possible effectiveness of the patch. Possibly others
 have the time to review?
I guess that wont go anywhere before 9.1?

I think at least the error code should be adjusted before 9.0.

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] Standalone backends run StartupXLOG in an incorrect environment

2010-04-19 Thread Tom Lane
Simon Riggs si...@2ndquadrant.com writes:
 On Mon, 2010-04-19 at 11:19 -0400, Tom Lane wrote:
 If you think we're at the point where this item is the main thing
 standing between us and beta, I'll go do something about it.  I've
 been waiting for the HS code to settle before trying to design a
 solution...

 I'm not hugely interested in supporting HS in standalone backends.

I'm not either.  The type of scenario that I'm worried about is someone
trying to use a standalone backend to clean up after a recovery failure.
Right now I'm afraid that could make things worse (ie create additional
corruption).

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] cost_rescan (was: match_unsorted_outer() vs. cost_nestloop())

2010-04-19 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 One problem with the current implementation of cost_rescan() is that
 it ignores caching effects.

Well, that's intentional, per the head comment for the function.
We might want to extend it later but I'd like to get some field
experience with what it's trying to model now.  I believe that it
is covering the first-order effects, and possible cache effects
would be second-order.

 It seems to be faster to rescan a
 materialize node than it is to rescan a seqscan of a table, even if
 there are no restriction clauses, presumably because you get to skip
 tuple visibility checks and maybe some other overhead, too.

Exactly.  IIRC, tuplestore's on-disk representation is also more compact
(less header overhead, no dead tuples, etc) so the amount of I/O needed
will also be less, if you're doing any at all.  But the code already
knows that scanning a tuplestore is cheaper than scanning a table ---
that doesn't seem to me to be relevant to the question of whether we
need to model cache effects in cost_rescan.

 Another potential problem is that materializing a whole-table seqscan
 to avoid repeating the tuple visibility checks may be a win in some
 strict sense, but there are externalities: it's also going to use a
 lot more memory/disk than just rescanning the table.

This is not specific to materialize, it is part of the generic problem
that we don't model the true costs of using work_mem in each of several
parts of a query.  There have been discussions about how to fix that
before, but no particularly good ideas have emerged.

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] Re: [COMMITTERS] pgsql: Tune GetSnapshotData() during Hot Standby by avoiding loop

2010-04-19 Thread Simon Riggs
On Mon, 2010-04-19 at 10:55 -0400, Tom Lane wrote:
 Simon Riggs si...@2ndquadrant.com writes:
  On Mon, 2010-04-19 at 17:44 +0300, Heikki Linnakangas wrote:
  Choices are
  1. Check RecoveryInProgress() once outside of lock, plus wild rumour of
  Murphy
  2. Check RecoveryInProgress() before and after holding lock
  3. Check RecoveryInProgress() while holding lock
  
  4. Check RecoveryInProgress() once outside of lock, and scan the
  ProcArray anyway, just in case. That's what we did before this patch.
  Document that takenDuringRecovery == true means that the snapshot was
  most likely taken during recovery, but there is some race conditions
  where takenDuringRecovery is true even though the snapshot was taken
  just after recovery finished. AFAICS all of the other current uses of
  takenDuringRecovery work fine with that.
 
  Checking RecoveryInProgress() is much cheaper than scanning the whole
  ProcArray, so (4) is definitely worse than 1-3.
 
 If the lock we're talking about is an LWLock, #3 is okay.  If it's a
 spinlock, not so much.

Just committed the following patch to implement option #3.

We test RecoveryInProgress() after the LWLockAcquire rather than before.

-- 
 Simon Riggs   www.2ndQuadrant.com
*** a/src/backend/storage/ipc/procarray.c
--- b/src/backend/storage/ipc/procarray.c
***
*** 1074,1081  GetSnapshotData(Snapshot snapshot)
  	 errmsg(out of memory)));
  	}
  
- 	snapshot-takenDuringRecovery = RecoveryInProgress();
- 
  	/*
  	 * It is sufficient to get shared lock on ProcArrayLock, even if we are
  	 * going to set MyProc-xmin.
--- 1074,1079 
***
*** 1091,1098  GetSnapshotData(Snapshot snapshot)
  	globalxmin = xmin = xmax;
  
  	/*
! 	 * If in recovery get any known assigned xids.
  	 */
  	if (!snapshot-takenDuringRecovery)
  	{
  		/*
--- 1089,1103 
  	globalxmin = xmin = xmax;
  
  	/*
! 	 * If we're in recovery then snapshot data comes from a different place,
! 	 * so decide which route we take before grab the lock. It is possible
! 	 * for recovery to end before we finish taking snapshot, and for newly
! 	 * assigned transaction ids to be added to the procarray. Xmax cannot
! 	 * change while we hold ProcArrayLock, so those newly added transaction
! 	 * ids would be filtered away, so we need not be concerned about them.
  	 */
+ 	snapshot-takenDuringRecovery = RecoveryInProgress();
+ 
  	if (!snapshot-takenDuringRecovery)
  	{
  		/*

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


Re: [HACKERS] Standalone backends run StartupXLOG in an incorrect environment

2010-04-19 Thread Tom Lane
I wrote:
 The point is that a standalone backend will fail to execute recovery
 correctly:
 http://archives.postgresql.org/pgsql-hackers/2009-09/msg01297.php

After digging around a bit, it seems like the cleanest solution would be
to move the responsibility for calling StartupXLOG in a standalone
backend into InitPostgres.  At the point where the latter currently has

/*
 * Initialize local process's access to XLOG, if appropriate.  In
 * bootstrap case we skip this since StartupXLOG() was run instead.
 */
if (!bootstrap)
(void) RecoveryInProgress();

we'd add a couple of lines to call StartupXLOG if !IsUnderPostmaster,
and then remove the call from postgres.c.  I haven't tested this yet
but it looks like the correct state has been set up at that point.
Anyone see any obvious holes in the idea?

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] Thoughts on pg_hba.conf rejection

2010-04-19 Thread Simon Riggs
On Thu, 2010-04-15 at 09:44 -0400, Tom Lane wrote:
 Maybe uaImplicitReject for the end-of-file case would be
 the most readable way.

uaImplicitReject capability added.

We're now free to bikeshed on exact wording. After much heavy thinking,
message is pg_hba.conf rejects... with no hint (yet?).

Point of note on giving information to the bad guys: if a
should-be-rejected connection request attempts to connect to a
non-existent database, we say database does not exist. If db does
exist we say pg_hba.conf rejects To me that looks like giving info
away... if an IP address range is rejected always then telling them
whether or not a particular database name exists seems like something I
would not wish to expose.

-- 
 Simon Riggs   www.2ndQuadrant.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] Standalone backends run StartupXLOG in an incorrect environment

2010-04-19 Thread Simon Riggs
On Mon, 2010-04-19 at 14:34 -0400, Tom Lane wrote:

 Anyone see any obvious holes in the idea?

Nothing springs to mind, so worth a try.

-- 
 Simon Riggs   www.2ndQuadrant.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] Standalone backends run StartupXLOG in an incorrect environment

2010-04-19 Thread Tom Lane
BTW, just for the archives' sake: it took a good long time to develop a
reproducible test case for this.  It seems that 99% of the WAL replay
code does *not* depend on the missing state.  I was eventually able to
reproduce the case originally reported, namely a crash during
btree_xlog_cleanup; but to get that you need (a) WAL to end between a
btree page split and insertion of the new parent record, and (b) have
the resulting insertion need to obtain a new btree page, ie there's
another split or newroot forced then, and (c) not have any available
pages in the index's FSM, so that we have to LockRelationForExtension,
which is what crashes for lack of a PGPROC.

So that probably explains why we went so long without recognizing the
bug.

This again points up the fact that WAL recovery isn't as well tested
as one could wish.  Koichi-san's efforts to create a test with 100%
coverage of all types of WAL records are good, but that'd not have
helped us to find this.  We should think about ways to provide better
test coverage of end-of-WAL cleanup.

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] Thoughts on pg_hba.conf rejection

2010-04-19 Thread Tom Lane
Simon Riggs si...@2ndquadrant.com writes:
 Point of note on giving information to the bad guys: if a
 should-be-rejected connection request attempts to connect to a
 non-existent database, we say database does not exist.

Yeah.  This was an acknowledged shortcoming of the changes to eliminate
flat-file storage of authentication information --- as of 9.0, it's
necessary to connect to some database in order to proceed with auth
checking.  We discussed it at the time and agreed it was an acceptable
loss.

The only way I can think of to improve that without going back to flat
files would be to develop a way for backends to switch databases after
initial startup, so that auth could be done in a predetermined database
(say, postgres) before switching to the requested DB.  This has enough
potential gotchas, in regards to catalog caching for instance, that I'm
not eager to go there.

Alternatively we could lie, and produce an auth failure message of some
sort rather than admitting the DB doesn't exist.  But that seems like
it's going to create enough confusion to not be acceptable.

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] Thoughts on pg_hba.conf rejection

2010-04-19 Thread Simon Riggs
On Mon, 2010-04-19 at 16:30 -0400, Tom Lane wrote:
 Simon Riggs si...@2ndquadrant.com writes:
  Point of note on giving information to the bad guys: if a
  should-be-rejected connection request attempts to connect to a
  non-existent database, we say database does not exist.
 
 Yeah.  This was an acknowledged shortcoming of the changes to eliminate
 flat-file storage of authentication information --- as of 9.0, it's
 necessary to connect to some database in order to proceed with auth
 checking.  

With code as currently, yes, though I see that there is a way to do
this. 

Rules that have an all in the database field of the hba can be applied
prior to attempting to select the database, as long as the all rule is
above any database-specific rules. So its possible, we just need to
special case the code so we call it once before db is assigned for all
rules only and then again later, as is now, including 100% of rules. (I
say 100% to avoid using the word all in two contexts in same sentence).

 We discussed it at the time and agreed it was an acceptable
 loss.
 
 The only way I can think of to improve that without going back to flat
 files would be to develop a way for backends to switch databases after
 initial startup, so that auth could be done in a predetermined database
 (say, postgres) before switching to the requested DB.  This has enough
 potential gotchas, in regards to catalog caching for instance, that I'm
 not eager to go there.
 
 Alternatively we could lie, and produce an auth failure message of some
 sort rather than admitting the DB doesn't exist.  But that seems like
 it's going to create enough confusion to not be acceptable.

-- 
 Simon Riggs   www.2ndQuadrant.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] Thoughts on pg_hba.conf rejection

2010-04-19 Thread Robert Haas
On Mon, Apr 19, 2010 at 4:30 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Simon Riggs si...@2ndquadrant.com writes:
 Point of note on giving information to the bad guys: if a
 should-be-rejected connection request attempts to connect to a
 non-existent database, we say database does not exist.

 Yeah.  This was an acknowledged shortcoming of the changes to eliminate
 flat-file storage of authentication information --- as of 9.0, it's
 necessary to connect to some database in order to proceed with auth
 checking.  We discussed it at the time and agreed it was an acceptable
 loss.

 The only way I can think of to improve that without going back to flat
 files would be to develop a way for backends to switch databases after
 initial startup, so that auth could be done in a predetermined database
 (say, postgres) before switching to the requested DB.  This has enough
 potential gotchas, in regards to catalog caching for instance, that I'm
 not eager to go there.

Would it be possible to set up a skeleton environment where we can
access shared catalogs only and then decide on which database we're
using later?

...Robert

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


Re: [HACKERS] Thoughts on pg_hba.conf rejection

2010-04-19 Thread Alvaro Herrera
Robert Haas escribió:
 On Mon, Apr 19, 2010 at 4:30 PM, Tom Lane t...@sss.pgh.pa.us wrote:

  The only way I can think of to improve that without going back to flat
  files would be to develop a way for backends to switch databases after
  initial startup, so that auth could be done in a predetermined database
  (say, postgres) before switching to the requested DB.  This has enough
  potential gotchas, in regards to catalog caching for instance, that I'm
  not eager to go there.
 
 Would it be possible to set up a skeleton environment where we can
 access shared catalogs only and then decide on which database we're
 using later?

Eh?  We already do that ... In fact the autovac launcher is always
connected to shared catalogs, without being connected to any one
database in particular (cf. get_database_list)

-- 
Alvaro Herrerahttp://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

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


Re: [HACKERS] Thoughts on pg_hba.conf rejection

2010-04-19 Thread Robert Haas
On Mon, Apr 19, 2010 at 5:04 PM, Alvaro Herrera
alvhe...@commandprompt.com wrote:
 Robert Haas escribió:
 On Mon, Apr 19, 2010 at 4:30 PM, Tom Lane t...@sss.pgh.pa.us wrote:

  The only way I can think of to improve that without going back to flat
  files would be to develop a way for backends to switch databases after
  initial startup, so that auth could be done in a predetermined database
  (say, postgres) before switching to the requested DB.  This has enough
  potential gotchas, in regards to catalog caching for instance, that I'm
  not eager to go there.

 Would it be possible to set up a skeleton environment where we can
 access shared catalogs only and then decide on which database we're
 using later?

 Eh?  We already do that ... In fact the autovac launcher is always
 connected to shared catalogs, without being connected to any one
 database in particular (cf. get_database_list)

Oh.  Then I'm confused.  Tom said: as of 9.0, it's necessary to
connect to some database in order to proceed with auth checking.  Why
is that necessary,  if we can access shared catalogs without it?

...Robert

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


Re: [HACKERS] Thoughts on pg_hba.conf rejection

2010-04-19 Thread Alvaro Herrera
Robert Haas escribió:
 On Mon, Apr 19, 2010 at 5:04 PM, Alvaro Herrera
 alvhe...@commandprompt.com wrote:
  Robert Haas escribió:
  On Mon, Apr 19, 2010 at 4:30 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 
   The only way I can think of to improve that without going back to flat
   files would be to develop a way for backends to switch databases after
   initial startup, so that auth could be done in a predetermined database
   (say, postgres) before switching to the requested DB.  This has enough
   potential gotchas, in regards to catalog caching for instance, that I'm
   not eager to go there.
 
  Would it be possible to set up a skeleton environment where we can
  access shared catalogs only and then decide on which database we're
  using later?
 
  Eh?  We already do that ... In fact the autovac launcher is always
  connected to shared catalogs, without being connected to any one
  database in particular (cf. get_database_list)
 
 Oh.  Then I'm confused.  Tom said: as of 9.0, it's necessary to
 connect to some database in order to proceed with auth checking.  Why
 is that necessary,  if we can access shared catalogs without it?

Hmm, yeah, why did he say that?  Maybe the order of operations during
startup is such that we only do auth checking after connecting to a
database for some reason.

Whatever it is, I don't think a badly worded error message is enough
grounds for fooling with this at this time of release process, though.
To be discussed for 9.1?

-- 
Alvaro Herrerahttp://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 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] Thoughts on pg_hba.conf rejection

2010-04-19 Thread Simon Riggs
On Mon, 2010-04-19 at 17:08 -0400, Robert Haas wrote:

 Oh.  Then I'm confused.  Tom said: as of 9.0, it's necessary to
 connect to some database in order to proceed with auth checking.  Why
 is that necessary

It's not, I just explained how to do it without.

-- 
 Simon Riggs   www.2ndQuadrant.com


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


[HACKERS] Privileges

2010-04-19 Thread Simon Riggs

There is a command to set privileges

  GRANT SELECT ON ALL TABLES IN SCHEMA foo TO PUBLIC;

and a command to set default privileges

  ALTER DEFAULT PRIVILEGES IN SCHEMA foo
  GRANT SELECT ON TABLES TO PUBLIC;

In the first command the ALL is required, whereas in the second command
the ALL must be absent.

ISTM that the ALL should be optional in both cases.
Same thing is true for FUNCTIONS and SEQUENCES.

Both options are new in 9.0.

Any objections to implementing this simple patch?

-- 
 Simon Riggs   www.2ndQuadrant.com
*** a/src/backend/parser/gram.y
--- b/src/backend/parser/gram.y
***
*** 4658,4663  privilege_target:
--- 4658,4671 
  	n-objs = $2;
  	$$ = n;
  }
+ 			| TABLES IN_P SCHEMA name_list
+ {
+ 	PrivTarget *n = (PrivTarget *) palloc(sizeof(PrivTarget));
+ 	n-targtype = ACL_TARGET_ALL_IN_SCHEMA;
+ 	n-objtype = ACL_OBJECT_RELATION;
+ 	n-objs = $4;
+ 	$$ = n;
+ }
  			| ALL TABLES IN_P SCHEMA name_list
  {
  	PrivTarget *n = (PrivTarget *) palloc(sizeof(PrivTarget));
***
*** 4666,4671  privilege_target:
--- 4674,4687 
  	n-objs = $5;
  	$$ = n;
  }
+ 			| SEQUENCES IN_P SCHEMA name_list
+ {
+ 	PrivTarget *n = (PrivTarget *) palloc(sizeof(PrivTarget));
+ 	n-targtype = ACL_TARGET_ALL_IN_SCHEMA;
+ 	n-objtype = ACL_OBJECT_SEQUENCE;
+ 	n-objs = $4;
+ 	$$ = n;
+ }
  			| ALL SEQUENCES IN_P SCHEMA name_list
  {
  	PrivTarget *n = (PrivTarget *) palloc(sizeof(PrivTarget));
***
*** 4674,4679  privilege_target:
--- 4690,4703 
  	n-objs = $5;
  	$$ = n;
  }
+ 			| FUNCTIONS IN_P SCHEMA name_list
+ {
+ 	PrivTarget *n = (PrivTarget *) palloc(sizeof(PrivTarget));
+ 	n-targtype = ACL_TARGET_ALL_IN_SCHEMA;
+ 	n-objtype = ACL_OBJECT_FUNCTION;
+ 	n-objs = $4;
+ 	$$ = n;
+ }
  			| ALL FUNCTIONS IN_P SCHEMA name_list
  {
  	PrivTarget *n = (PrivTarget *) palloc(sizeof(PrivTarget));
***
*** 4869,4877  DefACLAction:
  		;
  
  defacl_privilege_target:
! 			TABLES			{ $$ = ACL_OBJECT_RELATION; }
! 			| FUNCTIONS		{ $$ = ACL_OBJECT_FUNCTION; }
! 			| SEQUENCES		{ $$ = ACL_OBJECT_SEQUENCE; }
  		;
  
  
--- 4893,4904 
  		;
  
  defacl_privilege_target:
! 			ALL		TABLES			{ $$ = ACL_OBJECT_RELATION; }
! 			|		TABLES			{ $$ = ACL_OBJECT_RELATION; }
! 			| ALL	FUNCTIONS		{ $$ = ACL_OBJECT_FUNCTION; }
! 			| 	 	FUNCTIONS		{ $$ = ACL_OBJECT_FUNCTION; }
! 			| ALL	SEQUENCES		{ $$ = ACL_OBJECT_SEQUENCE; }
! 			| 		SEQUENCES		{ $$ = ACL_OBJECT_SEQUENCE; }
  		;
  
  

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


Re: [HACKERS] shared_buffers documentation

2010-04-19 Thread Bruce Momjian
Robert Haas wrote:
 On Mon, Apr 19, 2010 at 10:21 AM, Kevin Grittner
 kevin.gritt...@wicourts.gov wrote:
  Robert Haas robertmh...@gmail.com wrote:
 
  2. Reading the section on checkpoint_segments reminds me, again,
  that the current value seems extremely conservative on modern
  hardware. ?It's quite easy to hit this when doing large bulk data
  loads or even a big ol' CTAS. ?I think we should consider raising
  this for 9.1.
 
  Perhaps, but be aware the current default benchmarked better
  than a larger setting in bulk loads.
 
  http://archives.postgresql.org/pgsql-hackers/2009-06/msg01382.php
 
  The apparent reason is that when there were fewer of them the WAL
  files were re-used before the RAID controller flushed them from BBU
  cache, causing an overall reduction in disk writes. ?I have little
  doubt that *without* a good BBU cached controller a larger setting
  is better, but it's not universally true that bigger is better on
  this one.
 
 I don't actually know what's best.  I'm just concerned that we have a
 default in postgresql.conf and a tuning guide that says don't do
 that.  Maybe the tuning guide needs to be more nuanced, or maybe
 postgresql.conf needs to be changed, but it makes no sense to have
 them saying contradictory things.

The good news about checkpoint_segments is that you get a log file
warning message if the value should be increased, i.e. you are
checkpointing often than 30 seconds.

-- 
  Bruce Momjian  br...@momjian.ushttp://momjian.us
  EnterpriseDB http://enterprisedb.com

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


Re: [HACKERS] Thoughts on pg_hba.conf rejection

2010-04-19 Thread Alvaro Herrera
Simon Riggs escribió:
 On Mon, 2010-04-19 at 17:08 -0400, Robert Haas wrote:
 
  Oh.  Then I'm confused.  Tom said: as of 9.0, it's necessary to
  connect to some database in order to proceed with auth checking.  Why
  is that necessary
 
 It's not, I just explained how to do it without.

You mean purely using pg_hba.conf all rules?  That seems a bit
unsatisfactory ...

-- 
Alvaro Herrerahttp://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 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] shared_buffers documentation

2010-04-19 Thread Robert Haas
On Mon, Apr 19, 2010 at 5:36 PM, Bruce Momjian br...@momjian.us wrote:
 I don't actually know what's best.  I'm just concerned that we have a
 default in postgresql.conf and a tuning guide that says don't do
 that.  Maybe the tuning guide needs to be more nuanced, or maybe
 postgresql.conf needs to be changed, but it makes no sense to have
 them saying contradictory things.

 The good news about checkpoint_segments is that you get a log file
 warning message if the value should be increased, i.e. you are
 checkpointing often than 30 seconds.

Yeah.  I get that warning frequently when I'm creating test tables of
dummy data for PG devel purposes.  That's actually the main thing that
makes me think the default may be too low.

...Robert

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


Re: [HACKERS] Thoughts on pg_hba.conf rejection

2010-04-19 Thread Robert Haas
On Mon, Apr 19, 2010 at 5:12 PM, Alvaro Herrera
alvhe...@commandprompt.com wrote:
 Robert Haas escribió:
 On Mon, Apr 19, 2010 at 5:04 PM, Alvaro Herrera
 alvhe...@commandprompt.com wrote:
  Robert Haas escribió:
  On Mon, Apr 19, 2010 at 4:30 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 
   The only way I can think of to improve that without going back to flat
   files would be to develop a way for backends to switch databases after
   initial startup, so that auth could be done in a predetermined database
   (say, postgres) before switching to the requested DB.  This has enough
   potential gotchas, in regards to catalog caching for instance, that I'm
   not eager to go there.
 
  Would it be possible to set up a skeleton environment where we can
  access shared catalogs only and then decide on which database we're
  using later?
 
  Eh?  We already do that ... In fact the autovac launcher is always
  connected to shared catalogs, without being connected to any one
  database in particular (cf. get_database_list)

 Oh.  Then I'm confused.  Tom said: as of 9.0, it's necessary to
 connect to some database in order to proceed with auth checking.  Why
 is that necessary,  if we can access shared catalogs without it?

 Hmm, yeah, why did he say that?  Maybe the order of operations during
 startup is such that we only do auth checking after connecting to a
 database for some reason.

 Whatever it is, I don't think a badly worded error message is enough
 grounds for fooling with this at this time of release process, though.
 To be discussed for 9.1?

I'm not proposing to fix the issue right now; but I wanted to try to
understand it while it's fresh in my mind.  I'm still not seeing the
issue for some reason.

...Robert

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


Re: [HACKERS] Thoughts on pg_hba.conf rejection

2010-04-19 Thread Robert Haas
On Mon, Apr 19, 2010 at 5:22 PM, Simon Riggs si...@2ndquadrant.com wrote:
 On Mon, 2010-04-19 at 17:08 -0400, Robert Haas wrote:

 Oh.  Then I'm confused.  Tom said: as of 9.0, it's necessary to
 connect to some database in order to proceed with auth checking.  Why
 is that necessary

 It's not, I just explained how to do it without.

Your explanation seems to presuppose that we somehow can't process the
database-specific rules before selecting a database.  I don't
understand why that would be the case.  Why can't we just check all
the rules and then, if we decide to allow the connection, select the
database?

...Robert

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


Re: [HACKERS] master in standby mode croaks

2010-04-19 Thread Robert Haas
On Mon, Apr 19, 2010 at 5:31 AM, Fujii Masao masao.fu...@gmail.com wrote:
 On Mon, Apr 19, 2010 at 11:04 AM, Robert Haas robertmh...@gmail.com wrote:
 First of all, I wonder why the latter two need to affect the decision of
 whether additional information is written to WAL for HS. How about just
 removing XLogIsNeeded() condition from XLogStandbyInfoActive()?

 Bad idea, I think.  If XLogIsNeeded() is returning false and
 XLogStandbyInfoActive() is returning true, the resulting WAL will
 still be unusable for HS, at least AIUI.

 Probably No. Such a WAL will be usable for HS unless an unlogged
 operation (e.g., CLUSTER, CREATE TABLE AS SELECT, etc) happens.
 I think that the occurrence of an unlogged operation rather than
 XLogIsNeeded() itself must be checked in the standby, it's already
 been being checked. So just removing XLogIsNeeded() condition makes
 sense to me.

I think that's a bad idea.  Currently we have three possible types of
WAL-logging:

- just enough for crash recovery (archive_mode=off and max_wal_senders=0)
- enough for log-shipping replication (archive_mode=on or
max_wal_senders0, but recovery_connections=off)
- enough for log-shipping replication + hot standby (archive_mode=on
or max_wal_senders0, plus recovery_connections=on)

I'm not eager to add a fourth category where hot standby works unless
you do any of the things that break log-streaming in general.  That
seems hopelessly fragile and also fairly pointless.

...Robert

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


Re: [HACKERS] Thoughts on pg_hba.conf rejection

2010-04-19 Thread Simon Riggs
On Mon, 2010-04-19 at 17:52 -0400, Robert Haas wrote:
 On Mon, Apr 19, 2010 at 5:22 PM, Simon Riggs si...@2ndquadrant.com wrote:
  On Mon, 2010-04-19 at 17:08 -0400, Robert Haas wrote:
 
  Oh.  Then I'm confused.  Tom said: as of 9.0, it's necessary to
  connect to some database in order to proceed with auth checking.  Why
  is that necessary
 
  It's not, I just explained how to do it without.
 
 Your explanation seems to presuppose that we somehow can't process the
 database-specific rules before selecting a database.  I don't
 understand why that would be the case.  Why can't we just check all
 the rules and then, if we decide to allow the connection, select the
 database?

Some rules are user-specific, but I see that doesn't matter and you are
right. 

We can process the whole pg_hba.conf to see if it returns reject or
implicitreject before attempting to confirm the existence of any
database or any user. Any other result must be implemented during
ClientAuthentication(). So we may as well run the whole set of rules,
work out which rule applies and then remember that for later use. Just
as efficient, better security.

-- 
 Simon Riggs   www.2ndQuadrant.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] shared_buffers documentation

2010-04-19 Thread Bruce Momjian
Robert Haas wrote:
 On Mon, Apr 19, 2010 at 5:36 PM, Bruce Momjian br...@momjian.us wrote:
  I don't actually know what's best. ?I'm just concerned that we have a
  default in postgresql.conf and a tuning guide that says don't do
  that. ?Maybe the tuning guide needs to be more nuanced, or maybe
  postgresql.conf needs to be changed, but it makes no sense to have
  them saying contradictory things.
 
  The good news about checkpoint_segments is that you get a log file
  warning message if the value should be increased, i.e. you are
  checkpointing often than 30 seconds.
 
 Yeah.  I get that warning frequently when I'm creating test tables of
 dummy data for PG devel purposes.  That's actually the main thing that
 makes me think the default may be too low.

Well, the point is that you are getting it for _unusual_ circumstances. 
Seems it is only when you are getting it for typical workloads that it
should be increased.  However, this is the first time I am hearing that
battery-backed cache favors the default value.

-- 
  Bruce Momjian  br...@momjian.ushttp://momjian.us
  EnterpriseDB http://enterprisedb.com

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


Re: [HACKERS] shared_buffers documentation

2010-04-19 Thread Robert Haas
On Mon, Apr 19, 2010 at 6:06 PM, Bruce Momjian br...@momjian.us wrote:
 Robert Haas wrote:
 On Mon, Apr 19, 2010 at 5:36 PM, Bruce Momjian br...@momjian.us wrote:
  I don't actually know what's best. ?I'm just concerned that we have a
  default in postgresql.conf and a tuning guide that says don't do
  that. ?Maybe the tuning guide needs to be more nuanced, or maybe
  postgresql.conf needs to be changed, but it makes no sense to have
  them saying contradictory things.
 
  The good news about checkpoint_segments is that you get a log file
  warning message if the value should be increased, i.e. you are
  checkpointing often than 30 seconds.

 Yeah.  I get that warning frequently when I'm creating test tables of
 dummy data for PG devel purposes.  That's actually the main thing that
 makes me think the default may be too low.

 Well, the point is that you are getting it for _unusual_ circumstances.
 Seems it is only when you are getting it for typical workloads that it
 should be increased.

I guess.  I am not sure we should consider doing a large CTAS to be
an unusual workload, though.  Sure, most of us don't do that every
day, but what do we get out of having it be slow when we do decide to
do it?  Up until today, I had never heard anyone say that there was
any possible performance trade-off, and...

 However, this is the first time I am hearing that
 battery-backed cache favors the default value.

...if that's as bad as it gets, I'm still not sure we shouldn't
increase the default.  Most people will not have their first
experience of PG on a server with a battery-backed RAID controller,
I'm thinking.  And people who do have battery-backed RAID controllers
can tune the value down if need be.  I have never yet heard anyone
justify why all the values in postgresql.conf should be defined as
the lowest value that works best for at least 1 user.

Then again, I don't really know what I'm talking about.  I think we
should be listening very carefully to people who have spent a lot of
time tuning this and taking their advice on how it should be set by
default.

...Robert

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


Re: [HACKERS] shared_buffers documentation

2010-04-19 Thread Kevin Grittner
Robert Haas robertmh...@gmail.com wrote:
 
 However, this is the first time I am hearing that
 battery-backed cache favors the default value.
 
Well, it was discussed on the lists during a CommitFest.
 
 ...if that's as bad as it gets, I'm still not sure we shouldn't
 increase the default.  Most people will not have their first
 experience of PG on a server with a battery-backed RAID
 controller, I'm thinking.  And people who do have battery-backed
 RAID controllers can tune the value down if need be.  I have never
 yet heard anyone justify why all the values in postgresql.conf
 should be defined as the lowest value that works best for at
 least 1 user.
 
 Then again, I don't really know what I'm talking about.  I think
 we should be listening very carefully to people who have spent a
 lot of time tuning this and taking their advice on how it should
 be set by default.
 
I'm not sure we shouldn't change the default either.  There seems to
be a wealth of experience showing where a bigger value can help, and
a fairly narrow use case where (much to my surprise) the lower value
helped.  Perhaps this just fits under the be sure to test and tune
your own environment heading, although it is a direction people
might not even think to try without some hint that it can help.
 
FWIW, we use very different configurations for bulk loading (like
pg_dump piped to psql) than we do for production usage afterward. 
This has become part of my bag of tricks for bulk loads, but we
still use a larger number after the load.
 
Also, I haven't heard of any independent confirmation -- it could be
a quirk of our hardware and configuration?  Has anyone else
benchmarked this to see the impact on bulk loads with BBU cache?
 
-Kevin

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


Re: [HACKERS] shared_buffers documentation

2010-04-19 Thread Greg Smith

Kevin Grittner wrote:

Perhaps, but be aware the current default benchmarked better
than a larger setting in bulk loads.
 
http://archives.postgresql.org/pgsql-hackers/2009-06/msg01382.php
 
The apparent reason is that when there were fewer of them the WAL

files were re-used before the RAID controller flushed them from BBU
cache, causing an overall reduction in disk writes.  I have little
doubt that *without* a good BBU cached controller a larger setting
is better, but it's not universally true that bigger is better on
this one


After running some tests, I believe what you observed is more universal 
than that, because I've been able to replicate a performance drop from a 
checkpoint_segments increase on a system without a BBWC (laptop with 
write caching turned off) where I really expected it to help.  My 
working theory is that are a broader set of situations where limiting 
the working set of WAL files to a small number in order to decrease 
cache disruption applies than just when you've got hardware caching 
involved.


However, I believe the guidelines to increasing this parameter along 
with shared_buffers still applies.  The real case for wins with more 
segments is when you also have a large buffer cache, because that's 
where the write savings from postponed database writes to often used 
blocks becomes easy to measure.  I've found it difficult today to 
demonstrate a slam-dunk bulk loading improvement through 
checkpoint_segments increase when shared_buffers is fixed at its default 
of ~32MB.  If that keeps up, I might soon have enough data to bust the 
idea that it alone improves bulk loading performance when you haven't 
touched anything else in the default config, which was unexpected to 
me.  Will report back once I've got a full handle on it.


Thanks for reminding me about this counter example, it slipped by in 
that broader thread before and I didn't try doing that myself until 
today, to see that you're onto something there.


--
Greg Smith  2ndQuadrant US  Baltimore, MD
PostgreSQL Training, Services and Support
g...@2ndquadrant.com   www.2ndQuadrant.us


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


Re: [HACKERS] Thoughts on pg_hba.conf rejection

2010-04-19 Thread Tom Lane
Simon Riggs si...@2ndquadrant.com writes:
 With code as currently, yes, though I see that there is a way to do
 this. 

 Rules that have an all in the database field of the hba can be applied
 prior to attempting to select the database, as long as the all rule is
 above any database-specific rules.

Well, that's nice, but it's an awfully small subset of what the
pg_hba.conf rules might contain.  In particular you can't do anything
that involves group membership checks without access to the catalogs;
and I think a large fraction of installations that are exposed to
untrustworthy connections will be using password auth for them, which
means they still need to connect to the catalogs to get the password.

Now it's possible that we could have a prefilter that rejects
connections if they're coming from an IP address that cannot match
*any* of the pg_hba.conf rules.  Not sure how useful that would really
be in practice though.  It wouldn't do anything extra for people who
keep their DB server behind a firewall.

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] Thoughts on pg_hba.conf rejection

2010-04-19 Thread Tom Lane
Alvaro Herrera alvhe...@commandprompt.com writes:
 Robert Haas escribió:
 Would it be possible to set up a skeleton environment where we can
 access shared catalogs only and then decide on which database we're
 using later?

 Eh?  We already do that ... In fact the autovac launcher is always
 connected to shared catalogs, without being connected to any one
 database in particular (cf. get_database_list)

Hmm.  The AV launcher is only permitted to touch pg_database.
At the time there were considerable advantages to that restriction,
but subsequent changes (like getting rid of the need for manual
maintenance of pg_attribute entries for bootstrap catalogs) might
mean that it wouldn't be too painful to extend this capability to
pg_authid etc.  Could be worth thinking about.

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

2010-04-19 Thread Tom Lane
Simon Riggs si...@2ndquadrant.com writes:
 There is a command to set privileges

   GRANT SELECT ON ALL TABLES IN SCHEMA foo TO PUBLIC;

 and a command to set default privileges

   ALTER DEFAULT PRIVILEGES IN SCHEMA foo
   GRANT SELECT ON TABLES TO PUBLIC;

 In the first command the ALL is required, whereas in the second command
 the ALL must be absent.

 ISTM that the ALL should be optional in both cases.

I don't believe this is a good idea.  ALL in the second statement would
give a completely misleading impression, because it does *not* grant
privileges on all tables, in particular it doesn't affect existing
tables.  Conversely, leaving out ALL in the first statement would limit
our flexibility to insert additional options there in future.  (ALL is a
fully reserved word, TABLES isn't, so your proposal greatly increases
the odds of future syntactic conflicts.)

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] Thoughts on pg_hba.conf rejection

2010-04-19 Thread Robert Haas
On Mon, Apr 19, 2010 at 7:18 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Alvaro Herrera alvhe...@commandprompt.com writes:
 Robert Haas escribió:
 Would it be possible to set up a skeleton environment where we can
 access shared catalogs only and then decide on which database we're
 using later?

 Eh?  We already do that ... In fact the autovac launcher is always
 connected to shared catalogs, without being connected to any one
 database in particular (cf. get_database_list)

 Hmm.  The AV launcher is only permitted to touch pg_database.
 At the time there were considerable advantages to that restriction,
 but subsequent changes (like getting rid of the need for manual
 maintenance of pg_attribute entries for bootstrap catalogs) might
 mean that it wouldn't be too painful to extend this capability to
 pg_authid etc.  Could be worth thinking about.

Perhaps we should add a TODO.

...Robert

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


Re: [HACKERS] Thoughts on pg_hba.conf rejection

2010-04-19 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 On Mon, Apr 19, 2010 at 7:18 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Hmm.  The AV launcher is only permitted to touch pg_database.

 Perhaps we should add a TODO.

Actually, while I'm looking at that code, a more immediate TODO is
fix walsender.  Somebody has inserted an absolutely flight-of-fantasy
code path into InitPostgres.  (Hint: template1 can be dropped.
ESPECIALLY when you're deliberately not taking any lock on it.)

regards, tom lane

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


Re: [HACKERS] unresolved bugs

2010-04-19 Thread Robert Haas
On Thu, Jan 7, 2010 at 11:55 AM, Magnus Hagander mag...@hagander.net wrote:
 BUG #5245: Full Server Certificate Chain Not Sent to client
 http://archives.postgresql.org/pgsql-bugs/2009-12/msg00145.php

 Not clear to me whether this is a bug, or at least our bug.

 It's not a  bug, it's potentially a feature request :)

 Isn't that behavior entirely openssl's to determine?

 No, I think we can control that somehow.

 Let's leave it on the open items list, and I'll try to find some time
 to investigate it. Since it's behavior change, it's probably 8.5
 material, and not backpatch. Oh, and if somebody else has the time to
 investigate it, please be my guest :)

Magnus told me on IM today that he feels this should be reclassified
as a Todo, so I'm going to move it there from open items.

...Robert

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


Re: [HACKERS] row estimation off the mark when generate_series calls are involved

2010-04-19 Thread Takahiro Itagaki

Nikhil Sontakke nikhil.sonta...@enterprisedb.com wrote:

 I observed the following behavior on PG head:
 postgres=# explain verbose insert into public.x values 
 (generate_series(1,10));
 
 Insert (cost=0.00..0.01 rows=1 width=0)

Hmmm, there are differences between SELECT SRF() and SELECT * FROM SRF():

postgres=# EXPLAIN INSERT INTO public.x SELECT generate_series(1,10);
   QUERY PLAN

 Insert  (cost=0.00..0.02 rows=1 width=4)
   -  Result  (cost=0.00..0.01 rows=1 width=0)

postgres=# EXPLAIN INSERT INTO public.x SELECT * FROM generate_series(1,10);
  QUERY PLAN
--
 Insert  (cost=0.00..10.00 rows=1000 width=4)
   -  Function Scan on generate_series  (cost=0.00..10.00 rows=1000 width=4)


 I think the place where we set the
 targetlist of the result_plan to sub_tlist, immediately after that we
 should update the plan_rows estimate by walking this latest
 targetlist. I did that and now we seem to get proper row estimates.

I agree the estimation should be improved, but your patch *adds*
the estimated number of rows to the result:

postgres=# EXPLAIN INSERT INTO public.x SELECT generate_series(1,10);
QUERY PLAN
---
 Insert  (cost=0.00..12.52 rows=1002 width=4)
   -  Result  (cost=0.00..2.51 rows=1001 width=0)

Should it be 1000 rather than 1001?

Regards,
---
Takahiro Itagaki
NTT Open Source Software Center



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


Re: [HACKERS] Streaming replication and a disk full in primary

2010-04-19 Thread Robert Haas
On Fri, Apr 16, 2010 at 9:47 PM, Robert Haas robertmh...@gmail.com wrote:
 On Thu, Apr 15, 2010 at 6:13 PM, Robert Haas robertmh...@gmail.com wrote:
 On Thu, Apr 15, 2010 at 2:54 AM, Heikki Linnakangas
 heikki.linnakan...@enterprisedb.com wrote:
 Robert Haas wrote:
 I've realized another problem with this patch.  standby_keep_segments
 only controls the number of segments that we keep around for purposes
 of streaming: it doesn't affect archiving at all.  And of course, a
 standby server based on archiving is every bit as much of a standby
 server as one that uses streaming replication.  So at a minimum, the
 name of this GUC is very confusing.

 Hmm, I guess streaming_keep_segments would be more accurate. Somehow
 doesn't feel as good otherwise, though. Any other suggestions?

 I sort of feel like the correct description is something like
 num_extra_retained_wal_segments, but that's sort of long.  The actual
 behavior is not tied to streaming, although the use case is.

 thinks more

 How about wal_keep_segments?

Here's the patch.

...Robert


wal_keep_segments.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] plpgsql GUC variable: custom or built-in?

2010-04-19 Thread Robert Haas
On Thu, Nov 12, 2009 at 6:31 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Alvaro Herrera alvhe...@commandprompt.com writes:
 With all this fallout, I think it would be cleaner to step back and make
 it a regular GUC variable, not custom.  Pretending that plpgsql is
 somehow not an integral part of the system is not completely true
 anyway.  Yes, we're playing favorites in the PL camp here, but so what?

 True, but on the other hand, if plpgsql needs this to work nicely, then
 $YOURPL probably needs it too.

This thread is still on the open items list, as:

Improve behavior of SUSET GUC variables added by loadable modules?

Is there something that needs to be done here?  If so, what is it?

...Robert

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


Re: [HACKERS] shared_buffers documentation

2010-04-19 Thread Bruce Momjian
Robert Haas wrote:
  Well, the point is that you are getting it for _unusual_ circumstances.
  Seems it is only when you are getting it for typical workloads that it
  should be increased.
 
 I guess.  I am not sure we should consider doing a large CTAS to be
 an unusual workload, though.  Sure, most of us don't do that every
 day, but what do we get out of having it be slow when we do decide to
 do it?  Up until today, I had never heard anyone say that there was
 any possible performance trade-off, and...
 
  However, this is the first time I am hearing that
  battery-backed cache favors the default value.
 
 ...if that's as bad as it gets, I'm still not sure we shouldn't
 increase the default.  Most people will not have their first
 experience of PG on a server with a battery-backed RAID controller,
 I'm thinking.  And people who do have battery-backed RAID controllers
 can tune the value down if need be.  I have never yet heard anyone
 justify why all the values in postgresql.conf should be defined as
 the lowest value that works best for at least 1 user.
 
 Then again, I don't really know what I'm talking about.  I think we
 should be listening very carefully to people who have spent a lot of
 time tuning this and taking their advice on how it should be set by
 default.

The current default was just chosen to reduce the PG disk footprint.  It
probably should be increased, unless we find that the smaller working
set is a win in many cases.

-- 
  Bruce Momjian  br...@momjian.ushttp://momjian.us
  EnterpriseDB http://enterprisedb.com

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


Re: [HACKERS] pgindent and tabs in comments

2010-04-19 Thread Bruce Momjian
Peter Eisentraut wrote:
 On tor, 2010-04-15 at 20:56 -0400, Bruce Momjian wrote:
  Peter Eisentraut wrote:
   Apparently, pgindent replaces multiple spaces in comments by a tab
   (possibly subject to additional logic).  An example among thousands:
   
   http://git.postgresql.org/gitweb?p=postgresql.git;a=blobdiff_plain;f=src/backend/access/gin/ginentrypage.c;h=c23415c0075b5ec52f08e8ef698f7b7ec2f97b19;hp=5cbbc7455519eba6c37be465784a02b350065716;hb=aa1e9bb51c102b239340992f2fcce138edb39d8a;hpb=03ee49a016e69e7594978352df6da4e0bbd7d04a
   
   (or just rgrep -F '.TAB' the tree to see more).
   
   This doesn't make any sense to me.  Could this please be fixed, and if
   possible reverted sometime?
  
  Oh, that is an interesting example. What the code does is if there are
  several spaces and the next word is on a tab stop, the spaces are
  convered to tabs, except if we are in a string.  This conversion is done
  by 'entab' which we distribute in src/tools.  I am unclear how to fix
  this _except_ to remove all tabs if the line starts with '*', but that
  isn't foolproof, e.g.:
  
  *var = 12;
 
 Yeah, that explains it.  I don't have a good solution, unless entab
 wants to keep track when it's inside a comment.

I could add that code, but right now it isn't there.

-- 
  Bruce Momjian  br...@momjian.ushttp://momjian.us
  EnterpriseDB http://enterprisedb.com

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


Re: [HACKERS] shared_buffers documentation

2010-04-19 Thread Robert Haas
On Mon, Apr 19, 2010 at 9:23 PM, Bruce Momjian br...@momjian.us wrote:
 Robert Haas wrote:
  Well, the point is that you are getting it for _unusual_ circumstances.
  Seems it is only when you are getting it for typical workloads that it
  should be increased.

 I guess.  I am not sure we should consider doing a large CTAS to be
 an unusual workload, though.  Sure, most of us don't do that every
 day, but what do we get out of having it be slow when we do decide to
 do it?  Up until today, I had never heard anyone say that there was
 any possible performance trade-off, and...

  However, this is the first time I am hearing that
  battery-backed cache favors the default value.

 ...if that's as bad as it gets, I'm still not sure we shouldn't
 increase the default.  Most people will not have their first
 experience of PG on a server with a battery-backed RAID controller,
 I'm thinking.  And people who do have battery-backed RAID controllers
 can tune the value down if need be.  I have never yet heard anyone
 justify why all the values in postgresql.conf should be defined as
 the lowest value that works best for at least 1 user.

 Then again, I don't really know what I'm talking about.  I think we
 should be listening very carefully to people who have spent a lot of
 time tuning this and taking their advice on how it should be set by
 default.

 The current default was just chosen to reduce the PG disk footprint.  It
 probably should be increased, unless we find that the smaller working
 set is a win in many cases.

Yeah.  48MB is not much these days.

...Robert

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


Re: [HACKERS] plpgsql GUC variable: custom or built-in?

2010-04-19 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 This thread is still on the open items list, as:
 Improve behavior of SUSET GUC variables added by loadable modules?
 Is there something that needs to be done here?  If so, what is it?

Well, if we knew exactly what to do, it wouldn't still be on the list.
The problem is that making a custom variable SUSET doesn't really
work: the system will not accept a value that's assigned before the
loadable module is loaded, even if it was assigned by a superuser.
I suggested a fix in the referenced thread, but it's not exactly an
ideal fix.

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] plpgsql GUC variable: custom or built-in?

2010-04-19 Thread Robert Haas
On Mon, Apr 19, 2010 at 9:51 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Robert Haas robertmh...@gmail.com writes:
 This thread is still on the open items list, as:
 Improve behavior of SUSET GUC variables added by loadable modules?
 Is there something that needs to be done here?  If so, what is it?

 Well, if we knew exactly what to do, it wouldn't still be on the list.
 The problem is that making a custom variable SUSET doesn't really
 work: the system will not accept a value that's assigned before the
 loadable module is loaded, even if it was assigned by a superuser.
 I suggested a fix in the referenced thread, but it's not exactly an
 ideal fix.

Well, at this point the issue is deciding whether we're going to try
to do anything about this before beta.  If we don't know what the
right solution is, that sounds to me like no - in which case we
should move this from the open items list to the todo list.  Does that
sound reasonable to you?

...Robert

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


Re: [HACKERS] [DOCS] Streaming replication document improvements

2010-04-19 Thread Robert Haas
On Fri, Apr 2, 2010 at 2:06 AM, Fujii Masao masao.fu...@gmail.com wrote:
 On Fri, Apr 2, 2010 at 2:58 AM, Robert Haas robertmh...@gmail.com wrote:
 It's probably also easy to fix so that it doesn't NEED to be documented.

 The thing is, when dealing with new features, we reduce our overall
 maintenance burden if we get it right the first time.  Obviously it's
 too late for major changes, but minor adjustments to maintain the POLA
 seem like exactly what we SHOULD be doing right now.

 The attached patch implements the Heikki's proposal:

 --
 ReservedBackends = superuser_reserved_connections + max_wal_senders
 MaxBackends = max_connections + autovacuum_max_workers + max_wal_senders + 1
 --

 This change looks like minor adjustments rather than major changes.

Instead of doing this, could we just change the logic in InitPostgres?

Current logic says we hit the connection limit if:

if (!am_superuser 
ReservedBackends  0 
!HaveNFreeProcs(ReservedBackends))

Couldn't we just change this to:

if ((!am_superuser || am_walsender) 
ReservedBackends  0 
!HaveNFreeProcs(ReservedBackends))

Seems like that'd be a whole lot simpler, if it'll do the job...

...Robert

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


Re: [HACKERS] row estimation off the mark when generate_series calls are involved

2010-04-19 Thread Tom Lane
Takahiro Itagaki itagaki.takah...@oss.ntt.co.jp writes:
 Nikhil Sontakke nikhil.sonta...@enterprisedb.com wrote:
 I think the place where we set the
 targetlist of the result_plan to sub_tlist, immediately after that we
 should update the plan_rows estimate by walking this latest
 targetlist. I did that and now we seem to get proper row estimates.

 I agree the estimation should be improved, but your patch *adds*
 the estimated number of rows to the result:

I'm not very impressed with that patch, even discounting the
sum-vs-product thinko.  Trawling the tlist for SRFs will add a significant
number of cycles, to modify the rowcount in a way that is practically
always wrong (since the estimates for SRF output rowcounts are so bad).
What's more, most of the time we don't really care, because the
top-level rowcount estimate is of no interest for future planning
purposes.  It might be worth doing something about this inside
sub-selects, but not till we have less-bogus SRF rowcount estimates.

BTW, another reason for not being excited about this is that someday we
ought to disallow SRFs in the tlist altogether --- once we have LATERAL,
which might happen in 9.1, that will be a much more semantically
consistent way of getting the desired behavior.

regards, tom lane

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


Re: [HACKERS] plpgsql GUC variable: custom or built-in?

2010-04-19 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 On Mon, Apr 19, 2010 at 9:51 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 The problem is that making a custom variable SUSET doesn't really
 work: the system will not accept a value that's assigned before the
 loadable module is loaded, even if it was assigned by a superuser.
 I suggested a fix in the referenced thread, but it's not exactly an
 ideal fix.

 Well, at this point the issue is deciding whether we're going to try
 to do anything about this before beta.

The reason it seems of concern for 9.0 is that now we have a custom
SUSET variable in plpgsql.  If we don't fix this then we need to think
hard about the alternative of forcing the variable into the core code
to avoid the gotchas.

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] plpgsql GUC variable: custom or built-in?

2010-04-19 Thread Robert Haas
On Mon, Apr 19, 2010 at 10:29 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Robert Haas robertmh...@gmail.com writes:
 On Mon, Apr 19, 2010 at 9:51 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 The problem is that making a custom variable SUSET doesn't really
 work: the system will not accept a value that's assigned before the
 loadable module is loaded, even if it was assigned by a superuser.
 I suggested a fix in the referenced thread, but it's not exactly an
 ideal fix.

 Well, at this point the issue is deciding whether we're going to try
 to do anything about this before beta.

 The reason it seems of concern for 9.0 is that now we have a custom
 SUSET variable in plpgsql.  If we don't fix this then we need to think
 hard about the alternative of forcing the variable into the core code
 to avoid the gotchas.

Well, having reread your proposed solution, it sounds pretty
reasonable to me.  You're never going to be able to make totally
sensible decisions about GUCs if the code that defines those GUCs
isn't loaded, so requiring that the code be loaded before any GUCs are
set seems like a sensible thing to do.  On the other hand, if forcing
this into core gets a beta out the door sooner, maybe that's the way
to go, even though I wouldn't exactly classify it as an elegant
solution.

Or to put it another way - this thread has been sitting idle for 5
months; it's time to make a decision.

...Robert

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


Re: [HACKERS] Thoughts on pg_hba.conf rejection

2010-04-19 Thread Robert Haas
On Mon, Apr 19, 2010 at 8:11 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Robert Haas robertmh...@gmail.com writes:
 On Mon, Apr 19, 2010 at 7:18 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Hmm.  The AV launcher is only permitted to touch pg_database.

 Perhaps we should add a TODO.

 Actually, while I'm looking at that code, a more immediate TODO is
 fix walsender.  Somebody has inserted an absolutely flight-of-fantasy
 code path into InitPostgres.  (Hint: template1 can be dropped.
 ESPECIALLY when you're deliberately not taking any lock on it.)

Off-topic to that, but on-topic to the original topic of this thread,
check out this link that Karen Padir just blogged about on
planet.postgresql.org:

http://blog.metasploit.com/2010/02/postgres-fingerprinting.html

Assuming the situation really is as described here, I am wondering if
we should suppress the F, L, and R output in this and similar cases
and back-patch it all the way back.  This seems like it is entirely
too helpful.

...Robert

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