I applied this patch after a little bit of editorialization concerning
the points mentioned in this discussion:
Robert Haas writes:
> On Wed, Dec 9, 2009 at 9:33 PM, Takahiro Itagaki
> wrote:
>> Robert Haas wrote:
>>> Why does this patch #ifdef out the _PG_fini code in pg_stat_statements?
>>
>
On Wed, Dec 9, 2009 at 10:14 PM, Takahiro Itagaki
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:
>
> EXECUTE co
Robert Haas 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 other comman
On Wed, Dec 9, 2009 at 9:33 PM, Takahiro Itagaki
wrote:
> Robert Haas 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
> release
Robert Haas 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, removing #ifdef (enabl
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 th
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 EXPLA
On Tue, Dec 8, 2009 at 10:12 PM, Greg Smith 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
> like all the is
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 hook
Tom Lane 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 should have the a
Tom Lane escreveu:
> Bruce Momjian 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,
Tom Lane wrote:
> Bruce Momjian 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,
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, becaus
Bruce Momjian 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 off on it, if
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 p
Tom Lane wrote:
> Bruce Momjian 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
Bruce Momjian 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. Is there
Tom Lane wrote:
> Bruce Momjian 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:
https://commitfest.postgr
Bruce Momjian 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 subscription:
http:
I have applied this patch, with only a minor wording improvement:
Specify on to track DDL commands, which excludes
SELECT,
^^
Thanks.
---
Itagaki Takahiro wrote
Euler Taveira de Oliveira 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, why don't we
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.
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 wrote:
> > pg_stat_statements modu
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
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?
pg_stat_statements module also handle DDL commands as same as normal
queries. Fortunately, autovacuum calls call vacuum() directly,
so th
25 matches
Mail list logo
