Re: [HACKERS] SET CONSTRAINTS todo
I see what went wrong in my example. Unique constraints must have unique names since they create an index. I'll try again, sorry for the noise. --Dan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] SET CONSTRAINTS todo
I wanted to work on this todo item and I have a few questions about the semantics of it. Essentially, it is not possible to have more than one relname for a constraint, even though the comment in trigger.c says otherwise. I have used this code to test this: CREATE TABLE products ( product_no integer CONSTRAINT must_be_different UNIQUE DEFERRABLE, name text, price numeric ); CREATE TABLE products2 ( product_no integer CONSTRAINT must_be_different UNIQUE DEFERRABLE, name text, price numeric ); which results in the following error: ERROR: relation must_be_different already exists Therefore prefixing them with a table name does not seem to make sense. Additionally, there is already the feature of prefixing the constraint relname with a schema to limit the search space. Is the intention of the todo to allow the user to specify a tablename which limits the search path to that table's schema or is the feature to extend constraints to allow per table naming. In other words, would the feature allow multiple constraints of the same name in a schema since they would be table specific? --Dan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] CTE containing ambiguous columns
Can't you disambiguate it using a column list on beings? Sure, after I figured out what the real problem was. Maybe I'm a dope, but when I get an error cursor pointed at an ambiguous column reference, my thought is oh, I need to qualify that reference - not oh, some completely unrelated part of the query has an *-expansion that contains duplicate columns. Something like: HINT: alias contains multiple columns named colname ...would help a lot. I don't feel strongly about it, I just thought it was confusing. ...Robert +1 This error would be clearer with something as simple as putting the ^ in the right place and extremely clear with the above HINT. -- --Dan
[HACKERS] one line comment style
I'm going through a few files and trying to clean them up for style mostly and a bit of refactoring. I am curious about the preferred style for a one line comment. I see them in both of these forms and I would like to keep it consistent. /* a one line comment */ or /* * a one line comment */ Thoughts? -- --Dan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Rules: A Modest Proposal
On Mon, Oct 05, 2009 at 09:50:18AM +0300, Peter Eisentraut wrote: On Sun, 2009-10-04 at 18:24 -0700, Dan Colish wrote: I am not sure where that view implemenation is, but I doubt its stalled because of the rule system. It is. You can definitely create updatable views using rules. Sure you can, but they won't work in various significant corner cases. Search the archives for updatable views for details. I don't even want updatable views! I'm looking through those archives and its vague what killed them, but bad rules are definitely part of it. However, that doesn't mean you ditch the rule system because it didn't work for this particular situation. Maybe you could highlight some messages that point to the precise corner cases that make rules so bad? I would expect these corner cases would have nothing to do with updatable views, since they are such a bad idea to have automatically implemented. -- --Dan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Rules: A Modest Proposal
On Mon, Oct 05, 2009 at 11:28:13AM -0400, Alvaro Herrera wrote: Dan Colish wrote: On Mon, Oct 05, 2009 at 09:50:18AM +0300, Peter Eisentraut wrote: On Sun, 2009-10-04 at 18:24 -0700, Dan Colish wrote: You can definitely create updatable views using rules. Sure you can, but they won't work in various significant corner cases. Search the archives for updatable views for details. I don't even want updatable views! Why would you argue that point? They are specified in the SQL standard somewhere. I do not really think updatable views are needed. Maybe when the standard was written things are different; I guess you're talking about 2003. Just because something is in a standard, doesnt mean it has to be implemented. As long as you don't implement something outside of the standard, I do not have an issue. -- --Dan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Rules: A Modest Proposal
On Sun, Oct 04, 2009 at 11:42:45AM -0700, Josh Berkus wrote: There are already patches to deal with the first, at least for the kinds of VIEWs where this can be deduced automatically, and people are starting to take on the second. How would we deal with VIEWs which weren't simple enough for automated updating, then? I don't think that removing a major feature, one which some users have written applications around, is even feasible. What would be the benefit of this radical proposal? --Josh Berkus When you speak of writing to a view, what do you mean exactly? Are we saying refresh a view or update the parent tables of a view? -- --Dan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Rules: A Modest Proposal
On Sun, Oct 04, 2009 at 03:15:10PM -0400, Andrew Dunstan wrote: Dan Colish wrote: When you speak of writing to a view, what do you mean exactly? Are we saying refresh a view or update the parent tables of a view? He means INSERT, UPDATE and DELETE operations on the view. 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 How would you resolve where to perform these operations in the parent tables? I have not discovered a good way to determine which tables a user would desire to alter if the view contains a subset of data from the parent and these subsets do not include the primary keys. Even with primary keys as members of a view, there is a good potential for side effects. For example, consider the following tables: usernames, student_grades, course_listings If you have a view joining all three tables and delete one row in the view, you have the potential for deleting too much data from a parent table; ie, you choose to delete a username and its associated grades but also end up deleteing a course. You could also delete a course and end up deleting all the usernames and the grades associated. -- --Dan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Rules: A Modest Proposal
On Sun, Oct 04, 2009 at 08:54:56PM -0400, Alvaro Herrera wrote: David E. Wheeler wrote: On Oct 4, 2009, at 1:57 PM, David Fetter wrote: It's less about like or dislike and more about facing up to the reality that we've got a major legacy foot-gun left over from the experimentation of the Berkeley days. I think you're going to need to be a bit more concrete than that. In what way is it a foot-gun? What examples can you provide? What, exactly, are the issues? While I don't agree with David Fetter's premise, I think rehashing how we handle VIEWs would be a good step towards updatable views. Right now, the implementation of that is stalled precisely because of the rule system. I am not sure where that view implemenation is, but I doubt its stalled because of the rule system. You can definitely create updatable views using rules. However, I'm not sure updatable views are a good thing in most scenarios. I see way too much damage as a likely outcome. Rules are one of the great generative features of postgres and I see no reason to cut them. Features should not be limited just because they can be used incorrectly, since they can also be used in other correct/interesting ways we have yet to think up. -- --Dan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] ECPG patch views [moved from RRR list]
- Forwarded message from Robert Haas robertmh...@gmail.com -
Date: Sun, 27 Sep 2009 12:52:35 -0400
From: Robert Haas robertmh...@gmail.com
To: Boszormenyi Zoltan z...@cybertec.at
Cc: Dan Colish d...@unencrypted.org, pgsql-rrreview...@postgresql.org,
Jeff Janes jeff.ja...@gmail.com,
Hans-Juergen Schoenig h...@cybertec.at,
Michael Meskes mes...@postgresql.org
Subject: Re: CF 2009-09: initial reviewing assignments
2009/9/27 Boszormenyi Zoltan z...@cybertec.at:
On Sat, Sep 26, 2009 at 11:39:45AM -0700, Jeff Janes wrote:
Hi Robert,
Is there another patch you'd like me to work on?
Lock wait statistics says it needs review, but the last comment
suggests it is waiting on author.
Enhancements to COPY (error logging and autopartitioning) says it is
waiting on author but last comment suggests perhaps it is ready for
review.
I've taken a look at ECPG, but I couldn't make heads or tails of it.
I guess I could try harder :)
It looks like the ECPG patches are not independent and need to applied
in a particular order in order for them to apply cleanly to HEAD.
So I think I need some guidance on what I should be doing.
Thanks,
Jeff
I've been looking at the dynamic cursor names patch, so if you have any
insights I would really appreciate them. I am having some trouble fully
reviewing this patch because I am not very familiar with the ecpg code.
--
--Dan
Asketh and you shall be given. :-)
May I help you in understanding ECPG?
The dynamic cursorname patch was started on 8.3.7 first
and we moved to 8.4 (then 8.5) CVS because in 8.4,
ECPG grammar was rewritten to be auto-generated
from the core grammar. I had to dive in kneep deep
before I could modify it...
Basic thing is that ECPG modifies and extends the core
grammar in a way that
1) every token in ECPG is str type. New tokens are
defined in ecpg.tokens, types are defined in ecpg.type
2) most tokens from the core grammar are simply converted
to literals concatenated together to form the SQL string
passed to the server, this is done by parse.pl.
3) some rules need side-effects, actions are either added
or completely overridden (compared to the basic token
concatenation) for them, these are defined in ecpg.addons,
the rules for ecpg.addons are explained below.
4) new grammar rules are needed for ECPG metacommands.
These are in ecpg.trailer.
5) ecpg.header contains common functions, etc. used by
actions for grammar rules.
In ecpg.addons, every modified rule follows this pattern:
ECPG: dumpedtokens postfix
where dumpedtokens is simply tokens from core gram.y's
rules concatenated together. e.g. if gram.y has this:
ruleA: tokenA tokenB tokenC {...}
then dumpedtokens is ruleAtokenAtokenBtokenC.
postfix above can be:
a) block - the automatic rule created by parse.pl is completely
overridden, the code block has to be written completely as
it were in a plain bison grammar
b) rule - the automatic rule is extended on, so new syntaxes
are accepted for ruleA. E.g.:
ECPG: ruleAtokenAtokenBtokenC rule
| tokenD tokenE { action_code; }
...
It will be substituted with:
ruleA: original syntax forms and actions up to and including
tokenA tokenB tokenC
| tokenD tokenE { action_code; }
...
c) addon - the automatic action for the rule (SQL syntax constructed
from the tokens concatenated together) is prepended with a new
action code part. This code part is written as is's already inside
the { ... }
Multiple addon or block lines may appear together with the
new code block if the code block is common for those rules, which
is a very smart thing.
This was what I gathered from the code. The documentation
seems to be missing from the rewritten ECPG grammar in 8.4.
Michael, am I missing something?
Dan, please, start the review in light of the above.
If you have any questions, don't hesitate to ask.
Please move this discussion to -hackers, maybe with a link back to
this post. Good info, wrong place. :-)
...Robert
- End forwarded message -
I'm bumping this message to pg-hackers.
Here is a link to the archive as well:
http://archives.postgresql.org/pgsql-rrreviewers/2009-09/msg00039.php
--
--Dan
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Hot Standby on git
On Sat, Sep 26, 2009 at 10:45:17AM -0400, Mark Mielke wrote: On 09/26/2009 10:04 AM, Simon Riggs wrote: If you think there's something useful I could do, let me know and I'll take a look. I feel like I need a better way of unit testing new code. Some of the code in the patch is to handle corner cases, so recreating them is fairly hard. It is a nagging feeling that I am missing some knowledge here and would welcome some insight, or research, into better ways of doing general case unit testing. You might try and steal ideas from EasyMock / PowerMock - but not sure how well the ideas map to C. Generally it means allowing the functions to be called from a mock environment, where subroutine calls that might be called are stubbed out to return sample data that would simulate your scenario. Object oriented languages that require every object to provide an interface where most object methods can be overridden are more ideal for performing this sort of test. I rarely ever see this sort of stuff in FOSS projects, and never that I can remember in FOSS C projects. It's not easy, though. I assume you are doing it through code changing right now. Commenting out lines, replacing them with others, etc? Cheers, mark -- Mark Mielkem...@mielke.cc There are a variety of projects dedicated to creating C unit test frameworks. I don't have a lot of experience with them, but I have heard good things about check and cunit. Here's a link I found with a longer list of frameworks. http://www.opensourcetesting.org/unit_c.php -- --Dan -- Sent 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] pgbench: new feature allowing to launch shell commands
On Fri, Sep 18, 2009 at 03:10:14PM +0900, Michael Paquier wrote: You really should be returning a value at the point since the function signature defines a return type. If not the function should be void, which it cannot be in this context since it is used for boolean tests elsewhere. The returns in question are all part of error blocks and should return false. OK I got your point, I missed the part managing with CState, which is tested after doCustom. Another version of the patch is attached with this email. I must be doing something wrong when I am running this script. It is still throwing errors. Would you mind showing me the pgbench command you used to run it? Of course, here it is the list of commands I use: pgbench -i dbname (in case your database is called dbname) pgbench -c 10 -t 10 -f transaction_file_name.data dbname (customer and transaction numbers can also bet set as you want). Regards, -- Michael Paquier NTT OSSC OK, thank you for sending me the patch and the relivent commands. You're still not returning false where you should be, but that wasn't the issue I am seeing. I believe there is an issue in either the custom script you posted on the wiki or pgbench itself. Here is the script I am running, you might recongnize it :) \set nbranches :scale \set ntellers 10 * :scale \set naccounts 10 * :scale \setrandom aid 1 :naccounts \setrandom bid 1 :nbranches \setrandom tid 1 :ntellers \setrandom delta -5000 5000 \setrandom txidrand 0 1 START TRANSACTION; UPDATE pgbench_accounts SET abalance = abalance + :delta WHERE aid = :aid; SELECT abalance FROM pgbench_accounts WHERE aid = :aid; SELECT abalance FROM pgbench_accounts WHERE aid = :aid; UPDATE pgbench_tellers SET tbalance = tbalance + :delta WHERE tid = :tid; UPDATE pgbench_branches SET bbalance = bbalance + :delta WHERE bid = :bid; INSERT INTO pgbench_history (tid, bid, aid, delta, mtime) VALUES (:tid, :bid, :aid, :delta, CURRENT_TIMESTAMP); PREPARE TRANSACTION ':txidrand'; \shell ls -ll /tmp/log_data`date '+%Y%m%d'`_`date '+%k%M'` COMMIT PREPARED ':txidrand'; So I get this output with and with out the shell command in there, against the unpatched and patched version on pgbench. The commands you sent will not work with this script since it is using prepared statements. I am using this command to run the script. ./contrib/pgbench/pgbench -c 10 -j 2 -M prepared -f custom.script test From which, I get the following output: starting vacuum...end. Client 0 aborted in state 15: ERROR: bind message supplies 1 parameters, but prepared statement P0_15 requires 0 Client 1 aborted in state 15: ERROR: bind message supplies 1 parameters, but prepared statement P0_15 requires 0 Client 2 aborted in state 15: ERROR: bind message supplies 1 parameters, but prepared statement P0_15 requires 0 Client 5 aborted in state 15: ERROR: bind message supplies 1 parameters, but prepared statement P0_15 requires 0 Client 8 aborted in state 15: ERROR: bind message supplies 1 parameters, but prepared statement P0_15 requires 0 Client 6 aborted in state 15: ERROR: bind message supplies 1 parameters, but prepared statement P0_15 requires 0 Client 7 aborted in state 15: ERROR: bind message supplies 1 parameters, but prepared statement P0_15 requires 0 Client 3 aborted in state 15: ERROR: bind message supplies 1 parameters, but prepared statement P0_15 requires 0 Client 4 aborted in state 15: ERROR: bind message supplies 1 parameters, but prepared statement P0_15 requires 0 Client 9 aborted in state 15: ERROR: bind message supplies 1 parameters, but prepared statement P0_15 requires 0 transaction type: Custom query scaling factor: 1 query mode: prepared number of clients: 10 number of threads: 2 number of transactions per client: 10 number of transactions actually processed: 0/100 tps = 0.00 (including connections establishing) tps = 0.00 (excluding connections establishing) Since it occurs on both versions, I'm having some trouble identifying whether this is a bug in the script or a bug in pgbench. Any help is appreciated. -- --Dan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] generic copy options
On Fri, Sep 18, 2009 at 10:21:08AM -0700, Josh Berkus wrote: On 9/17/09 3:54 PM, Greg Smith wrote: On Thu, 17 Sep 2009, Dan Colish wrote: - Performance appears to be the same although I don't have a good way for testing this at the moment Here's what I do to generate simple COPY performance test cases: Is there any reason to think that *this* copy patch will affect performance at all? -- Josh Berkus PostgreSQL Experts Inc. www.pgexperts.com Nope, but it was on the checklist and I was being thorough. -- Dan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] generic copy options
On Fri, Sep 18, 2009 at 10:31:21AM -0700, Josh Berkus wrote: Nope, but it was on the checklist and I was being thorough. That's a good thing. I was just seeing if I needed to get involved in performance testing. -- Josh Berkus PostgreSQL Experts Inc. www.pgexperts.com I always say, the more tests the better. From the tests I ran it was clear the parser did not change speed. Might be good to have someone confirm that. -- Dan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] generic copy options
On Thu, Sep 17, 2009 at 11:07:33AM -0400, Emmanuel Cecchet wrote: Tom Lane wrote: I wonder though if we couldn't simplify matters. Offhand it seems to me that psql doesn't need to validate the command's syntax fully. All it really needs to do is find the target filename and replace it with STDIN/STDOUT. Could we have it just treat the remainder of the line literally, and not worry about the details of what the options might be? Let the backend worry about throwing an error if they're bad. New version with the simplified psql. Still supports the 7.3 syntax. I am going to look into porting the other COPY enhancements (error logging and autopartitioning) on this implementation. We might come up with new ideas for the documentation side of things with more options. manu -- Emmanuel Cecchet Aster Data Systems Web: http://www.asterdata.com Hi, I've been working on a review of this patch and currently its failing regression tests. Here's the regression.diff. I'm going to spend some time today trying to figure out if the tests need to change of there is an actual issue in the patch. Just wanted to give a heads up. -- --Dan *** /home/dc0lish/workspace/postgresql/src/test/regress/expected/copy.out 2009-09-17 11:45:04.041818319 -0700 --- /home/dc0lish/workspace/postgresql/src/test/regress/results/copy.out 2009-09-17 11:45:14.215152558 -0700 *** *** 72,88 1,a,1 2,b,2 -- Repeat the above tests with the new 8.5 option syntax ! copy copytest to '/home/manu/workspace/Postgres8.5-COPY/src/test/regress/results/copytest.csv' (csv); truncate copytest2; ! copy copytest2 from '/home/manu/workspace/Postgres8.5-COPY/src/test/regress/results/copytest.csv' (csv); select * from copytest except select * from copytest2; style | test | filler ---+--+ (0 rows) truncate copytest2; ! copy copytest to '/home/manu/workspace/Postgres8.5-COPY/src/test/regress/results/copytest.csv' (csv, csv_quote , csv_escape E'\\'); ! copy copytest2 from '/home/manu/workspace/Postgres8.5-COPY/src/test/regress/results/copytest.csv' (csv, csv_quote , csv_escape E'\\'); select * from copytest except select * from copytest2; style | test | filler ---+--+ --- 72,88 1,a,1 2,b,2 -- Repeat the above tests with the new 8.5 option syntax ! copy copytest to '/home/dc0lish/workspace/postgresql/src/test/regress/results/copytest.csv' (csv); truncate copytest2; ! copy copytest2 from '/home/dc0lish/workspace/postgresql/src/test/regress/results/copytest.csv' (csv); select * from copytest except select * from copytest2; style | test | filler ---+--+ (0 rows) truncate copytest2; ! copy copytest to '/home/dc0lish/workspace/postgresql/src/test/regress/results/copytest.csv' (csv, csv_quote , csv_escape E'\\'); ! copy copytest2 from '/home/dc0lish/workspace/postgresql/src/test/regress/results/copytest.csv' (csv, csv_quote , csv_escape E'\\'); select * from copytest except select * from copytest2; style | test | filler ---+--+ *** *** 95,111 1,a,1 2,b,2 -- Repeat the above tests with the new 8.5 option syntax from psql ! \copy copytest to '/home/manu/workspace/Postgres8.5-COPY/src/test/regress/results/copytest.csv' (csv) truncate copytest2; ! \copy copytest2 from '/home/manu/workspace/Postgres8.5-COPY/src/test/regress/results/copytest.csv' (csv) select * from copytest except select * from copytest2; style | test | filler ---+--+ (0 rows) truncate copytest2; ! \copy copytest to '/home/manu/workspace/Postgres8.5-COPY/src/test/regress/results/copytest.csv' (csv, csv_quote , csv_escape E'\\') ! \copy copytest2 from '/home/manu/workspace/Postgres8.5-COPY/src/test/regress/results/copytest.csv' (csv, csv_quote , csv_escape E'\\') select * from copytest except select * from copytest2; style | test | filler ---+--+ --- 95,111 1,a,1 2,b,2 -- Repeat the above tests with the new 8.5 option syntax from psql ! \copy copytest to '/home/dc0lish/workspace/postgresql/src/test/regress/results/copytest.csv' (csv) truncate copytest2; ! \copy copytest2 from '/home/dc0lish/workspace/postgresql/src/test/regress/results/copytest.csv' (csv) select * from copytest except select * from copytest2; style | test | filler ---+--+ (0 rows) truncate copytest2; ! \copy copytest to '/home/dc0lish/workspace/postgresql/src/test/regress/results/copytest.csv' (csv, csv_quote , csv_escape E'\\') ! \copy copytest2 from '/home/dc0lish/workspace/postgresql/src/test/regress/results/copytest.csv' (csv, csv_quote , csv_escape E'\\') select * from copytest except select * from copytest2; style | test | filler ---+--+ ==
Re: [HACKERS] generic copy options
On Thu, Sep 17, 2009 at 02:56:07PM -0400, Emmanuel Cecchet wrote: Dan, My bad, I copy/pasted the hard link in output/copy.source instead of @abs_build...@. Here is a complete version of the patch with the fix on output/copy.source. Emmanuel Hooray, that works just fine now. I guess I should have seen that... -- --Dan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] generic copy options
On Thu, Sep 17, 2009 at 02:56:07PM -0400, Emmanuel Cecchet wrote: Dan, My bad, I copy/pasted the hard link in output/copy.source instead of @abs_build...@. Here is a complete version of the patch with the fix on output/copy.source. Emmanuel Emmanuel, Thanks for getting that back so quickly. As I said before, it applies cleanly and passes regression tests. I'm reading through the changes now. When you get a moment could you send me the patch as a context diff? -- --Dan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] generic copy options
Hi, I have read through the patch a few times and it looks OK. The additions to the COPY syntax work as expected and as agreed upon based on the thread. Below are some points from my checklist. - Patch applies cleanly - Included new tests and documentation - Well commented - Documentation is clearly written - Produced no error or warning on compile - When compiled passes all tests - Syntax works as expected - Performance appears to be the same although I don't have a good way for testing this at the moment - Patch integrates well with current backend copy functions - Patch cleanly extends the psql \copy feature Any further thoughts on this patch? I think its pretty much ready. -- --Dan -- Sent 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] pgbench: new feature allowing to launch shell commands
On Thu, Sep 17, 2009 at 09:56:44AM +0900, Michael Paquier wrote: Hi all, Sorry for my late reply. There is no other update for this patch since the 13th of August, at least until today. The new patch is attached By the way I worked on the comments that Dan and Gabriel pointed out. I added a check on system such as to prevent an error from this side. By the way, I noticed that the way return values are managed in doCustom and in process_commands is different. Such as to make an homogeneous code, return values are managed the same way in both functions in the patch, explaining why I did not return a specific value when file commands are treated in doCustom. You really should be returning a value at the point since the function signature defines a return type. If not the function should be void, which it cannot be in this context since it is used for boolean tests elsewhere. The returns in question are all part of error blocks and should return false. Here is also as wanted a simple transaction that permits to use this function: \set nbranches :scale \set naccounts 10 * :scale \setrandom aid 1 :naccounts \setrandom bid 1 :nbranches \setrandom delta -5000 5000 \setrandom txidrand 0 1 START TRANSACTION; UPDATE pgbench_accounts SET abalance = abalance + :delta WHERE aid = :aid; SELECT abalance FROM pgbench_accounts WHERE aid = :aid; UPDATE pgbench_branches SET bbalance = bbalance + :delta WHERE bid = :bid; PREPARE TRANSACTION ':txidrand'; \shell ls -ll $PGDATA/pg_twophase COMMIT PREPARED ':txidrand'; I must be doing something wrong when I am running this script. It is still throwing errors. Would you mind showing me the pgbench command you used to run it? -- --Dan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] generic copy options
On Thu, Sep 17, 2009 at 07:10:35PM -0400, Andrew Dunstan wrote: Greg Smith wrote: On Thu, 17 Sep 2009, Dan Colish wrote: - Performance appears to be the same although I don't have a good way for testing this at the moment Here's what I do to generate simple COPY performance test cases: CREATE TABLE t (i integer); INSERT INTO t SELECT x FROM generate_series(1,10) AS x; \timing COPY t TO '/some/file' WITH [options]; BEGIN; TRUNCATE TABLE t; COPY t FROM '/some/file' WITH [options]; COMMIT; You can adjust the size of the generated table based on whether you want to minimize (small number) or maximize (big number) the impact of the setup overhead relative to actual processing time. Big numbers make sense if there's a per-row change, small ones if it's mainly COPY setup that's been changed if you want a small bit of data to test against. An example with one column in it is a good test case for seeing whether per-row impact has gone up. You'd want something with a wider row for other types of performance tests. The reason for the BEGIN/COMMIT there is that form utilizes an optimization that lowers WAL volume when doing the COPY insertion, which makes it more likely you'll be testing performance of the right thing. I usually prefer to test with a table that is more varied than anything you can make with generate_series. When I tested my ragged copy patch the other day I copied 1,000,000 rows out of a large table with a mixture of dates, strings, numbers and nulls. But then, it has a (tiny) per field overhead so I wanted to make sure that was well represented in the test. You are certainly right about wrapping it in begin/truncate/commit (and when you do make sure that archiving is not on). You probably want to make sure that the file is not on the same disk as the database, to avoid disk contention. Or, better, make sure that it is in OS file system cache, or on a RAM disk. cheers andrew If someone with a more significant setup can run tests that would ideal. I only have my laptop which is a single disk and fairly underpowered. That said, here are my results running the script above, it looks like the pach improves performance. I would really interested to see results on a larger data set and heavier iron. -- --Dan Without Patch: CREATE TABLE INSERT 0 10 Timing is on. COPY 10 Time: 83.273 ms BEGIN Time: 0.412 ms TRUNCATE TABLE Time: 0.357 ms COPY 10 Time: 140.911 ms COMMIT Time: 4.909 ms With Patch: CREATE TABLE INSERT 0 10 Timing is on. COPY 10 Time: 80.205 ms BEGIN Time: 0.351 ms TRUNCATE TABLE Time: 0.346 ms COPY 10 Time: 124.303 ms COMMIT Time: 4.130 ms -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] generic copy options
On Thu, Sep 17, 2009 at 07:45:45PM -0400, Andrew Dunstan wrote: Dan Colish wrote: CREATE TABLE INSERT 0 10 Timing is on. COPY 10 Time: 83.273 ms BEGIN Time: 0.412 ms TRUNCATE TABLE Time: 0.357 ms COPY 10 Time: 140.911 ms COMMIT Time: 4.909 ms Anything that doesn't have times that are orders of magnitude greater than this is pretty much useless as a measurement of COPY performance, IMNSHO. In this particular test, to check for paring times, I'd be inclined to do copy repeatedly (i.e. probably quite a few thousand times) from an empty file to test the speed. Something like: select current_timestamp; begin; truncate; copy;copy;copy; ... commit; select current_timestamp; (tests like this are really a good case for DO ' something'; - we could put a loop in the DO.) cheers andrew Ok, so I ran something like you suggested and did a simple copy from an empty file to just test the parsing. I have the COPY statement run 3733 times in the transaction block and did the select timestamps, but I still only was a few milliseconds difference between the two versions. Maybe a more complex copy statment could be a better test of the parser, but I do not see a significant difference of parsing speed here. -- --Dan -- Sent 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] pgbench: new feature allowing to launch shell commands
On Tue, Sep 15, 2009 at 09:53:11PM -0400, Stephen Frost wrote: Michael, I just wanted to follow-up on your pgbench patch. The latest version that I see is from August 13th. Is that the correct patch to be reviewing? Do you have any other updates on it? Thanks! Stephen Hi! Some comments about this patch: - You have DOS-style carriage returns, which interfere with the patch application on Unix systems. - We'd like to see return specifically return a value (lines 1008 and 1022 in the patched version) - We'd like to see something done with the return value from system (line 1026 in patched version) - pg_bench functions as expected, however, your example script given on the wiki page for this patch fails (http://wiki.postgresql.org/wiki/Pgbench:_shell_command). Can we have an example that works so we can check it out? It's not really clear to us how this will be useful to others. Thanks! Gabrielle Dan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [RRR] [HACKERS] CommitFest 2009-09 Plans and Call for Reviewers
On Thu, Sep 10, 2009 at 08:23:00AM -0700, David E. Wheeler wrote: On Sep 9, 2009, at 6:13 PM, Robert Haas wrote: Hopefully this plan is acceptable to everyone. If not, please feel free to reply here. +1 And I'm available to review again, of course. Best, David -- Sent via pgsql-rrreviewers mailing list (pgsql-rrreview...@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-rrreviewers Please count me in. -- --Dan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Executor documentation
Hi, I've been looking through the current state of docuemtation, including comments, with respect to the executor code and I would like to improve upon their condition. If anyone has notes, pseudocode, thoughts on how it all really works, or anything that can help me out, I'd really appreciate it. Currently, I'm just going through and finding comments that need more, then adding to them. I'd also like to add to the readme where it's calling for more documentation; ie, the XXX line. I think this cleanup will really help flush out some of the confusion I've had with the executor and maybe identify areas of it that can be improved. Thanks in advance, -- --Dan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Executor Material
I found these docs to very helpful for understand how the backend works, but for the executor specifically, I think following the code is best. http://anoncvs.postgresql.org/cvsweb.cgi/~checkout~/pgsql/src/tools/backend/index.html -- --Dan On Tue, Aug 04, 2009 at 05:35:04PM -0300, Edson Ramiro wrote: Hi all, Does someone has some material which explain how the executor works? I'm looking for the internal processing of a query in the _executor_. Thanks for help Edson Ramiro -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Convert stmt back into queryString
I am currently trying to convert an insertstmt back into a const char *queryString, but I can't find an existing function to do this for the life of me. I will write one if none exits, but I figured I ask here first. Unfortunately, nodeToString is not quite right for what I'm doing. Thanks in advance. -- --Dan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Convert stmt back into queryString
On Tue, Aug 04, 2009 at 10:00:24PM -0400, Tom Lane wrote: Dan Colish d...@unencrypted.org writes: I am currently trying to convert an insertstmt back into a const char *queryString, but I can't find an existing function to do this for the life of me. I will write one if none exits, but I figured I ask here first. Unfortunately, nodeToString is not quite right for what I'm doing. Thanks in advance. Hmm, you mean a Query, or a raw unanalyzed InsertStmt? If the former, ruleutils.c will help. If the latter, be prepared to write a lot of code; there's nothing closer than nodeToString, and even that is pretty incomplete for raw grammar output nodes IIRC. regards, tom lane In this case, its a raw InsertStmt. I would like to pass this back to parse_analyze, but I need to have a queryString to go with that call, so crafting a function to rate that seems to be the only way, atm. -- --Dan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Convert stmt back into queryString
On Tue, Aug 04, 2009 at 10:55:07PM -0400, Tom Lane wrote: Dan Colish d...@unencrypted.org writes: On Tue, Aug 04, 2009 at 10:00:24PM -0400, Tom Lane wrote: Hmm, you mean a Query, or a raw unanalyzed InsertStmt? In this case, its a raw InsertStmt. I would like to pass this back to parse_analyze, but I need to have a queryString to go with that call, so crafting a function to rate that seems to be the only way, atm. Hm, so you have an InsertStmt but not the text it was generated from? Where? By and large the design plan is that the source text should still be available anyplace that's dealing with raw parsetrees. I believe you can just pass NULL as the querystring --- the only thing you lose from that is syntax location pointers in error messages. But in ordinary situations you shouldn't have to. (Also, the fact that that's what the string is used for means that ginning up a string from the nodetree is a bit pointless. It won't contain the detail that it's meant to provide.) regards, tom lane Well the problem with declaring it null is that the first parse_analyze Assert will fail. Assert(sourceText != NULL); /* required as of 8.4 */ What I am doing here is taking one Create stmt of a new type and breaking it up into the various operations I need it to perform. One is a this InsertStmt. Another is a CreateStmt. I'll also need to add CreateTrigStmt's. Recently, I've been thinking this would be best to do in gram.y and then pass those parts as distinct nodes. I still might hit this queryString issue although. I haven't thought about that enough. -- --Dan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
