[HACKERS] CTAS not honoring NOT NULL, DEFAULT modifiers
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
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
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
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
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
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
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
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
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')
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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())
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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?
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
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
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
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?
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?
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
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
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?
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?
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
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