Re: [HACKERS] MERGE SQL Statement
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
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
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
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
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
"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
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
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
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
"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
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
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
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
"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
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
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
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
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
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
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
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
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
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
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
[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
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
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
-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
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
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
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
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
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
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
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
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
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
-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
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
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
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
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
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
-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
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
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
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
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
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
-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
[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
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
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
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
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
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
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)
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
[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
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
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
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
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
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
[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
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
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
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
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
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
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
"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
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
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
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
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
"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
"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
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