From: | Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | Neil Conway <neilc(at)samurai(dot)com>, Brendan Jurd <direvus(at)gmail(dot)com>, pgsql-patches(at)postgresql(dot)org |
Subject: | Re: Additional current timestamp values |
Date: | 2006-04-23 03:33:54 |
Message-ID: | 200604230333.k3N3Xsn20238@candle.pha.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers pgsql-patches |
Tom Lane wrote:
> Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> > Tom Lane wrote:
> >> The patch as given strikes me as pretty broken --- it does not advance
> >> statement_timestamp when I would expect (AFAICS it only sets it during
> >> transaction start).
>
> > Uh, it does advance:
>
> But not once per statement --- in reality, you get a fairly arbitrary
> behavior that will advance in some cases and not others when dealing
> with a multi-statement querystring. Your example showing that it fails
> to advance in a psql -c string shows this ... don't you think most
> people would call that a bug?
>
> If it's "statement" timestamp then I think it ought to advance once per
> SQL statement, which this isn't doing. (As I already said, though, that
> isn't the behavior I really want. My point is just that the code's
> behavior is an extremely strange, nonintuitive definition of the word
> "statement".)
>
> > I have always been confused if
> > statement_timeout times queries inside server-side functions, for
> > example. I don't think it should.
>
> That's exactly my point; I agree that we don't want it doing that,
> but that being the case, "statement" isn't a great name for the units
> that we are actually processing. We're really wanting to do these
> things once per client command, or maybe per client query would be a
> better name.
I have updated my patch based on community comments. One cleanup is
that I now set statement_timestamp(), and then base
transaction_timestamp() (aka now()) on the statement_timestamp of BEGIN,
which is a much cleaner API.
As far as how often statement_timestamp() is called, when a "Q" query
arrives, it calls exec_simple_query(), which calls start_xact_command()
before it parses anything, setting the transaction start. It is called
inside the per-command loop, but it does nothing unless
finish_xact_command() was called to finish a transaction.
(Is there some double-processing here for BEGIN because it will re-run
the initialization stuff?)
I also documented how statement_timestamp behaves when multiple
statements are in the same query string, and when called from functions.
One side-affect of tracking transaction_timestamp based on
statement_timestamp() is if multiple statements are sent in a single
query string, and multiple transactions are used, statement_timestamp
will be advanced so transaction_timestamp() can vary. Again, not ideal,
but probably the cleanest we are going to be able to do. If we decided
to just have statement_timestamp be the arrival of the string always, we
are going to incur additional gettimeofday() calls and the code is going
to be more complex.
FYI, this is exactly how statement_timeout behaves, and no one has
complained about it.
The only other approach would be to put the statement_timestamp()
setting call in exec_simple_query(), and in all the protocol-level
functions, and fastpath. You then also need to do a separate call for
transaction_timestamp() because you want that to advance if multiple
transactions are in the same query string.
If we want to take that approach, should statement_timeout code also be
moved around?
See my other post about the use of the term "statement". I don't think
most people think about sending multiple statements, so if we document
its behavior, that is good enough.
--
Bruce Momjian http://candle.pha.pa.us
EnterpriseDB http://www.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
Attachment | Content-Type | Size |
---|---|---|
unknown_filename | text/plain | 13.4 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Christopher Browne | 2006-04-23 03:48:10 | Re: plperl on AIX |
Previous Message | Bruce Momjian | 2006-04-23 03:04:15 | Re: Additional current timestamp values |
From | Date | Subject | |
---|---|---|---|
Next Message | Bruce Momjian | 2006-04-23 03:36:38 | Re: Removal of backward-compatibility docs mentions |
Previous Message | Bruce Momjian | 2006-04-23 03:04:15 | Re: Additional current timestamp values |