Re: [HACKERS] MERGE SQL Statement

2008-04-16 Thread Simon Riggs
On Wed, 2008-04-16 at 22:18 -0400, Robert Treat wrote:

> > * USING query can be a VALUES clause if we wish to do single/few row
> > operations, so MERGE can be used for bulk-loading and OLTP
> >
> > * There is no RETURNING option for MERGE, nor for any INSERT/UPDATE
> > sub-clauses
> 
> Is there a reason for this? A returning for any insert/updated rows would be 
> great, especially if your doing single row merges via the values clause. 

It's non-standard, is the first. Second would be complexity and third
would be difficulty in doing this usefully since you'd probably also
want to know which WHEN clause fired and whether the result for that row
was an I, U or D. Perhaps a later extension, but certainly not first
shot.

> > * WHERE CURRENT OF cursor is not supported anywhere
> > * The join can't be recursive, so no WITH support (common expressions,
> > i.e. non-recursive WITH are supported by SQLServer 2008)
> > * conditions would not allow sub-selects
> >
> 
> same question on both of these, is there some technical reason for this? I'd 
> imagine people would like both of these options. 
> 
> > * MERGE would work on base tables only, just like COPY
> > * Changes are made only to that single table
> > * Cannot update a column mentioned in the ON clause cos that would make
> > my head hurt too much
> >
> 
> Hmm... if the matching test is always done first, istm you could then update 
> the columns that matched on, since you don't really care about the value of 
> the on column once you have the row.  

I think you can update all columns without difficulty.

> > * MERGE will perform a left outer join between source on left and target
> > on right. There must be no more than 1 row from table-ref for each row
> > in the table. Each row in the table can only be updated once during each
> > MERGE statement. Each non-matching row in the table-ref will result in
> > one INSERT into table.
> >
> > * WHEN clauses are tested in the order specified. If the AND condition
> > returns false then we skip onto the next WHEN clause. We stop once a
> > WHEN clause activates, so only *one* action is ever activated for each
> > row.
> >
> > * AND clauses need not form a complete set, i.e. it is possible that no
> > action will result. It is also possible that some WHEN clauses will
> > never activate because of the execution order; we would not try to
> > prevent this, just document it as a possible user error.
> >
> 
> Just curious if any of these behaviors come from the spec?  or maybe from 
> other databases?  they don't seem unreasonable in general though.  

First two come from spec following clarifications from other
implementations. The last point about the AND clauses not necessarily
covering 100% of cases follows from the other points.

> > * MERGE will respect Triggers, but not Rules since the rules behaviour
> > is roughly orthogonal to the WHEN clauses
> 
> Should there be a new rule option?  ie. ON MERGE rules ? 

Maybe, but not as part of this project.

> > * MERGE fires UPDATE and INSERT triggers according to which WHEN clause
> > is activated (if any)
> >
> > * It's unclear whether MERGE should activate statement-level triggers,
> > or not. Few of the above sources are explicit either way on this point.
> > DB2 always runs both UPDATE and INSERT statement-level triggers, whether
> > or not rows have been changed; I would suggest we do that also for
> > before triggers. For after statement triggers I would suggest that we
> > track how many updates and inserts are caused and if updates > 0 then we
> > activate the after statement for update triggers, and if inserts > 0
> > then we activate the after statement for insert triggers. If a statement
> > level trigger is activated by both update and insert then it would be
> > possible for both TRIGGER_FIRED_BY_UPDATE() and
> > TRIGGER_FIRED_BY_DELETE() to be true (for statement level triggers
> > only), which would be a change from what we do now, even if the old
> > behaviour was not explicitly mutually exclusive. In both cases I suggest
> > we run Update triggers before Insert triggers consistently for both
> > before and after statement triggers.
> >
> 
> It would seem wierd that a before update statement level trigger would fire 
> and not the matching after update statement level trigger. I can probably 
> live with this behavior though, but seems like a note worth documenting. 

Agreed. I think this aspect can be re-visited when we see all of the
test cases.

> > * The number of rows changed should be (inserts + updates) which should
> > be < number of rows returned by table-ref. It would be good to get
> > access to the number of rows inserted and updated, so I propose that we
> > return a NOTICE statement with this information.
> >
> 
> How do you handle deletes? IE. If I merge two tables and I end up with no 
> updates or inserts but 100 deletes, is the number of affected rows 0 ? 

No, 100. I meant: add those as well.

> Nice work, hope my comments will b

Re: [HACKERS] Patch for Prevent pg_dump/pg_restore from being affected by statement_timeout

2008-04-16 Thread Heikki Linnakangas

Greg Sabino Mullane wrote:

I agree that we should do that, but the thread on -hackers ("Autovacuum
vs statement_timeout") wasn't totally conclusive. Greg Sabine Mullane
and Peter Eisentraut argued that we shouldn't, but neither provided a
plausible use case where a statement_timeout on restoring a dump would
be useful. I'm thinking we should apply the patch unless someone comes
up with one.


I don't think it's fair to simply discard the use cases provided as
"implausible" and demand one more to your liking.


Sorry if I missed it in the original thread, but what is the use case 
you have in mind?


--
  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] Lessons from commit fest

2008-04-16 Thread Heikki Linnakangas

Alvaro Herrera wrote:

Bruce Momjian wrote:

Alvaro Herrera wrote:



Maybe this means that we should give pgindent a run over the back
branches.

Yea, that thought has crossed our minds, but the problem is that there
is little testing of back branches so it would be risky.


That's a fair point, though I wonder how can a code indenter be so
broken that it broke code in ways not detected by our buildfarm.


Something like this:

if (foo)
{
do something;
do something else;
}
...

->

if (foo)
do something;
do something else;
...

I wouldn't be too surprised if something like that happened with one of 
the complex macros, like PG_TRY/CATCH.


--
  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] MERGE SQL Statement

2008-04-16 Thread Robert Treat
On Wednesday 16 April 2008 14:58, Simon Riggs wrote:
> I've analysed various flavours of MERGE command to understand and
> propose what we should use for PostgreSQL.
>
> The results aren't what you'd expect from a quick flick through the
> standard, so lets look at my main concerns:
>
> 1. The simplest syntax is for SQL:2003. The syntax for DB2, SQL Server
> and Oracle is more complex, with SQL:2008(final draft) being very
> similar to DB2 and SQL Server, so unlikely to be a point of contention
> in the standard. I suggest we go with the latter, but yes, its still in
> draft (yawn).
>
> 2. MySQL and Teradata have their own syntax for the row-oriented Upsert
> operation. Both of those are more useful (IMHO) than MERGE for OLTP
> apps, while MERGE is very useful for bulk data loads. I'm open to the
> idea that we do something like this in addition to MERGE.
>
> 3. The way MERGE works is to define a left outer join between source and
> target, then specify a series of WHEN clauses that may or may not apply.
> It **isn't** just a simple Update/Insert and so much of what we have
> discussed previously goes straight in the trash. AFAICS the way it is
> specified to work it would be fairly straightforward to cause race
> conditions and failures when using multiple concurrent MERGE statements.
>
> General Example of the recommended syntax for PostgreSQL
>
> MERGE INTO Stock S  /* target */
>
> USING DailySales DS   /* source table */
>
> ON S.Item = DS.Item   /* left outer join source to target */
>
> WHEN MATCHED AND (QtyOnHand - QtySold = 0) THEN
>
>   /* delete item if no stock remaining */
> DELETE
>
> WHEN MATCHED THEN /* No AND clause, so drop thru */
>
>  /* update value if some remains */
> UPDATE SET QtyOnHand = QtyOnHand - QtySold
>
> WHEN NOT MATCHED THEN
>
> /* insert a row if the stock is new */
> INSERT VALUES (Item, QtySold)
> ;
>
> So lets look at the syntaxes and then review how it might work.
>
> SYNTAX
> ==
> SQL:2003
> 
> MERGE INTO target [AS correlation-name]
> USING [table-ref | subquery]
> ON 
> [WHEN MATCHED THEN MergeUpdate]
> [WHEN NOT MATCHED THEN MergeInsert]
>
> Oracle 11g
> --
> MERGE INTO target [AS correlation-name]
> USING [table-ref | subquery]
> ON 
> [WHEN MATCHED THEN MergeUpdate
>   WHERE  DELETE WHERE ]
> [WHEN NOT MATCHED THEN MergeInsert
>   WHERE ]
>
> Differences from SQL:2003 are
> * Update and Insert have WHERE clauses on them
> * Oracle allows multiple WHEN ... WHERE clauses
> * Oracle allows an error logging clause also
> * optional DELETE statement as part of the UPDATE, so you can only
> DELETE what you update (yeh, really)
> * WHEN MATCHED/WHEN NOT MATCHED must be in fixed order, only
>
> IBM DB2
> ---
> MERGE INTO target [AS correlation-name]
> USING [table-ref | subquery]
> ON 
> [WHEN MATCHED [AND ] THEN MergeUpdate | MergeDelete]
> [WHEN NOT MATCHED [AND ] THEN MergeInsert | SignalClause]
> [ELSE IGNORE]
>
> Differences from SQL:2003 are
> * Update and Insert have AND clauses on them (like WHERE...)
> * DB2 allows multiple WHEN ... AND clauses
> * DELETE is also a full-strength option, not part of the MergeUpdate
> clause as it is in Oracle
> * DB2 allows a SIGNAL statement, similar to RAISE
> * ELSE IGNORE is an optional syntax, which does nothing
>
> SQL Server 2008
> ---
>
> MERGE [INTO] target [AS correlation-name]
> USING [table-ref | subquery]
> ON 
> [WHEN MATCHED [AND ] THEN MergeUpdate | MergeDelete]
> [WHEN NOT MATCHED [AND ] THEN MergeInsert]
>
> Differences from SQL:2003 are
> * Update and Insert have AND clauses on them (like WHERE...)
> * DB2 allows multiple WHEN ... AND clauses
> * DELETE is also a full-strength option, not part of the MergeUpdate
> clause as it is in Oracle
>
> SQL:2008
> 
>
> MERGE INTO target [AS correlation-name]
> USING [table-ref | subquery]
> ON 
> [WHEN MATCHED [AND ] THEN MergeUpdate]
> [WHEN NOT MATCHED [AND ] THEN MergeInsert]
>
> Differences from SQL:2003 are
> * Update and Insert have AND clauses on them (like WHERE...)
> * Allows multiple WHEN ... AND clauses
>
> Alternate Syntax
> 
>
> MySQL supports
> * REPLACE INTO
> * INSERT ... ON DUPLICATE KEY UPDATE ...
>
> Teradata supports
> * UPDATE ... ELSE INSERT ...
> * MERGE with an additional error logging clause
>
> The Teradata and Oracle error logging clauses are very cute and I
> suggest we do something similar for COPY, at least.
>
> Proposed Syntax for PostgreSQL
> ==
>
> MERGE INTO table [[AS] alias]
> USING [table-ref | query]
> ON join-condition
> [WHEN MATCHED [AND condition] THEN MergeUpdate | DELETE]
> [WHEN NOT MATCHED [AND condition] THEN MergeInsert]
>
> MergeUpdate is
> UPDATE SET { column = { expression | DEFAULT } |
>   ( column [, ...] ) = ( { expression | DEFAULT } [, ...] ) }
> [, ...]
> (yes, there is no WHERE clause here

Re: [HACKERS] How to submit a patch

2008-04-16 Thread Andrew Dunstan



Gregory Stark wrote:

That's why waiting until feature freeze was so awful from my point of view.
There was never any time left to return patches to the author so Tom ended up
reworking any patches we really wanted.
  


Some patches went back and forth a few times even after feature freeze.

Many patches last feature freeze seemed to me at least to be of lower 
quality  and/or higher complexity than usual, and we had a huge number. 
Part of the idea behind commit-fest is to avoid that, and so kicking 
patches back to the author is  more likely to occur, I think.


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] How to submit a patch

2008-04-16 Thread Gregory Stark
"Heikki Linnakangas" <[EMAIL PROTECTED]> writes:

> Workflow A:
>
> 1. You post patch to pgsql-patches
> 2. a committer picks it up immediately, and commits it.

I'm more interested in knowing what happens when a committer *doesn't* commit
it. Personally I would almost rather a committer not commit my patch but
instead return feedback on the first go-around than commit it.

I would rather hear about any objections and fix them myself and in the
process learn how to do it better next time. It just isn't as good a learning
experience when I read the commit diffs and have to try to figure out what the
rationale was for the changes.

That's why waiting until feature freeze was so awful from my point of view.
There was never any time left to return patches to the author so Tom ended up
reworking any patches we really wanted.

-- 
  Gregory Stark
  EnterpriseDB  http://www.enterprisedb.com
  Ask me about EnterpriseDB's PostGIS 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] count(*) performance improvement ideas

2008-04-16 Thread Stephen Denne
PFC wrote:
> Let's try this quick & dirty implementation of a local 
> count-delta cache  
> using a local in-memory hashtable (ie. {}).

> CREATE OR REPLACE FUNCTION update_count( key TEXT, delta INTEGER )
>RETURNS INTEGER
> AS $$
>  if key in GD:
>  GD[key] += delta
>  else:
>  GD[key] = delta
>  return GD[key]
> $$ LANGUAGE plpythonu;

Thanks for the code, this seems to be very much what I was looking for.

I don't know plpythonu (nor python), just read a few docs now:
"The global dictionary SD is available to store data between function calls. 
This variable is private static data. The global dictionary GD is public data, 
available to all Python functions within a session. Use with care."

Does session == transaction or connection?
I don't understand the difference between SD and GD, private and public. Where 
are the context boundaries?

Regards,
Stephen Denne.

Disclaimer:
At the Datamail Group we value team commitment, respect, achievement, customer 
focus, and courage. This email with any attachments is confidential and may be 
subject to legal privilege.  If it is not intended for you please advise by 
reply immediately, destroy it and do not copy, disclose or use it in any way.
__
  This email has been scanned by the DMZGlobal Business Quality
  Electronic Messaging Suite.
Please see http://www.dmzglobal.com/dmzmessaging.htm for details.
__


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


Re: [HACKERS] count(*) performance improvement ideas

2008-04-16 Thread Stephen Denne
Tom Lane wrote
> Transaction commit is an exceedingly subtle and carefully structured
> thing.  Throwing random user-defined code into it ain't gonna happen.

Deferred constraint triggers currently run random user-defined code. This'll do 
me.

Regards,
Stephen Denne.

Disclaimer:
At the Datamail Group we value team commitment, respect, achievement, customer 
focus, and courage. This email with any attachments is confidential and may be 
subject to legal privilege.  If it is not intended for you please advise by 
reply immediately, destroy it and do not copy, disclose or use it in any way.
__
  This email has been scanned by the DMZGlobal Business Quality
  Electronic Messaging Suite.
Please see http://www.dmzglobal.com/dmzmessaging.htm for details.
__



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


Re: [HACKERS] count(*) performance improvement ideas

2008-04-16 Thread PFC



The whole thing is a bit of an abuse of what the mechanism
was intended
for, and so I'm not sure we should rejigger GUC's behavior to make it
more pleasant, but on the other hand if we're not ready to provide a
better substitute ...


In my experiments with materialized views, I identified these problems  
as "minor" difficulties. Resolving them would allow further abuse ;)


Let's try this quick & dirty implementation of a local count-delta cache  
using a local in-memory hashtable (ie. {}).
Writing the results to stable storage in an ON COMMIT trigger is left as  
an exercise to the reader ;)

Performance isn't that bad, calling the trigger takes about 50 us.
Oldskool implementation with a table is at the end, it's about 10x slower.

Example :

INSERT INTO victim1 (key) VALUES ('one'),('two'),('two');
INSERT 0 3
Temps : 1,320 ms
test=# SELECT * FROM get_count();
 key | cnt
-+-
 two |   2
 one |   1


CREATE OR REPLACE FUNCTION clear_count(  )
  RETURNS VOID
AS $$
GD.clear()
$$ LANGUAGE plpythonu;

CREATE OR REPLACE FUNCTION update_count( key TEXT, delta INTEGER )
  RETURNS INTEGER
AS $$
if key in GD:
GD[key] += delta
else:
GD[key] = delta
return GD[key]
$$ LANGUAGE plpythonu;

CREATE TYPE count_data AS ( key TEXT, cnt INTEGER );

CREATE OR REPLACE FUNCTION get_count( )
RETURNS SETOF count_data
AS $$
return GD.iteritems()
$$ LANGUAGE plpythonu;


CREATE TABLE victim( id SERIAL PRIMARY KEY, key TEXT NOT NULL );
INSERT INTO victim (key) SELECT (random() * 300)::INTEGER::TEXT FROM  
generate_series( 1,10 );


CREATE TABLE victim1( id SERIAL PRIMARY KEY, key TEXT NOT NULL );

\timing
INSERT INTO victim1 SELECT * FROM victim;
TRUNCATE TABLE victim1;

SELECT clear_count();
INSERT INTO victim1 SELECT * FROM victim RETURNING update_count( key, 1 );
SELECT * FROM get_count();
TRUNCATE TABLE victim1;

CREATE OR REPLACE FUNCTION counter_trigger_f()
RETURNS TRIGGER
LANGUAGE plpgsql
AS
$$
DECLARE
BEGIN
IF TG_OP = 'INSERT' THEN
PERFORM update_count( NEW.key, 1 );
RETURN NEW;
ELSEIF TG_OP = 'UPDATE' THEN
-- update topic
IF NEW.key != OLD.key THEN
PERFORM update_count( OLD.key, -1 ), update_count( NEW.key, 1  
);

END IF;
RETURN NEW;
ELSE-- DELETE
PERFORM update_count( OLD.key, -1 );
RETURN OLD;
END IF;
END;
$$;

CREATE TRIGGER count_trigger BEFORE INSERT OR UPDATE OR DELETE ON victim1  
FOR EACH ROW EXECUTE PROCEDURE counter_trigger_f();


SELECT clear_count();
INSERT INTO victim1 SELECT * FROM victim;
SELECT * FROM get_count();

SELECT clear_count();
TRUNCATE TABLE victim1;
INSERT INTO victim1 (key) VALUES ('one'),('two'),('two');
SELECT * FROM get_count();
DELETE FROM victim1 WHERE key='two';
SELECT * FROM get_count();
UPDATE victim1 SET key='three' WHERE key='one';
SELECT * FROM get_count();
DELETE FROM victim1;
SELECT * FROM get_count();


CREATE TABLE counts( key TEXT PRIMARY KEY, total INTEGER NOT NULL DEFAULT  
0 );


CREATE OR REPLACE FUNCTION table_counter_trigger_f()
RETURNS TRIGGER
LANGUAGE plpgsql
AS
$$
DECLARE
BEGIN
IF TG_OP = 'INSERT' THEN
UPDATE counts SET total=total+1 WHERE key=NEW.key;
IF NOT FOUND THEN INSERT INTO counts (key,total) VALUES  
(NEW.key,1); END IF;

RETURN NEW;
ELSEIF TG_OP = 'UPDATE' THEN
-- update topic
IF NEW.key != OLD.key THEN
UPDATE counts SET total=total-1 WHERE key=OLD.key;
UPDATE counts SET total=total+1 WHERE key=NEW.key;
IF NOT FOUND THEN INSERT INTO counts (key,total) VALUES  
(NEW.key,1); END IF;

END IF;
RETURN NEW;
ELSE-- DELETE
UPDATE counts SET total=total-1 WHERE key=OLD.key;
RETURN OLD;
END IF;
END;
$$;

CREATE TABLE victim2( id SERIAL PRIMARY KEY, key TEXT NOT NULL );

CREATE TRIGGER table_count_trigger BEFORE INSERT OR UPDATE OR DELETE ON  
victim2 FOR EACH ROW EXECUTE PROCEDURE table_counter_trigger_f();

SELECT * FROM counts;
TRUNCATE TABLE victim2;
INSERT INTO victim2 SELECT * FROM victim;



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


Re: [HACKERS] count(*) performance improvement ideas

2008-04-16 Thread Tom Lane
"Stephen Denne" <[EMAIL PROTECTED]> writes:
> Aside: It is currently more cumbersome to get a function to run, if needed, 
> at commit. Ideal solution would be something like "EXECUTE ON COMMIT 
> my_function()" or maybe "SAVEPOINT my_name ON COMMIT my_function()", but 
> these suggestions are made without investigating what provision the SQL 
> standard has made to address this need.

There is none, and the reason seems pretty obvious to me.  What if your
"on commit" function fails?  Or if you have two, and the second one
fails?  Or even more to the point, the second one does something that
the first one expected to see the effects of?

Transaction commit is an exceedingly subtle and carefully structured
thing.  Throwing random user-defined code into it ain't gonna happen.

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] Lessons from commit fest

2008-04-16 Thread Bruce Momjian
Bruce Momjian wrote:
> Tom Lane wrote:
> > Bruce Momjian <[EMAIL PROTECTED]> writes:
> > > I am reviewing the psql wrap patch and just used pgindent today to clean
> > > it up.  (pgindent did not add any extra spacing changes.)  Patch
> > > reviewers should probably be able to run pgindent.
> > 
> > Well, that means nobody in the world can review except you, because
> > nobody else has ever reported success in duplicating your pgindent
> > results.  I know I haven't been able to.
> > 
> > If you really believe the above then you need to try a bit harder
> > to find a portable version of indent that we all can use.
> 
> The source code of pgindent I use is on our ftp site.  find_typedefs
> should now work on Linux as well as BSD.  Not sure what else would be a
> problem.

Also I can put up a web page where you can upload or email your C file
and get a formatted version back.

-- 
  Bruce Momjian  <[EMAIL PROTECTED]>http://momjian.us
  EnterpriseDB http://enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

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


Re: [HACKERS] Lessons from commit fest

2008-04-16 Thread Bruce Momjian
Tom Lane wrote:
> Bruce Momjian <[EMAIL PROTECTED]> writes:
> > I am reviewing the psql wrap patch and just used pgindent today to clean
> > it up.  (pgindent did not add any extra spacing changes.)  Patch
> > reviewers should probably be able to run pgindent.
> 
> Well, that means nobody in the world can review except you, because
> nobody else has ever reported success in duplicating your pgindent
> results.  I know I haven't been able to.
> 
> If you really believe the above then you need to try a bit harder
> to find a portable version of indent that we all can use.

The source code of pgindent I use is on our ftp site.  find_typedefs
should now work on Linux as well as BSD.  Not sure what else would be a
problem.

-- 
  Bruce Momjian  <[EMAIL PROTECTED]>http://momjian.us
  EnterpriseDB http://enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

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


Re: [HACKERS] Lessons from commit fest

2008-04-16 Thread Tom Lane
Bruce Momjian <[EMAIL PROTECTED]> writes:
> I am reviewing the psql wrap patch and just used pgindent today to clean
> it up.  (pgindent did not add any extra spacing changes.)  Patch
> reviewers should probably be able to run pgindent.

Well, that means nobody in the world can review except you, because
nobody else has ever reported success in duplicating your pgindent
results.  I know I haven't been able to.

If you really believe the above then you need to try a bit harder
to find a portable version of indent that we all can use.

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] Lessons from commit fest

2008-04-16 Thread Gregory Stark
"Alvaro Herrera" <[EMAIL PROTECTED]> writes:

> I suggested to you awhile back that we could keep a typedef file on the
> repository, saving one step.

That kind of sucks since it means you get conflicts when you update if you've
run it yourself.

Is there a reason we can't write makefiles which generate this file?

-- 
  Gregory Stark
  EnterpriseDB  http://www.enterprisedb.com
  Ask me about EnterpriseDB's RemoteDBA services!

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


Re: [HACKERS] Patch for Prevent pg_dump/pg_restore from being affected by statement_timeout

2008-04-16 Thread Alex Hunsaker
On Wed, Apr 16, 2008 at 4:54 PM, Alvaro Herrera
<[EMAIL PROTECTED]> wrote:
> Joshua D. Drake escribió:
>
> > That is an interesting idea. Something like:
>  >
>  > pg_restore -E "SET STATEMENT_TIMEOUT=0; SET MAINTENANCE_WORK_MEM=1G" ?
>
>  We already have it -- it's called PGOPTIONS.
>

Ok but is not the purpose of the patch to turn off statement_timeout
by *default* in pg_restore/pg_dump?

Here is an updated patch for I posted above (with the command line
option --use-statement-timeout) for pg_dump and pg_restore.

(sorry If I hijacked your patch Josh :) )


pg_dump_restore_statement_timeout.diff
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] count(*) performance improvement ideas

2008-04-16 Thread Stephen Denne
Tom Lane wrote
> "Stephen Denne" <[EMAIL PROTECTED]> writes:
> > From: Tom Lane [mailto:[EMAIL PROTECTED]
> >> As for 2) and 3), can't you look into the pg_settings view?
> 
> > pg_settings view doesn't contain custom variables created 
> on the fly,
> 
> Really?  [ pokes around ... ]  Hm, you're right, because
> add_placeholder_variable() sets the GUC_NO_SHOW_ALL flag, and in this
> usage it'll never be cleared.  I wonder if we should change that.
> 
> The whole thing is a bit of an abuse of what the mechanism 
> was intended
> for, and so I'm not sure we should rejigger GUC's behavior to make it
> more pleasant, but on the other hand if we're not ready to provide a
> better substitute ...

In my experiments with materialized views, I identified these problems as 
"minor" difficulties. Resolving them would allow further abuse ;)

Aside: It is currently more cumbersome to get a function to run, if needed, at 
commit. Ideal solution would be something like "EXECUTE ON COMMIT 
my_function()" or maybe "SAVEPOINT my_name ON COMMIT my_function()", but these 
suggestions are made without investigating what provision the SQL standard has 
made to address this need.

My use of mv.initialized means I can create variables when initializing a 
transaction, and afterwards know that they have values, but what I can't easily 
do is use those variables to identify which grouping keys have been updated. To 
do that I select & conditionally insert to a table for that explicit purpose. 
If select doesn't find the key, then I create variables named after that key, 
with zero values.

Performance and efficiency-wise which would be better way of keeping track 
of grouping keys used in a transaction?:
1) Create a temp table, on commit drop, for the transaction, storing grouping 
keys affected.
2) Use a persistent table, storing txid and grouping keys affected, deleting 
txid rows at commit.
3) Use pg_settings, storing tx local grouping keys affected, existence check 
via catching an exception, listing via checking existence for all possible 
values (a possibility in my scenario).

Speed is my priority, low disk IO is a probable means to that end, which is why 
I investigated using variables.

Basically, (3) isn't a viable option, so what are the trade-offs between 
creating a temporary table per transaction, or using rows in a permanent table 
with a txid column?

Here are some more plpgsql code fragments:

   mv := 'mv.' || view_name || '.' || key_value || '.';

When recording a grouping key as being affected by the transaction, create the 
variables with zeroes:

   PERFORM set_config(mv||'documents', '0', true);
   PERFORM set_config(mv||'last_addition', 'null', true);

In an insert trigger:

   PERFORM set_config(mv||'documents', 
(current_setting(mv||'documents')::bigint + 1)::text, true);
   PERFORM set_config(mv||'last_addition', now()::text, true);

In the defferred till commit trigger:

  UPDATE materialized_view set 
 documents=documents+current_setting(mv||'documents')::bigint, 
 
last_addition=greatest(last_addition,nullif(current_setting(mv||'last_addition'),'null')::timestamp)
 where 
 group_id = key_values.key_value;


Regards,
Stephen Denne.

Disclaimer:
At the Datamail Group we value team commitment, respect, achievement, customer 
focus, and courage. This email with any attachments is confidential and may be 
subject to legal privilege.  If it is not intended for you please advise by 
reply immediately, destroy it and do not copy, disclose or use it in any way.
__
  This email has been scanned by the DMZGlobal Business Quality
  Electronic Messaging Suite.
Please see http://www.dmzglobal.com/dmzmessaging.htm for details.
__



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


Re: [HACKERS] Lessons from commit fest

2008-04-16 Thread Bruce Momjian
Chris Browne wrote:
> > That is much a more radical use of pgindent than it has had in the past
> > but it is certainly possible.
> 
> Well, supposing you're cleaning up a patch after someone has generated
> it in bad style, it would seem like rather less work to use pgindent
> to impose style policy right away rather than simulating its effects
> manually.

I am reviewing the psql wrap patch and just used pgindent today to clean
it up.  (pgindent did not add any extra spacing changes.)  Patch
reviewers should probably be able to run pgindent.

-- 
  Bruce Momjian  <[EMAIL PROTECTED]>http://momjian.us
  EnterpriseDB http://enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

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


Re: [HACKERS] How to submit a patch

2008-04-16 Thread Greg Smith

On Wed, 16 Apr 2008, Joshua D. Drake wrote:


I've added a redirect at http://wiki.postgresql.org/wiki/CommitFest
which currently points to May, but should be updated whenever we close
a commitfest against new submissions.


We should also update the FAQ.


I wouldn't bother with that yet.  That whole area of the Wiki is still 
moving around a bit, and I expect some more usefully targetted pages will 
emerge ("How to submit a patch" comes to mind).  Having a stable 
CommitFest URL is handy, but I don't think it's where the FAQ should be 
sending people.


--
* Greg Smith [EMAIL PROTECTED] http://www.gregsmith.com Baltimore, MD

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


Re: [HACKERS] Patch for Prevent pg_dump/pg_restore from being affected by statement_timeout

2008-04-16 Thread Alvaro Herrera
Joshua D. Drake escribió:
> On Wed, 16 Apr 2008 18:50:28 -0400
> Andrew Dunstan <[EMAIL PROTECTED]> wrote:

> > Actually, it's probably more important to be selectable at restore
> > time than at dump time, so if you're doing just one ...

I think the patch posted by Joshua at the start of this thread does
that.

> > This whole thing set me wondering whether or not we should provide a 
> > more general command-line facility to psql and pg_restore, and maybe 
> > others, to do some session setup before running their commands.
> 
> That is an interesting idea. Something like:
> 
> pg_restore -E "SET STATEMENT_TIMEOUT=0; SET MAINTENANCE_WORK_MEM=1G" ?

We already have it -- it's called PGOPTIONS.

-- 
Alvaro Herrerahttp://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

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


Re: [HACKERS] Patch for Prevent pg_dump/pg_restore from being affected by statement_timeout

2008-04-16 Thread Joshua D. Drake
On Wed, 16 Apr 2008 18:50:28 -0400
Andrew Dunstan <[EMAIL PROTECTED]> wrote:

> 
> 
> Alex Hunsaker wrote:
> >
> >
> > Sorry if i missed the obvious reason not to do this... but if its a
> > command line option the user can choose.  Why not something like
> > this (i did it for pg_dump only...)
> >
> >
> >   
> Actually, it's probably more important to be selectable at restore
> time than at dump time, so if you're doing just one ...
> 
> This whole thing set me wondering whether or not we should provide a 
> more general command-line facility to psql and pg_restore, and maybe 
> others, to do some session setup before running their commands.
> 
> Of course, there's no reason we couldn't have both.

That is an interesting idea. Something like:

pg_restore -E "SET STATEMENT_TIMEOUT=0; SET MAINTENANCE_WORK_MEM=1G" ?

Sincerely,

Joshua D. Drake

> 
> cheers
> 
> andrew
> 


-- 
The PostgreSQL Company since 1997: http://www.commandprompt.com/ 
PostgreSQL Community Conference: http://www.postgresqlconference.org/
United States PostgreSQL Association: http://www.postgresql.us/
Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate



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


Re: [HACKERS] Patch for Prevent pg_dump/pg_restore from being affected by statement_timeout

2008-04-16 Thread Andrew Dunstan



Alex Hunsaker wrote:



Sorry if i missed the obvious reason not to do this... but if its a
command line option the user can choose.  Why not something like this
(i did it for pg_dump only...)


  
Actually, it's probably more important to be selectable at restore time 
than at dump time, so if you're doing just one ...


This whole thing set me wondering whether or not we should provide a 
more general command-line facility to psql and pg_restore, and maybe 
others, to do some session setup before running their commands.


Of course, there's no reason we couldn't have both.

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] Lessons from commit fest

2008-04-16 Thread Tom Lane
Alvaro Herrera <[EMAIL PROTECTED]> writes:
> Bruce Momjian wrote:
>> Alvaro Herrera wrote:
>>> Maybe this means that we should give pgindent a run over the back
>>> branches.
>> 
>> Yea, that thought has crossed our minds, but the problem is that there
>> is little testing of back branches so it would be risky.

> That's a fair point, though I wonder how can a code indenter be so
> broken that it broke code in ways not detected by our buildfarm.

Yeah, the buildfarm has changed that equation a little.

I think that if we were going to do something like switch to GNU indent,
we'd really have to do that (re-indent the back branches) to maintain
our sanity.  And again anytime we moved to a new release of indent.

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 for Prevent pg_dump/pg_restore from being affected by statement_timeout

2008-04-16 Thread Alex Hunsaker
On Wed, Apr 16, 2008 at 4:32 PM, Joshua D. Drake <[EMAIL PROTECTED]> wrote:
> On Wed, 16 Apr 2008 15:22:31 -0700
>
> "Joshua D. Drake" <[EMAIL PROTECTED]> wrote:
>
>
> > On Wed, 16 Apr 2008 22:17:30 -
>  > "Greg Sabino Mullane" <[EMAIL PROTECTED]> wrote:
>  >
>  > > I don't think it's fair to simply discard the use cases provided as
>  > > "implausible" and demand one more to your liking. I strongly dislike
>  > > having a giant dump file written that has non-vital configuration
>  > > variables embedded in the top of it, precluding any user choice
>  > > whatsoever[1]. As before, where are the reports of all the people
>  > > having their manual restorations interrupted by a statement_timeout?
>  >
>  > Calling me, wondering why in the world it is happening.
>
>  Sorry couldn't help myself. Anyway, in an attempt to be productive, I
>  will say that your "where are all the reports" is about as valid as,
>  "Where are all the users besides yourself arguing about this having to
>  edit a backup file?"
>
>  This is a real problem and unless we can find more people to
>  substantiate your claim, I think the consensus is to ensure that people
>  don't get bit by statement timeout when attempting to do a restore.
>
>  I vote in favor of the one less foot gun approach.

Sorry if i missed the obvious reason not to do this... but if its a
command line option the user can choose.  Why not something like this
(i did it for pg_dump only...)

diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index ed1b33d..bf9365a 100644
*** a/src/bin/pg_dump/pg_dump.c
--- /bsrc/bin/pg_dump/pg_dump.c
*** main(int argc, char **argv)
*** 225,230 
--- 225,231 
int outputNoOwner = 0;
static int  use_setsessauth = 0;
static int  disable_triggers = 0;
+   static int  use_statement_timeout = 0;
char   *outputSuperuser = NULL;

RestoreOptions *ropt;
*** main(int argc, char **argv)
*** 267,272 
--- 268,274 
{"disable-dollar-quoting", no_argument,
&disable_dollar_quoting, 1},
{"disable-triggers", no_argument, &disable_triggers, 1},
{"use-set-session-authorization", no_argument,
&use_setsessauth, 1},
+   {"use-statement-timeout", no_argument,
&use_statement_timeout, 1},

{NULL, 0, NULL, 0}
};
*** main(int argc, char **argv)
*** 419,424 
--- 421,428 
disable_triggers = 1;
else if (strcmp(optarg,
"use-set-session-authorization") == 0)
use_setsessauth = 1;
+   else if (strcmp(optarg,
"use-statement-timeout") == 0)
+   use_statement_timeout = 1;
else
{
fprintf(stderr,
*** main(int argc, char **argv)
*** 571,576 
--- 575,583 
 */
do_sql_command(g_conn, "BEGIN");

+   if (!use_statement_timeout)
+   do_sql_command(g_conn, "SET statement_timeout = 0;");
+
do_sql_command(g_conn, "SET TRANSACTION ISOLATION LEVEL SERIALIZABLE");

/* Select the appropriate subquery to convert user IDs to names */
*** help(const char *progname)
*** 771,776 
--- 778,784 
printf(_("  --use-set-session-authorization\n"
 "  use SESSION
AUTHORIZATION commands instead of\n"
"  ALTER OWNER commands to set
ownership\n"));
+   printf(_("  --use-statement-timeout respect statement_timeout\n"));

printf(_("\nConnection options:\n"));
printf(_("  -h, --host=HOSTNAME  database server host or
socket directory\n"));

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


Re: [HACKERS] Patch for Prevent pg_dump/pg_restore from being affected by statement_timeout

2008-04-16 Thread Joshua D. Drake
On Wed, 16 Apr 2008 15:22:31 -0700
"Joshua D. Drake" <[EMAIL PROTECTED]> wrote:

> On Wed, 16 Apr 2008 22:17:30 -
> "Greg Sabino Mullane" <[EMAIL PROTECTED]> wrote:
>  
> > I don't think it's fair to simply discard the use cases provided as
> > "implausible" and demand one more to your liking. I strongly dislike
> > having a giant dump file written that has non-vital configuration
> > variables embedded in the top of it, precluding any user choice
> > whatsoever[1]. As before, where are the reports of all the people
> > having their manual restorations interrupted by a statement_timeout?
> 
> Calling me, wondering why in the world it is happening.

Sorry couldn't help myself. Anyway, in an attempt to be productive, I
will say that your "where are all the reports" is about as valid as,
"Where are all the users besides yourself arguing about this having to
edit a backup file?"

This is a real problem and unless we can find more people to
substantiate your claim, I think the consensus is to ensure that people
don't get bit by statement timeout when attempting to do a restore.

I vote in favor of the one less foot gun approach. 

Sincerely,

Joshua D. Drake

-- 
The PostgreSQL Company since 1997: http://www.commandprompt.com/ 
PostgreSQL Community Conference: http://www.postgresqlconference.org/
United States PostgreSQL Association: http://www.postgresql.us/
Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate



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


Re: [HACKERS] Lessons from commit fest

2008-04-16 Thread Chris Browne
[EMAIL PROTECTED] (Bruce Momjian) writes:
> Chris Browne wrote:
>> >> Would it be a terrible idea to...
>> >> 
>> >> - Draw the indent code from NetBSD into src/tools/pgindent
>> >> - Build it _in place_ inside the code tree (e.g. - don't assume 
>> >>   it will get installed in /usr/local/bin)
>> >> - Thus have the ability to run it in place?
>> >
>> > Yes, but it bloats our code and people still need to generate the
>> > typedefs and follow the instructions.  The other problem is if they run
>> > it on a file they have modified, it is going to adjust places they
>> > didn't touch, thereby making the patch harder to review.
>> 
>> The bloat is 154K, on a project with something around 260MB of code.
>> I don't think this is a particlarly material degree of bloat.
>
> You mean 37kb vs 13MB, right?  That is the tarball sizes I see.

Hmm.  I was apparently badly counting the size of the sources.  I ran
a find | wc command that seemed to report 260MB of code.  A "du" on a
"make distclean" tree gives me 104MB.  At any rate, that's only out by
a bit more than a binary order of magnitude ;-).

I was thinking about the 154K of source code sitting in CVS, not the
(yes, lower) cost of it in a tarball.

Seems immaterial either way...

>> If we ran pgindent really frequently, there would admittedly be the
>> difference that there would be a lot of little cases of
>> changes-from-pgindent being committed along the way, but [1] might it
>> not be cheaper to accept that cost, with the concomittant benefit that
>> you could tell patchers "Hey, run pgindent before submitting that
>> patch, and that'll clean up a number of of the issues."  Yes, it
>> doesn't address code changes like typedef generation, but that never
>> was an argument against running pgindent...
>
> That is much a more radical use of pgindent than it has had in the past
> but it is certainly possible.

Well, supposing you're cleaning up a patch after someone has generated
it in bad style, it would seem like rather less work to use pgindent
to impose style policy right away rather than simulating its effects
manually.

I'm not proposing anything *really* radical, like migrating away from
CVS, after all!  ;-)
-- 
let name="cbbrowne" and tld="linuxfinances.info" in String.concat "@" 
[name;tld];;
http://linuxfinances.info/info/lisp.html
There was a young lady of Crewe
Whose limericks stopped at line two. 

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


Re: [HACKERS] Patch for Prevent pg_dump/pg_restore from being affected by statement_timeout

2008-04-16 Thread Joshua D. Drake
On Wed, 16 Apr 2008 22:17:30 -
"Greg Sabino Mullane" <[EMAIL PROTECTED]> wrote:
 
> I don't think it's fair to simply discard the use cases provided as
> "implausible" and demand one more to your liking. I strongly dislike
> having a giant dump file written that has non-vital configuration
> variables embedded in the top of it, precluding any user choice
> whatsoever[1]. As before, where are the reports of all the people
> having their manual restorations interrupted by a statement_timeout?

Calling me, wondering why in the world it is happening.

Joshua D. Drake

-- 
The PostgreSQL Company since 1997: http://www.commandprompt.com/ 
PostgreSQL Community Conference: http://www.postgresqlconference.org/
United States PostgreSQL Association: http://www.postgresql.us/
Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate



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


Re: [HACKERS] Timely reporting of COPY errors

2008-04-16 Thread Martijn van Oosterhout
On Wed, Apr 16, 2008 at 05:22:17PM -0400, Tom Lane wrote:
> I dunno about "intentional", but the API exposed by libpq for COPY
> doesn't really permit any other behavior: you push all the data and
> then look to see if it worked or not.

Oh? I expected the PQputData would return -1 as stated by the
documentation, at which point I would call PQendCopy and retrieve the
resultset.

> Even if we had some way of letting the application notice that the copy
> had already failed, I don't see that psql could do very much with it,
> at least not for COPY FROM STDIN.  It's got to read through the source
> data anyway or it'll be out of sync with the script file.

psql could ignore the result of PQputData if it wanted, no big deal
there.

> We could possibly fix libpq to start dropping the data on the floor
> if it sees an error reply already pending, but that's only going
> to be an incremental change.

At the very least the documentation needs to be improved. For example,
no NOTICEs will be processed either *unless* there are enough to cause
the backend to block. At which point they will all be processed at
once. But the first step would be to get libpq to even notice the
error. I'm confused as to why PQconsumeInput doesn't work.

Have a nice day,
-- 
Martijn van Oosterhout   <[EMAIL PROTECTED]>   http://svana.org/kleptog/
> Please line up in a tree and maintain the heap invariant while 
> boarding. Thank you for flying nlogn airlines.


signature.asc
Description: Digital signature


Re: [HACKERS] Patch for Prevent pg_dump/pg_restore from being affected by statement_timeout

2008-04-16 Thread Greg Sabino Mullane

-BEGIN PGP SIGNED MESSAGE-
Hash: RIPEMD160


> I agree that we should do that, but the thread on -hackers ("Autovacuum
> vs statement_timeout") wasn't totally conclusive. Greg Sabine Mullane
> and Peter Eisentraut argued that we shouldn't, but neither provided a
> plausible use case where a statement_timeout on restoring a dump would
> be useful. I'm thinking we should apply the patch unless someone comes
> up with one.

I don't think it's fair to simply discard the use cases provided as
"implausible" and demand one more to your liking. I strongly dislike
having a giant dump file written that has non-vital configuration
variables embedded in the top of it, precluding any user choice
whatsoever[1]. As before, where are the reports of all the people
having their manual restorations interrupted by a statement_timeout?

[1] Short of editing a potentially GB-size file, or using
some sed/shell shenanigans to strip out the suspect setting.

- --
Greg Sabino Mullane [EMAIL PROTECTED]
End Point Corporation
PGP Key: 0x14964AC8 200804161814
http://biglumber.com/x/web?pk=2529DF6AB8F79407E94445B4BC9B906714964AC8
-BEGIN PGP SIGNATURE-

iEYEAREDAAYFAkgGetEACgkQvJuQZxSWSsg+4ACghvlBkIth1aBiGpFPFPj+HWgf
iyEAnj+WK9MQL+ZQqKoTcLOe/lvoNkkV
=Nlyg
-END PGP SIGNATURE-


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


Re: [HACKERS] Lessons from commit fest

2008-04-16 Thread Alvaro Herrera
Bruce Momjian wrote:
> Alvaro Herrera wrote:

> > Maybe this means that we should give pgindent a run over the back
> > branches.
> 
> Yea, that thought has crossed our minds, but the problem is that there
> is little testing of back branches so it would be risky.

That's a fair point, though I wonder how can a code indenter be so
broken that it broke code in ways not detected by our buildfarm.

-- 
Alvaro Herrerahttp://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

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


Re: [HACKERS] DROP DATABASE vs patch to not remove files right away

2008-04-16 Thread Andrew Dunstan



Tom Lane wrote:

Heikki Linnakangas <[EMAIL PROTECTED]> writes:
  

Tom Lane wrote:


ISTM that we must fix the bgwriter so that ForgetDatabaseFsyncRequests
causes PendingUnlinkEntrys for the doomed DB to be thrown away too.
  


  
Because of the asynchronous nature of ForgetDatabaseFsyncRequests, this 
still isn't enough, I'm afraid.



Hm.  I notice that there is no bug on Windows because dropdb forces a
checkpoint before it starts to remove files.  Maybe the sanest solution
is to just do that on all platforms.


  


IIRC we did that to avoid an unlink problem :-)

I certainly don't see any particular reason not to do it everywhere.

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] Lessons from commit fest

2008-04-16 Thread Bruce Momjian
Alvaro Herrera wrote:
> Tom Lane wrote:
> > Bruce Momjian <[EMAIL PROTECTED]> writes:
> 
> > > I hate to say it but pgindent formatting changes are usually made in
> > > cases where you or the community want pgindent to improve its indenting. 
> > > :-)
> > > So we improve pgindent but pay for it in backpatching difficulties.  :-(
> > 
> > Yeah, I know ... it's why I've pretty much stopped complaining about
> > pgindent, even though there are lots of little things it does that
> > I don't especially care for.
> 
> Maybe this means that we should give pgindent a run over the back
> branches.

Yea, that thought has crossed our minds, but the problem is that there
is little testing of back branches so it would be risky.

-- 
  Bruce Momjian  <[EMAIL PROTECTED]>http://momjian.us
  EnterpriseDB http://enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

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


Re: [HACKERS] Lessons from commit fest

2008-04-16 Thread Alvaro Herrera
Tom Lane wrote:
> Bruce Momjian <[EMAIL PROTECTED]> writes:

> > I hate to say it but pgindent formatting changes are usually made in
> > cases where you or the community want pgindent to improve its indenting. :-)
> > So we improve pgindent but pay for it in backpatching difficulties.  :-(
> 
> Yeah, I know ... it's why I've pretty much stopped complaining about
> pgindent, even though there are lots of little things it does that
> I don't especially care for.

Maybe this means that we should give pgindent a run over the back
branches.

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

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


Re: [HACKERS] DROP DATABASE vs patch to not remove files right away

2008-04-16 Thread Tom Lane
Heikki Linnakangas <[EMAIL PROTECTED]> writes:
> Tom Lane wrote:
>> ISTM that we must fix the bgwriter so that ForgetDatabaseFsyncRequests
>> causes PendingUnlinkEntrys for the doomed DB to be thrown away too.

> Because of the asynchronous nature of ForgetDatabaseFsyncRequests, this 
> still isn't enough, I'm afraid.

Hm.  I notice that there is no bug on Windows because dropdb forces a
checkpoint before it starts to remove files.  Maybe the sanest solution
is to just do that on all platforms.

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] Lessons from commit fest

2008-04-16 Thread Tom Lane
Bruce Momjian <[EMAIL PROTECTED]> writes:
> Tom Lane wrote:
>> It's bad enough that Bruce whacks around his copy from time to time :-(.

> I hate to say it but pgindent formatting changes are usually made in
> cases where you or the community want pgindent to improve its indenting. :-)
> So we improve pgindent but pay for it in backpatching difficulties.  :-(

Yeah, I know ... it's why I've pretty much stopped complaining about
pgindent, even though there are lots of little things it does that
I don't especially care 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] Timely reporting of COPY errors

2008-04-16 Thread Alvaro Herrera
Tom Lane wrote:

> We could possibly fix libpq to start dropping the data on the floor
> if it sees an error reply already pending, but that's only going
> to be an incremental change.

I think this incremental change makes a lot of sense.  What point is
there in transmitting the data over the network, if the backend is in
error state?

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

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


Re: [HACKERS] Lessons from commit fest

2008-04-16 Thread Bruce Momjian
Tom Lane wrote:
> The main problem I see with "pgindent early and often" is that it only
> works well if everyone is using exactly the same pgindent code (and
> exactly the same typedef list).  Otherwise you just get buried in
> useless whitespace diffs.
> 
> It's bad enough that Bruce whacks around his copy from time to time :-(.
> I would say that the single greatest annoyance for maintaining our back
> branches is that patches tend to not back-patch cleanly, and well over
> half the time it's because of random reformatting done by pgindent
> to code that hadn't changed at all, but it had formatted differently
> the prior year. 

I hate to say it but pgindent formatting changes are usually made in
cases where you or the community want pgindent to improve its indenting. :-)
So we improve pgindent but pay for it in backpatching difficulties.  :-(

> I guess an advantage of maintaining our own fork is that there'd be Only
> One True pgindent, thereby alleviating that problem; but I'm still not
> eager to go there.  If we were going to do it, I'd really wish that we
> could standardize on a version that didn't need a typedef list.  The

I don't think that is possible.  GNU indent 2.2.9 requires the same -T
option:

   You  must  use the -T option to tell indent the name of all the
   type names in your program that are defined by typedef.  -T can be
   specified more than once, and all names specified are used.  For
   example, if your program contains

typedef unsigned long CODE_ADDR;
typedef enum {red, blue, green} COLOR;

   you would use the options -T CODE_ADDR -T COLOR

astyle doesn't seem to require it but perhaps it just mishandles them. 
As I remember there isn't anything about the C grammar that can tell
identify a typedef when it needs to.

> But in any case, this is all focusing on trivialities.  The stuff
> pgindent can fix is, by definition, stuff that committers don't really
> have to worry about during patch review.  The stuff I'm worried about
> is at a higher level than that.

Agreed.  Reformatting is small compared to understanding the patch,
adjusting in the comments, testing, documentation, etc.

-- 
  Bruce Momjian  <[EMAIL PROTECTED]>http://momjian.us
  EnterpriseDB http://enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

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


Re: [HACKERS] Timely reporting of COPY errors

2008-04-16 Thread Tom Lane
Martijn van Oosterhout <[EMAIL PROTECTED]> writes:
> I notice that while doing bulk-loads that any errors detected by the
> backend arn't noticed by libpq until right at the end. Is this
> intentional?

I dunno about "intentional", but the API exposed by libpq for COPY
doesn't really permit any other behavior: you push all the data and
then look to see if it worked or not.

Even if we had some way of letting the application notice that the copy
had already failed, I don't see that psql could do very much with it,
at least not for COPY FROM STDIN.  It's got to read through the source
data anyway or it'll be out of sync with the script file.

We could possibly fix libpq to start dropping the data on the floor
if it sees an error reply already pending, but that's only going
to be an incremental change.

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: [PATCHES] [HACKERS] Text <-> C string

2008-04-16 Thread Brendan Jurd
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On Thu, Apr 17, 2008 at 1:16 AM, Tom Lane  wrote:
> "Brendan Jurd"  writes:
>
> >> If we don't want to meddle with xmltype/bytea/VarChar at all, we'll
>  >> have to revert those changes, and I'll have to seriously scale back
>  >> the cleanup patch I was about to post.
>
>  > Still not sure where we stand on the above.  To cast, or not to cast?
>
>  I dunno.  I know there was previously some handwaving about representing
>  XML values in some more intelligent fashion than a plain text string,
>  but I have no idea if anyone is likely to do anything about it in the
>  foreseeable future.
>

Well ... if somebody does want to change the representation of xml
down the road, he's going to have to touch all the sites where the
code converts to and from cstring anyway, right?

So the only real difference this will make is that, instead of having
to replace four lines of VARDATA/memcpy per site, he'll have to
replace a single function call.  That doesn't seem like a negative.

With that in mind, please find attached my followup patch.  It cleans
up another 21 sites manually copying between cstring and varlena, for
a net reduction of 115 lines of code.

I didn't attempt to work through every reference to VARDATA, but I did
look at every hit from a  `grep -Rn VARDATA . | grep memcpy`.

All regression tests passed (contrib tests included) on gentoo.

Patch added to commitfest queue.

Cheers,
BJ
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.7 (GNU/Linux)
Comment: http://getfiregpg.org

iD8DBQFIBm105YBsbHkuyV0RAmqfAKCfyNyFdciqX4QV81sG9MhPt+KXuACfe694
3d/ICZF6yqV6K20X3TVX+So=
=CvRM
-END PGP SIGNATURE-


text-cstring-followup.diff.bz2
Description: BZip2 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] Lessons from commit fest

2008-04-16 Thread Tom Lane
Chris Browne <[EMAIL PROTECTED]> writes:
> Would it be a terrible idea to...
> 
> - Draw the indent code from NetBSD into src/tools/pgindent

I am not real eager to become maintainers of our own indent fork, which
is what you propose.  (Just for starters, what will we have to do to
make it run on non-BSD systems?)

> We are presently at the extreme position where pgindent is run once in
> a very long time (~ once a year), at pretty considerable cost, and
> with the associated cost that a whole lot of indentation problems are
> managed by hand.

Yeah.  One reason for that is that the typedef problem makes it a pretty
manual process.

The main problem I see with "pgindent early and often" is that it only
works well if everyone is using exactly the same pgindent code (and
exactly the same typedef list).  Otherwise you just get buried in
useless whitespace diffs.

It's bad enough that Bruce whacks around his copy from time to time :-(.
I would say that the single greatest annoyance for maintaining our back
branches is that patches tend to not back-patch cleanly, and well over
half the time it's because of random reformattings done by pgindent
to code that hadn't changed at all, but it had formatted differently
the prior year. 

For the same reason, my take on your "random whitespace changes are
acceptable" theory is not no but hell no.  It's gonna cost us,
permanently, in manual patch adjustments if we allow the repository to
get cluttered with content-free diffs.

I guess an advantage of maintaining our own fork is that there'd be Only
One True pgindent, thereby alleviating that problem; but I'm still not
eager to go there.  If we were going to do it, I'd really wish that we
could standardize on a version that didn't need a typedef list.  The
random whitespace changes you get after changing the typedef list are
another thorn in my side.

But in any case, this is all focusing on trivialities.  The stuff
pgindent can fix is, by definition, stuff that committers don't really
have to worry about during patch review.  The stuff I'm worried about
is at a higher level than 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] Timely reporting of COPY errors

2008-04-16 Thread Stephen Frost
Martijn,

* Martijn van Oosterhout ([EMAIL PROTECTED]) wrote:
> Is there anything that can be done? I've tried putting in
> PQconsumeInput in places but it doesn't appear to help.

I certainly hope something can be done, I've noticed this exact same
issue myself and it's very annoying.  I've resorted to watching 'top' on
the server and hitting ctrl-c when it goes 'idle' but my psql hasn't
returned yet.

Thanks,

Stephen


signature.asc
Description: Digital signature


[HACKERS] Timely reporting of COPY errors

2008-04-16 Thread Martijn van Oosterhout
Hi,

I notice that while doing bulk-loads that any errors detected by the
backend arn't noticed by libpq until right at the end. Is this
intentional? Looking at the code we have this comment in putCopyData:

  /*
   * Process any NOTICE or NOTIFY messages that might be pending in the
   * input buffer.  Since the server might generate many notices during the
   * COPY, we want to clean those out reasonably promptly to prevent
   * indefinite expansion of the input buffer.  (Note: the actual read of
   * input data into the input buffer happens down inside pqSendSome, but
   * it's not authorized to get rid of the data again.)
   */

Except that pqSendSome won't try reading anything until it has a
problem writing. Since the backend will consume copy data indefinitly,
the error message sits in the kernel buffers until the end.

Is there anything that can be done? I've tried putting in
PQconsumeInput in places but it doesn't appear to help.

Any ideas?
-- 
Martijn van Oosterhout   <[EMAIL PROTECTED]>   http://svana.org/kleptog/
> Please line up in a tree and maintain the heap invariant while 
> boarding. Thank you for flying nlogn airlines.


signature.asc
Description: Digital signature


Re: [HACKERS] How to submit a patch

2008-04-16 Thread Andrew Dunstan



Merlin Moncure wrote:

I've been mostly following this discussion on
this particular topic but have absolutely no idea what, if anything,
to do on the wiki in terms of submitting patch. 
  


I think the short answer right now to this and to Joshua's original 
question is: to submit a patch do what has always been done, i.e. send 
the patch to the mailing list. You can also add the patch to the list on 
the wiki, but if you don't, it won't be forgotten.


All the rest is for reviewers/committers.

I've been wondering idly and probably pointlessly if we should try 
adopting something like the methodology of the Usenet Oracle, which, if 
you ask it a question will usually send you one to answer in return. 
Obviously we can't always do that, but maybe we should be asking people 
who are obviously competent enough (judging by the quality and 
complexity of the patches they submit) to step up to the plate more on 
reviewing patches. No amount of process will substitute for more reviewers.


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] How to submit a patch

2008-04-16 Thread Joshua D. Drake
On Thu, 17 Apr 2008 06:19:49 +1000
"Brendan Jurd" <[EMAIL PROTECTED]> wrote:

> -BEGIN PGP SIGNED MESSAGE-
> Hash: SHA1
> 
> On Thu, Apr 17, 2008 at 6:03 AM, Merlin Moncure  wrote:
> >  One small point here.  I've been mostly following this discussion
> > on this particular topic but have absolutely no idea what, if
> > anything, to do on the wiki in terms of submitting patch.  There
> > was a spot related to the commit fest where we kept an to date
> > version of our patch which I can't find any more (might be lousy
> > search skills on my end).
> >
> 
> Fair enough.  The current commitfest page is at
> http://wiki.postgresql.org/wiki/CommitFest:May
> 
> I agree that it's not well linked from the front page of the wiki (you
> have to follow "Development information" -> "Project management
> documentation" -> "Patches for May Commitfest").
> 
> I've added a redirect at http://wiki.postgresql.org/wiki/CommitFest
> which currently points to May, but should be updated whenever we close
> a commitfest against new submissions.  That way submitters will always
> have one URL to visit to add their stuff.

We should also update the FAQ.

Joshua D. Drake
-- 
The PostgreSQL Company since 1997: http://www.commandprompt.com/ 
PostgreSQL Community Conference: http://www.postgresqlconference.org/
United States PostgreSQL Association: http://www.postgresql.us/
Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate



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


Re: [HACKERS] How to submit a patch

2008-04-16 Thread Brendan Jurd
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On Thu, Apr 17, 2008 at 6:03 AM, Merlin Moncure  wrote:
>  One small point here.  I've been mostly following this discussion on
>  this particular topic but have absolutely no idea what, if anything,
>  to do on the wiki in terms of submitting patch.  There was a spot
>  related to the commit fest where we kept an to date version of our
>  patch which I can't find any more (might be lousy search skills on my
>  end).
>

Fair enough.  The current commitfest page is at
http://wiki.postgresql.org/wiki/CommitFest:May

I agree that it's not well linked from the front page of the wiki (you
have to follow "Development information" -> "Project management
documentation" -> "Patches for May Commitfest").

I've added a redirect at http://wiki.postgresql.org/wiki/CommitFest
which currently points to May, but should be updated whenever we close
a commitfest against new submissions.  That way submitters will always
have one URL to visit to add their stuff.

>  There seems to be information scattered all over the place with
>  various overlapping lists whose function and location are changing
>  constantly.  This is not a gripe by any meansit just that you
>  might get a little more help from the grass roots on the wiki as the
>  process settles down and there are examples of what to do.
>

I agree, and in fact I've just recently added some documentation about
how to add your patch to the wiki up the top of the CommitFest:May
page; I invite you to take a look and post back if you have any
comments.

Cheers,
BJ
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.7 (GNU/Linux)
Comment: http://getfiregpg.org

iD8DBQFIBl9f5YBsbHkuyV0RAoP8AKDAOaXzhsC7ap0/AVf0F6SqxHOZwACfQ4LR
JJQcrRKbxoIzQHRbja6gqBw=
=tLDr
-END PGP SIGNATURE-

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


Re: [HACKERS] How to submit a patch

2008-04-16 Thread Bruce Momjian
Heikki Linnakangas wrote:
> Based on my observations, there's basically three different workflows a 
> patch can follow (assuming the patch gets committed in the end):
> 
> Workflow A:
> 
> 1. You post patch to pgsql-patches
> 2. a committer picks it up immediately, and commits it.
> 
> Workflow B:
> 
> 1. You post a patch to pgsql-patches
> 2. You add a link to the wiki page of the next commit fest
> 3. A committer picks up the patch from the wiki page, and commits it
> 
> Workflow C:
> 
> 1. You post a patch to pgsql-patches
> 2. Bruce adds the patch to the unapplied patches queue after a while
> 3. At the beginning of the next commit fest, Alvaro (with the help from 
> others, I hope) goes through the patches queue, and puts a link to the 
> wiki page of the next commit fest
> 4. A committer picks up the patch from the wiki page, and commits it

Yep, that's pretty accurate.

-- 
  Bruce Momjian  <[EMAIL PROTECTED]>http://momjian.us
  EnterpriseDB http://enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

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


Re: [HACKERS] How to submit a patch

2008-04-16 Thread Bruce Momjian
Brendan Jurd wrote:
> >  OK.  FYI, what would be really nice would be for someone to review and
> >  apply the patch or give the author feedback so we could avoid adding it
> >  to the wiki at all.
> 
> Bruce,
> 
> Yes, that would be nice!  But not likely in practice, unless your
> patch happens to immediately catch the interest of a suitably
> qualified person with commit privileges.
> 
> However, I don't know of any way the maintenance of the wiki is an
> addition to your workload.  I feel that the onus of adding the patch
> to the wiki should be on the submitter, and we've already had some
> success getting submitters to add their own patches.  And Alvaro has
> already offered to pick up the slack in cases where the submitter
> fails to add their patch to the queue.
> 
> Every patch that somebody else adds to the wiki is another patch you
> don't have to add to your queue, so how can this be anything but a
> plus for you?

Yes, but unless people actually applies patches from the wiki, it
doesn't help me --- adding patches to my queue is zero cost for me.

-- 
  Bruce Momjian  <[EMAIL PROTECTED]>http://momjian.us
  EnterpriseDB http://enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

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


Re: [HACKERS] Lessons from commit fest

2008-04-16 Thread Bruce Momjian
Chris Browne wrote:
> >> Would it be a terrible idea to...
> >> 
> >> - Draw the indent code from NetBSD into src/tools/pgindent
> >> - Build it _in place_ inside the code tree (e.g. - don't assume 
> >>   it will get installed in /usr/local/bin)
> >> - Thus have the ability to run it in place?
> >
> > Yes, but it bloats our code and people still need to generate the
> > typedefs and follow the instructions.  The other problem is if they run
> > it on a file they have modified, it is going to adjust places they
> > didn't touch, thereby making the patch harder to review.
> 
> The bloat is 154K, on a project with something around 260MB of code.
> I don't think this is a particlarly material degree of bloat.

You mean 37kb vs 13MB, right?  That is the tarball sizes I see.

> If we ran pgindent really frequently, there would admittedly be the
> difference that there would be a lot of little cases of
> changes-from-pgindent being committed along the way, but [1] might it
> not be cheaper to accept that cost, with the concomittant benefit that
> you could tell patchers "Hey, run pgindent before submitting that
> patch, and that'll clean up a number of of the issues."  Yes, it
> doesn't address code changes like typedef generation, but that never
> was an argument against running pgindent...

That is much a more radical use of pgindent than it has had in the past
but it is certainly possible.

-- 
  Bruce Momjian  <[EMAIL PROTECTED]>http://momjian.us
  EnterpriseDB http://enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

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


Re: [HACKERS] How to submit a patch

2008-04-16 Thread Merlin Moncure
On Wed, Apr 16, 2008 at 3:47 PM, Brendan Jurd <[EMAIL PROTECTED]> wrote:
>  On Thu, Apr 17, 2008 at 5:17 AM, Bruce Momjian
>  > Andrew Dunstan wrote:
>  >  > Bruce Momjian wrote:
>  >  > >
>
> >  > > This is one of the reasons I didn't want to add wiki maintenance to my
>  >  > > already full workload.  Instead I am having to field complaints.
>  >  >
>  >  > I didn't mean to complain about anything. Personally, I'm in favor of
>  >  > reducing your workload.
>  >
>  >  OK.  FYI, what would be really nice would be for someone to review and
>  >  apply the patch or give the author feedback so we could avoid adding it
>  >  to the wiki at all.
>  Yes, that would be nice!  But not likely in practice, unless your
>  patch happens to immediately catch the interest of a suitably
>  qualified person with commit privileges.
>
>  However, I don't know of any way the maintenance of the wiki is an
>  addition to your workload.  I feel that the onus of adding the patch
>  to the wiki should be on the submitter, and we've already had some
>  success getting submitters to add their own patches.  And Alvaro has
>  already offered to pick up the slack in cases where the submitter
>  fails to add their patch to the queue.

One small point here.  I've been mostly following this discussion on
this particular topic but have absolutely no idea what, if anything,
to do on the wiki in terms of submitting patch.  There was a spot
related to the commit fest where we kept an to date version of our
patch which I can't find any more (might be lousy search skills on my
end).

There seems to be information scattered all over the place with
various overlapping lists whose function and location are changing
constantly.  This is not a gripe by any meansit just that you
might get a little more help from the grass roots on the wiki as the
process settles down and there are examples of what to do.

merlin

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


Re: [HACKERS] DROP DATABASE vs patch to not remove files right away

2008-04-16 Thread Heikki Linnakangas

Tom Lane wrote:

Actually ... what if the same DB OID and relfilenode get re-made before
the checkpoint?  Then we'd be unlinking live data.  This is improbable
but hardly less so than the scenario the PendingUnlinkEntry code was
put in to prevent.

ISTM that we must fix the bgwriter so that ForgetDatabaseFsyncRequests
causes PendingUnlinkEntrys for the doomed DB to be thrown away too.


Because of the asynchronous nature of ForgetDatabaseFsyncRequests, this 
still isn't enough, I'm afraid.


1. DROP TABLE footable;
2. Checkpoint begins.
3. DROP DATABASE foodb;
4. CREATE DATABASE bardb; -- bardb gets same OID as foodb, and a table 
copied from template database, let's call it bartable, gets same OID as 
footable
5. Checkpoint processes pending unlink for footable, but removes 
bartable instead


Needs more thought, after some sleep...

--
  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] How to submit a patch

2008-04-16 Thread Brendan Jurd
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On Thu, Apr 17, 2008 at 5:17 AM, Bruce Momjian
 wrote:
> Andrew Dunstan wrote:
>  > Bruce Momjian wrote:
>  > >
>  > > This is one of the reasons I didn't want to add wiki maintenance to my
>  > > already full workload.  Instead I am having to field complaints.
>  >
>  > I didn't mean to complain about anything. Personally, I'm in favor of
>  > reducing your workload.
>
>  OK.  FYI, what would be really nice would be for someone to review and
>  apply the patch or give the author feedback so we could avoid adding it
>  to the wiki at all.

Bruce,

Yes, that would be nice!  But not likely in practice, unless your
patch happens to immediately catch the interest of a suitably
qualified person with commit privileges.

However, I don't know of any way the maintenance of the wiki is an
addition to your workload.  I feel that the onus of adding the patch
to the wiki should be on the submitter, and we've already had some
success getting submitters to add their own patches.  And Alvaro has
already offered to pick up the slack in cases where the submitter
fails to add their patch to the queue.

Every patch that somebody else adds to the wiki is another patch you
don't have to add to your queue, so how can this be anything but a
plus for you?

Cheers,
BJ


-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.7 (GNU/Linux)
Comment: http://getfiregpg.org

iD8DBQFIBlfb5YBsbHkuyV0RAs6tAKDNR3CbqFC4YwROaoiwNMXSWKXWTgCbBaz1
pO90zcotv+/UMJrotfDTg/M=
=IY5R
-END PGP SIGNATURE-

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


Re: [HACKERS] Lessons from commit fest

2008-04-16 Thread Chris Browne
[EMAIL PROTECTED] (Bruce Momjian) writes:
> Chris Browne wrote:
>> [EMAIL PROTECTED] (Bruce Momjian) writes:
>> > Magnus Hagander wrote:
>> >> > And I think adopting surrounding naming, commeting, coding conventions
>> >> > should come naturally as it can aide in copy-pasting too :)
>> >> 
>> >> I think pg_indent has to be made a lot more portable and easy to use
>> >> before that can happen :-) I've run it once or twice on linux machines,
>> >> and it comes out with huge changes compared to what Bruce gets on his
>> >> machine. Other times, it doesn't :-) So yeah, it could be that it just
>> >> needs to be made easier to use, because I may certainly have done
>> >> something wrong.
>> >
>> > Agreed, pgindent is too cumbersome to require patch submitters to use. 
>> > One idea would be to allow C files to be emailed and the indented
>> > version automatically returned via email.
>> 
>> Would it be a terrible idea to...
>> 
>> - Draw the indent code from NetBSD into src/tools/pgindent
>> - Build it _in place_ inside the code tree (e.g. - don't assume 
>>   it will get installed in /usr/local/bin)
>> - Thus have the ability to run it in place?
>
> Yes, but it bloats our code and people still need to generate the
> typedefs and follow the instructions.  The other problem is if they run
> it on a file they have modified, it is going to adjust places they
> didn't touch, thereby making the patch harder to review.

The bloat is 154K, on a project with something around 260MB of code.
I don't think this is a particlarly material degree of bloat.

If it is included in src/tools/pgindent, you can add in a Makefile
such that it is automatically built, so the cost of running it goes
way down, so that it could be run all the time rather than once in a
great long while.

If it was being run *all* the time, would we not expect to find that
we would be seeing relatively smaller sets of changes coming out of
it?

We are presently at the extreme position where pgindent is run once in
a very long time (~ once a year), at pretty considerable cost, and
with the associated cost that a whole lot of indentation problems are
managed by hand.

If we ran pgindent really frequently, there would admittedly be the
difference that there would be a lot of little cases of
changes-from-pgindent being committed along the way, but [1] might it
not be cheaper to accept that cost, with the concomittant benefit that
you could tell patchers "Hey, run pgindent before submitting that
patch, and that'll clean up a number of of the issues."  Yes, it
doesn't address code changes like typedef generation, but that never
was an argument against running pgindent...

[1] In cases where the differences primarily fall from differences in
indentation, "cvs diff -u" can drop out those differences when reading
the effects of a patch...
-- 
let name="cbbrowne" and tld="cbbrowne.com" in name ^ "@" ^ tld;;
http://linuxdatabases.info/info/sap.html
"A study  in the  Washington Post says  that women have  better verbal
skills than  men. I  just want to  say to  the authors of  that study:
Duh." -- Conan O' Brien

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


Re: [HACKERS] How to submit a patch

2008-04-16 Thread Bruce Momjian
Andrew Dunstan wrote:
> 
> 
> Bruce Momjian wrote:
> > Frankly, I am getting pretty tired of people complaining about what I am
> > doing.  Perhaps I should just stop and let everyone else deal with
> > things.  I have lots of things I would rather be doing.
> >
> > This is one of the reasons I didn't want to add wiki maintenance to my
> > already full workload.  Instead I am having to field complaints.
> >
> >   
> 
> I didn't mean to complain about anything. Personally, I'm in favor of 
> reducing your workload.

OK.  FYI, what would be really nice would be for someone to review and
apply the patch or give the author feedback so we could avoid adding it
to the wiki at all.

-- 
  Bruce Momjian  <[EMAIL PROTECTED]>http://momjian.us
  EnterpriseDB http://enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

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


[HACKERS] MERGE SQL Statement

2008-04-16 Thread Simon Riggs

I've analysed various flavours of MERGE command to understand and
propose what we should use for PostgreSQL.

The results aren't what you'd expect from a quick flick through the
standard, so lets look at my main concerns:

1. The simplest syntax is for SQL:2003. The syntax for DB2, SQL Server
and Oracle is more complex, with SQL:2008(final draft) being very
similar to DB2 and SQL Server, so unlikely to be a point of contention
in the standard. I suggest we go with the latter, but yes, its still in
draft (yawn).

2. MySQL and Teradata have their own syntax for the row-oriented Upsert
operation. Both of those are more useful (IMHO) than MERGE for OLTP
apps, while MERGE is very useful for bulk data loads. I'm open to the
idea that we do something like this in addition to MERGE.

3. The way MERGE works is to define a left outer join between source and
target, then specify a series of WHEN clauses that may or may not apply.
It **isn't** just a simple Update/Insert and so much of what we have
discussed previously goes straight in the trash. AFAICS the way it is
specified to work it would be fairly straightforward to cause race
conditions and failures when using multiple concurrent MERGE statements.

General Example of the recommended syntax for PostgreSQL

MERGE INTO Stock S  /* target */

USING DailySales DS   /* source table */

ON S.Item = DS.Item   /* left outer join source to target */

WHEN MATCHED AND (QtyOnHand - QtySold = 0) THEN

/* delete item if no stock remaining */
DELETE 

WHEN MATCHED THEN /* No AND clause, so drop thru */

 /* update value if some remains */
UPDATE SET QtyOnHand = QtyOnHand - QtySold

WHEN NOT MATCHED THEN

/* insert a row if the stock is new */
INSERT VALUES (Item, QtySold)
;

So lets look at the syntaxes and then review how it might work.

SYNTAX
==
SQL:2003

MERGE INTO target [AS correlation-name]
USING [table-ref | subquery]
ON 
[WHEN MATCHED THEN MergeUpdate]
[WHEN NOT MATCHED THEN MergeInsert]

Oracle 11g 
--
MERGE INTO target [AS correlation-name]
USING [table-ref | subquery]
ON 
[WHEN MATCHED THEN MergeUpdate 
  WHERE  DELETE WHERE ]
[WHEN NOT MATCHED THEN MergeInsert
  WHERE ]

Differences from SQL:2003 are
* Update and Insert have WHERE clauses on them
* Oracle allows multiple WHEN ... WHERE clauses
* Oracle allows an error logging clause also
* optional DELETE statement as part of the UPDATE, so you can only
DELETE what you update (yeh, really)
* WHEN MATCHED/WHEN NOT MATCHED must be in fixed order, only

IBM DB2
---
MERGE INTO target [AS correlation-name]
USING [table-ref | subquery]
ON 
[WHEN MATCHED [AND ] THEN MergeUpdate | MergeDelete]
[WHEN NOT MATCHED [AND ] THEN MergeInsert | SignalClause]
[ELSE IGNORE]

Differences from SQL:2003 are
* Update and Insert have AND clauses on them (like WHERE...)
* DB2 allows multiple WHEN ... AND clauses
* DELETE is also a full-strength option, not part of the MergeUpdate
clause as it is in Oracle
* DB2 allows a SIGNAL statement, similar to RAISE
* ELSE IGNORE is an optional syntax, which does nothing

SQL Server 2008
---

MERGE [INTO] target [AS correlation-name]
USING [table-ref | subquery]
ON 
[WHEN MATCHED [AND ] THEN MergeUpdate | MergeDelete]
[WHEN NOT MATCHED [AND ] THEN MergeInsert]

Differences from SQL:2003 are
* Update and Insert have AND clauses on them (like WHERE...)
* DB2 allows multiple WHEN ... AND clauses
* DELETE is also a full-strength option, not part of the MergeUpdate
clause as it is in Oracle

SQL:2008


MERGE INTO target [AS correlation-name]
USING [table-ref | subquery]
ON 
[WHEN MATCHED [AND ] THEN MergeUpdate]
[WHEN NOT MATCHED [AND ] THEN MergeInsert]

Differences from SQL:2003 are
* Update and Insert have AND clauses on them (like WHERE...)
* Allows multiple WHEN ... AND clauses

Alternate Syntax


MySQL supports
* REPLACE INTO
* INSERT ... ON DUPLICATE KEY UPDATE ... 

Teradata supports
* UPDATE ... ELSE INSERT ...
* MERGE with an additional error logging clause

The Teradata and Oracle error logging clauses are very cute and I
suggest we do something similar for COPY, at least.

Proposed Syntax for PostgreSQL
==

MERGE INTO table [[AS] alias]
USING [table-ref | query]
ON join-condition
[WHEN MATCHED [AND condition] THEN MergeUpdate | DELETE]
[WHEN NOT MATCHED [AND condition] THEN MergeInsert]

MergeUpdate is
UPDATE SET { column = { expression | DEFAULT } |
  ( column [, ...] ) = ( { expression | DEFAULT } [, ...] ) }
[, ...]
(yes, there is no WHERE clause here)

MergeInsert is
INSERT [ ( column [, ...] ) ]
{ DEFAULT VALUES | VALUES ( { expression | DEFAULT } [, ...] )
[, ...]}
(no subquery allowed)


Notes and behaviours


* It is possible for concurrent MERGE statements to cause duplicate
INSERT violations because of a race condition between wh

Re: [HACKERS] How to submit a patch

2008-04-16 Thread Alvaro Herrera
Andrew Dunstan wrote:

> BTW, I don't see why the wiki can't pick up patches that were submitted  
> before it started.

Of course it can.  This one wasn't added initially because I didn't see it.

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

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


Re: [HACKERS] How to submit a patch

2008-04-16 Thread Joshua D. Drake
On Wed, 16 Apr 2008 14:24:34 -0400 (EDT)
Bruce Momjian <[EMAIL PROTECTED]> wrote:

> > If you are going to review the patch and apply or reply to the
> > author, at one point is it supposed to be on the wiki?
> 
> I have no idea.  If not dealt with, it will be on my web page once the
> next commit fest starts.
> 

This is the exact reason I posted the question :).


Joshua D. Drake

-- 
The PostgreSQL Company since 1997: http://www.commandprompt.com/ 
PostgreSQL Community Conference: http://www.postgresqlconference.org/
United States PostgreSQL Association: http://www.postgresql.us/
Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate



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


Re: [HACKERS] Commit fest queue

2008-04-16 Thread Bryce Nesbitt

Peter Eisentraut wrote:

Am Dienstag, 15. April 2008 schrieb Zdenek Kotala:
  

JavaDB and Firebird community use Jira


Jira had already been rejected many years ago because it is not open source.
  
Jira is also tremendously slow.  Not a good system when an individual 
has to move quickly through a lot of reports.


-Bryce


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


Re: [HACKERS] How to submit a patch

2008-04-16 Thread Heikki Linnakangas

Joshua D. Drake wrote:

On Wed, 16 Apr 2008 13:59:50 -0400
Andrew Dunstan <[EMAIL PROTECTED]> wrote:

If you posted it last month then it was too late for the commit-fest 
that started on March 1, IIRC, so the fact that you didn't get

feedback is hardly surprising - a commit-fest is like a
mini-feature-freeze.


O.k. then what happens at that point? It wasn't in the queue for May (I
had to add it)


There is different viewpoints on how it should happen. Hopefully the 
picture will be clearer after one or two more commit fests.


Based on my observations, there's basically three different workflows a 
patch can follow (assuming the patch gets committed in the end):


Workflow A:

1. You post patch to pgsql-patches
2. a committer picks it up immediately, and commits it.

Workflow B:

1. You post a patch to pgsql-patches
2. You add a link to the wiki page of the next commit fest
3. A committer picks up the patch from the wiki page, and commits it

Workflow C:

1. You post a patch to pgsql-patches
2. Bruce adds the patch to the unapplied patches queue after a while
3. At the beginning of the next commit fest, Alvaro (with the help from 
others, I hope) goes through the patches queue, and puts a link to the 
wiki page of the next commit fest

4. A committer picks up the patch from the wiki page, and commits it

--
  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] Race conditions in relcache load (again)

2008-04-16 Thread Tom Lane
I wrote:
> I realized that we failed to plug all the gaps of this type,
> because relcache.c contains *internal* cache load/reload operations
> that aren't protected.  In particular the LOAD_CRIT_INDEX macro
> calls invoke relcache load on indexes that aren't locked.  So they'd
> be at risk from a concurrent REINDEX or similar on those system
> indexes.  RelationReloadIndexInfo seems at risk as well.

On closer analysis, LOAD_CRIT_INDEX is clearly broken here, but the
other places I was worried about seem safe, because they are all
used to rebuild relcache entries that are being held open by
higher-level code; and the contract is that there should be a lock
protecting any open relcache entry.

I've committed a fix that adds locking to LOAD_CRIT_INDEX and adds
some comments about the other cases.

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] How to submit a patch

2008-04-16 Thread Andrew Dunstan



Bruce Momjian wrote:

Frankly, I am getting pretty tired of people complaining about what I am
doing.  Perhaps I should just stop and let everyone else deal with
things.  I have lots of things I would rather be doing.

This is one of the reasons I didn't want to add wiki maintenance to my
already full workload.  Instead I am having to field complaints.

  


I didn't mean to complain about anything. Personally, I'm in favor of 
reducing your workload.


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] How to submit a patch

2008-04-16 Thread Bruce Momjian
Joshua D. Drake wrote:
> On Wed, 16 Apr 2008 14:13:53 -0400 (EDT)
> Bruce Momjian <[EMAIL PROTECTED]> wrote:
> 
> > > > I review it and apply or reply to the author.  The wiki had
> > > > started being updated after your submission so this is a
> > > > transitionary phase.
> > > 
> > > Wait... apply where? The wiki? or to the tree?
> > 
> > Apply to CVS.
> > 
> 
> O.k. so patches may be applied through the development cycle but no
> patches will be accepted after -X- date for a current commit fest. So
> my question is:
> 
> If you are going to review the patch and apply or reply to the author,
> at one point is it supposed to be on the wiki?

I have no idea.  If not dealt with, it will be on my web page once the
next commit fest starts.

-- 
  Bruce Momjian  <[EMAIL PROTECTED]>http://momjian.us
  EnterpriseDB http://enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

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


Re: [HACKERS] How to submit a patch

2008-04-16 Thread Bruce Momjian
Andrew Dunstan wrote:
> > If you posted it last month then it was too late for the
> > commit-fest that started on March 1, IIRC, so the fact that you
> > didn't get feedback is hardly surprising - a commit-fest is like a
> > mini-feature-freeze.
> >   
>  O.k. then what happens at that point? It wasn't in the queue for
>  May (I had to add it)
>  
> >>> I review it and apply or reply to the author.  The wiki had started
> >>> being updated after your submission so this is a transitionary phase.
> >>>   
> >> Wait... apply where? The wiki? or to the tree?
> >> 
> >
> > Apply to CVS.
> >
> >   
> 
> Bruce, I know you don't mean this, but it reads like you are undertaking 
> to review and apply all patches.

Didn't you read "review it and apply or reply to the author".

> BTW, I don't see why the wiki can't pick up patches that were submitted 
> before it started.

True.

Frankly, I am getting pretty tired of people complaining about what I am
doing.  Perhaps I should just stop and let everyone else deal with
things.  I have lots of things I would rather be doing.

This is one of the reasons I didn't want to add wiki maintenance to my
already full workload.  Instead I am having to field complaints.

-- 
  Bruce Momjian  <[EMAIL PROTECTED]>http://momjian.us
  EnterpriseDB http://enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

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


Re: [HACKERS] Patch for Prevent pg_dump/pg_restore from being affected by statement_timeout

2008-04-16 Thread Joshua D. Drake
On Wed, 16 Apr 2008 21:20:17 +0300
Heikki Linnakangas <[EMAIL PROTECTED]> wrote:

> > My patch addresses all three, unless I am misunderstanding your
> > meaning. The patch does the following:
> > 
> > After connection with pg_dump it executes set statement_timeout = 0;
> > This fixed the pg_dump timeout issue.
> > 
> > It also writes set statement_timeout = 0 into the archive file,
> > which fixed pg_restore and psql.
> 
> Oh, ok, I misread the patch. Sorry for the noise.
> 

:)

Joshua D. Drake

-- 
The PostgreSQL Company since 1997: http://www.commandprompt.com/ 
PostgreSQL Community Conference: http://www.postgresqlconference.org/
United States PostgreSQL Association: http://www.postgresql.us/
Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate



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


Re: [HACKERS] How to submit a patch

2008-04-16 Thread Joshua D. Drake
On Wed, 16 Apr 2008 14:13:53 -0400 (EDT)
Bruce Momjian <[EMAIL PROTECTED]> wrote:

> > > I review it and apply or reply to the author.  The wiki had
> > > started being updated after your submission so this is a
> > > transitionary phase.
> > 
> > Wait... apply where? The wiki? or to the tree?
> 
> Apply to CVS.
> 

O.k. so patches may be applied through the development cycle but no
patches will be accepted after -X- date for a current commit fest. So
my question is:

If you are going to review the patch and apply or reply to the author,
at one point is it supposed to be on the wiki?

Joshua D. Drake


-- 
The PostgreSQL Company since 1997: http://www.commandprompt.com/ 
PostgreSQL Community Conference: http://www.postgresqlconference.org/
United States PostgreSQL Association: http://www.postgresql.us/
Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate



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


Re: [HACKERS] Patch for Prevent pg_dump/pg_restore from being affected by statement_timeout

2008-04-16 Thread Heikki Linnakangas

Joshua D. Drake wrote:

On Wed, 16 Apr 2008 21:04:17 +0300
Heikki Linnakangas <[EMAIL PROTECTED]> wrote:

To quote Tom:

I think we need to be careful to distinguish three situations:

* statement_timeout during pg_dump
* statement_timeout during pg_restore
* statement_timeout during psql reading a pg_dump script file
This patch addresses the third situation, but leaves open the 1st and 
the 2nd. IMO, we should set statement_timeout = 0 in them as well, 
unless someone comes up with plausible use case for using a non-zero 
statement_timeout.


My patch addresses all three, unless I am misunderstanding your
meaning. The patch does the following:

After connection with pg_dump it executes set statement_timeout = 0;
This fixed the pg_dump timeout issue.

It also writes set statement_timeout = 0 into the archive file, which
fixed pg_restore and psql.


Oh, ok, I misread the patch. Sorry for the noise.

--
  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] How to submit a patch

2008-04-16 Thread Andrew Dunstan



Bruce Momjian wrote:

Joshua D. Drake wrote:
  

On Wed, 16 Apr 2008 14:10:30 -0400 (EDT)
Bruce Momjian <[EMAIL PROTECTED]> wrote:



Joshua D. Drake wrote:
  

On Wed, 16 Apr 2008 13:59:50 -0400
Andrew Dunstan <[EMAIL PROTECTED]> wrote:



If you posted it last month then it was too late for the
commit-fest that started on March 1, IIRC, so the fact that you
didn't get feedback is hardly surprising - a commit-fest is like a
mini-feature-freeze.
  

O.k. then what happens at that point? It wasn't in the queue for
May (I had to add it)


I review it and apply or reply to the author.  The wiki had started
being updated after your submission so this is a transitionary phase.
  

Wait... apply where? The wiki? or to the tree?



Apply to CVS.

  


Bruce, I know you don't mean this, but it reads like you are undertaking 
to review and apply all patches.


BTW, I don't see why the wiki can't pick up patches that were submitted 
before it started.


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] How to submit a patch

2008-04-16 Thread Bruce Momjian
Joshua D. Drake wrote:
> On Wed, 16 Apr 2008 14:10:30 -0400 (EDT)
> Bruce Momjian <[EMAIL PROTECTED]> wrote:
> 
> > Joshua D. Drake wrote:
> > > On Wed, 16 Apr 2008 13:59:50 -0400
> > > Andrew Dunstan <[EMAIL PROTECTED]> wrote:
> > > 
> > > > 
> > > > If you posted it last month then it was too late for the
> > > > commit-fest that started on March 1, IIRC, so the fact that you
> > > > didn't get feedback is hardly surprising - a commit-fest is like a
> > > > mini-feature-freeze.
> > > 
> > > O.k. then what happens at that point? It wasn't in the queue for
> > > May (I had to add it)
> > 
> > I review it and apply or reply to the author.  The wiki had started
> > being updated after your submission so this is a transitionary phase.
> 
> Wait... apply where? The wiki? or to the tree?

Apply to CVS.

-- 
  Bruce Momjian  <[EMAIL PROTECTED]>http://momjian.us
  EnterpriseDB http://enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

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


Re: [HACKERS] How to submit a patch

2008-04-16 Thread Joshua D. Drake
On Wed, 16 Apr 2008 14:10:30 -0400 (EDT)
Bruce Momjian <[EMAIL PROTECTED]> wrote:

> Joshua D. Drake wrote:
> > On Wed, 16 Apr 2008 13:59:50 -0400
> > Andrew Dunstan <[EMAIL PROTECTED]> wrote:
> > 
> > > 
> > > If you posted it last month then it was too late for the
> > > commit-fest that started on March 1, IIRC, so the fact that you
> > > didn't get feedback is hardly surprising - a commit-fest is like a
> > > mini-feature-freeze.
> > 
> > O.k. then what happens at that point? It wasn't in the queue for
> > May (I had to add it)
> 
> I review it and apply or reply to the author.  The wiki had started
> being updated after your submission so this is a transitionary phase.

Wait... apply where? The wiki? or to the tree?

Joshua D. Drake 


-- 
The PostgreSQL Company since 1997: http://www.commandprompt.com/ 
PostgreSQL Community Conference: http://www.postgresqlconference.org/
United States PostgreSQL Association: http://www.postgresql.us/
Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate



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


Re: [HACKERS] How to submit a patch

2008-04-16 Thread Bruce Momjian
Joshua D. Drake wrote:
> On Wed, 16 Apr 2008 13:59:50 -0400
> Andrew Dunstan <[EMAIL PROTECTED]> wrote:
> 
> > 
> > If you posted it last month then it was too late for the commit-fest 
> > that started on March 1, IIRC, so the fact that you didn't get
> > feedback is hardly surprising - a commit-fest is like a
> > mini-feature-freeze.
> 
> O.k. then what happens at that point? It wasn't in the queue for May (I
> had to add it)

I review it and apply or reply to the author.  The wiki had started
being updated after your submission so this is a transitionary phase.

-- 
  Bruce Momjian  <[EMAIL PROTECTED]>http://momjian.us
  EnterpriseDB http://enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

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


Re: [HACKERS] Patch for Prevent pg_dump/pg_restore from being affected by statement_timeout

2008-04-16 Thread Joshua D. Drake
On Wed, 16 Apr 2008 21:04:17 +0300
Heikki Linnakangas <[EMAIL PROTECTED]> wrote:

> To quote Tom:
> > I think we need to be careful to distinguish three situations:
> > 
> > * statement_timeout during pg_dump
> > * statement_timeout during pg_restore
> > * statement_timeout during psql reading a pg_dump script file
> 
> This patch addresses the third situation, but leaves open the 1st and 
> the 2nd. IMO, we should set statement_timeout = 0 in them as well, 
> unless someone comes up with plausible use case for using a non-zero 
> statement_timeout.

My patch addresses all three, unless I am misunderstanding your
meaning. The patch does the following:

After connection with pg_dump it executes set statement_timeout = 0;
This fixed the pg_dump timeout issue.

It also writes set statement_timeout = 0 into the archive file, which
fixed pg_restore and psql.

> 
> Ps. If you want to save the committer a couple of minutes of valuable 
> time, you can fix the indentation to use tabs instead of spaces, and 
> remove the spurious whitespace change on the empty line.
> 

I can do that. Thanks for the feedback.

Joshua D. Drake

-- 
The PostgreSQL Company since 1997: http://www.commandprompt.com/ 
PostgreSQL Community Conference: http://www.postgresqlconference.org/
United States PostgreSQL Association: http://www.postgresql.us/
Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate



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


Re: [HACKERS] Patch for Prevent pg_dump/pg_restore from being affected by statement_timeout

2008-04-16 Thread Heikki Linnakangas

Joshua D. Drake wrote:

What is the feedback on this patch? Is there anything I need to do to
get it into the next commit fest?


Yes, go add it to the wiki page ;-):
http://wiki.postgresql.org/wiki/CommitFest:May

I agree that we should do that, but the thread on -hackers ("Autovacuum 
vs statement_timeout") wasn't totally conclusive. Greg Sabine Mullane 
and Peter Eisentraut argued that we shouldn't, but neither provided a 
plausible use case where a statement_timeout on restoring a dump would 
be useful. I'm thinking we should apply the patch unless someone comes 
up with one.


To quote Tom:

I think we need to be careful to distinguish three situations:

* statement_timeout during pg_dump
* statement_timeout during pg_restore
* statement_timeout during psql reading a pg_dump script file


This patch addresses the third situation, but leaves open the 1st and 
the 2nd. IMO, we should set statement_timeout = 0 in them as well, 
unless someone comes up with plausible use case for using a non-zero 
statement_timeout.


Ps. If you want to save the committer a couple of minutes of valuable 
time, you can fix the indentation to use tabs instead of spaces, and 
remove the spurious whitespace change on the empty line.


--
  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] How to submit a patch

2008-04-16 Thread Joshua D. Drake
On Wed, 16 Apr 2008 13:59:50 -0400
Andrew Dunstan <[EMAIL PROTECTED]> wrote:

> 
> If you posted it last month then it was too late for the commit-fest 
> that started on March 1, IIRC, so the fact that you didn't get
> feedback is hardly surprising - a commit-fest is like a
> mini-feature-freeze.

O.k. then what happens at that point? It wasn't in the queue for May (I
had to add it)

> 
> As for the rest, we are still feeling our way a bit, as should have
> been apparent from the list emails, so formal documentation is
> probably premature.

I assumed the docs would be subject to change but "something" that
gives one off and not often patch submitters a clue is probably useful.
It also allows us to actually discuss a pattern of behavior we are
starting versus bouncing through 50 threads trying to figure out what's
next.

Sincerely,

Joshua D. Drake


-- 
The PostgreSQL Company since 1997: http://www.commandprompt.com/ 
PostgreSQL Community Conference: http://www.postgresqlconference.org/
United States PostgreSQL Association: http://www.postgresql.us/
Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate



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


Re: [HACKERS] How to submit a patch

2008-04-16 Thread Andrew Dunstan



Joshua D. Drake wrote:

Hello,

Could someone break out exactly what the process is "now" for
submitting a patch? Last month I sent a patch for pg_dump which never
got feedback (at least on thread). I just asked and alvaro asked me to
add it to the commitfest page. Which I have done but I think we need to
known all the steps and get it documented.

I have:

post to pgsql-hackers with idea, take feedback, code to consensus
post to pgsql-patches, await feedback
 ...
?
  


If you posted it last month then it was too late for the commit-fest 
that started on March 1, IIRC, so the fact that you didn't get feedback 
is hardly surprising - a commit-fest is like a mini-feature-freeze.


As for the rest, we are still feeling our way a bit, as should have been 
apparent from the list emails, so formal documentation is probably 
premature.


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


[HACKERS] How to submit a patch

2008-04-16 Thread Joshua D. Drake
Hello,

Could someone break out exactly what the process is "now" for
submitting a patch? Last month I sent a patch for pg_dump which never
got feedback (at least on thread). I just asked and alvaro asked me to
add it to the commitfest page. Which I have done but I think we need to
known all the steps and get it documented.

I have:

post to pgsql-hackers with idea, take feedback, code to consensus
post to pgsql-patches, await feedback
 ...
?

Joshua D. Drake

-- 
The PostgreSQL Company since 1997: http://www.commandprompt.com/ 
PostgreSQL Community Conference: http://www.postgresqlconference.org/
United States PostgreSQL Association: http://www.postgresql.us/
Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate



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


Re: [HACKERS] Patch for Prevent pg_dump/pg_restore from being affected by statement_timeout

2008-04-16 Thread Joshua D. Drake
On Wed, 16 Apr 2008 13:36:50 -0400
Alvaro Herrera <[EMAIL PROTECTED]> wrote:

> Joshua D. Drake wrote:
> 
> > What is the feedback on this patch? Is there anything I need to do
> > to get it into the next commit fest?
> 
> Please add it to the commitfest wiki page.
> 
> http://wiki.postgresql.org/wiki/CommitFest:May
> 

Done: http://wiki.postgresql.org/wiki/CommitFest:May#Pending_patches

Joshua D. Drake


-- 
The PostgreSQL Company since 1997: http://www.commandprompt.com/ 
PostgreSQL Community Conference: http://www.postgresqlconference.org/
United States PostgreSQL Association: http://www.postgresql.us/
Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate



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


Re: [HACKERS] Patch for Prevent pg_dump/pg_restore from being affected by statement_timeout

2008-04-16 Thread Alvaro Herrera
Joshua D. Drake wrote:

> What is the feedback on this patch? Is there anything I need to do to
> get it into the next commit fest?

Please add it to the commitfest wiki page.

http://wiki.postgresql.org/wiki/CommitFest:May

-- 
Alvaro Herrerahttp://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

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


Re: [HACKERS] Lessons from commit fest

2008-04-16 Thread Alvaro Herrera
Bruce Momjian wrote:
> Chris Browne wrote:

> > Would it be a terrible idea to...
> > 
> > - Draw the indent code from NetBSD into src/tools/pgindent
> > - Build it _in place_ inside the code tree (e.g. - don't assume 
> >   it will get installed in /usr/local/bin)
> > - Thus have the ability to run it in place?
> 
> Yes, but it bloats our code and people still need to generate the
> typedefs and follow the instructions.

I suggested to you awhile back that we could keep a typedef file on the
repository, saving one step.

I don't see the problem with keeping the BSD indent for now, until we
figure out the set of options GNU indent needs to behave properly.  It's
not all that much code, after all.


> The other problem is if they run it on a file they have modified, it
> is going to adjust places they didn't touch, thereby making the patch
> harder to review.

That should be rare if pgindent is run frequently, I think.  (And
anyway, there are tools to remove unwanted hunks from patches if the
author so desires.)

I guess the point here is that pgindent is a tool that should _help_
developers.  Making it harder to run is not helping anyone.

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

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


Re: [HACKERS] Patch for Prevent pg_dump/pg_restore from being affected by statement_timeout

2008-04-16 Thread Joshua D. Drake
On Tue, 11 Mar 2008 11:07:27 -0700
"Joshua D. Drake" <[EMAIL PROTECTED]> wrote:

> -BEGIN PGP SIGNED MESSAGE-
> Hash: SHA1
> 
> On Tue, 11 Mar 2008 10:46:23 -0700
> "Joshua D. Drake" <[EMAIL PROTECTED]> wrote:
> 
> And the -c version :) (thanks bruce)
> 
> Joshua D. Drake
> 

What is the feedback on this patch? Is there anything I need to do to
get it into the next commit fest?

Joshua D. Drake


-- 
The PostgreSQL Company since 1997: http://www.commandprompt.com/ 
PostgreSQL Community Conference: http://www.postgresqlconference.org/
United States PostgreSQL Association: http://www.postgresql.us/
Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate



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


Re: [HACKERS] Lessons from commit fest

2008-04-16 Thread Bruce Momjian
Chris Browne wrote:
> [EMAIL PROTECTED] (Bruce Momjian) writes:
> > Magnus Hagander wrote:
> >> > And I think adopting surrounding naming, commeting, coding conventions
> >> > should come naturally as it can aide in copy-pasting too :)
> >> 
> >> I think pg_indent has to be made a lot more portable and easy to use
> >> before that can happen :-) I've run it once or twice on linux machines,
> >> and it comes out with huge changes compared to what Bruce gets on his
> >> machine. Other times, it doesn't :-) So yeah, it could be that it just
> >> needs to be made easier to use, because I may certainly have done
> >> something wrong.
> >
> > Agreed, pgindent is too cumbersome to require patch submitters to use. 
> > One idea would be to allow C files to be emailed and the indented
> > version automatically returned via email.
> 
> Would it be a terrible idea to...
> 
> - Draw the indent code from NetBSD into src/tools/pgindent
> - Build it _in place_ inside the code tree (e.g. - don't assume 
>   it will get installed in /usr/local/bin)
> - Thus have the ability to run it in place?

Yes, but it bloats our code and people still need to generate the
typedefs and follow the instructions.  The other problem is if they run
it on a file they have modified, it is going to adjust places they
didn't touch, thereby making the patch harder to review.

-- 
  Bruce Momjian  <[EMAIL PROTECTED]>http://momjian.us
  EnterpriseDB http://enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

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


Re: [HACKERS] [PATCHES] Avahi support for Postgresql

2008-04-16 Thread Bruce Momjian

Added to TODO:

* Implement the non-threaded Avahi service discovery protocol

  http://archives.postgresql.org/pgsql-hackers/2008-02/msg00939.php
  http://archives.postgresql.org/pgsql-patches/2008-02/msg00097.php
  http://archives.postgresql.org/pgsql-hackers/2008-03/msg01211.php
  http://archives.postgresql.org/pgsql-patches/2008-04/msg1.php


---

Alvaro Herrera wrote:
> Bruce Momjian wrote:
> > Alvaro Herrera wrote:
> > > Mathias Hasselmann wrote:
> > > 
> > > > The patches were in my initial mail, but now I've also uploaded them to
> > > > my personal site for convenience:
> > > > 
> > > > http://taschenorakel.de/files/pgsql-avahi-support/
> > > 
> > > Hmm, a quick look at the third patch reveals that it is using the
> > > "threaded" Avahi client.  That's a showstopper.  Please consider using
> > > some other approach -- writing our own handlers for AvahiPoll would seem
> > > apropos.
> > 
> > Based on the threading usage in this patch, I assume it is rejected, and
> > that we don't want a TODO item for this.
> 
> This particular patch should be rejected, but we certainly do need a
> TODO item IMHO.
> 
> -- 
> Alvaro Herrerahttp://www.CommandPrompt.com/
> PostgreSQL Replication, Consulting, Custom Development, 24x7 support

-- 
  Bruce Momjian  <[EMAIL PROTECTED]>http://momjian.us
  EnterpriseDB http://enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

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


Re: [HACKERS] Lessons from commit fest

2008-04-16 Thread Chris Browne
[EMAIL PROTECTED] (Bruce Momjian) writes:
> Magnus Hagander wrote:
>> > And I think adopting surrounding naming, commeting, coding conventions
>> > should come naturally as it can aide in copy-pasting too :)
>> 
>> I think pg_indent has to be made a lot more portable and easy to use
>> before that can happen :-) I've run it once or twice on linux machines,
>> and it comes out with huge changes compared to what Bruce gets on his
>> machine. Other times, it doesn't :-) So yeah, it could be that it just
>> needs to be made easier to use, because I may certainly have done
>> something wrong.
>
> Agreed, pgindent is too cumbersome to require patch submitters to use. 
> One idea would be to allow C files to be emailed and the indented
> version automatically returned via email.

Would it be a terrible idea to...

- Draw the indent code from NetBSD into src/tools/pgindent
- Build it _in place_ inside the code tree (e.g. - don't assume 
  it will get installed in /usr/local/bin)
- Thus have the ability to run it in place?
-- 
let name="cbbrowne" and tld="linuxfinances.info" in name ^ "@" ^ tld;;
http://cbbrowne.com/info/lisp.html
"It worked about as well as sticking a blender in the middle of a lime
plantation and hoping they'll make margaritas out of themselves."
-- Frederick J. Polsky v1.0

-- 
Sent 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_terminate_backend() issues

2008-04-16 Thread Bruce Momjian
Tom Lane wrote:
> Bruce Momjian <[EMAIL PROTECTED]> writes:
> > Tom Lane wrote:
> >> The closest thing I can think of to an automated test is to run repeated
> >> sets of the parallel regression tests, and each time SIGTERM a randomly
> >> chosen backend at a randomly chosen time.  Then see if anything "funny"
> 
> > Yep, that was my plan, plus running the parallel regression tests you
> > get the possibility of >2 backends.
> 
> I was intentionally suggesting only one kill per test cycle.  Multiple
> kills will probably create an O(N^2) explosion in the set of possible
> downstream-failure deltas.  I doubt you'd really get any improvement
> in testing coverage to justify the much larger amount of hand validation
> needed.

No, the point is that you have >2 backends to choose from to kill.

-- 
  Bruce Momjian  <[EMAIL PROTECTED]>http://momjian.us
  EnterpriseDB http://enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

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


Re: [HACKERS] Lessons from commit fest

2008-04-16 Thread Bruce Momjian
Chris Browne wrote:
> [EMAIL PROTECTED] (Tom Lane) writes:
> > Magnus Hagander <[EMAIL PROTECTED]> writes:
> >> I think pg_indent has to be made a lot more portable and easy to use
> >> before that can happen :-) I've run it once or twice on linux machines,
> >> and it comes out with huge changes compared to what Bruce gets on his
> >> machine.
> >
> > Yeah, I've had no luck with it either.
> >
> > Every so often there are discussions of going over to GNU indent
> > instead.  Presumably that would solve the portability problem.
> > The last time we tried it (which was a long time ago) it seemed
> > to have too many bugs and idiosyncrasies of its own, but it would
> > be worth a fresh round of experimenting IMHO.
> 
> Well, GNU indent is now on version 2.2.9, and has evidently addressed
> *some* problems with it.
> 
> Unfortunately, the pgindent README does not actually specify what any
> of the actual problems with GNU indent are, thus making it pretty much
> impossible to evaluate whether or not any of the subsequent releases
> might have addressed any of those problems.
> 
> I doubt that the pgindent issues have been addressed.

What I did was to run pgindent on the source tree, make a copy, then run
GNU indent on it and diff -r.  I can try that test again in the next few
months.

-- 
  Bruce Momjian  <[EMAIL PROTECTED]>http://momjian.us
  EnterpriseDB http://enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

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


Re: [HACKERS] Lessons from commit fest

2008-04-16 Thread Joshua D. Drake
On Wed, 16 Apr 2008 12:36:50 -0400 (EDT)
Bruce Momjian <[EMAIL PROTECTED]> wrote:

> 
> If the patch submitters hasn't read the developers' FAQ, we assume
> they are not interested in improving the style of their patch.

I think that point is fairly flawed in consideration. I know for a fact
that I have had to repeat even to long time contributors source
references for information even though they new about it previously.
Yourself included.

People get in a hurry, they should be reminded with reference.

JD, thanks for the patch but you missed foo.. please check 1.5 of the
Developers FAQ.

Sincerely,

Joshua D. Drake




-- 
The PostgreSQL Company since 1997: http://www.commandprompt.com/ 
PostgreSQL Community Conference: http://www.postgresqlconference.org/
United States PostgreSQL Association: http://www.postgresql.us/
Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate



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


Re: [HACKERS] Lessons from commit fest

2008-04-16 Thread Tom Lane
Chris Browne <[EMAIL PROTECTED]> writes:
> [EMAIL PROTECTED] (Tom Lane) writes:
>> Every so often there are discussions of going over to GNU indent
>> instead.  Presumably that would solve the portability problem.
>> The last time we tried it (which was a long time ago) it seemed
>> to have too many bugs and idiosyncrasies of its own, but it would
>> be worth a fresh round of experimenting IMHO.

> Well, GNU indent is now on version 2.2.9, and has evidently addressed
> *some* problems with it.

> Unfortunately, the pgindent README does not actually specify what any
> of the actual problems with GNU indent are, thus making it pretty much
> impossible to evaluate whether or not any of the subsequent releases
> might have addressed any of those problems.

This wouldn't be hard for someone to investigate.  Check out our CVS
from immediately after the last pgindent run.  Run GNU indent on it.
See what options result in the smallest diff, and what the diff looks
like in terms of making the code look nicer or less nice.

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] pg_terminate_backend() issues

2008-04-16 Thread Tom Lane
Bruce Momjian <[EMAIL PROTECTED]> writes:
> Tom Lane wrote:
>> The closest thing I can think of to an automated test is to run repeated
>> sets of the parallel regression tests, and each time SIGTERM a randomly
>> chosen backend at a randomly chosen time.  Then see if anything "funny"

> Yep, that was my plan, plus running the parallel regression tests you
> get the possibility of >2 backends.

I was intentionally suggesting only one kill per test cycle.  Multiple
kills will probably create an O(N^2) explosion in the set of possible
downstream-failure deltas.  I doubt you'd really get any improvement
in testing coverage to justify the much larger amount of hand validation
needed.

It also strikes me that you could make some simple alterations to the
regression tests to reduce the set of observable downstream deltas.
For example, anyplace where a test loads a table with successive INSERTs
and that table is used by later tests, wrap the INSERT sequence with
BEGIN/END.  Then there is only one possible downstream delta (empty
table) and not N different possibilities for an N-row table.

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] Lessons from commit fest

2008-04-16 Thread Chris Browne
[EMAIL PROTECTED] (Tom Lane) writes:
> Magnus Hagander <[EMAIL PROTECTED]> writes:
>> I think pg_indent has to be made a lot more portable and easy to use
>> before that can happen :-) I've run it once or twice on linux machines,
>> and it comes out with huge changes compared to what Bruce gets on his
>> machine.
>
> Yeah, I've had no luck with it either.
>
> Every so often there are discussions of going over to GNU indent
> instead.  Presumably that would solve the portability problem.
> The last time we tried it (which was a long time ago) it seemed
> to have too many bugs and idiosyncrasies of its own, but it would
> be worth a fresh round of experimenting IMHO.

Well, GNU indent is now on version 2.2.9, and has evidently addressed
*some* problems with it.

Unfortunately, the pgindent README does not actually specify what any
of the actual problems with GNU indent are, thus making it pretty much
impossible to evaluate whether or not any of the subsequent releases
might have addressed any of those problems.

I doubt that the pgindent issues have been addressed.
-- 
output = reverse("ofni.sesabatadxunil" "@" "enworbbc")
http://linuxfinances.info/info/sgml.html
"In elementary school, in case of  fire you have to line up quietly in
a single  file line from  smallest to tallest.  What is the  logic? Do
tall people burn slower?" -- Warren Hutcherson

-- 
Sent 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_terminate_backend() issues

2008-04-16 Thread Bruce Momjian
Tom Lane wrote:
> Magnus Hagander <[EMAIL PROTECTED]> writes:
> > Tom Lane wrote:
> >> I'm willing to enable a SIGTERM-based pg_terminate_backend for 8.4
> >> if there is some reasonable amount of testing done during this
> >> development cycle to try to expose any problems.
> 
> > If someone can come up with an automated script to do this kind of
> > testing, I can commit a VM or three to running this 24/7 for a month,
> > easily... But I don't trust myself in coming up with a test-case that's
> > good enough :-P
> 
> The closest thing I can think of to an automated test is to run repeated
> sets of the parallel regression tests, and each time SIGTERM a randomly
> chosen backend at a randomly chosen time.  Then see if anything "funny"

Yep, that was my plan, plus running the parallel regression tests you
get the possibility of >2 backends.

> happens.  The hard part here is distinguishing expected from unexpected
> regression outputs, especially in view of the fact that some of the
> tests depend on database contents set up by earlier tests.

Oh, that is a problem.  I was going to get the typical errors, sort them
and put them in a file.  Then when I run the test, I sort the log again
and use diff | grep '<' to see an differences.  If there are cases that
generate new errors on a previous failure, I will have to add that
output too.

> I'm thinking that you could automatically discard the regression diff
> for the specific test that got SIGTERM'd, as long as it looked like
> the normal output up to the point where the "terminated by
> administrator" error appears.  Then what you'd have is the potential for
> downstream failures due to things not being created, which *should* fall
> into a fairly stylized set of possible diffs.  So get the script to
> throw away any diffs that exactly match ones seen previously.  Run it
> for awhile, and then hand-validate the set of diffs that it's saved
> ... or if any of 'em look funny, report.

Yep, see above.

> One gotcha I can think of is that killing the prepared_xacts test
> can leave you with open 2PC transactions, which will interfere with
> starting the next cycle of the tests (you have to kill them before you
> can dropdb).  But you could add a "rollback prepared" to the driver
> script to clean out any uncommitted prepared xact.

Good idea.

-- 
  Bruce Momjian  <[EMAIL PROTECTED]>http://momjian.us
  EnterpriseDB http://enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

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


Re: [HACKERS] pg_terminate_backend() issues

2008-04-16 Thread Tom Lane
Magnus Hagander <[EMAIL PROTECTED]> writes:
> Tom Lane wrote:
>> I'm willing to enable a SIGTERM-based pg_terminate_backend for 8.4
>> if there is some reasonable amount of testing done during this
>> development cycle to try to expose any problems.

> If someone can come up with an automated script to do this kind of
> testing, I can commit a VM or three to running this 24/7 for a month,
> easily... But I don't trust myself in coming up with a test-case that's
> good enough :-P

The closest thing I can think of to an automated test is to run repeated
sets of the parallel regression tests, and each time SIGTERM a randomly
chosen backend at a randomly chosen time.  Then see if anything "funny"
happens.  The hard part here is distinguishing expected from unexpected
regression outputs, especially in view of the fact that some of the
tests depend on database contents set up by earlier tests.

I'm thinking that you could automatically discard the regression diff
for the specific test that got SIGTERM'd, as long as it looked like
the normal output up to the point where the "terminated by
administrator" error appears.  Then what you'd have is the potential for
downstream failures due to things not being created, which *should* fall
into a fairly stylized set of possible diffs.  So get the script to
throw away any diffs that exactly match ones seen previously.  Run it
for awhile, and then hand-validate the set of diffs that it's saved
... or if any of 'em look funny, report.

One gotcha I can think of is that killing the prepared_xacts test
can leave you with open 2PC transactions, which will interfere with
starting the next cycle of the tests (you have to kill them before you
can dropdb).  But you could add a "rollback prepared" to the driver
script to clean out any uncommitted prepared xact.

Whether this is workable or not depends on the size of the set of
"expected" downstream-failure diffs.  My gut feeling from many years of
watching regression test crashes is that it'd be large but not
completely impractical to look through by hand.

I haven't time to write something like that myself, but offhand it seems
like it could be done without more than a day or so's work, especially
if you start from the buildfarm infrastructure.

BTW, don't forget to include autovac workers in the set of SIGTERM
target candidates.

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] Lessons from commit fest

2008-04-16 Thread Bruce Momjian
Magnus Hagander wrote:
> > And I think adopting surrounding naming, commeting, coding conventions
> > should come naturally as it can aide in copy-pasting too :)
> 
> I think pg_indent has to be made a lot more portable and easy to use
> before that can happen :-) I've run it once or twice on linux machines,
> and it comes out with huge changes compared to what Bruce gets on his
> machine. Other times, it doesn't :-) So yeah, it could be that it just
> needs to be made easier to use, because I may certainly have done
> something wrong.

Agreed, pgindent is too cumbersome to require patch submitters to use. 
One idea would be to allow C files to be emailed and the indented
version automatically returned via email.

-- 
  Bruce Momjian  <[EMAIL PROTECTED]>http://momjian.us
  EnterpriseDB http://enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

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


Re: [HACKERS] Lessons from commit fest

2008-04-16 Thread Bruce Momjian
Brendan Jurd wrote:
> >  The idea that we "fix" stylistic issues on the fly is not sustainable.
> >  We should offer help and mentorship to new patch submitters in all
> >  areas (including stylistic) but they should do the work. It is the only
> >  way we will mold them to submit patches in the proper way.
> >
> 
> I agree.  As a submitter I would much rather get an email saying e.g.
> "Hey, your patch is nice but the code style sticks out like a sore
> thumb.  Please adopt surrounding naming convention and fix your
> indentation per the rules at [link]." than have it fixed silently on
> its way to being committed.
> 
> With the former I learn something and get to improve my own work.
> With the latter, my next patch is probably going to have the exact
> same problem, which is in the long term just making extra work for the
> reviewers.

If the patch submitters hasn't read the developers' FAQ, we assume they
are not interested in improving the style of their patch.

-- 
  Bruce Momjian  <[EMAIL PROTECTED]>http://momjian.us
  EnterpriseDB http://enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

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


Re: [HACKERS] pg_terminate_backend() issues

2008-04-16 Thread Bruce Momjian
Tom Lane wrote:
> Bruce Momjian <[EMAIL PROTECTED]> writes:
> > I am starting to think we need to analyze the source code rather than
> > testing, because of what we are testing for.
> 
> I was thinking about that a bit more, in connection with trying to
> verbalize why I don't like your patch ;-)
> 
> The fundamental assumption here is that we think that proc_exit() from
> the main loop of PostgresMain() should be safe, because that's exercised
> anytime a client quits or dies unexpectedly, and so it should be a
> well-tested path.  What is lacking such test coverage is the many code
> paths that start proc_exit() from any random CHECK_FOR_INTERRUPTS()
> point in the backend.  Some of those points might be (in fact are known
> to be, as of today) holding locks or otherwise in a state that prevents
> a clean proc_exit().
> 
> Your patch proposes to replace that code path by one that fakes a
> QueryCancel error and then does proc_exit() once control reaches the
> outer level.  While that certainly *should* work (ignoring the question
> of whether control gets to the outer level in any reasonable amount of
> time), it's a bit of a stretch to claim that it's a well-tested case.
> At the moment it would only arise if a QueryCancel interrupt arrived at
> approximately the same time that the client died or sent a disconnect
> message, which is surely far less probable than client death by itself.
> So I don't buy the argument that this is a better-tested path, and
> I definitely don't want to introduce new complexity in order to make it
> happen like that.

I would like my use of SIGINT to be like someone pressing ^C and then \q
in psql, but I can see that the SIGINT might be delivered in places
where libpq would not deliver it, like during shutdown.  Hopefully that
could be addressed, but I agree using SIGTERM is better and more useful.

> Now, as to whether a SIGTERM-style exit is safe.  With the exception
> of the PG_CATCH issues that we already know about, any other case seems
> like a clear coding error: you'd have to be doing something that you
> know could throw an error, without having made provision to clean up
> whatever unclean state you are in.  It'd be nice to think that we
> haven't been that silly anywhere.  Experience however teaches that such
> an assumption is hubris.
> 
> I think that the way forward is to fix the PG_CATCH issues (which I will
> try to get done later this week) and then get someone to mount a serious
> effort to break it, as outlined in previous discussions:
> http://archives.postgresql.org/pgsql-hackers/2006-08/msg00174.php

I was thinking of running two parallel regression tests and killing one
at random and see if the others complete.  One problem is that the
regression tests issue a lot of server error messages so somehow I would
need to filter out the normal errors, which I think I could do.

Magnus has offered to test too.

> I'm willing to enable a SIGTERM-based pg_terminate_backend for 8.4
> if there is some reasonable amount of testing done during this
> development cycle to try to expose any problems.  If no one is willing
> to do any such testing, then as far as I'm concerned there is no market
> for the feature and we should drop it from TODO.

Agreed, but the feature is going to be asked for again soon so we would
probably have to put it on the features we don't want, and that is going
to look pretty odd.

-- 
  Bruce Momjian  <[EMAIL PROTECTED]>http://momjian.us
  EnterpriseDB http://enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

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


[HACKERS] /etc/init.d/postgresql status error

2008-04-16 Thread Gaetano Mendola
I have just installed the rpm found at:

http://www.postgresql.org/ftp/binary/v8.2.7/linux/rpms/redhat/rhel-4-i386/

and the status option generates an error:

$ /etc/init.d/postgresql status
pidof: invalid options on command line!

pidof: invalid options on command line!

-p is stopped


I was able to fix it in the following mode:

  status)
#   status -p /var/run/postmaster.${PGPORT}.pid
status postmaster
script_result=$?
;;

Commented the original line and replaced with the one just below.

Regards
Gaetano Mendola


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


Re: [HACKERS] Improving planner variable handling

2008-04-16 Thread Gregory Stark

"Tom Lane" <[EMAIL PROTECTED]> writes:

> Heikki Linnakangas <[EMAIL PROTECTED]> writes:
>> I wonder if this would help to clean up the equivalence class hacks in 
>> Greg's ordered append patch?
>
> Pretty hard to say at this early stage, 

I've started to have some doubts myself about stuffing all these vars into the
same equivalence class. My concern here is actually performance. I'm concerned
that if you use a partitioned table within a larger query once we've generated
the paths for the partitioned table we don't want to then carry that baggage
of hundreds or possibly thousands of variables for planning the whole rest of
the query above that rel. They'll never be relevant again after that.

The more these ideas start fitting together in my head the more I think Tom's
original instinct would perform better. I was concerned that pulling up and
pushing down pathkey lists sounded like a lot of extra work. But at least you
would only have to pay that for that one level, not carry it along and pay for
it for the rest of the planning.

-- 
  Gregory Stark
  EnterpriseDB  http://www.enterprisedb.com
  Get trained by Bruce Momjian - ask me about EnterpriseDB's PostgreSQL 
training!

-- 
Sent 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_terminate_backend() issues

2008-04-16 Thread Tom Lane
Gregory Stark <[EMAIL PROTECTED]> writes:
> No, I meant the earlier patch which you rejected with the flag in MyProc. I
> realize there were other issues but the initial concern was that it wouldn't
> respond promptly because it would wait for CHECK_FOR_INTERRUPTS.

No, that's not the concern in the least.  The concern is that something
could trap the attempted throwing of the error *after*
CHECK_FOR_INTERRUPTS has noticed the signal.

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: [PATCHES] [HACKERS] Text <-> C string

2008-04-16 Thread Andrew Dunstan



Tom Lane wrote:

"Brendan Jurd" <[EMAIL PROTECTED]> writes:
  

If we don't want to meddle with xmltype/bytea/VarChar at all, we'll
have to revert those changes, and I'll have to seriously scale back
the cleanup patch I was about to post.
  


  

Still not sure where we stand on the above.  To cast, or not to cast?



I dunno.  I know there was previously some handwaving about representing
XML values in some more intelligent fashion than a plain text string,
but I have no idea if anyone is likely to do anything about it in the
foreseeable future.


  


It seems very unlikely to me.

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] Improving planner variable handling

2008-04-16 Thread Tom Lane
Heikki Linnakangas <[EMAIL PROTECTED]> writes:
> I wonder if this would help to clean up the equivalence class hacks in 
> Greg's ordered append patch?

Pretty hard to say at this early stage, but the more I think about it
the more I like the idea of never using the same Var to represent two
values that aren't guaranteed equal.  The maybe-its-null business has
been a thorn in the side of any sort of close semantic analysis for
a long time, but somehow I never perceived what a bogus idea that was.
I'm almost more excited about that than about fixing the original
problem ...

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] DROP DATABASE vs patch to not remove files right away

2008-04-16 Thread Heikki Linnakangas

Tom Lane wrote:

Heikki Linnakangas <[EMAIL PROTECTED]> writes:
Florian suggested a scheme where the xid and epoch is embedded in the 
filename, but that's unnecessarily complex. We could just make 
relfilenode a 64-bit integer. 2^64 should be enough for everyone.


Doesn't fix the problem unless DB and TS OIDs become int64 too;


Remember what the original scenario was:

1. DROP TABLE foo;
2. BEGIN; CREATE TABLE bar; COPY TO bar ...; COMMIT -- bar gets the same 
relfilenode as "foo", by chance

3. 
4. WAL replay of "DROP TABLE foo" deletes the data inserted at step 2. 
Because the table was created in the same transaction, we skipped WAL 
logging the inserts, and the data is lost.


Even if we can get a collision in DB or tablespace OIDs, we can't get a 
collision at step 2 if we don't reuse relfilenodes.



in fact, given that we generate relfilenodes off the OID counter,
it's difficult to see how you do this without making *all* OIDs
64-bit.


By generating them off a new 64-bit counter instead of the OID counter. 
Or by extending the OID counter to 64-bits, but only using the lower 64 
bits for OIDs.



Plus you're assuming that the machine has working 64-bit ints.
There's a large difference in my mind between saying "bigint
doesn't work right if you don't have working int64" and "we don't
guarantee the safety of your data if you don't have int64".
I'm not prepared to rip out the non-collision code until such
time as we irrevocably refuse to build on machines without int64.


Hmm. We could make it a struct of two int4s if we have to, couldn't we?

--
  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: [PATCHES] [HACKERS] Text <-> C string

2008-04-16 Thread Tom Lane
"Brendan Jurd" <[EMAIL PROTECTED]> writes:
>> If we don't want to meddle with xmltype/bytea/VarChar at all, we'll
>> have to revert those changes, and I'll have to seriously scale back
>> the cleanup patch I was about to post.

> Still not sure where we stand on the above.  To cast, or not to cast?

I dunno.  I know there was previously some handwaving about representing
XML values in some more intelligent fashion than a plain text string,
but I have no idea if anyone is likely to do anything about it in the
foreseeable future.

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] pg_terminate_backend() issues

2008-04-16 Thread Gregory Stark
"Tom Lane" <[EMAIL PROTECTED]> writes:

> Gregory Stark <[EMAIL PROTECTED]> writes:
>> "Tom Lane" <[EMAIL PROTECTED]> writes:
>>> No, we wouldn't, because a SIGTERM can only actually fire at a
>>> CHECK_FOR_INTERRUPTS() call.  You'd just need to be sure there wasn't
>>> one in the cleanup code.
>
>> Wait, huh? In that case I don't see what advantage any of this has over
>> Bruce's patch. And his approach seemed a lot more robust.
>
> Maybe I missed something, but I thought he was just proposing some
> macro syntactic sugar over the same code that I described.

No, I meant the earlier patch which you rejected with the flag in MyProc. I
realize there were other issues but the initial concern was that it wouldn't
respond promptly because it would wait for CHECK_FOR_INTERRUPTS. But if
sigterm was never handled except at a CHECK_FOR_INTERRUPTS then that was never
a factor.

-- 
  Gregory Stark
  EnterpriseDB  http://www.enterprisedb.com
  Ask me about EnterpriseDB's Slony Replication support!

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


Re: [HACKERS] pg_terminate_backend() issues

2008-04-16 Thread Tom Lane
Gregory Stark <[EMAIL PROTECTED]> writes:
> "Tom Lane" <[EMAIL PROTECTED]> writes:
>> No, we wouldn't, because a SIGTERM can only actually fire at a
>> CHECK_FOR_INTERRUPTS() call.  You'd just need to be sure there wasn't
>> one in the cleanup code.

> Wait, huh? In that case I don't see what advantage any of this has over
> Bruce's patch. And his approach seemed a lot more robust.

Maybe I missed something, but I thought he was just proposing some
macro syntactic sugar over the same code that I described.

I'm not against syntactic sugar, but it doesn't seem to be totally
clean; I think you would need two repetitions of the function and
arg anyway:

PG_ENSURE_ERROR_CLEANUP(cleanup_function, arg);
{
... risky code here ...
}
PG_END_ENSURE_ERROR_CLEANUP(cleanup_function, arg);

because both the "before" and "after" parts need to know the function
name and the arg to pass it.  The only way to avoid duplication would be
if the whole controlled code block was actually an argument to the
macro:

PG_ENSURE_ERROR_CLEANUP(cleanup_function, arg,
... risky code here ...
);

but there are very good reasons not to do that when the controlled
code can be large; for instance that #if can't be used portably
in a macro argument.

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   >