Re: [HACKERS] WIP: RangeTypes

2011-01-14 Thread Jeff Davis
Updated patch.

Summary of changes:

  * More generic functions

  * pg_dump support

  * remove typmod support until it can be done correctly

  * added some tests

There is still quite a bit left, including (numbers match up with
previous TODO list):

  1. Generic functions -- still more work to do here. Handling the
combination of continuous range semantics with NULLs requires quite a
lot of special cases, because it's hard to share code among functions.
Even something as simple as equals is not as trivial as it sounds.
Perhaps I'm missing some cleaner abstractions, or perhaps I'm
over-thinking the null semantics.

  3. perhaps fix typmod

  4. documentation

  5. more tests

  7. better parser


Regards,
Jeff Davis


rangetypes-20110114.patch.gz
Description: GNU Zip compressed data

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


Re: [HACKERS] pg_depend explained

2011-01-14 Thread Joel Jacobson
2011/1/12 Alvaro Herrera alvhe...@commandprompt.com:
 I think this code should live in the Wiki somewhere:
 http://wiki.postgresql.org/wiki/Snippets

This file contains only the relevant remapping of pg_depend, folding
the internal linkages properly:

https://github.com/gluefinance/pov/blob/master/sql/schema/pov/views/pg_depend_remapped.sql

It can be tested stand-alone and does not require any other components
from the pov project.

Can I create a wiki snippet myself or do I need some kind of admin access?

-- 
Best regards,

Joel Jacobson
Glue Finance

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


Re: [HACKERS] pg_depend explained

2011-01-14 Thread Magnus Hagander
On Fri, Jan 14, 2011 at 09:39, Joel Jacobson j...@gluefinance.com wrote:
 2011/1/12 Alvaro Herrera alvhe...@commandprompt.com:
 I think this code should live in the Wiki somewhere:
 http://wiki.postgresql.org/wiki/Snippets

 This file contains only the relevant remapping of pg_depend, folding
 the internal linkages properly:

 https://github.com/gluefinance/pov/blob/master/sql/schema/pov/views/pg_depend_remapped.sql

 It can be tested stand-alone and does not require any other components
 from the pov project.

 Can I create a wiki snippet myself or do I need some kind of admin access?

Absolutely, no admin access required. As long as you have (or create)
a community account, you can edit or create pages.

-- 
 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] Streaming base backups

2011-01-14 Thread Heikki Linnakangas

On 14.01.2011 08:45, Fujii Masao wrote:

On Fri, Jan 14, 2011 at 4:13 AM, Magnus Hagandermag...@hagander.net  wrote:

At the end of the backup by walsender, it forces a switch to a new
WAL file and waits until the last WAL file has been archived. So we
should change postmaster so that it doesn't cause the archiver to
end before walsender ends when shutdown is requested?


Um. I have to admit I'm not entirely following what you mean enough to
confirm it, but it *sounds* correct :-)

What scenario exactly is the problematic one?


1. Smart shutdown is requested while walsender is sending a backup.
2. Shutdown causes archiver to end.
  (Though shutdown sends SIGUSR2 to walsender to exit, walsender
   running backup doesn't respond for now)
3. At the end of backup, walsender calls do_pg_stop_backup, which
  forces a switch to a new WAL file and waits until the last WAL file has
  been archived.
  *BUT*, since archiver has already been dead, walsender waits for
  that forever.


Not only does it wait forever, but it writes the end-of-backup WAL 
record after bgwriter has already exited and written the shutdown 
checkpoint record.


I think postmaster should treat a walsender as a regular backend, until 
it has started streaming.


We can achieve that by starting up the child as PM_CHILD_ACTIVE, and 
changing the state to PM_CHILD_WALSENDER later, when streaming is 
started. Looking at the postmaster.c, that should be safe, postmaster 
will treat a backend as a regular backend anyway until it has connected 
to shared memory. It is *not* safe to switch a walsender back to a 
regular process, but we have no need to do 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] SSI patch version 8

2011-01-14 Thread Anssi Kääriäinen

On 01/14/2011 02:21 AM, Kevin Grittner wrote:

I hope you have no objection to having the code you wrote included
in the test suite which is part of the patch.  Well, if you do, I'll
pull it back out and invent something similar...  ;-)

No objection.

 - Anssi

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


Re: [HACKERS] [RRR] reviewers needed!

2011-01-14 Thread Dimitri Fontaine
Robert Haas robertmh...@gmail.com writes:
 So far I have 6 people who have volunteered to be round-robin
 reviewers, and 7 people who are listed as reviewers on the CF site
 already.  That leaves 45 patches without a reviewer, plus whatever
 comes in in the next day or so.  This is not going to work unless a
 lot more people pitch in.

I will enroll later this week-end or early next week, have some other
duties meanwhile.

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

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


[HACKERS] Recovery control functions

2011-01-14 Thread Simon Riggs

Functions to control recovery, to aid PITR and Hot Standby.
pg_is_xlog_replay_paused()
pg_xlog_replay_pause()
pg_xlog_replay_resume()

recovery.conf parameter: pause_at_recovery_target (bool)

-- 
 Simon Riggs   http://www.2ndQuadrant.com/books/
 PostgreSQL Development, 24x7 Support, Training and Services
 
diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml
index 17a1d34..5b90e9e 100644
--- a/doc/src/sgml/ref/alter_table.sgml
+++ b/doc/src/sgml/ref/alter_table.sgml
@@ -42,7 +42,8 @@ ALTER TABLE replaceable class=PARAMETERname/replaceable
 ALTER [ COLUMN ] replaceable class=PARAMETERcolumn/replaceable SET ( replaceable class=PARAMETERattribute_option/replaceable = replaceable class=PARAMETERvalue/replaceable [, ... ] )
 ALTER [ COLUMN ] replaceable class=PARAMETERcolumn/replaceable RESET ( replaceable class=PARAMETERattribute_option/replaceable [, ... ] )
 ALTER [ COLUMN ] replaceable class=PARAMETERcolumn/replaceable SET STORAGE { PLAIN | EXTERNAL | EXTENDED | MAIN }
-ADD replaceable class=PARAMETERtable_constraint/replaceable
+ADD replaceable class=PARAMETERtable_constraint/replaceable [ NOT VALID ]
+VALIDATE CONSTRAINT replaceable class=PARAMETERconstraint_name/replaceable
 DROP CONSTRAINT [ IF EXISTS ]  replaceable class=PARAMETERconstraint_name/replaceable [ RESTRICT | CASCADE ]
 DISABLE TRIGGER [ replaceable class=PARAMETERtrigger_name/replaceable | ALL | USER ]
 ENABLE TRIGGER [ replaceable class=PARAMETERtrigger_name/replaceable | ALL | USER ]
@@ -220,11 +221,27 @@ ALTER TABLE replaceable class=PARAMETERname/replaceable
/varlistentry
 
varlistentry
-termliteralADD replaceable class=PARAMETERtable_constraint/replaceable/literal/term
+termliteralADD replaceable class=PARAMETERtable_constraint/replaceable
+  [ NOT VALID ]/literal/term
 listitem
  para
   This form adds a new constraint to a table using the same syntax as
-  xref linkend=SQL-CREATETABLE.
+  xref linkend=SQL-CREATETABLE. Newly added foreign key constraints can
+  also be defined as literalNOT VALID/literal to avoid the
+  potentially lengthy initial check that must otherwise be performed.
+  Constraint checks are skipped at create table time, so
+  xref linkend=SQL-CREATETABLE does not contain this option.
+ /para
+/listitem
+   /varlistentry
+
+   varlistentry
+termliteralVALIDATE CONSTRAINT/literal/term
+listitem
+ para
+  This form validates a foreign key constraint that was previously created
+  as literalNOT VALID/literal. Constraints already marked valid do not
+  cause an error response.
  /para
 /listitem
/varlistentry
diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index 4c55db7..7cc521d 100644
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -1835,6 +1835,7 @@ StoreRelCheck(Relation rel, char *ccname, Node *expr,
 		  CONSTRAINT_CHECK,		/* Constraint Type */
 		  false,	/* Is Deferrable */
 		  false,	/* Is Deferred */
+		  true,		/* Is Validated */
 		  RelationGetRelid(rel),		/* relation */
 		  attNos,		/* attrs in the constraint */
 		  keycount,		/* # attrs in the constraint */
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index 4dd89e1..a19b139 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -777,6 +777,7 @@ index_create(Oid heapRelationId,
 		   constraintType,
 		   deferrable,
 		   initdeferred,
+		   true,
 		   heapRelationId,
 		   indexInfo-ii_KeyAttrNumbers,
 		   indexInfo-ii_NumIndexAttrs,
diff --git a/src/backend/catalog/pg_constraint.c b/src/backend/catalog/pg_constraint.c
index 3a38518..6619eed 100644
--- a/src/backend/catalog/pg_constraint.c
+++ b/src/backend/catalog/pg_constraint.c
@@ -46,6 +46,7 @@ CreateConstraintEntry(const char *constraintName,
 	  char constraintType,
 	  bool isDeferrable,
 	  bool isDeferred,
+	  bool isValidated,
 	  Oid relId,
 	  const int16 *constraintKey,
 	  int constraintNKeys,
@@ -158,6 +159,7 @@ CreateConstraintEntry(const char *constraintName,
 	values[Anum_pg_constraint_contype - 1] = CharGetDatum(constraintType);
 	values[Anum_pg_constraint_condeferrable - 1] = BoolGetDatum(isDeferrable);
 	values[Anum_pg_constraint_condeferred - 1] = BoolGetDatum(isDeferred);
+	values[Anum_pg_constraint_convalidated - 1] = BoolGetDatum(isValidated);
 	values[Anum_pg_constraint_conrelid - 1] = ObjectIdGetDatum(relId);
 	values[Anum_pg_constraint_contypid - 1] = ObjectIdGetDatum(domainId);
 	values[Anum_pg_constraint_conindid - 1] = ObjectIdGetDatum(indexRelId);
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index f3bd565..8041a9b 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -254,6 +254,7 @@ static void 

Re: [HACKERS] Recovery control functions

2011-01-14 Thread Simon Riggs
On Fri, 2011-01-14 at 11:09 +, Simon Riggs wrote:
 Functions to control recovery, to aid PITR and Hot Standby.
 pg_is_xlog_replay_paused()
 pg_xlog_replay_pause()
 pg_xlog_replay_resume()
 
 recovery.conf parameter: pause_at_recovery_target (bool)

And now with the correct patch.

-- 
 Simon Riggs   http://www.2ndQuadrant.com/books/
 PostgreSQL Development, 24x7 Support, Training and Services
 
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index d15..55fa70d 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -14173,6 +14173,63 @@ postgres=# SELECT * FROM pg_xlogfile_name_offset(pg_stop_backup());
/table
 
para
+The functions shown in xref
+linkend=functions-recovery-control-table control the progress of recovery.
+These functions may be executed only during recovery.
+   /para
+
+   table id=functions-recovery-control-table
+titleRecovery Control Functions/title
+tgroup cols=3
+ thead
+  rowentryName/entry entryReturn Type/entry entryDescription/entry
+  /row
+ /thead
+
+ tbody
+  row
+   entry
+literalfunctionpg_is_xlog_replay_paused()/function/literal
+/entry
+   entrytypebool/type/entry
+   entryTrue if recovery is paused.
+   /entry
+  /row
+  row
+   entry
+literalfunctionpg_xlog_replay_pause()/function/literal
+/entry
+   entrytypevoid/type/entry
+   entryPauses recovery immediately.
+   /entry
+  /row
+  row
+   entry
+literalfunctionpg_xlog_replay_resume()/function/literal
+/entry
+   entrytypevoid/type/entry
+   entryRestarts recovery if it was paused.
+   /entry
+  /row
+ /tbody
+/tgroup
+   /table
+
+   para
+While recovery is paused no further database changes are applied.
+If in hot standby, all queries will see the same consistent snapshot
+of the database, and no query conflicts will be generated.
+   /para
+
+   para
+If streaming replication is disabled, the paused state may continue
+indefinitely without problem. While streaming replication is in
+progress WAL records will continue to be received, which will
+eventually fill available disk space, depending upon the duration of
+the pause, the rate of WAL generation and available disk space.
+   /para
+
+   para
 The functions shown in xref linkend=functions-admin-dbsize calculate
 the disk space usage of database objects.
/para
diff --git a/doc/src/sgml/recovery-config.sgml b/doc/src/sgml/recovery-config.sgml
index bd9dfd1..b5f9cfa 100644
--- a/doc/src/sgml/recovery-config.sgml
+++ b/doc/src/sgml/recovery-config.sgml
@@ -228,6 +228,31 @@ restore_command = 'copy C:\\server\\archivedir\\%f %p'  # Windows
   /listitem
  /varlistentry
 
+ varlistentry id=pause-at-recovery-target
+   xreflabel=pause_at_recovery_target
+  termvarnamepause_at_recovery_target/varname
+(typeboolean/type)
+  /term
+  indexterm
+primaryvarnamepause_at_recovery_target/ recovery parameter/primary
+  /indexterm
+  listitem
+   para
+Specifies whether recovery should pause when the recovery target
+is reached. The default is true, if a recovery target is set.
+This is intended to allow queries to be executed against the
+database to check if this recovery target is the most desirable
+point for recovery. The paused state can be resumed by using
+functionpg_xlog_replay_resume()/ (See
+xref linkend=functions-recovery-control-table), which then
+causes recovery to end. If this recovery target is not the
+desired stopping point, then shutdown the server, change the
+recovery target settings to a later target and restart to
+continue recovery.
+   /para
+  /listitem
+ /varlistentry
+
  /variablelist
/sect1
 
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 818a59f..6a8c500 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -184,6 +184,7 @@ static char *recoveryEndCommand = NULL;
 static char *archiveCleanupCommand = NULL;
 static RecoveryTargetType recoveryTarget = RECOVERY_TARGET_UNSET;
 static bool recoveryTargetInclusive = true;
+static bool recoveryPauseAtTarget = false;
 static TransactionId recoveryTargetXid;
 static TimestampTz recoveryTargetTime;
 
@@ -416,6 +417,8 @@ typedef struct XLogCtlData
 	XLogRecPtr	recoveryLastRecPtr;
 	/* timestamp of last COMMIT/ABORT record replayed (or being replayed) */
 	TimestampTz recoveryLastXTime;
+	/* Are we requested to pause recovery? */
+	bool		recoveryPause;
 
 	slock_t		info_lck;		/* locks shared variables shown above */
 } XLogCtlData;
@@ -563,6 +566,9 @@ static void readRecoveryCommandFile(void);
 static void exitArchiveRecovery(TimeLineID endTLI,
 	uint32 endLogId, uint32 endLogSeg);
 static bool 

Re: [HACKERS] ALTER TABLE ... ADD FOREIGN KEY ... NOT ENFORCED

2011-01-14 Thread Simon Riggs
On Mon, 2010-12-13 at 17:15 +, Peter Geoghegan wrote:
 On 13 December 2010 16:08, Robert Haas robertmh...@gmail.com wrote:
  On Mon, Dec 13, 2010 at 10:49 AM, Simon Riggs si...@2ndquadrant.com wrote:
  2. pg_validate_foreign_key('constraint name');
  Returns immediately if FK is valid
  Returns SETOF rows that violate the constraint, or if no rows are
  returned it updates constraint to show it is now valid.
  Lock held: AccessShareLock
 
  I'm less sure about this part.  I think there should be a DDL
  statement to validate the foreign key.  The return the problem rows
  behavior could be done some other way, or just left to the user to
  write their own query.
 
 +1. I think that a DDL statement is more appropriate, because it makes
 the process sort of symmetrical.

Patch to implement the proposed feature attached, for CFJan2011.

2 sub-command changes:

ALTER TABLE foo ADD FOREIGN KEY fkoo ... NOT VALID;

ALTER TABLE foo VALIDATE CONSTRAINT fkoo;

-- 
 Simon Riggs   http://www.2ndQuadrant.com/books/
 PostgreSQL Development, 24x7 Support, Training and Services
 
diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml
index 17a1d34..5b90e9e 100644
--- a/doc/src/sgml/ref/alter_table.sgml
+++ b/doc/src/sgml/ref/alter_table.sgml
@@ -42,7 +42,8 @@ ALTER TABLE replaceable class=PARAMETERname/replaceable
 ALTER [ COLUMN ] replaceable class=PARAMETERcolumn/replaceable SET ( replaceable class=PARAMETERattribute_option/replaceable = replaceable class=PARAMETERvalue/replaceable [, ... ] )
 ALTER [ COLUMN ] replaceable class=PARAMETERcolumn/replaceable RESET ( replaceable class=PARAMETERattribute_option/replaceable [, ... ] )
 ALTER [ COLUMN ] replaceable class=PARAMETERcolumn/replaceable SET STORAGE { PLAIN | EXTERNAL | EXTENDED | MAIN }
-ADD replaceable class=PARAMETERtable_constraint/replaceable
+ADD replaceable class=PARAMETERtable_constraint/replaceable [ NOT VALID ]
+VALIDATE CONSTRAINT replaceable class=PARAMETERconstraint_name/replaceable
 DROP CONSTRAINT [ IF EXISTS ]  replaceable class=PARAMETERconstraint_name/replaceable [ RESTRICT | CASCADE ]
 DISABLE TRIGGER [ replaceable class=PARAMETERtrigger_name/replaceable | ALL | USER ]
 ENABLE TRIGGER [ replaceable class=PARAMETERtrigger_name/replaceable | ALL | USER ]
@@ -220,11 +221,27 @@ ALTER TABLE replaceable class=PARAMETERname/replaceable
/varlistentry
 
varlistentry
-termliteralADD replaceable class=PARAMETERtable_constraint/replaceable/literal/term
+termliteralADD replaceable class=PARAMETERtable_constraint/replaceable
+  [ NOT VALID ]/literal/term
 listitem
  para
   This form adds a new constraint to a table using the same syntax as
-  xref linkend=SQL-CREATETABLE.
+  xref linkend=SQL-CREATETABLE. Newly added foreign key constraints can
+  also be defined as literalNOT VALID/literal to avoid the
+  potentially lengthy initial check that must otherwise be performed.
+  Constraint checks are skipped at create table time, so
+  xref linkend=SQL-CREATETABLE does not contain this option.
+ /para
+/listitem
+   /varlistentry
+
+   varlistentry
+termliteralVALIDATE CONSTRAINT/literal/term
+listitem
+ para
+  This form validates a foreign key constraint that was previously created
+  as literalNOT VALID/literal. Constraints already marked valid do not
+  cause an error response.
  /para
 /listitem
/varlistentry
diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index 4c55db7..7cc521d 100644
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -1835,6 +1835,7 @@ StoreRelCheck(Relation rel, char *ccname, Node *expr,
 		  CONSTRAINT_CHECK,		/* Constraint Type */
 		  false,	/* Is Deferrable */
 		  false,	/* Is Deferred */
+		  true,		/* Is Validated */
 		  RelationGetRelid(rel),		/* relation */
 		  attNos,		/* attrs in the constraint */
 		  keycount,		/* # attrs in the constraint */
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index 4dd89e1..a19b139 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -777,6 +777,7 @@ index_create(Oid heapRelationId,
 		   constraintType,
 		   deferrable,
 		   initdeferred,
+		   true,
 		   heapRelationId,
 		   indexInfo-ii_KeyAttrNumbers,
 		   indexInfo-ii_NumIndexAttrs,
diff --git a/src/backend/catalog/pg_constraint.c b/src/backend/catalog/pg_constraint.c
index 3a38518..6619eed 100644
--- a/src/backend/catalog/pg_constraint.c
+++ b/src/backend/catalog/pg_constraint.c
@@ -46,6 +46,7 @@ CreateConstraintEntry(const char *constraintName,
 	  char constraintType,
 	  bool isDeferrable,
 	  bool isDeferred,
+	  bool isValidated,
 	  Oid relId,
 	  const int16 *constraintKey,
 	  int constraintNKeys,
@@ -158,6 +159,7 @@ CreateConstraintEntry(const char 

Re: [HACKERS] Streaming base backups

2011-01-14 Thread Magnus Hagander
On Fri, Jan 14, 2011 at 11:19, Heikki Linnakangas
heikki.linnakan...@enterprisedb.com wrote:
 On 14.01.2011 08:45, Fujii Masao wrote:

 On Fri, Jan 14, 2011 at 4:13 AM, Magnus Hagandermag...@hagander.net
  wrote:

 At the end of the backup by walsender, it forces a switch to a new
 WAL file and waits until the last WAL file has been archived. So we
 should change postmaster so that it doesn't cause the archiver to
 end before walsender ends when shutdown is requested?

 Um. I have to admit I'm not entirely following what you mean enough to
 confirm it, but it *sounds* correct :-)

 What scenario exactly is the problematic one?

 1. Smart shutdown is requested while walsender is sending a backup.
 2. Shutdown causes archiver to end.
      (Though shutdown sends SIGUSR2 to walsender to exit, walsender
       running backup doesn't respond for now)
 3. At the end of backup, walsender calls do_pg_stop_backup, which
      forces a switch to a new WAL file and waits until the last WAL file
 has
      been archived.
      *BUT*, since archiver has already been dead, walsender waits for
      that forever.

 Not only does it wait forever, but it writes the end-of-backup WAL record
 after bgwriter has already exited and written the shutdown checkpoint
 record.

 I think postmaster should treat a walsender as a regular backend, until it
 has started streaming.

 We can achieve that by starting up the child as PM_CHILD_ACTIVE, and
 changing the state to PM_CHILD_WALSENDER later, when streaming is started.
 Looking at the postmaster.c, that should be safe, postmaster will treat a
 backend as a regular backend anyway until it has connected to shared memory.
 It is *not* safe to switch a walsender back to a regular process, but we
 have no need to do that.

Seems reasonable to me.

I've applied a patch that exits base backups when the postmaster is
shutting down - I'm happily waiting for Heikki to submit one that
changes the shutdown logic in the postmaster :-)

-- 
 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] [PATCH] Return command tag 'REPLACE X' for CREATE OR REPLACE statements.

2011-01-14 Thread Robert Haas
2011/1/13 KaiGai Kohei kai...@ak.jp.nec.com:
 I tried to pick up this patch to review.

 It seems to me fine, enough simple and works as explained in the
 implementation level, apart from reasonability of this feature.
 (Tom was not 100% agree with this feature 1.5month ago.)

Did you check whether this updated the code for 100% of the object
types where this could apply?

 I'm not certain whether the current regression test should be
 updated, or not. The pg_regress launches psql with -q option,
 so completionTag is always ignored.

Well, I don't see any easy way of regression testing it, then.  Am I
missing something?

Also, I don't really like the way this spreads knowledge of the
completionTag out all over the backend.  I think it would be better to
follow the existing model used by the COPY and COMMIT commands,
whereby the return value indicates what happened and
standard_ProcessUtility() uses that to set the command tag.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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


Re: [HACKERS] Recovery control functions

2011-01-14 Thread Magnus Hagander
On Fri, Jan 14, 2011 at 12:15, Simon Riggs si...@2ndquadrant.com wrote:
 On Fri, 2011-01-14 at 11:09 +, Simon Riggs wrote:
 Functions to control recovery, to aid PITR and Hot Standby.
 pg_is_xlog_replay_paused()
 pg_xlog_replay_pause()
 pg_xlog_replay_resume()

 recovery.conf parameter: pause_at_recovery_target (bool)


Awesome, I've been waiting for these! :-)

How hard would it be to have a pg_xlog_replay_until(xlog location or
timestamp), to have it resume recovery up to that point and then
pause again?

-- 
 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] Recovery control functions

2011-01-14 Thread Heikki Linnakangas

On 14.01.2011 13:15, Simon Riggs wrote:

 /*
+ * Recheck shared recoveryPause by polling.
+ *
+ * XXX It might seem we should do this via a shared Latch, but
+ * currently we only support one shared latch per process and
+ * that is already taken for Startup process. Polling is used
+ * in other places in xlog.c already, so not a concern.
+ */


There is no such limitation with latches.

--
  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] Recovery control functions

2011-01-14 Thread Simon Riggs
On Fri, 2011-01-14 at 12:41 +0100, Magnus Hagander wrote:
 On Fri, Jan 14, 2011 at 12:15, Simon Riggs si...@2ndquadrant.com wrote:
  On Fri, 2011-01-14 at 11:09 +, Simon Riggs wrote:
  Functions to control recovery, to aid PITR and Hot Standby.
  pg_is_xlog_replay_paused()
  pg_xlog_replay_pause()
  pg_xlog_replay_resume()
 
  recovery.conf parameter: pause_at_recovery_target (bool)
 
 
 Awesome, I've been waiting for these! :-)
 
 How hard would it be to have a pg_xlog_replay_until(xlog location or
 timestamp), to have it resume recovery up to that point and then
 pause again?

You can already do that for timestamps. 

What you can't do is dynamically set recovery targets via functions,
since currently that is set via recovery.conf parameters. Which requires
restart.

Jaime has a separate patch about recovery targets as well, which does
some more of what you want.

Some things are straightforward, some things require overhaul of the
recovery.conf mechanisms, which is not the right time to do that, nor
have we even discussed let alone agreed what we would change it to.

-- 
 Simon Riggs   http://www.2ndQuadrant.com/books/
 PostgreSQL Development, 24x7 Support, Training and Services
 


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


Re: [HACKERS] Recovery control functions

2011-01-14 Thread Simon Riggs
On Fri, 2011-01-14 at 13:47 +0200, Heikki Linnakangas wrote:
 On 14.01.2011 13:15, Simon Riggs wrote:
   /*
  + * Recheck shared recoveryPause by polling.
  + *
  + * XXX It might seem we should do this via a shared Latch, but
  + * currently we only support one shared latch per process and
  + * that is already taken for Startup process. Polling is used
  + * in other places in xlog.c already, so not a concern.
  + */
 
 There is no such limitation with latches.

SIGUSR1 handler can only handle one shared latch

-- 
 Simon Riggs   http://www.2ndQuadrant.com/books/
 PostgreSQL Development, 24x7 Support, Training and Services
 


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


Re: [HACKERS] Recovery control functions

2011-01-14 Thread Heikki Linnakangas

On 14.01.2011 14:01, Simon Riggs wrote:

On Fri, 2011-01-14 at 13:47 +0200, Heikki Linnakangas wrote:

On 14.01.2011 13:15, Simon Riggs wrote:

  /*
+ * Recheck shared recoveryPause by polling.
+ *
+ * XXX It might seem we should do this via a shared Latch, but
+ * currently we only support one shared latch per process and
+ * that is already taken for Startup process. Polling is used
+ * in other places in xlog.c already, so not a concern.
+ */


There is no such limitation with latches.


SIGUSR1 handler can only handle one shared latch


You can only *wait* for one latch at a time, but you can own more than 
that. AFAICS you would never need to wait for the recovery-pause-latch 
at the same time as the other latch.


(That you can't wait for more than one latch at a time isn't a 
limitation of the SIGUSR1 handler either. The signal handler and the 
self-pipe mechanism wouldn't need any changes to support multi-latch 
waits. We're just missing a WaitMultipleLatches() function that would 
check the is_set flag on multiple latches.)


--
  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] reviewers needed!

2011-01-14 Thread Robert Haas
On Thu, Jan 13, 2011 at 4:54 PM, Robert Haas robertmh...@gmail.com wrote:
 On Tue, Jan 11, 2011 at 9:17 PM, Robert Haas robertmh...@gmail.com wrote:
 [ abject plea for reviewers ]

 [ second abject please for reviewers ]

OK, I believe I've sent an off-list email to everyone who volunteered
to review and asked me to assign them a patch, which totaled 11
people.

If you got an email of this type, and the patch you were assigned is
OK, then please go to commitfest.postgresql.org, edit your patch, and
put your name next to it so other people know you've grabbed it.  Then
review it and post your review on -hackers.  Then add a comment to the
patch with your review and mark it as either Waiting on Author or
Ready for Committer.

If you did not get an email of this type and want one, email me and
I'll assign you a patch.

If you are a committer or an experience reviewer looking for a
challenge, please pick up a hard patch and review it, as many people
are not up to picking these up.  Some good choices: Change
pg_last_xlog_receive_location not to move backwards (is this safe?),
SSPI client auth on non-Windows systems (needs Windows expertise), FDW
API (complicated), Range Types (also complicated), ALTER EXTENSION
UPGRADE (needs design comments as well as code review).

At this point, our goal should be to get all patches that are marked
as Needs Review to either Waiting on Author or Ready for
Committer as quickly as possible.  All help is appreciated!

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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


Re: [HACKERS] [COMMITTERS] pgsql: Exit from base backups when shutdown is requested

2011-01-14 Thread Heikki Linnakangas

On 14.01.2011 14:10, Simon Riggs wrote:

Please make that the last commit in this area for next week.

I want to get a base from which to solidify the sync rep code, and
associated patches.

I'm not saying there will be a code clash, but there may be a clash of
detail or intention that will make life more difficult for testing.


I'm sorry, but it doesn't work that way. There's at least one serious 
bug in there still 
(http://archives.postgresql.org/message-id/4d302326.1000...@enterprisedb.com), 
and the multiple-concurrent-backups patch is pending review/commit. And 
there's one more feature me  Magnus would like to get in before the 
release: including all the WAL files required to restore from the backup 
in the backup tar itself - we'll see if that makes the release or not.


None of that should conflict seriously with the synch rep code, although 
it's hard to tell without seeing the patch. And even if it does, we'll 
just resolve the conflict, merge conflicts are not such a big deal.


--
  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] Recovery control functions

2011-01-14 Thread Simon Riggs
On Fri, 2011-01-14 at 14:08 +0200, Heikki Linnakangas wrote:
 On 14.01.2011 14:01, Simon Riggs wrote:
  On Fri, 2011-01-14 at 13:47 +0200, Heikki Linnakangas wrote:
  On 14.01.2011 13:15, Simon Riggs wrote:
/*
  + * Recheck shared recoveryPause by polling.
  + *
  + * XXX It might seem we should do this via a shared Latch, but
  + * currently we only support one shared latch per process and
  + * that is already taken for Startup process. Polling is used
  + * in other places in xlog.c already, so not a concern.
  + */
 
  There is no such limitation with latches.
 
  SIGUSR1 handler can only handle one shared latch
 
 You can only *wait* for one latch at a time, but you can own more than 
 that. AFAICS you would never need to wait for the recovery-pause-latch 
 at the same time as the other latch.

Yes, I understand.

Trouble is, if you wait on Latch X and other processes send wakeup
assuming you were waiting on Latch Y, then this will erroneously wake
you up.

So a process can have more than one shared latch, BUT other processes
don't know and can't tell which latch you're waiting on. Yes, you can
get around that, but not via the direct support of the latch module, as
currently written.

Polling is fine in that case, and we already do elsewhere, so it wasn't
critical for me to extend latch support to implement this. 

 (That you can't wait for more than one latch at a time isn't a 
 limitation of the SIGUSR1 handler either. The signal handler and the 
 self-pipe mechanism wouldn't need any changes to support multi-latch 
 waits. We're just missing a WaitMultipleLatches() function that would 
 check the is_set flag on multiple latches.)

Something for the future.

-- 
 Simon Riggs   http://www.2ndQuadrant.com/books/
 PostgreSQL Development, 24x7 Support, Training and Services
 


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


Re: [HACKERS] [COMMITTERS] pgsql: Exit from base backups when shutdown is requested

2011-01-14 Thread Magnus Hagander
On Fri, Jan 14, 2011 at 13:16, Heikki Linnakangas
heikki.linnakan...@enterprisedb.com wrote:
 On 14.01.2011 14:10, Simon Riggs wrote:

 Please make that the last commit in this area for next week.

 I want to get a base from which to solidify the sync rep code, and
 associated patches.

 I'm not saying there will be a code clash, but there may be a clash of
 detail or intention that will make life more difficult for testing.

 I'm sorry, but it doesn't work that way. There's at least one serious bug in
 there still
 (http://archives.postgresql.org/message-id/4d302326.1000...@enterprisedb.com),
 and the multiple-concurrent-backups patch is pending review/commit. And
 there's one more feature me  Magnus would like to get in before the
 release: including all the WAL files required to restore from the backup in
 the backup tar itself - we'll see if that makes the release or not.

There's one more before that, which is the walsender parser patch. I
intend to get that in RSN, but the include all wal files will be a
while, though, I don't think either of us has started that one?

-- 
 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] SQL/MED - FDW API

2011-01-14 Thread Shigeru HANADA
Attached are rebased version of patches for FDW API.

To make review easier, I split core functionality into 3 patches. 
Please apply these patches in the following order.

1) fdw_handler - this patch adds HANDLER option to both syntax and
catalog of FOREIGN DATA WRAPPER.

2) foreign_scan - this patch adds following: ForeignScan executor-node,
hooks in planner and executor, and FdwRoutine (FDW API).

3) fdw_catalog_lookup - this patch adds GetForeignTable() whicch
returns ForeignTable object, similar to GetForeignDataWrapper(),
GetForeignServer(), and GetUserMapping().  This function is assumed to
be used by FDWs.

You would be able to test these patches with 20110114 version of file_fdw
wrapper patches which will be posted in another thread.

Regards,
--
Shigeru Hanada


20110114-fdw_handler.patch.gz
Description: Binary data


20110114-foreign_scan.patch.gz
Description: Binary data


20110114-fdw_catalog_lookup.patch.gz
Description: Binary data

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


[HACKERS] Re: [COMMITTERS] pgsql: Exit from base backups when shutdown is requested

2011-01-14 Thread Simon Riggs
On Fri, 2011-01-14 at 14:16 +0200, Heikki Linnakangas wrote:
 On 14.01.2011 14:10, Simon Riggs wrote:
  Please make that the last commit in this area for next week.
 
  I want to get a base from which to solidify the sync rep code, and
  associated patches.
 
  I'm not saying there will be a code clash, but there may be a clash of
  detail or intention that will make life more difficult for testing.
 
 I'm sorry, but it doesn't work that way. There's at least one serious 
 bug in there still 

I'm not clear why you haven't submitted this code to the CommitFest,
especially if it clearly hasn't been very well tested.

 None of that should conflict seriously with the synch rep code, although 
 it's hard to tell without seeing the patch. And even if it does, we'll 
 just resolve the conflict, merge conflicts are not such a big deal.

It's even easier if you don't cause them in the first place.

My request is not unreasonable. Please be helpful, as you would expect
me to be if the roles were reversed.

-- 
 Simon Riggs   http://www.2ndQuadrant.com/books/
 PostgreSQL Development, 24x7 Support, Training and Services
 


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


Re: [HACKERS] Recovery control functions

2011-01-14 Thread Heikki Linnakangas

On 14.01.2011 14:18, Simon Riggs wrote:

On Fri, 2011-01-14 at 14:08 +0200, Heikki Linnakangas wrote:

On 14.01.2011 14:01, Simon Riggs wrote:

On Fri, 2011-01-14 at 13:47 +0200, Heikki Linnakangas wrote:

On 14.01.2011 13:15, Simon Riggs wrote:

   /*
+ * Recheck shared recoveryPause by polling.
+ *
+ * XXX It might seem we should do this via a shared Latch, but
+ * currently we only support one shared latch per process and
+ * that is already taken for Startup process. Polling is used
+ * in other places in xlog.c already, so not a concern.
+ */


There is no such limitation with latches.


SIGUSR1 handler can only handle one shared latch


You can only *wait* for one latch at a time, but you can own more than
that. AFAICS you would never need to wait for the recovery-pause-latch
at the same time as the other latch.


Yes, I understand.

Trouble is, if you wait on Latch X and other processes send wakeup
assuming you were waiting on Latch Y, then this will erroneously wake
you up.


The signal will wake up the process, but WaitLatch will quickly go back 
to sleep if it wasn't for the latch we're waiting for. I don't think 
that causes any meaningful performance issues.


So that's all handled within the latch code.

--
  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: Exit from base backups when shutdown is requested

2011-01-14 Thread Robert Haas
On Fri, Jan 14, 2011 at 7:25 AM, Simon Riggs si...@2ndquadrant.com wrote:
 My request is not unreasonable. Please be helpful, as you would expect
 me to be if the roles were reversed.

I can't remember anyone other than you ever requesting that.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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


Re: [HACKERS] [COMMITTERS] pgsql: Exit from base backups when shutdown is requested

2011-01-14 Thread Heikki Linnakangas

On 14.01.2011 14:25, Simon Riggs wrote:

On Fri, 2011-01-14 at 14:16 +0200, Heikki Linnakangas wrote:

None of that should conflict seriously with the synch rep code, although
it's hard to tell without seeing the patch. And even if it does, we'll
just resolve the conflict, merge conflicts are not such a big deal.


It's even easier if you don't cause them in the first place.

My request is not unreasonable. Please be helpful, as you would expect
me to be if the roles were reversed.


Feel free to submit the patch against the snapshot of today. I will fix 
any merge conflict that arises from changes to the base-backup stuff.


--
  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] Recovery control functions

2011-01-14 Thread Simon Riggs
On Fri, 2011-01-14 at 14:27 +0200, Heikki Linnakangas wrote:

  Trouble is, if you wait on Latch X and other processes send wakeup
  assuming you were waiting on Latch Y, then this will erroneously wake
  you up.
 
 The signal will wake up the process, but WaitLatch will quickly go back 
 to sleep if it wasn't for the latch we're waiting for. I don't think 
 that causes any meaningful performance issues.

Neither does polling.

 So that's all handled within the latch code.

I'll adjust the comment.

-- 
 Simon Riggs   http://www.2ndQuadrant.com/books/
 PostgreSQL Development, 24x7 Support, Training and Services
 


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


Re: [HACKERS] Allowing multiple concurrent base backups

2011-01-14 Thread Magnus Hagander
On Thu, Jan 13, 2011 at 22:32, Heikki Linnakangas
heikki.linnakan...@enterprisedb.com wrote:
 On 13.01.2011 22:57, Josh Berkus wrote:

 On 1/13/11 12:11 PM, Robert Haas wrote:

 That's going to depend on the situation.  If the database fits in
 memory, then it's just going to work.  If it fits on disk, it's less
 obvious whether it'll be good or bad, but an arbitrary limitation here
 doesn't serve us well.

 FWIW, if we had this feature right now in 9.0 we (PGX) would be using
 it.  We run into the case of DB in memory, multiple slaves fairly often
 these days.

 Anyway, here's an updated patch with all the known issues fixed.

It's kind of crude that do_pg_stop_backup() has to parse the
backuplabel with sscanf() when it's coming from a non-exclusive backup
- we could pass the location as a parameter instead, to make it
cleaner. There might be a point to it being the same in both
codepaths, but I'm not sure it's that important...

Doesn't this code put the backup label in *every* tarfile, and not
just the main one? From what I can tell the code in
SendBackupDirectory() relies on labelfile being NULL in tar files for
other tablespaces, but the caller never sets this.

Finally, I'd move the addition of the labelfile to it's own function,
but that's just me ;)


-- 
 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] Error code for terminating connection due to conflict with recovery

2011-01-14 Thread Robert Haas
On Thu, Jan 13, 2011 at 7:31 PM, Tatsuo Ishii is...@postgresql.org wrote:
 Tatsuo Ishii is...@postgresql.org writes:
 Please add this to the currently open CommitFest:
 https://commitfest.postgresql.org/action/commitfest_view/open

 Done. Comments are welcome. Unless there's objection, I will commit it
 this weekend.

 If you're expecting anyone to actually *review* it during the CF,
 that's a bit premature.

 No problem to wait for longer. I will wait by the end of January for
 the present.

Review:

The only possible point of concern I see here is the naming of the C
identifier.  Everything else in class 40 uses ERRCODE_T_R_whatever,
with T_R standing for transaction rollback.  It's not obvious to me
that that convention has any real value, but perhaps we ought to
follow it here for the sake of consistency?

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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


Re: [HACKERS] Database file copy

2011-01-14 Thread Bruce Momjian
Alvaro Herrera wrote:
 Excerpts from Bruce Momjian's message of jue ene 13 00:05:53 -0300 2011:
  Srini Raghavan wrote:
   Thank you very much for reviewing, appreciate the feedback.? As pointed 
   out by 
   you, it is always best to test it out with the latest version, so, I 
   tested the 
   same approach with postgres 9.0.2 on windows just now, and it works! 
   
   
   I forgot to mention earlier that in addition to setting 
   vacuum_freeze_table_age 
   to 0, vacuum_freeze_min_age must also be set to 0 to reset xmin with the 
   FrozenXid. 
  
  I wonder if you should be using VACUUM FREEZE instead of having to set
  variables.  
 
 The documentation says you shouldn't:
 
 FREEZE
 Selects aggressive freezing of tuples. Specifying FREEZE is equivalent to
 performing VACUUM with the vacuum_freeze_min_age parameter set to zero. The
 FREEZE option is deprecated and will be removed in a future release; set the
 parameter instead.
   http://www.postgresql.org/docs/9.0/static/sql-vacuum.html

I didn't know that.  I added the -z(freeze) option to vacuumdb in 8.4
for use by pg_upgrade.

I think the original idea was that people should never need to freeze
anything, but it turns out pg_upgrade and this user need it so maybe
depricating is not a good idea.  I guess pg_upgrade could call vacuumdb
with a PGOPTIONS flag to force a vacuum_freeze_min_age value.

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

  + It's impossible for everything to be true. +

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


Re: [HACKERS] LOCK for non-tables

2011-01-14 Thread Robert Haas
On Tue, Jan 11, 2011 at 8:31 PM, Robert Haas robertmh...@gmail.com wrote:
 In that case, can I have some comments on approaches mentioned in my OP?

Tom - I am willing to implement this if you think it's valuable, but
I'd like your input on the syntax.

http://archives.postgresql.org/pgsql-hackers/2011-01/msg00472.php

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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


Re: [HACKERS] Database file copy

2011-01-14 Thread Robert Haas
On Fri, Jan 14, 2011 at 9:13 AM, Bruce Momjian br...@momjian.us wrote:
 Alvaro Herrera wrote:
 Excerpts from Bruce Momjian's message of jue ene 13 00:05:53 -0300 2011:
  Srini Raghavan wrote:
   Thank you very much for reviewing, appreciate the feedback.? As pointed 
   out by
   you, it is always best to test it out with the latest version, so, I 
   tested the
   same approach with postgres 9.0.2 on windows just now, and it works!
  
  
   I forgot to mention earlier that in addition to setting 
   vacuum_freeze_table_age
   to 0, vacuum_freeze_min_age must also be set to 0 to reset xmin with the
   FrozenXid.
 
  I wonder if you should be using VACUUM FREEZE instead of having to set
  variables.

 The documentation says you shouldn't:

 FREEZE
 Selects aggressive freezing of tuples. Specifying FREEZE is equivalent to
 performing VACUUM with the vacuum_freeze_min_age parameter set to zero. The
 FREEZE option is deprecated and will be removed in a future release; set the
 parameter instead.
       http://www.postgresql.org/docs/9.0/static/sql-vacuum.html

 I didn't know that.  I added the -z(freeze) option to vacuumdb in 8.4
 for use by pg_upgrade.

 I think the original idea was that people should never need to freeze
 anything, but it turns out pg_upgrade and this user need it so maybe
 depricating is not a good idea.  I guess pg_upgrade could call vacuumdb
 with a PGOPTIONS flag to force a vacuum_freeze_min_age value.

I'd rather remove the deprecating warning.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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


Re: [HACKERS] [PATCH] Return command tag 'REPLACE X' for CREATE OR REPLACE statements.

2011-01-14 Thread Alvaro Herrera
Excerpts from Robert Haas's message of vie ene 14 08:40:07 -0300 2011:

 Also, I don't really like the way this spreads knowledge of the
 completionTag out all over the backend.  I think it would be better to
 follow the existing model used by the COPY and COMMIT commands,
 whereby the return value indicates what happened and
 standard_ProcessUtility() uses that to set the command tag.

Yeah, that looks ugly.  However it's already ugly elsewhere: for example
see PerformPortalFetch.  I am not sure if it should be this patch's
responsability to clean that stuff up.  (Maybe we should decree that at
least this patch shouldn't make the situation worse.)

-- 
Álvaro Herrera alvhe...@commandprompt.com
The PostgreSQL Company - Command Prompt, Inc.
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] Database file copy

2011-01-14 Thread Alvaro Herrera
Excerpts from Robert Haas's message of vie ene 14 11:18:16 -0300 2011:

 I'd rather remove the deprecating warning.

+1

-- 
Álvaro Herrera alvhe...@commandprompt.com
The PostgreSQL Company - Command Prompt, Inc.
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] [PATCH] Return command tag 'REPLACE X' for CREATE OR REPLACE statements.

2011-01-14 Thread Robert Haas
On Fri, Jan 14, 2011 at 9:24 AM, Alvaro Herrera
alvhe...@commandprompt.com wrote:
 Excerpts from Robert Haas's message of vie ene 14 08:40:07 -0300 2011:

 Also, I don't really like the way this spreads knowledge of the
 completionTag out all over the backend.  I think it would be better to
 follow the existing model used by the COPY and COMMIT commands,
 whereby the return value indicates what happened and
 standard_ProcessUtility() uses that to set the command tag.

 Yeah, that looks ugly.  However it's already ugly elsewhere: for example
 see PerformPortalFetch.  I am not sure if it should be this patch's
 responsability to clean that stuff up.  (Maybe we should decree that at
 least this patch shouldn't make the situation worse.)

Agreed: it's not the patch's job to clean it up, but it shouldn't make
the situation worse.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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


Re: [HACKERS] Database file copy

2011-01-14 Thread Kevin Grittner
Alvaro Herrera alvhe...@commandprompt.com wrote:
 Excerpts from Robert Haas's message:
 
 I'd rather remove the deprecating warning.
 
 +1
 
+1
 
-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] Add support for logging the current role

2011-01-14 Thread Dimitri Fontaine
Tom Lane t...@sss.pgh.pa.us writes:
 Another little problem with the quick and dirty solution is that stuff
 that's important enough to warrant a log_line_prefix escape is generally
 thought to be important enough to warrant inclusion in CSV logs.  That
 would imply adding a column and taking the resultant compatibility hit.

Well if we're down to adding columns to the CSV format, what about
adding an explicit column where to output the query duration as an
interval literal, rather than putting it in the query string (IIRC)?

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

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


Re: [HACKERS] kill -KILL: What happens?

2011-01-14 Thread Alvaro Herrera
Excerpts from Robert Haas's message of vie ene 14 00:03:53 -0300 2011:
 On Thu, Jan 13, 2011 at 8:28 PM, Tom Lane t...@sss.pgh.pa.us wrote:

  True.  It strikes me also that the postmaster does provide some services
  other than accepting new connections:
 
  * ensuring that everybody gets killed if a backend crashes

  While you could probably live without these in the scenario of let my
  honking big query finish before restarting, you would not want to do
  without them in unattended operation.
 
 Yep.  I'm pretty doubtful that you're going to want them even in that
 case, but you're surely not going to want them in unattended
 operation.

I'm sure you don't want that.  The reason postmaster causes a restart of
all backends in case one of them crashes is that it could have left some
corrupted state behind.  If postmaster dies, and then another backend
crashes, then your backend running your honking big query could run
across corrupted state and then you'd be in serious trouble.

-- 
Álvaro Herrera alvhe...@commandprompt.com
The PostgreSQL Company - Command Prompt, Inc.
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] Error code for terminating connection due to conflict with recovery

2011-01-14 Thread Tatsuo Ishii
 Review:
 
 The only possible point of concern I see here is the naming of the C
 identifier.  Everything else in class 40 uses ERRCODE_T_R_whatever,
 with T_R standing for transaction rollback.  It's not obvious to me
 that that convention has any real value, but perhaps we ought to
 follow it here for the sake of consistency?

Yeah. Actually at first I used T_R convention. After a few seconds
thought, I realized that T_R is not appropreate by the same reason
you feel. Possible other argument might be Terminating connection
always involves transaction rollback. So using T_R is ok. I'm not
sure this argument is reasonable enough though.
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese: http://www.sraoss.co.jp

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


Re: [HACKERS] kill -KILL: What happens?

2011-01-14 Thread Kevin Grittner
Alvaro Herrera alvhe...@commandprompt.com wrote:
 
 If postmaster dies, and then another backend crashes, then your
 backend running your honking big query could run across
 corrupted state and then you'd be in serious trouble.
 
Worst of all, it could give bogus results without error.  I really
don't see a production use case for letting backends continue after
postmaster failure -- unless you only kinda, sorta care whether
committed data is actually retrievable or reported data is actually
accurate.
 
-Kevin

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


[HACKERS] Maintenance downtime for commitfest.postgresql.org and media.postgresql.org

2011-01-14 Thread Stefan Kaltenbrunner

Hi all!


In preperation of some further improvments to our infrastructure, the 
sysadmin team needs to move coridian.postgresql.org (aka 
commitfest.postgresql.org) and mintaka.postgresql.org 
(media.postgresql.org) to a different host within the same datacenter.


We are planning to do that move starting Sunday January 16th 12:00 to 
13:00 (GMT).


The expected downtime is ~30min per VM and we will send a all is good 
again after the work is done - it would be good to not make any chances 
to the commitfest interface until you see that notice.
In preparation for the move we also have reduced the TTLs of the 
affected records down to 5 minutes to make the DNS-migration go as fast 
as possible.




Stefan

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


Re: [HACKERS] kill -KILL: What happens?

2011-01-14 Thread Florian Pflug
On Jan14, 2011, at 17:22 , Kevin Grittner wrote:

 Alvaro Herrera alvhe...@commandprompt.com wrote:
 
 If postmaster dies, and then another backend crashes, then your
 backend running your honking big query could run across
 corrupted state and then you'd be in serious trouble.
 
 Worst of all, it could give bogus results without error.  I really
 don't see a production use case for letting backends continue after
 postmaster failure -- unless you only kinda, sorta care whether
 committed data is actually retrievable or reported data is actually
 accurate.

I gather that the behaviour we want is for normal backends to exit
once the postmaster is gone, and for utility processes (bgwriter, ...)
to exit once all the backends are gone.

The test program I posted in this thread proves that FIFOs and select()
can be used to implement this, if we're ready to check for EOF on the
socket in CHECK_FOR_INTERRUPTS() every few seconds. Is this a viable
route to take?

best regards,
Florian Pflug


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


Re: [HACKERS] Add support for logging the current role

2011-01-14 Thread Andrew Dunstan



On 01/12/2011 08:59 PM, Tom Lane wrote:


I'm not actually concerned about adding a few extra cycles to SET ROLE
for this.  What bothered me more was the cost of adding another output
column to CSV log mode.  That's not something you're going to be able to
finesse such that only people who care pay the cost.




I think it's time to revisit the design of CSV logs again, now we have 
two or three releases worth of experience with it. It needs some 
flexibility and refinement.


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] Add support for logging the current role

2011-01-14 Thread Stephen Frost
* Tom Lane (t...@sss.pgh.pa.us) wrote:
 I seem to recall that the assign hook for role stores the string form of
 the role name anyway.  

Indeed it does, and it's already exposed through show_role() since it's
needed in guc.c.  Based on my review and understanding of the comments
and calls, it also doesn't do anything particularly complicated or any
syscache searches or anything.

 It wouldn't track the
 effects of RENAME ROLE against an actively-used role, but then again
 neither does %u.

Right, I didn't specifically point that out in the documentation
changes, but I can if people feel it's neceessary.

 What bothered me more was the cost of adding another output
 column to CSV log mode.  That's not something you're going to be able to
 finesse such that only people who care pay the cost.

I definitely feel this is something that we should be logging in the CSV
also, and you're right, there doesn't appear to be a way to do that
without just outright changing the format and causing people to have to
update anything/everything that uses it.  I have a hard time with the
idea that we'll commit to never changing that format though, so do we
want to provide a way for users to specify the format (ala
log_line_prefix), or just ask users to expect and deal with format
changes when they happen..?

I noticed Dimitri would like another change to the CSV log format (which
looked reasonable to me, asking to have something split out from the
query string itself), it'd certainly be better to change both in the
same release than split them across two (of course, we might come up
with something else in the future...).

I have to admit to being a bit suprised the CSV logging wasn't
implemented with a 'format' type option.  I'm not sure if I have the
cycles or even if we would want to try and add that now, but it
strikes me as something we should probably do.

Updated patch attached, included new comments for elog.c too.

Thanks,

Stephen

commit 4e27ab79ef9b0d0c3c9824d672e06160dd227cc2
Author: Stephen Frost sfr...@snowman.net
Date:   Wed Jan 12 12:22:16 2011 -0500

Improve comments at the top of elog.c

Add in some comments about how certain usually available backend
systems may be unavailable or which won't function properly in
elog.c due to the current transaction being in a failed state.

commit d3ca4063ba8e16930278947c32c336b5b80cdaba
Author: Stephen Frost sfr...@snowman.net
Date:   Fri Jan 14 11:19:45 2011 -0500

Add %U option to log_line_prefix for current role

This adds a new option to log_line_prefix (%U) to allow the current
role to be logged, which is valuable information when an application
or user is using SET ROLE and roles which are set 'noinherit'.

This also changes the current definition of %u to be 'Session user',
to avoid confusion when a superuser uses 'SET SESSION AUTHORIZATION'.
Otherwise, a log might read 'login_user none' but actually be running
as a different user due to SET SESSION AUTHORIZATION.  The 'username'
field for CSV logging was also updated to be 'Session user'.  Note:
SET SESSION AUTHORIZATION is only allowed for superusers, and the
logged username will only change if SET SESSION AUTHORIZATION is
called, so this is unlikely to have signifigant user impact.

Last, but certainly not least, role_name was added as a new column to
the CSV log output and corresponding example CSV table definition.
This is a user-visible change which should be called out in the release
notes.
*** a/doc/src/sgml/config.sgml
--- b/doc/src/sgml/config.sgml
***
*** 3504,3510  local0.*/var/log/postgresql
  /row
  row
   entryliteral%u/literal/entry
!  entryUser name/entry
   entryyes/entry
  /row
  row
--- 3504,3523 
  /row
  row
   entryliteral%u/literal/entry
!  entrySession user name, typically the user name which was used
!  to authenticate to productnamePostgreSQL/productname with,
!  but can be changed by a superuser, see commandSET SESSION
!  AUTHORIZATION//entry
!  entryyes/entry
! /row
! row
!  entryliteral%U/literal/entry
!  entryCurrent role name, when set with commandSET ROLE/;
!  the current role identifier is relevant for permission checking;
!  Returns 'none' if the current role matches the session user.
!  Note: Log messages from inside literalSECURITY DEFINER/
!  functions will show the calling role, not the effective role
!  inside the literalSECURITY DEFINER/ function/entry
   entryyes/entry
  /row
  row
***
*** 3731,3737  FROM pg_stat_activity;
  (acronymCSV/) format,
  with these columns:
  

Re: [HACKERS] Add support for logging the current role

2011-01-14 Thread Tom Lane
Andrew Dunstan and...@dunslane.net writes:
 On 01/12/2011 08:59 PM, Tom Lane wrote:
 I'm not actually concerned about adding a few extra cycles to SET ROLE
 for this.  What bothered me more was the cost of adding another output
 column to CSV log mode.  That's not something you're going to be able to
 finesse such that only people who care pay the cost.

 I think it's time to revisit the design of CSV logs again, now we have 
 two or three releases worth of experience with it. It needs some 
 flexibility and refinement.

It would definitely be nice to support optional columns a little better.
I'm not even sure whether the runtime overhead is worth worrying about
(maybe it is, maybe it isn't, I have no data).  But I do know that
adding a column to the CSV output format spec causes a flag day for
users.  How can we avoid that?

regards, tom lane

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


Re: [HACKERS] kill -KILL: What happens?

2011-01-14 Thread Robert Haas
On Fri, Jan 14, 2011 at 11:28 AM, Florian Pflug f...@phlo.org wrote:
 I gather that the behaviour we want is for normal backends to exit
 once the postmaster is gone, and for utility processes (bgwriter, ...)
 to exit once all the backends are gone.

 The test program I posted in this thread proves that FIFOs and select()
 can be used to implement this, if we're ready to check for EOF on the
 socket in CHECK_FOR_INTERRUPTS() every few seconds. Is this a viable
 route to take?

I don't think there's much point in getting excited about the order in
which things exit.  If we're agreed (and we seem to be, modulo Tom)
that the backends should exit quickly if the postmaster dies, then
worrying about whether the utility processes exit slightly before or
slightly after that doesn't excite me very much.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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


Re: [HACKERS] Error code for terminating connection due to conflict with recovery

2011-01-14 Thread Robert Haas
On Fri, Jan 14, 2011 at 10:53 AM, Tatsuo Ishii is...@postgresql.org wrote:
 Review:

 The only possible point of concern I see here is the naming of the C
 identifier.  Everything else in class 40 uses ERRCODE_T_R_whatever,
 with T_R standing for transaction rollback.  It's not obvious to me
 that that convention has any real value, but perhaps we ought to
 follow it here for the sake of consistency?

 Yeah. Actually at first I used T_R convention. After a few seconds
 thought, I realized that T_R is not appropreate by the same reason
 you feel. Possible other argument might be Terminating connection
 always involves transaction rollback. So using T_R is ok. I'm not
 sure this argument is reasonable enough though.

Looking at this a bit more carefully, I notice that there are two
cases when a recovery conflict occurs:

- we cancel the currently running statement, or
- we kill the whole connection

Should those use the same error code, or different ones?  This patch
doesn't appear to update all the places where recovery conflicts can
occur, which is probably not ideal.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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


Re: [HACKERS] Add support for logging the current role

2011-01-14 Thread Stephen Frost
* Tom Lane (t...@sss.pgh.pa.us) wrote:
 Andrew Dunstan and...@dunslane.net writes:
  I think it's time to revisit the design of CSV logs again, now we have 
  two or three releases worth of experience with it. It needs some 
  flexibility and refinement.
 
 It would definitely be nice to support optional columns a little better.
 I'm not even sure whether the runtime overhead is worth worrying about
 (maybe it is, maybe it isn't, I have no data).  But I do know that
 adding a column to the CSV output format spec causes a flag day for
 users.  How can we avoid that?

My first thought would be to have a 'log_csv_format' GUC that's very
similar to 'log_line_prefix' (and uses the same variables if
possible..).  We could then ship a default in postgresql.conf that
matches what the current format is while adding the other options if
people want to use them.

If we could have all the processing to generate that line go through the
same function for log_line_prefix and log_csv_format, that'd be even
better.  Makes me tempted to throw out the current notion of
'log_line_*prefix*' and replace it with 'log_line_*format*' to match
exactly the 'log_csv_format' that I'm proposing.  That'd undoubtably
cause more user headaches tho... :(

Thanks,

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] Error code for terminating connection due to conflict with recovery

2011-01-14 Thread Tom Lane
Tatsuo Ishii is...@postgresql.org writes:
 Review:
 The only possible point of concern I see here is the naming of the C
 identifier.  Everything else in class 40 uses ERRCODE_T_R_whatever,
 with T_R standing for transaction rollback.  It's not obvious to me
 that that convention has any real value, but perhaps we ought to
 follow it here for the sake of consistency?

 Yeah. Actually at first I used T_R convention. After a few seconds
 thought, I realized that T_R is not appropreate by the same reason
 you feel. Possible other argument might be Terminating connection
 always involves transaction rollback. So using T_R is ok. I'm not
 sure this argument is reasonable enough though.

This is not only a matter of some macro name or other.  According to the
SQL standard, class 40 itself is defined as transaction rollback.
If the error condition can't reasonably be regarded as a subcase of
that, you're making a bad choice of SQLSTATE code.

regards, tom lane

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


Re: [HACKERS] [PATCH] Return command tag 'REPLACE X' for CREATE OR REPLACE statements.

2011-01-14 Thread Tom Lane
Alvaro Herrera alvhe...@commandprompt.com writes:
 Excerpts from Robert Haas's message of vie ene 14 08:40:07 -0300 2011:
 Also, I don't really like the way this spreads knowledge of the
 completionTag out all over the backend.  I think it would be better to
 follow the existing model used by the COPY and COMMIT commands,
 whereby the return value indicates what happened and
 standard_ProcessUtility() uses that to set the command tag.

 Yeah, that looks ugly.  However it's already ugly elsewhere: for example
 see PerformPortalFetch.  I am not sure if it should be this patch's
 responsability to clean that stuff up.  (Maybe we should decree that at
 least this patch shouldn't make the situation worse.)

I thought we were going to reject the patch outright anyway.  The
compatibility consequences of changing command tags are not worth the
benefit, independently of how ugly the backend-side code may or may
not be.

regards, tom lane

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


Re: [HACKERS] [PATCH] Return command tag 'REPLACE X' for CREATE OR REPLACE statements.

2011-01-14 Thread Simon Riggs
On Fri, 2011-01-14 at 12:07 -0500, Tom Lane wrote:

 I thought we were going to reject the patch outright anyway.  The
 compatibility consequences of changing command tags are not worth the
 benefit, independently of how ugly the backend-side code may or may
 not be.

+1

-- 
 Simon Riggs   http://www.2ndQuadrant.com/books/
 PostgreSQL Development, 24x7 Support, Training and Services
 


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


Re: [HACKERS] Database file copy

2011-01-14 Thread Tom Lane
Kevin Grittner kevin.gritt...@wicourts.gov writes:
 Alvaro Herrera alvhe...@commandprompt.com wrote:
 I'd rather remove the deprecating warning.

 +1
 
 +1

The reason for wanting to deprecate and ultimately remove that syntax is
so we can get rid of FREEZE as a reserved word.

We could probably still allow the new-style syntax VACUUM (FREEZE) ...
but VACUUM FREEZE really needs to be killed.  pg_upgrade is NOT a
good reason to have a nonstandard reserved word in the grammar.

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: Exit from base backups when shutdown is requested

2011-01-14 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 On Fri, Jan 14, 2011 at 7:25 AM, Simon Riggs si...@2ndquadrant.com wrote:
 My request is not unreasonable. Please be helpful, as you would expect
 me to be if the roles were reversed.

 I can't remember anyone other than you ever requesting that.

Yes.  As Heikki said, it doesn't work that way.

regards, tom lane

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


[HACKERS] Add ENCODING option to COPY

2011-01-14 Thread Hitoshi Harada
Here's the patch to add ENCODING option to COPY command.
The fundamental issue was explained months ago:

http://archives.postgresql.org/message-id/AANLkTikCt6bHXZjO_oX+JS7+G=jaq7gvzpu0owjcs...@mail.gmail.com

In short, client_encoding is not appropriate for copy operation so we
should need the specialized option for it. Robert Haas agreed with its
need later in the thread. Thanks.

The patch overrides client_encoding by the added ENCODING option, and
restores it as soon as copy is done. I see some complaints ask to use
pg_do_encoding_conversion() instead of
pg_client_to_server/server_to_client(), but the former will surely add
slight overhead per reading line and I believe copy is
performance-sensitive command.

I'll add this to the CF app.

Regards,

-- 
Hitoshi Harada


copy_encoding.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] Error code for terminating connection due to conflict with recovery

2011-01-14 Thread Simon Riggs
On Fri, 2011-01-14 at 12:04 -0500, Tom Lane wrote:
 Tatsuo Ishii is...@postgresql.org writes:
  Review:
  The only possible point of concern I see here is the naming of the C
  identifier.  Everything else in class 40 uses ERRCODE_T_R_whatever,
  with T_R standing for transaction rollback.  It's not obvious to me
  that that convention has any real value, but perhaps we ought to
  follow it here for the sake of consistency?
 
  Yeah. Actually at first I used T_R convention. After a few seconds
  thought, I realized that T_R is not appropreate by the same reason
  you feel. Possible other argument might be Terminating connection
  always involves transaction rollback. So using T_R is ok. I'm not
  sure this argument is reasonable enough though.
 
 This is not only a matter of some macro name or other.  According to the
 SQL standard, class 40 itself is defined as transaction rollback.
 If the error condition can't reasonably be regarded as a subcase of
 that, you're making a bad choice of SQLSTATE code.

This whole thing is confused. No change is appropriate here at all.

We issue ERRCODE_T_R_SERIALIZATION_FAILURE almost all of the time for
recovery conflicts.

We issue ERRCODE_ADMIN_SHUTDOWN only if the conflict is non-retryable,
which occurs if someone drops the database that the user was connected
to when they get kicked off. That code exists specifically to inform the
user that they *cannot* reconnect. So pgpool should not be trying to
trap that error and reconnect.

So the whole basis for the existence of this patch is moot.

-- 
 Simon Riggs   http://www.2ndQuadrant.com/books/
 PostgreSQL Development, 24x7 Support, Training and Services
 


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


Re: [HACKERS] Database file copy

2011-01-14 Thread Kevin Grittner
Tom Lane t...@sss.pgh.pa.us wrote:
 
 The reason for wanting to deprecate and ultimately remove that
 syntax is so we can get rid of FREEZE as a reserved word.
 
 We could probably still allow the new-style syntax VACUUM (FREEZE)
 
Oh, OK.  I can go along with that.  If we're going that route,
though, shouldn't we be getting support for the new syntax added, so
there can be a release or two supporting both?
 
-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] limiting hint bit I/O

2011-01-14 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 On Thu, Jan 13, 2011 at 10:43 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 This appears to remove the BM_JUST_DIRTIED logic.  Please explain why
 that's not completely broken.  Even if it isn't completely broken,
 it would seem better to do something like that as a separate patch.

 Well, the only point of BM_JUST_DIRTIED is to detect whether BM_DIRTY
 has been set while a buffer write is in progress.  With this patch,
 only BM_HINT_BITS can be set while the buffer write is in progress;
 BM_DIRTY cannot.  Perhaps one could make the argument that this would
 be a good cleanup anyway: in the unpatched code, BM_DIRTY can only be
 set while a buffer I/O is in progress if it is set due to a hint-bit
 update, and then we don't really care if the update gets lost.
 Although that seems a bit confusing...

[ thinks some more... ]  If memory serves, the BM_JUST_DIRTIED mechanism
dates from a time when checkpoints would write dirty buffers without
taking any lock on them; if somebody changed the page meanwhile, the
buffer was just considered to remain dirty.  We later decided that was
a bad idea and set up the current arrangement whereby only hint-bit
changes are allowed while a write is in progress.  So you're right that
it would be dead code if we don't consider that a hint-bit change is
really dirtying the page.  I'm not for removing it altogether though,
because it seems like something we could possibly want again in the
future (for instance, we might decide to go back to write-without-lock
to reduce lock contention).  It's not like we are short of buffer flag
bits.  Moreover this whole business of not treating hint-bit setting as
a page-dirtying operation is completely experimental/unproven IMO, so it
would be better to keep the patch footprint as small as possible.  I'd
suggest leaving BM_JUST_DIRTIED as-is and just adding BM_HINT_BITS_DIRTY
as a new flag.

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] limiting hint bit I/O

2011-01-14 Thread Robert Haas
On Fri, Jan 14, 2011 at 12:47 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Robert Haas robertmh...@gmail.com writes:
 On Thu, Jan 13, 2011 at 10:43 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 This appears to remove the BM_JUST_DIRTIED logic.  Please explain why
 that's not completely broken.  Even if it isn't completely broken,
 it would seem better to do something like that as a separate patch.

 Well, the only point of BM_JUST_DIRTIED is to detect whether BM_DIRTY
 has been set while a buffer write is in progress.  With this patch,
 only BM_HINT_BITS can be set while the buffer write is in progress;
 BM_DIRTY cannot.  Perhaps one could make the argument that this would
 be a good cleanup anyway: in the unpatched code, BM_DIRTY can only be
 set while a buffer I/O is in progress if it is set due to a hint-bit
 update, and then we don't really care if the update gets lost.
 Although that seems a bit confusing...

 [ thinks some more... ]  If memory serves, the BM_JUST_DIRTIED mechanism
 dates from a time when checkpoints would write dirty buffers without
 taking any lock on them; if somebody changed the page meanwhile, the
 buffer was just considered to remain dirty.  We later decided that was
 a bad idea and set up the current arrangement whereby only hint-bit
 changes are allowed while a write is in progress.  So you're right that
 it would be dead code if we don't consider that a hint-bit change is
 really dirtying the page.  I'm not for removing it altogether though,
 because it seems like something we could possibly want again in the
 future (for instance, we might decide to go back to write-without-lock
 to reduce lock contention).  It's not like we are short of buffer flag
 bits.  Moreover this whole business of not treating hint-bit setting as
 a page-dirtying operation is completely experimental/unproven IMO, so it
 would be better to keep the patch footprint as small as possible.  I'd
 suggest leaving BM_JUST_DIRTIED as-is and just adding BM_HINT_BITS_DIRTY
 as a new flag.

I have some concerns about that proposal, but it might be the right
way to go.  Before we get too far off into the weeds, though, let's
back up and talk about something more fundamental: this seems to be
speeding up the first run by 6x at the expense of slowing down many
subsequent runs by 10-15%.  Does that make this whole idea dead on
arrival?

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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


Re: [HACKERS] Allowing multiple concurrent base backups

2011-01-14 Thread Simon Riggs
On Thu, 2011-01-13 at 23:32 +0200, Heikki Linnakangas wrote:
 On 13.01.2011 22:57, Josh Berkus wrote:
  On 1/13/11 12:11 PM, Robert Haas wrote:
  That's going to depend on the situation.  If the database fits in
  memory, then it's just going to work.  If it fits on disk, it's less
  obvious whether it'll be good or bad, but an arbitrary limitation here
  doesn't serve us well.
 
  FWIW, if we had this feature right now in 9.0 we (PGX) would be using
  it.  We run into the case of DB in memory, multiple slaves fairly often
  these days.
 
 Anyway, here's an updated patch with all the known issues fixed.

It's good we have this as an option and I like the way you've done this.

-- 
 Simon Riggs   http://www.2ndQuadrant.com/books/
 PostgreSQL Development, 24x7 Support, Training and Services
 


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


Re: [HACKERS] FOR KEY LOCK foreign keys

2011-01-14 Thread David E. Wheeler
On Jan 13, 2011, at 1:58 PM, Alvaro Herrera wrote:

 Something else that might be of interest: the patch as presented here
 does NOT solve the deadlock problem originally presented by Joel
 Jacobson.  It does solve the second, simpler example I presented in my
 blog article referenced above, however.  I need to have a closer look at
 that problem to figure out if we could fix the deadlock too.

Sounds like a big win already. Should this be considered a WIP patch, though, 
if you still plan to look at Joel's deadlock example?

Best,

David


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


Re: [HACKERS] limiting hint bit I/O

2011-01-14 Thread Kevin Grittner
Robert Haas robertmh...@gmail.com wrote:
 
 this seems to be speeding up the first run by 6x at the expense of
 slowing down many subsequent runs by 10-15%.
 
If the overall throughput when measured far enough out to have hit a
steady state again is anywhere in the neighborhood of the unpatched
throughput, the leveling of the response times has enough value to
merit the change.  At least in my world.
 
-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] Database file copy

2011-01-14 Thread Tom Lane
Kevin Grittner kevin.gritt...@wicourts.gov writes:
 Tom Lane t...@sss.pgh.pa.us wrote:
 The reason for wanting to deprecate and ultimately remove that
 syntax is so we can get rid of FREEZE as a reserved word.

 Oh, OK.  I can go along with that.  If we're going that route,
 though, shouldn't we be getting support for the new syntax added, so
 there can be a release or two supporting both?

You mean like 9.0?

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] limiting hint bit I/O

2011-01-14 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 On Fri, Jan 14, 2011 at 12:47 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Moreover this whole business of not treating hint-bit setting as
 a page-dirtying operation is completely experimental/unproven IMO, so it
 would be better to keep the patch footprint as small as possible.

 I have some concerns about that proposal, but it might be the right
 way to go.  Before we get too far off into the weeds, though, let's
 back up and talk about something more fundamental: this seems to be
 speeding up the first run by 6x at the expense of slowing down many
 subsequent runs by 10-15%.  Does that make this whole idea dead on
 arrival?

Well, it reinforces my opinion that it's experimental ;-).  But first
run of what, exactly?  And are you sure you're taking a wholistic view
of the costs/benefits?

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] limiting hint bit I/O

2011-01-14 Thread Robert Haas
On Fri, Jan 14, 2011 at 1:02 PM, Kevin Grittner
kevin.gritt...@wicourts.gov wrote:
 Robert Haas robertmh...@gmail.com wrote:
 this seems to be speeding up the first run by 6x at the expense of
 slowing down many subsequent runs by 10-15%.

 If the overall throughput when measured far enough out to have hit a
 steady state again is anywhere in the neighborhood of the unpatched
 throughput, the leveling of the response times has enough value to
 merit the change.  At least in my world.

I think it would eventually settle down to the same speed, but it
might take a really long time.  I got impatient before I got that far.
 I'm hoping some will pick it up and play with it some more (hint,
hint).

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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


Re: [HACKERS] limiting hint bit I/O

2011-01-14 Thread Robert Haas
On Fri, Jan 14, 2011 at 1:06 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Robert Haas robertmh...@gmail.com writes:
 On Fri, Jan 14, 2011 at 12:47 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Moreover this whole business of not treating hint-bit setting as
 a page-dirtying operation is completely experimental/unproven IMO, so it
 would be better to keep the patch footprint as small as possible.

 I have some concerns about that proposal, but it might be the right
 way to go.  Before we get too far off into the weeds, though, let's
 back up and talk about something more fundamental: this seems to be
 speeding up the first run by 6x at the expense of slowing down many
 subsequent runs by 10-15%.  Does that make this whole idea dead on
 arrival?

 Well, it reinforces my opinion that it's experimental ;-).  But first
 run of what, exactly?

See the test case in my OP.  The runs in question are select sum(1) from s.

 And are you sure you're taking a wholistic view
 of the costs/benefits?

No.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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


Re: [HACKERS] FOR KEY LOCK foreign keys

2011-01-14 Thread Robert Haas
On Fri, Jan 14, 2011 at 1:00 PM, David E. Wheeler da...@kineticode.com wrote:
 On Jan 13, 2011, at 1:58 PM, Alvaro Herrera wrote:

 Something else that might be of interest: the patch as presented here
 does NOT solve the deadlock problem originally presented by Joel
 Jacobson.  It does solve the second, simpler example I presented in my
 blog article referenced above, however.  I need to have a closer look at
 that problem to figure out if we could fix the deadlock too.

 Sounds like a big win already. Should this be considered a WIP patch, though, 
 if you still plan to look at Joel's deadlock example?

Alvaro, are you planning to add this to the CF?

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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


Re: [HACKERS] Re: [COMMITTERS] pgsql: Exit from base backups when shutdown is requested

2011-01-14 Thread Simon Riggs
On Fri, 2011-01-14 at 07:30 -0500, Robert Haas wrote:
 On Fri, Jan 14, 2011 at 7:25 AM, Simon Riggs si...@2ndquadrant.com wrote:
  My request is not unreasonable. Please be helpful, as you would expect
  me to be if the roles were reversed.
 
 I can't remember anyone other than you ever requesting that.

Meaning what? You're playing compliment to my originality? Thanks ;-)

-- 
 Simon Riggs   http://www.2ndQuadrant.com/books/
 PostgreSQL Development, 24x7 Support, Training and Services
 


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


Re: [HACKERS] Database file copy

2011-01-14 Thread Kevin Grittner
Tom Lane t...@sss.pgh.pa.us wrote: 
 Kevin Grittner kevin.gritt...@wicourts.gov writes:
 
 shouldn't we be getting support for the new syntax added, so
 there can be a release or two supporting both?
 
 You mean like 9.0?
 
Yeah, just like that.
 
If we're going to be supporting that long term, we should probably
change the note about FREEZE being deprecated, though.
 
So, still +1 on removing the wording about FREEZE being deprecated,
but instead we should mention what actually *is* deprecated (the
omission of the parentheses).
 
-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] [PATCH] Return command tag 'REPLACE X' for CREATE OR REPLACE statements.

2011-01-14 Thread Robert Haas
On Fri, Jan 14, 2011 at 12:07 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Alvaro Herrera alvhe...@commandprompt.com writes:
 Excerpts from Robert Haas's message of vie ene 14 08:40:07 -0300 2011:
 Also, I don't really like the way this spreads knowledge of the
 completionTag out all over the backend.  I think it would be better to
 follow the existing model used by the COPY and COMMIT commands,
 whereby the return value indicates what happened and
 standard_ProcessUtility() uses that to set the command tag.

 Yeah, that looks ugly.  However it's already ugly elsewhere: for example
 see PerformPortalFetch.  I am not sure if it should be this patch's
 responsability to clean that stuff up.  (Maybe we should decree that at
 least this patch shouldn't make the situation worse.)

 I thought we were going to reject the patch outright anyway.  The
 compatibility consequences of changing command tags are not worth the
 benefit, independently of how ugly the backend-side code may or may
 not be.

My previous response to this criticism was here:

http://archives.postgresql.org/pgsql-hackers/2010-11/msg01899.php

Your response, which seemed at least partially in agreement, is here:

http://archives.postgresql.org/pgsql-hackers/2010-11/msg01901.php

If we're going to reject this patch on backwards-compatibility
grounds, we need to make an argument that the backward-compatibility
hazards are a real concern.  So, again, has anyone complained about
the changes we made in this area in 9.0?  And under what circumstances
do we foresee someone relying on the command tag of a command that
always returns the same tag?  I'm as quick as anyone to bow before a
compelling argument, but I don't think anyone's made such an argument.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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


Re: [HACKERS] FOR KEY LOCK foreign keys

2011-01-14 Thread Alvaro Herrera
Excerpts from David E. Wheeler's message of vie ene 14 15:00:48 -0300 2011:
 On Jan 13, 2011, at 1:58 PM, Alvaro Herrera wrote:
 
  Something else that might be of interest: the patch as presented here
  does NOT solve the deadlock problem originally presented by Joel
  Jacobson.  It does solve the second, simpler example I presented in my
  blog article referenced above, however.  I need to have a closer look at
  that problem to figure out if we could fix the deadlock too.
 
 Sounds like a big win already. Should this be considered a WIP patch, though, 
 if you still plan to look at Joel's deadlock example?

Not necessarily -- we can implement that as a later refinement/improvement.

-- 
Álvaro Herrera alvhe...@commandprompt.com
The PostgreSQL Company - Command Prompt, Inc.
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] FOR KEY LOCK foreign keys

2011-01-14 Thread Alvaro Herrera
Excerpts from Robert Haas's message of vie ene 14 15:08:27 -0300 2011:
 On Fri, Jan 14, 2011 at 1:00 PM, David E. Wheeler da...@kineticode.com 
 wrote:
  On Jan 13, 2011, at 1:58 PM, Alvaro Herrera wrote:
 
  Something else that might be of interest: the patch as presented here
  does NOT solve the deadlock problem originally presented by Joel
  Jacobson.  It does solve the second, simpler example I presented in my
  blog article referenced above, however.  I need to have a closer look at
  that problem to figure out if we could fix the deadlock too.
 
  Sounds like a big win already. Should this be considered a WIP patch, 
  though, if you still plan to look at Joel's deadlock example?
 
 Alvaro, are you planning to add this to the CF?

Eh, yes.

-- 
Álvaro Herrera alvhe...@commandprompt.com
The PostgreSQL Company - Command Prompt, Inc.
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] limiting hint bit I/O

2011-01-14 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 On Fri, Jan 14, 2011 at 1:06 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Well, it reinforces my opinion that it's experimental ;-).  But first
 run of what, exactly?

 See the test case in my OP.  The runs in question are select sum(1) from 
 s.

 And are you sure you're taking a wholistic view
 of the costs/benefits?

 No.

Well, IMO it would be a catastrophic mistake to evaluate a patch like
this on the basis of any single test case, let alone one as simplistic
as that.  I would observe in particular that your test case creates a
table containing only one distinct value of xmin, which means that the
single-transaction cache in transam.c is 100% effective, which doesn't
seem to me to be a very realistic test condition.  I think this is
vastly understating the cost of missing hint bits.

So what it needs now is a lot more testing.  pg_bench might be worth
trying if you want something with minimal development effort, though
I'm not sure if its clog access pattern is particularly realistic
either.

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] limiting hint bit I/O

2011-01-14 Thread Kevin Grittner
Robert Haas robertmh...@gmail.com wrote:
 
 I'm hoping some will pick it up and play with it some more (hint,
 hint).
 
That was a bit of a pun, eh?
 
Anyway, there are so many ideas in this area, it's hard to keep them
all straight.  Personally, if I was going to start with something,
it would probably be to better establish what the impact is on
various workloads of *eliminating* hint bits.  If the impact is
negative to a significant degree, my next step might be to try
background *freezing* of tuples (in a manner somewhat similar to
what you've done in this test) with the hint bits gone.
 
I know some people find them useful for forensics to a degree that
they would prefer not to see this, but I think it makes sense to
establish what cost people are paying every day to maintain forensic
information in this format.  In previous discussions there has been
some talk about being able to get better forensics from WAL files if
certain barriers could be overcome -- having hard numbers on the
performance benefits which might also accrue might put that work in
a different perspective.
 
-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] [PATCH] Return command tag 'REPLACE X' for CREATE OR REPLACE statements.

2011-01-14 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 If we're going to reject this patch on backwards-compatibility
 grounds, we need to make an argument that the backward-compatibility
 hazards are a real concern.  So, again, has anyone complained about
 the changes we made in this area in 9.0?

That 9.0 change was far less invasive than this: it only added a count
field to SELECT and CTAS result tags.  Quite aside from the fact that
the tag name stayed the same, in the SELECT case it's unlikely anyone
would have checked the tag at all rather than just testing for
PQresultStatus() == PGRES_TUPLES_OK.  So it was basically only changing
the result for *one* command type.  I don't think it's a good basis for
arguing that this patch won't cause problems.

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] limiting hint bit I/O

2011-01-14 Thread Robert Haas
On Fri, Jan 14, 2011 at 1:34 PM, Kevin Grittner
kevin.gritt...@wicourts.gov wrote:
 Robert Haas robertmh...@gmail.com wrote:

 I'm hoping some will pick it up and play with it some more (hint,
 hint).

 That was a bit of a pun, eh?

Unintentional...

 Anyway, there are so many ideas in this area, it's hard to keep them
 all straight.  Personally, if I was going to start with something,
 it would probably be to better establish what the impact is on
 various workloads of *eliminating* hint bits.  If the impact is
 negative to a significant degree, my next step might be to try
 background *freezing* of tuples (in a manner somewhat similar to
 what you've done in this test) with the hint bits gone.

Background freezing plays havoc with Hot Standby, and this test is
sufficient to show that eliminating hint bits altogether would a
significant regression on some workloads.  I don't think either of
those ideas can get off the ground.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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


Re: [HACKERS] limiting hint bit I/O

2011-01-14 Thread Tom Lane
Kevin Grittner kevin.gritt...@wicourts.gov writes:
 Anyway, there are so many ideas in this area, it's hard to keep them
 all straight.  Personally, if I was going to start with something,
 it would probably be to better establish what the impact is on
 various workloads of *eliminating* hint bits.
 
 I know some people find them useful for forensics to a degree that
 they would prefer not to see this,

Um, yeah, I think you're having a problem keeping all the ideas straight
;-).  The argument about forensics has to do with how soon we're willing
to freeze tuples, ie replace the XID with a constant.  Not about hint
bits.

regards, tom lane

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


Re: [HACKERS] [PATCH] Return command tag 'REPLACE X' for CREATE OR REPLACE statements.

2011-01-14 Thread Robert Haas
On Fri, Jan 14, 2011 at 1:35 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Robert Haas robertmh...@gmail.com writes:
 If we're going to reject this patch on backwards-compatibility
 grounds, we need to make an argument that the backward-compatibility
 hazards are a real concern.  So, again, has anyone complained about
 the changes we made in this area in 9.0?

 That 9.0 change was far less invasive than this: it only added a count
 field to SELECT and CTAS result tags.  Quite aside from the fact that
 the tag name stayed the same, in the SELECT case it's unlikely anyone
 would have checked the tag at all rather than just testing for
 PQresultStatus() == PGRES_TUPLES_OK.  So it was basically only changing
 the result for *one* command type.  I don't think it's a good basis for
 arguing that this patch won't cause problems.

Yeah, but that one command tag was SELECT.  That's a pretty commonly
used command.  Most production environments probably use all of the
commands affected by this patch together an order of magnitude less
often than they use SELECT.

Again, on what basis are we arguing that people are going to be
looking at the command tag of a command that always returns the same
tag?  That seems pretty darn unlikely to me.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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


Re: [HACKERS] Error code for terminating connection due to conflict with recovery

2011-01-14 Thread Robert Haas
On Fri, Jan 14, 2011 at 12:29 PM, Simon Riggs si...@2ndquadrant.com wrote:
 This whole thing is confused. No change is appropriate here at all.

 We issue ERRCODE_T_R_SERIALIZATION_FAILURE almost all of the time for
 recovery conflicts.

 We issue ERRCODE_ADMIN_SHUTDOWN only if the conflict is non-retryable,
 which occurs if someone drops the database that the user was connected
 to when they get kicked off. That code exists specifically to inform the
 user that they *cannot* reconnect. So pgpool should not be trying to
 trap that error and reconnect.

CheckRecoveryConflictDeadlock() uses ERRCODE_QUERY_CANCELLED.
ProcessInterrupts() sometimes uses ERRCODE_T_R_SERIALIZATION_FAILURE
and sometimes uses ERRCODE_ADMIN_SHUTDOWN.  It seems to me that it
wouldn't be a bad thing to be a bit more consistent, and perhaps to
have dedicated error codes for recovery conflicts.  This bit strikes
me as particularly strange:

else if (RecoveryConflictPending  RecoveryConflictRetryable)
{
pgstat_report_recovery_conflict(RecoveryConflictReason);
ereport(FATAL,

(errcode(ERRCODE_T_R_SERIALIZATION_FAILURE),
  errmsg(terminating connection due to
conflict with recovery),
 errdetail_recovery_conflict()));
}
else if (RecoveryConflictPending)
{
pgstat_report_recovery_conflict(RecoveryConflictReason);
ereport(FATAL,
(errcode(ERRCODE_ADMIN_SHUTDOWN),
  errmsg(terminating connection due to
conflict with recovery),
 errdetail_recovery_conflict()));
}

That's the same error message at the same severity level with two
different SQLSTATEs depending on RecoveryConflictRetryable.  Seems a
bit cryptic.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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


Re: [HACKERS] limiting hint bit I/O

2011-01-14 Thread Robert Haas
On Fri, Jan 14, 2011 at 1:42 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Kevin Grittner kevin.gritt...@wicourts.gov writes:
 Anyway, there are so many ideas in this area, it's hard to keep them
 all straight.  Personally, if I was going to start with something,
 it would probably be to better establish what the impact is on
 various workloads of *eliminating* hint bits.

 I know some people find them useful for forensics to a degree that
 they would prefer not to see this,

 Um, yeah, I think you're having a problem keeping all the ideas straight
 ;-).  The argument about forensics has to do with how soon we're willing
 to freeze tuples, ie replace the XID with a constant.  Not about hint
 bits.

Those things are related, though.  Freezing sooner could be viewed as
an alternative to hint bits.  Trouble is, it breaks Hot Standby,
badly.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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


Re: [HACKERS] limiting hint bit I/O

2011-01-14 Thread Kevin Grittner
Robert Haas robertmh...@gmail.com wrote:
 
 Background freezing plays havoc with Hot Standby
 
I must have missed or forgotten the issue of background vacuums
and hot standby.  Can you summarize why that's worse than hitting
thresholds where autovacuum is freezing things?
 
 this test is sufficient to show that eliminating hint bits
 altogether would a significant regression on some workloads.
 
That wasn't clear to me from what you posted -- I thought that the
reduced performance might be partly (largely? mostly?) due to
competition with the background writer's work pushing the hinted
pages out.  Maybe I'm missing something or you didn't post
everything you observed in this regard
 
-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] LOCK for non-tables

2011-01-14 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 Tom - I am willing to implement this if you think it's valuable, but
 I'd like your input on the syntax.
 http://archives.postgresql.org/pgsql-hackers/2011-01/msg00472.php

It looks to me like the reason why there's a shift/reduce conflict is
not so much that TABLE is optional as that we allow the syntax

LOCK tablename NOWAIT

If that weren't possible, then a table name would have to be followed by
EOL or IN (which is full-reserved), while an optional object type name
could not be followed by either, so there would be no shift/reduce
conflict.  So we broke it when we added NOWAIT, not when TABLE was made
optional.

So it looks to me like there are at least two fixes other than the ones
you enumerated:

1. Make NOWAIT a reserved word.  Not good, but perhaps better than
reserving all the different object type names.

2. Disallow the above abbreviated syntax; allow NOWAIT only after an
explicit IN ... MODE phrase.  This would probably break a couple of
applications, but I bet a lot fewer than changing the longer-established
parts of the command syntax would break.

I think #2 might be the best choice here.

regards, tom lane

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


Re: [HACKERS] limiting hint bit I/O

2011-01-14 Thread Kevin Grittner
Robert Haas robertmh...@gmail.com wrote:
 
 Freezing sooner could be viewed as an alternative to hint bits.
 
Exactly.  And as your test showed, things run faster frozen than
unfrozen with hint bits set.
 
 Trouble is, it breaks Hot Standby, badly.
 
You're really starting to worry me here.  Both for performance and
to reduce the WAN bandwidth demands of our backup strategy we are
very aggressive with our freezing.  Do off-hours VACUUM (FREEZE)
runs break hot standby?  Autovacuum freezing?  What are the
symptoms?
 
-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] LOCK for non-tables

2011-01-14 Thread Simon Riggs
On Fri, 2011-01-14 at 13:58 -0500, Tom Lane wrote:
 Robert Haas robertmh...@gmail.com writes:
  Tom - I am willing to implement this if you think it's valuable, but
  I'd like your input on the syntax.
  http://archives.postgresql.org/pgsql-hackers/2011-01/msg00472.php
 
 It looks to me like the reason why there's a shift/reduce conflict is
 not so much that TABLE is optional as that we allow the syntax
 
   LOCK tablename NOWAIT
 
 If that weren't possible, then a table name would have to be followed by
 EOL or IN (which is full-reserved), while an optional object type name
 could not be followed by either, so there would be no shift/reduce
 conflict.  So we broke it when we added NOWAIT, not when TABLE was made
 optional.
 
 So it looks to me like there are at least two fixes other than the ones
 you enumerated:
 
 1. Make NOWAIT a reserved word.  Not good, but perhaps better than
 reserving all the different object type names.
 
 2. Disallow the above abbreviated syntax; allow NOWAIT only after an
 explicit IN ... MODE phrase.  This would probably break a couple of
 applications, but I bet a lot fewer than changing the longer-established
 parts of the command syntax would break.
 
 I think #2 might be the best choice here.

There's a similar issue on my new syntax for skipping the validation
check on FKs. I'd appreciate it if you could propose a solution there
also. I'm not sure whether I solved it, or am adding issues for the
future.

-- 
 Simon Riggs   http://www.2ndQuadrant.com/books/
 PostgreSQL Development, 24x7 Support, Training and Services
 


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


Re: [HACKERS] [PATCH] Return command tag 'REPLACE X' for CREATE OR REPLACE statements.

2011-01-14 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 On Fri, Jan 14, 2011 at 1:35 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 That 9.0 change was far less invasive than this: it only added a count
 field to SELECT and CTAS result tags.  Quite aside from the fact that
 the tag name stayed the same, in the SELECT case it's unlikely anyone
 would have checked the tag at all rather than just testing for
 PQresultStatus() == PGRES_TUPLES_OK.

 Yeah, but that one command tag was SELECT.  That's a pretty commonly
 used command.

You're ignoring the point that people would probably use PQresultStatus
in preference to checking the tag at all, when dealing with SELECT.
psql itself is an example --- it never looks at the tag, nor shows it to
the user, in the SELECT case.  That patch only really changed the
exposed behavior for CREATE TABLE AS SELECT / SELECT INTO, neither of
which can be claimed to be hugely popular things for programs to issue.

The other side of the argument that needs to be considered is what the
benefit is.  There was a fairly clear functional gain from reporting
the rowcount for CTAS.  I'm less convinced that sending back REPLACE
is a big benefit worth taking big compatibility risks for.

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] LOCK for non-tables

2011-01-14 Thread Robert Haas
On Fri, Jan 14, 2011 at 1:58 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Robert Haas robertmh...@gmail.com writes:
 Tom - I am willing to implement this if you think it's valuable, but
 I'd like your input on the syntax.
 http://archives.postgresql.org/pgsql-hackers/2011-01/msg00472.php

 It looks to me like the reason why there's a shift/reduce conflict is
 not so much that TABLE is optional as that we allow the syntax

        LOCK tablename NOWAIT

 If that weren't possible, then a table name would have to be followed by
 EOL or IN (which is full-reserved), while an optional object type name
 could not be followed by either, so there would be no shift/reduce
 conflict.  So we broke it when we added NOWAIT, not when TABLE was made
 optional.

Hmm, OK.

 So it looks to me like there are at least two fixes other than the ones
 you enumerated:

 1. Make NOWAIT a reserved word.  Not good, but perhaps better than
 reserving all the different object type names.

 2. Disallow the above abbreviated syntax; allow NOWAIT only after an
 explicit IN ... MODE phrase.  This would probably break a couple of
 applications, but I bet a lot fewer than changing the longer-established
 parts of the command syntax would break.

 I think #2 might be the best choice here.

That strikes me as pretty unintuitive.  I'd rather take the hit of
forcing people to write LOCK TABLE foo instead of just LOCK foo
than try to explain why they have to include IN ACCESS EXCLUSIVE
MODE if they want to stick NOWAIT on the end.  However, I guess
it's a matter of opinion so... anyone else have an opinion?

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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


Re: [HACKERS] limiting hint bit I/O

2011-01-14 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 On Fri, Jan 14, 2011 at 1:42 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Um, yeah, I think you're having a problem keeping all the ideas straight
 ;-).  The argument about forensics has to do with how soon we're willing
 to freeze tuples, ie replace the XID with a constant.  Not about hint
 bits.

 Those things are related, though.  Freezing sooner could be viewed as
 an alternative to hint bits.

Freezing sooner isn't likely to reduce I/O compared to hint bits.  What
that does is create I/O that you *have* to execute ... both in the pages
themselves, and in WAL.

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] limiting hint bit I/O

2011-01-14 Thread Robert Haas
On Fri, Jan 14, 2011 at 2:01 PM, Kevin Grittner
kevin.gritt...@wicourts.gov wrote:
 Trouble is, it breaks Hot Standby, badly.

 You're really starting to worry me here.  Both for performance and
 to reduce the WAN bandwidth demands of our backup strategy we are
 very aggressive with our freezing.  Do off-hours VACUUM (FREEZE)
 runs break hot standby?  Autovacuum freezing?  What are the
 symptoms?

Freezing removes XIDs, so latestRemovedXid advances.  VACUUM (FREEZE)
is fine if you do it when there are no queries running on your Hot
Standby server, but if there ARE queries running on the Hot Standby
server, they'll be cancelled once max_standby_delay expires.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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


Re: [HACKERS] Determining client_encoding from client locale

2011-01-14 Thread Peter Eisentraut
On lör, 2009-07-25 at 01:41 -0500, Jaime Casanova wrote:
 On Fri, Jul 24, 2009 at 2:23 AM, Magnus Hagandermag...@hagander.net wrote:
 
  1) it introduces a dependency for -lpgport when compiling a client
  that uses libpq
 http://archives.postgresql.org/pgsql-hackers/2009-07/msg01511.php
 
  For other parts of libpgport that are needed, we pull in the
  individual source files. We specifically *don't* link libpq with
  libpgport, for a reason. There's a comment in the Makefile that
  explains why.
 
 
 ok, attached a version that modifies src/interfaces/libpq/Makefile to
 push chklocale.o and eliminate the dependency on libpgport, this
 change also fixes the compile problem on windows

I have adjusted your old patch for the current tree, and it seems to
work.  I think it was just forgotten last time because the move to
PQconnectdbParams had to happen first.  But I'll throw it back into the
ring now.
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index fe661b8..cb3f6e3 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -245,6 +245,21 @@ PGconn *PQconnectdbParams(const char **keywords, const char **values, int expand
  /listitem
 /varlistentry
 
+varlistentry id=libpq-connect-client-encoding xreflabel=client_encoding
+ termliteralclient_encoding/literal/term
+ listitem
+ para
+  This sets the varnameclient_encoding/varname
+  configuration parameter for this connection.  In addition to
+  the values accepted by the corresponding server option, you
+  can use literalauto/literal to determine the right
+  encoding from the current locale in the client
+  (envarLC_CTYPE/envar environment variable on Unix
+  systems).
+ /para
+ /listitem
+/varlistentry
+
 varlistentry id=libpq-connect-options xreflabel=options
  termliteraloptions/literal/term
  listitem
@@ -6341,6 +6356,16 @@ myEventProc(PGEventId evtId, void *evtInfo, void *passThrough)
   linkend=libpq-connect-connect-timeout connection parameter.
  /para
 /listitem
+
+listitem
+ para
+  indexterm
+   primaryenvarPGCLIENTENCODING/envar/primary
+  /indexterm
+  envarPGCLIENTENCODING/envar behaves the same as xref
+  linkend=libpq-connect-client-encoding connection parameter.
+ /para
+/listitem
/itemizedlist
   /para
 
@@ -6377,17 +6402,6 @@ myEventProc(PGEventId evtId, void *evtInfo, void *passThrough)
 listitem
  para
   indexterm
-   primaryenvarPGCLIENTENCODING/envar/primary
-  /indexterm
-  envarPGCLIENTENCODING/envar sets the default client character
-  set encoding.  (Equivalent to literalSET client_encoding TO
-  .../literal.)
- /para
-/listitem
-
-listitem
- para
-  indexterm
primaryenvarPGGEQO/envar/primary
   /indexterm
   envarPGGEQO/envar sets the default mode for the genetic query
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index 5f61561..1716b3b 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -593,6 +593,17 @@ $ userinputpsql service=myservice sslmode=require/userinput
 privileges, server is not running on the targeted host, etc.),
 applicationpsql/application will return an error and terminate.
 /para
+
+para
+ If at least one of standard input or standard output are a
+ terminal, then applicationpsql/application sets the client
+ encoding to quoteauto/quote, which will detect the
+ appropriate client encoding from the locale settings
+ (envarLC_CTYPE/envar environment variable on Unix systems).
+ If this doesn't work out as expected, the client encoding can be
+ overridden using the environment
+ variable envarPGCLIENTENCODING/envar.
+/para
   /refsect2
 
   refsect2 id=R2-APP-PSQL-4
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 962c13c..37f696a 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -1475,7 +1475,7 @@ do_connect(char *dbname, char *user, char *host, char *port)
 
 	while (true)
 	{
-#define PARAMS_ARRAY_SIZE	7
+#define PARAMS_ARRAY_SIZE	8
 		const char **keywords = pg_malloc(PARAMS_ARRAY_SIZE * sizeof(*keywords));
 		const char **values = pg_malloc(PARAMS_ARRAY_SIZE * sizeof(*values));
 
@@ -1491,8 +1491,10 @@ do_connect(char *dbname, char *user, char *host, char *port)
 		values[4] = dbname;
 		keywords[5] = fallback_application_name;
 		values[5] = pset.progname;
-		keywords[6] = NULL;
-		values[6] = NULL;
+		keywords[6] = client_encoding;
+		values[6] = (pset.notty || getenv(PGCLIENTENCODING)) ? NULL : auto;
+		keywords[7] = NULL;
+		values[7] = NULL;
 
 		n_conn = PQconnectdbParams(keywords, values, true);
 
diff --git a/src/bin/psql/startup.c b/src/bin/psql/startup.c
index 4f3815a..539f8be 100644
--- a/src/bin/psql/startup.c
+++ b/src/bin/psql/startup.c
@@ -171,7 

Re: [HACKERS] limiting hint bit I/O

2011-01-14 Thread Robert Haas
On Fri, Jan 14, 2011 at 1:52 PM, Kevin Grittner
kevin.gritt...@wicourts.gov wrote:
 Robert Haas robertmh...@gmail.com wrote:

 Background freezing plays havoc with Hot Standby

 I must have missed or forgotten the issue of background vacuums
 and hot standby.  Can you summarize why that's worse than hitting
 thresholds where autovacuum is freezing things?

The critical issue is whether the tuples get frozen while they're
still invisible to some transactions on the standby server.  That's
when you get query cancellations.

 this test is sufficient to show that eliminating hint bits
 altogether would a significant regression on some workloads.

 That wasn't clear to me from what you posted -- I thought that the
 reduced performance might be partly (largely? mostly?) due to
 competition with the background writer's work pushing the hinted
 pages out.  Maybe I'm missing something or you didn't post
 everything you observed in this regard

Well, let me put together a quick patch that obliterates hint bits
entirely, and we can measure that.  The background writer has always
pushed out hint bit pages; I think the reduced performance was
probably due to needing to reset hint bits on pages that we threw away
without pushing them out.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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


Re: [HACKERS] limiting hint bit I/O

2011-01-14 Thread Robert Haas
On Fri, Jan 14, 2011 at 2:09 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Robert Haas robertmh...@gmail.com writes:
 On Fri, Jan 14, 2011 at 1:42 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Um, yeah, I think you're having a problem keeping all the ideas straight
 ;-).  The argument about forensics has to do with how soon we're willing
 to freeze tuples, ie replace the XID with a constant.  Not about hint
 bits.

 Those things are related, though.  Freezing sooner could be viewed as
 an alternative to hint bits.

 Freezing sooner isn't likely to reduce I/O compared to hint bits.  What
 that does is create I/O that you *have* to execute ... both in the pages
 themselves, and in WAL.

It depends on which way you tilt your head - right now, we rewrite
each table 3x - once to populate, once to hint, and once to freeze.
If the table is doomed to survive long enough to go through all three
of those, then freezing is better than hinting.  Of course, that's not
always the case, but people keep complaining about the way this shakes
out.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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


Re: [HACKERS] Database file copy

2011-01-14 Thread Tom Lane
Kevin Grittner kevin.gritt...@wicourts.gov writes:
 If we're going to be supporting that long term, we should probably
 change the note about FREEZE being deprecated, though.
 
 So, still +1 on removing the wording about FREEZE being deprecated,
 but instead we should mention what actually *is* deprecated (the
 omission of the parentheses).

If we're going to do that, we should deprecate the unparenthesized
syntax altogether, with an eye to de-reserving VERBOSE and ANALYZE
as well.

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] limiting hint bit I/O

2011-01-14 Thread Kevin Grittner
Tom Lane t...@sss.pgh.pa.us wrote:
 Robert Haas robertmh...@gmail.com writes:
 
 Those things are related, though.  Freezing sooner could be
 viewed as an alternative to hint bits.
 
 Freezing sooner isn't likely to reduce I/O compared to hint bits. 
 What that does is create I/O that you *have* to execute ... both
 in the pages themselves, and in WAL.
 
In an environment where the vast majority of tuples live long enough
to need to be frozen anyway, freezing sooner doesn't really do that
to you.  Granted, explicit freezing off-hours prevents autovacuum
from doing that to you in large bursts at unexpected times, but if
you're comparing background writer freezing to autovacuum freezing,
I'm not clear on where the extra pain comes from.
 
I am assuming that the background writer would be sane about how it
did this, of course.  We could all set up straw man implementations
which would clobber performance.  I suspect that you can envision a
hueristic which would be no more bothersome than autovacuum.
 
-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] limiting hint bit I/O

2011-01-14 Thread Kevin Grittner
Robert Haas robertmh...@gmail.com wrote:
 
 The critical issue is whether the tuples get frozen while they're
 still invisible to some transactions on the standby server. 
 That's when you get query cancellations.
 
Oh, OK; I get that.  That seems easy enough to at least mitigate to
a large degree by some threshold GUC.  But of course, the longer you
wait to freeze so that you don't cancel queries on the standby, the
more you pay to recalculate visibility, so it'd be a fussy thing to
tune.  Perhaps such freeze information could be queued until a safe
time on the standby.  (Now that I've learned the joys of SLRU, I can
see all sorts of possible uses for it)
 
 Well, let me put together a quick patch that obliterates hint bits
 entirely, and we can measure that.  The background writer has
 always pushed out hint bit pages; I think the reduced performance
 was probably due to needing to reset hint bits on pages that we
 threw away without pushing them out.
 
It would be good to confirm and quantify.
 
-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] [PATCH] Return command tag 'REPLACE X' for CREATE OR REPLACE statements.

2011-01-14 Thread Robert Haas
On Fri, Jan 14, 2011 at 2:07 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Robert Haas robertmh...@gmail.com writes:
 On Fri, Jan 14, 2011 at 1:35 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 That 9.0 change was far less invasive than this: it only added a count
 field to SELECT and CTAS result tags.  Quite aside from the fact that
 the tag name stayed the same, in the SELECT case it's unlikely anyone
 would have checked the tag at all rather than just testing for
 PQresultStatus() == PGRES_TUPLES_OK.

 Yeah, but that one command tag was SELECT.  That's a pretty commonly
 used command.

 You're ignoring the point that people would probably use PQresultStatus
 in preference to checking the tag at all, when dealing with SELECT.

I would assume they would also use PQresultStatus() when checking
whether CREATE OR REPLACE FUNCTION worked.  Even if they were using
PQcmdStatus() for some reason, which seems like an odd thing to do,
there'd be no reason to check for anything beyond is it empty?.  The
idea that there are massive amounts of code out there that are
expecting the command tag to be *exactly* CREATE FUNCTION and will
break if it differs by a byte seems quite improbable.  Can you produce
an example of any such code?

 The other side of the argument that needs to be considered is what the
 benefit is.  There was a fairly clear functional gain from reporting
 the rowcount for CTAS.  I'm less convinced that sending back REPLACE
 is a big benefit worth taking big compatibility risks for.

Asserting that there are big compatibility risks doesn't make it so
- you've offered no evidence of that.  Even if a handful of people had
complained about that one, I would still felt it was a good change,
but it doesn't seem that there are any at all.  I classify this as one
of a dozen or two minor usability enhancements that we make in every
release, and most people don't care, and those who do go oh, that's
handy.  I think before we reject a patch for breaking things, we
ought to be able to identify either some actual application that is
broken by it, or at least some reasonably plausible coding pattern
that would blow up.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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


Re: [HACKERS] LOCK for non-tables

2011-01-14 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 On Fri, Jan 14, 2011 at 1:58 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 2. Disallow the above abbreviated syntax; allow NOWAIT only after an
 explicit IN ... MODE phrase.  This would probably break a couple of
 applications, but I bet a lot fewer than changing the longer-established
 parts of the command syntax would break.

 That strikes me as pretty unintuitive.  I'd rather take the hit of
 forcing people to write LOCK TABLE foo instead of just LOCK foo
 than try to explain why they have to include IN ACCESS EXCLUSIVE
 MODE if they want to stick NOWAIT on the end.  However, I guess
 it's a matter of opinion so... anyone else have an opinion?

It doesn't seem amazingly unintuitive to me; the syntax diagram just
changes from

LOCK ... [ IN lockmode MODE ] [ NOWAIT ]

to

LOCK ... [ IN lockmode MODE [ NOWAIT ] ]

If it had been that way to start with, nobody would have questioned it.

In any case I'd rather break apps using LOCK foo NOWAIT than break
every application using any form of LOCK at all, which is what I think
your proposal will amount to in practice.  People aren't that eager to
write useless noise words, which is what TABLE has been up to now in
this command.

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] Database file copy

2011-01-14 Thread Kevin Grittner
Tom Lane t...@sss.pgh.pa.us wrote: 
 Kevin Grittner kevin.gritt...@wicourts.gov writes:
 
 So, still +1 on removing the wording about FREEZE being
 deprecated, but instead we should mention what actually *is*
 deprecated (the omission of the parentheses).
 
 If we're going to do that, we should deprecate the unparenthesized
 syntax altogether, with an eye to de-reserving VERBOSE and ANALYZE
 as well.
 
+1
 
-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] limiting hint bit I/O

2011-01-14 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 On Fri, Jan 14, 2011 at 2:09 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Freezing sooner isn't likely to reduce I/O compared to hint bits.  What
 that does is create I/O that you *have* to execute ... both in the pages
 themselves, and in WAL.

 It depends on which way you tilt your head - right now, we rewrite
 each table 3x - once to populate, once to hint, and once to freeze.
 If the table is doomed to survive long enough to go through all three
 of those, then freezing is better than hinting.  Of course, that's not
 always the case, but people keep complaining about the way this shakes
 out.

The people whose tables are mostly insert-only complain about it, but
that's not the majority of our userbase IMO.  We just happen to have a
couple of particularly vocal ones, like Berkus.

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] LOCK for non-tables

2011-01-14 Thread Dimitri Fontaine
Le 14 janv. 2011 à 20:08, Robert Haas robertmh...@gmail.com a écrit :

 On Fri, Jan 14, 2011 at 1:58 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 
 So it looks to me like there are at least two fixes other than the ones
 you enumerated:
 
 1. Make NOWAIT a reserved word.  Not good, but perhaps better than
 reserving all the different object type names.
 
 2. Disallow the above abbreviated syntax; allow NOWAIT only after an
 explicit IN ... MODE phrase.  This would probably break a couple of
 applications, but I bet a lot fewer than changing the longer-established
 parts of the command syntax would break.
 
 I think #2 might be the best choice here.

+1

 
 That strikes me as pretty unintuitive.  I'd rather take the hit of
 forcing people to write LOCK TABLE foo instead of just LOCK foo
 than try to explain why they have to include IN ACCESS EXCLUSIVE
 MODE if they want to stick NOWAIT on the end.  However, I guess
 it's a matter of opinion so... anyone else have an opinion?

Since you ask, see above. :)

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


Re: [HACKERS] LOCK for non-tables

2011-01-14 Thread Simon Riggs
On Fri, 2011-01-14 at 14:48 -0500, Tom Lane wrote:

 In any case I'd rather break apps using LOCK foo NOWAIT than break
 every application using any form of LOCK at all, which is what I think
 your proposal will amount to in practice. 

Can I suggest that we don't break anything at all?

pg_lock_object(objectname, objecttype, mode);
or
pg_lock_sequence(name, mode);

is all we need...

-- 
 Simon Riggs   http://www.2ndQuadrant.com/books/
 PostgreSQL Development, 24x7 Support, Training and Services
 


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


Re: [HACKERS] SQL/MED - FDW API

2011-01-14 Thread Andrew Dunstan



On 01/14/2011 07:23 AM, Shigeru HANADA wrote:

You would be able to test these patches with 20110114 version of file_fdw
wrapper patches which will be posted in another thread.



Have you actually posted this version of file_fdw? I haven't seen it.

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] LOCK for non-tables

2011-01-14 Thread Tom Lane
Simon Riggs si...@2ndquadrant.com writes:
 On Fri, 2011-01-14 at 14:48 -0500, Tom Lane wrote:
 In any case I'd rather break apps using LOCK foo NOWAIT than break
 every application using any form of LOCK at all, which is what I think
 your proposal will amount to in practice. 

 Can I suggest that we don't break anything at all?

 pg_lock_object(objectname, objecttype, mode);
 or
 pg_lock_sequence(name, mode);

 is all we need...

No, that will not work at all.  LOCK has to be a utility command.
A function called by SELECT isn't a substitute, because SELECT
will acquire a transaction snapshot before executing the function,
and that breaks many use cases for locks.

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


  1   2   >