I applied this patch after a little bit of editorialization concerning
the points mentioned in this discussion:
Robert Haas robertmh...@gmail.com writes:
On Wed, Dec 9, 2009 at 9:33 PM, Takahiro Itagaki
itagaki.takah...@oss.ntt.co.jp wrote:
Robert Haas robertmh...@gmail.com wrote:
Why does
Why does this patch #ifdef out the _PG_fini code in pg_stat_statements?
Where you check for INSERT, UPDATE, and DELETE return codes in
pgss_ProcessUtility, I think this deserves a comment explaining that
these could occur as a result of EXECUTE. It wasn't obvious to me,
anyway.
It seems to me
Robert Haas robertmh...@gmail.com wrote:
Why does this patch #ifdef out the _PG_fini code in pg_stat_statements?
That's because _PG_fini is never called in current releases.
We could remove it completely, but I'd like to leave it for future
releases where _PG_fini callback is re-enabled.
Or,
On Wed, Dec 9, 2009 at 9:33 PM, Takahiro Itagaki
itagaki.takah...@oss.ntt.co.jp wrote:
Robert Haas robertmh...@gmail.com wrote:
Why does this patch #ifdef out the _PG_fini code in pg_stat_statements?
That's because _PG_fini is never called in current releases.
We could remove it completely,
Robert Haas robertmh...@gmail.com wrote:
Like this?
/*
* Parse command tag to retrieve the number of affected rows.
* COPY command returns COPY tag. EXECUTE command might return INSERT,
* UPDATE, or DELETE tags, but we cannot retrieve the number of rows
* for SELECT. We assume
On Wed, Dec 9, 2009 at 10:14 PM, Takahiro Itagaki
itagaki.takah...@oss.ntt.co.jp wrote:
I'm confused by the we cannot retrieve the number of rows for SELECT
part. Can you clarify that?
Ah, I meant the SELECT was EXECUTE of SELECT.
If I use internal structure names, the explanation will be:
It looks like the last round of issues on this patch only came from
Tom's concerns, so I'm not sure if anyone but Tom (or a similarly picky
alternate committer) is going to find anything else in a re-review. It
looks to me like all the issues were sorted out anyway. Euler, you're
off the
On Tue, Dec 8, 2009 at 10:12 PM, Greg Smith g...@2ndquadrant.com wrote:
It looks like the last round of issues on this patch only came from Tom's
concerns, so I'm not sure if anyone but Tom (or a similarly picky alternate
committer) is going to find anything else in a re-review. It looks to me
Robert Haas wrote:
Since Itagaki Takahiro is now a committer, I sort of assumed he would
be committing his own patches.
Maybe, but I wasn't going to be the one to suggest that Tom get cut out
of the loop after he raised a list of issues with the patch already.
I think the situation for
Tom Lane t...@sss.pgh.pa.us wrote:
... and now that I have, I find at least four highly questionable
things about it:
1. The placement of the hook. Why is it three lines down in
ProcessUtility? It's probably reasonable to have the Assert first,
but I don't see why the hook function
Tom Lane escreveu:
Bruce Momjian br...@momjian.us writes:
So, if someone writes a patch, and it is reviewed, and the patch author
updates the patch and replies, it still should be reviewed again before
being committed?
Well, that's for the reviewer to say --- if the update satisfies his
I have applied this patch, with only a minor wording improvement:
Specify literalon/ to track DDL commands, which excludes
commandSELECT/,
^^
Thanks.
---
Bruce Momjian br...@momjian.us writes:
I have applied this patch, with only a minor wording improvement:
Uh, we weren't even done reviewing this were we?
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your
Tom Lane wrote:
Bruce Momjian br...@momjian.us writes:
I have applied this patch, with only a minor wording improvement:
Uh, we weren't even done reviewing this were we?
Uh, I am new to this commitfest wiki thing, but it did have a review by
Euler Taveira de Oliveira:
Bruce Momjian br...@momjian.us writes:
Tom Lane wrote:
Uh, we weren't even done reviewing this were we?
Uh, I am new to this commitfest wiki thing, but it did have a review by
Euler Taveira de Oliveira:
https://commitfest.postgresql.org/action/patch_view?id=196
and the author replied.
Tom Lane wrote:
Bruce Momjian br...@momjian.us writes:
Tom Lane wrote:
Uh, we weren't even done reviewing this were we?
Uh, I am new to this commitfest wiki thing, but it did have a review by
Euler Taveira de Oliveira:
https://commitfest.postgresql.org/action/patch_view?id=196
I wrote:
It wasn't marked Ready For Committer, so presumably the reviewer
wasn't done with it. I know I hadn't looked at it at all, because
I was waiting for the commitfest review process to finish.
... and now that I have, I find at least four highly questionable
things about it:
1. The
Bruce Momjian br...@momjian.us writes:
So, if someone writes a patch, and it is reviewed, and the patch author
updates the patch and replies, it still should be reviewed again before
being committed?
Well, that's for the reviewer to say --- if the update satisfies his
concerns, he should sign
OK, reverted and placed back in Needs Review status.
---
Tom Lane wrote:
I wrote:
It wasn't marked Ready For Committer, so presumably the reviewer
wasn't done with it. I know I hadn't looked at it at all, because
I
Tom Lane wrote:
Bruce Momjian br...@momjian.us writes:
So, if someone writes a patch, and it is reviewed, and the patch author
updates the patch and replies, it still should be reviewed again before
being committed?
Well, that's for the reviewer to say --- if the update satisfies his
Euler Taveira de Oliveira eu...@timbira.com wrote:
The functionality is divided in two parts. The first part is a hook in the
utility module. The idea is capture the commands that doesn't pass through
executor. I'm afraid that that hook will be used only for capturing non-DML
queries. If so,
Itagaki Takahiro escreveu:
Here is a patch to add ProcessUtility_hook to handle all DDL
commands in user modules. (ProcessUtility_hook_20091021.patch)
It adds a global variable 'ProcessUtility_hook' and
export the original function as 'standard_ProcessUtility'.
I've reviewed your patch. It
Itagaki Takahiro escreveu:
We can hook SELECT, INSERT, UPDATE and DELETE with ExecutorXxx_hooks,
but there is no way to hook DDL commands. Can I submit a patch to add
ProcessUtility_hook?
+1. Hey, I was thinking about that yesterday. ;)
pg_stat_statements module also handle DDL commands as
Here is a patch to add ProcessUtility_hook to handle all DDL
commands in user modules. (ProcessUtility_hook_20091021.patch)
It adds a global variable 'ProcessUtility_hook' and
export the original function as 'standard_ProcessUtility'.
Euler Taveira de Oliveira eu...@timbira.com wrote:
24 matches
Mail list logo
