Re: [PATCHES] Additional current timestamp values

2006-04-24 Thread Bruce Momjian

Patch applied, and catalog version updated.

---

Bruce Momjian wrote:
> 
> Here is an updated patch.  I broke out the statement_timestamp and
> statement_timeout handling into separate functions, initialize_command()
> and finalize_command(), which call the xact start/stop internally.
> 
> This clears up the API because now start/stop xact can be called
> independent of the statement tracking operations.
> 
> It also makes all commands arriving in a single query string have the
> same statement_timestamp (even if in different transactions), and share
> the same statement_timeout timer.
> 
> I have also documented that statement_timeout is the tracked from the
> time the command arrives at the server.
> 
> ---
> 
> Bruce Momjian wrote:
> > 
> > I am not happy with my patch and am going to try a more comprehensive
> > restructuring --- will post later.
> > 
> > ---
> > 
> > Bruce Momjian wrote:
> > > Tom Lane wrote:
> > > > Bruce Momjian  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
>   EnterpriseDBhttp://www.enterprisedb

Re: [PATCHES] Additional current timestamp values

2006-04-23 Thread Bruce Momjian

Here is an updated patch.  I broke out the statement_timestamp and
statement_timeout handling into separate functions, initialize_command()
and finalize_command(), which call the xact start/stop internally.

This clears up the API because now start/stop xact can be called
independent of the statement tracking operations.

It also makes all commands arriving in a single query string have the
same statement_timestamp (even if in different transactions), and share
the same statement_timeout timer.

I have also documented that statement_timeout is the tracked from the
time the command arrives at the server.

---

Bruce Momjian wrote:
> 
> I am not happy with my patch and am going to try a more comprehensive
> restructuring --- will post later.
> 
> ---
> 
> Bruce Momjian wrote:
> > Tom Lane wrote:
> > > Bruce Momjian  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
  EnterpriseDBhttp://www.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +
Index: doc/src/sgml/config.sgml
===
RCS file: /cvsroot/pgsql/doc/src/sgml/config.sgml,v
retrieving revision 1.55
diff -c -c -r1.55 config.sgml
*** doc/src/sgml/config.sgml23 Apr 2006 03:39:48 -  1.55
--- doc/src/sgml/config.sgm

Re: [PATCHES] Additional current timestamp values

2006-04-23 Thread Bruce Momjian

I am not happy with my patch and am going to try a more comprehensive
restructuring --- will post later.

---

Bruce Momjian wrote:
> Tom Lane wrote:
> > Bruce Momjian  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
>   EnterpriseDBhttp://www.enterprisedb.com
> 
>   + If your life is a hard drive, Christ can be your backup. +

> Index: doc/src/sgml/func.sgml
> ===
> RCS file: /cvsroot/pgsql/doc/src/sgml/func.sgml,v
> retrieving revision 1.313
> diff -c -c -r1.313 func.sgml
> *** doc/src/sgml/func.sgml10 Mar 2006 20:15:25 -  1.313
> --- doc/src/sgml/func.sgml23 Apr 2006 02:26:19 -
> ***
> *** 5303,5308 
> --- 5303,5317 
>   now
>  
>  
> + transaction_timestamp
> +
> +
> + statement_timestamp
> +
> +
> + clock_timestamp
> +
> +
>   timeofday
>  
>   
> ***
> *** 5358,5364 
>  
>   
> current_timestamp
>   timestamp with time zone
> ! Date and time; see  linkend="functions-datetime-current">
>   
>   
>   
> --- 5367,5373 
>  
>   
> current_timestamp
>   timestamp with time zone
> ! Date and time of start of current transaction; see  linkend="functions-datetime-current">
>   
>   
>   
> ***
> *** 5474,5481 
>  
>   now()
>   timestamp with tim

Re: [PATCHES] Additional current timestamp values

2006-04-22 Thread Bruce Momjian
Tom Lane wrote:
> Bruce Momjian  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
  EnterpriseDBhttp://www.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +
Index: doc/src/sgml/func.sgml
===
RCS file: /cvsroot/pgsql/doc/src/sgml/func.sgml,v
retrieving revision 1.313
diff -c -c -r1.313 func.sgml
*** doc/src/sgml/func.sgml  10 Mar 2006 20:15:25 -  1.313
--- doc/src/sgml/func.sgml  23 Apr 2006 02:26:19 -
***
*** 5303,5308 
--- 5303,5317 
  now
 
 
+ transaction_timestamp
+
+
+ statement_timestamp
+
+
+ clock_timestamp
+
+
  timeofday
 
  
***
*** 5358,5364 
 
  
current_timestamp
  timestamp with time zone
! Date and time; see 
  
  
  
--- 5367,5373 
 
  
current_timestamp
  timestamp with time zone
! Date and time of start of current transaction; see 
  
  
  
***
*** 5474,5481 
 
  now()
  timestamp with time zone
! Current date and time (equivalent to
!  current_timestamp); see 
  
  
  
--- 5483,5518 
 
  now()
  timestamp with time zone
! Date and time of start of current transaction (equivalent to
!  CURRENT_TIMESTAMP); see 
! 
! 
! 
!
! 
!
! 
transaction_timestamp()
! timestamp with time zone
! Date and time of start of current transaction (equivalent to
!  CURRENT_TIMEST

Re: [PATCHES] Additional current timestamp values

2006-04-22 Thread Bruce Momjian
Kevin Grittner wrote:
> >>> On Mon, Mar 20, 2006 at  7:58 pm, in message
> <[EMAIL PROTECTED]>, Bruce Momjian
>  wrote: 
> > Tom Lane wrote:
> >> 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.
> 
> >> "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.
> > 
> > Right.
> 
> What about "query string"?  If you want to include the term "client", I
> would find "client query string" less confusing than "client command" or
> "client query".  If it's not always in the form of a string, maybe
> "client xxx batch", where xxx could be statement, request, command, or
> query.

We could use something like query_arrived_timestamp or something like
that, but it kind of confuses the distinction between it and
transaction_timestamp(), and for 99% of users, they don't even realize
you can send multiple statements in a single query.  I am thinking we
call it statement_timestamp (like statement_timeout) and just document
its behavior.  No one has problems with statement_timeout(), and that
has the exact same behavior as statement_timestamp() will have.

-- 
  Bruce Momjian   http://candle.pha.pa.us
  EnterpriseDBhttp://www.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

---(end of broadcast)---
TIP 2: Don't 'kill -9' the postmaster


Re: [PATCHES] Additional current timestamp values

2006-04-01 Thread Bruce Momjian
Tom Lane wrote:
> "Kevin Grittner" <[EMAIL PROTECTED]> writes:
> >>> "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.
> >> 
> >> Right.
> 
> > What about "query string"?  If you want to include the term "client", I
> > would find "client query string" less confusing than "client command" or
> > "client query".
> 
> "Query string" is a term we've used in the past, and it shows up in the
> source code.  I could live with that, but I'm not sure if it's got any
> good connotations for people who haven't got their hands dirty in the
> code ...

Peter hammered us quite a bit in the past that we send "statements", not
just queries (aka SELECT).  It is hard to argue with that.

-- 
  Bruce Momjian   http://candle.pha.pa.us

  + If your life is a hard drive, Christ can be your backup. +

---(end of broadcast)---
TIP 6: explain analyze is your friend


Re: [PATCHES] Additional current timestamp values

2006-03-30 Thread Tom Lane
"Kevin Grittner" <[EMAIL PROTECTED]> writes:
>>> "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.
>> 
>> Right.

> What about "query string"?  If you want to include the term "client", I
> would find "client query string" less confusing than "client command" or
> "client query".

"Query string" is a term we've used in the past, and it shows up in the
source code.  I could live with that, but I'm not sure if it's got any
good connotations for people who haven't got their hands dirty in the
code ...

regards, tom lane

---(end of broadcast)---
TIP 3: Have you checked our extensive FAQ?

   http://www.postgresql.org/docs/faq


Re: [PATCHES] Additional current timestamp values

2006-03-30 Thread Kevin Grittner
>>> On Mon, Mar 20, 2006 at  7:58 pm, in message
<[EMAIL PROTECTED]>, Bruce Momjian
 wrote: 
> Tom Lane wrote:
>> 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.

>> "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.
> 
> Right.

What about "query string"?  If you want to include the term "client", I
would find "client query string" less confusing than "client command" or
"client query".  If it's not always in the form of a string, maybe
"client xxx batch", where xxx could be statement, request, command, or
query.

-Kevin


---(end of broadcast)---
TIP 6: explain analyze is your friend


Re: [PATCHES] Additional current timestamp values

2006-03-20 Thread Bruce Momjian
Tom Lane wrote:
> Bruce Momjian  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?

I assume RULES should have the same statement_timestamp and I figured my
approach was the only one that would work.

> 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 suppose so.

> > 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.

Right.

-- 
  Bruce Momjian   http://candle.pha.pa.us
  SRA OSS, Inc.   http://www.sraoss.com

  + If your life is a hard drive, Christ can be your backup. +

---(end of broadcast)---
TIP 2: Don't 'kill -9' the postmaster


Re: [PATCHES] Additional current timestamp values

2006-03-20 Thread Tom Lane
Bruce Momjian  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.

regards, tom lane

---(end of broadcast)---
TIP 6: explain analyze is your friend


Re: [PATCHES] Additional current timestamp values

2006-03-20 Thread Bruce Momjian
Tom Lane wrote:
> Neil Conway <[EMAIL PROTECTED]> writes:
> > Bruce Momjian wrote:
> >> One trick is that these should be the same:
> >> test=> SELECT statement_timestamp(), transaction_timestamp();
> 
> > Should they be?
> 
> ISTM that the most useful definition of "statement_timestamp" is really
> "time of arrival of the latest interactive command from the client", and
> as such it should not be tied to statement start per se at all.

I see your point.

> I'd be in favor of doing gettimeofday() upon receiving a client message,
> reporting that value directly for statement_timestamp, and copying it
> during transaction start to obtain the value to use for
> transaction_timestamp.  I don't much like the idea of doing a
> gettimeofday() per SQL statement, especially not if that's taken to mean
> every SQL statement issued by PL functions (and if it doesn't mean that,
> "statement_timestamp" seems like the wrong name).  One gettimeofday()
> per client message doesn't seem too horrible though, since that's
> certainly going to require at least a couple of kernel calls anyway.
> 
> Possibly we should call it "command_timestamp" not "statement_timestamp"
> to help reduce confusion.
> 
> 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).  I don't like it stylistically either: ISTM either

Uh, it does advance:

test=> BEGIN;
BEGIN
test=> SELECT statement_timestamp(), transaction_timestamp();
 statement_timestamp  | transaction_timestamp
--+---
 2006-03-20 18:49:17.88062-05 | 2006-03-20 18:49:11.922934-05
(1 row)

test=> SELECT statement_timestamp(), transaction_timestamp();
  statement_timestamp  | transaction_timestamp
---+---
 2006-03-20 18:49:19.176823-05 | 2006-03-20 18:49:11.922934-05
(1 row)


start_xact_command() is kind of badly worded.  It calls
StartTransactionCommand(), which might or might not start a transaction,
then it does statement_timeout setup.  I have always been confused if
statement_timeout times queries inside server-side functions, for
example.  I don't think it should.

> these things are the responsibility of xact.c or they are the
> responsibility of postgres.c, it is not sensible to have both modules
> assigning to statement_timestamp.

It was done to minimize code change and limit the number of
gettimeofday() calls.

> BTW, now that I look at it, the "statement_timeout" GUC variable seems
> to have much of the same confusion about whether "statement" is
> equivalent to "interactive command" or not.

True.

-- 
  Bruce Momjian   http://candle.pha.pa.us
  SRA OSS, Inc.   http://www.sraoss.com

  + If your life is a hard drive, Christ can be your backup. +

---(end of broadcast)---
TIP 5: don't forget to increase your free space map settings


Re: [PATCHES] Additional current timestamp values

2006-03-20 Thread Tom Lane
Neil Conway <[EMAIL PROTECTED]> writes:
> Bruce Momjian wrote:
>> One trick is that these should be the same:
>> test=> SELECT statement_timestamp(), transaction_timestamp();

> Should they be?

ISTM that the most useful definition of "statement_timestamp" is really
"time of arrival of the latest interactive command from the client", and
as such it should not be tied to statement start per se at all.

I'd be in favor of doing gettimeofday() upon receiving a client message,
reporting that value directly for statement_timestamp, and copying it
during transaction start to obtain the value to use for
transaction_timestamp.  I don't much like the idea of doing a
gettimeofday() per SQL statement, especially not if that's taken to mean
every SQL statement issued by PL functions (and if it doesn't mean that,
"statement_timestamp" seems like the wrong name).  One gettimeofday()
per client message doesn't seem too horrible though, since that's
certainly going to require at least a couple of kernel calls anyway.

Possibly we should call it "command_timestamp" not "statement_timestamp"
to help reduce confusion.

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).  I don't like it stylistically either: ISTM either
these things are the responsibility of xact.c or they are the
responsibility of postgres.c, it is not sensible to have both modules
assigning to statement_timestamp.

BTW, now that I look at it, the "statement_timeout" GUC variable seems
to have much of the same confusion about whether "statement" is
equivalent to "interactive command" or not.

regards, tom lane

---(end of broadcast)---
TIP 5: don't forget to increase your free space map settings


Re: [PATCHES] Additional current timestamp values

2006-03-20 Thread Bruce Momjian
Neil Conway wrote:
> Bruce Momjian wrote:
> > Peter Eisentraut wrote:
> >> The most common complaint that I recall is that current_timestamp 
> >> returns the transaction timestamp rather than the statement timestamp, 
> >> which is what many expect.  How does your patch address that?
> > 
> > No, we believe the standard requires it.
> 
> My copy of SQL 200n has the following to say:
> 
> Annex C, paragraph 16:
> 
>  "The time of evaluation of the CURRENT_DATE, CURRENT_TIME, and
>  CURRENT_TIMESTAMP functions during the execution of an
>  SQL-statement is implementation-dependent."
> 
> 6.31, :
> 
>  (1) The s CURRENT_DATE, CURRENT_TIME,
>  and CURRENT_TIMESTAMP respectively return the current date,
>  current time, and current timestamp; the time and timestamp values
>  are returned with time zone displacement equal to the current
>  default time zone displacement of the SQL-session. [...]
> 
>  (2) Let S be an  that is not generally
>  contained in a . All s
>  that are contained in s that are generally
>  contained, without an intervening  whose subject
>  routines do not include an SQL function, either in S without an
>  intervening  or in an   statement> contained in the  of a trigger
>  activated as a consequence of executing S, are effectively evaluated
>  simultaneously. The time of evaluation of a   function> during the execution of S and its activated triggers is
>  implementation-dependent.

OK, so we just decided transaction timestamp is the most logical value
for CURRENT_TIMESTAMP.  Anyway, this might mean we should have
transaction_timestamp for completeness.  Not sure.

-- 
  Bruce Momjian   http://candle.pha.pa.us
  SRA OSS, Inc.   http://www.sraoss.com

  + If your life is a hard drive, Christ can be your backup. +

---(end of broadcast)---
TIP 5: don't forget to increase your free space map settings


Re: [PATCHES] Additional current timestamp values

2006-03-20 Thread Bruce Momjian
Neil Conway wrote:
> Bruce Momjian wrote:
> > CURRENT_TIMESTAMP might not be the
> > transaction start time on other database systems.
> > For this reason, and for completeness,
> > transaction_timestamp is provided.
> 
> Well, transaction_timestamp() is even more unlikely to be the 
> transaction start time on other database systems :) If the user wants 
> non-standard syntax for getting the timestamp at which the current 
> transaction began, we already have now().

True, which is why I brought it up.  I think a good argument can be made
that we don't need two non-standard ways of specifying the transaction
timestamp, but we need to decide that as a group.

> > One trick is that these should be the same:
> > 
> > test=> SELECT statement_timestamp(), transaction_timestamp();
> 
> Should they be? It seems quite reasonable to me that the DBMS begins a 
> transaction internally (setting transaction_timestamp()), and then a 
> short while later begins executing the statement submitted by the user, 
> at which point statement_timestamp() is set.
> 
> Perhaps ensuring they are identical for single-statement transactions is 
> the best behavior, I just don't think this is required behavior.

Yea, perhaps it isn't required, but it seems like a good idea.  It will
avoid confusion and seems logical.  :-)

> > And these should be the same:
> > 
> > $ psql -c '
> > INSERT INTO t VALUES (statement_timestamp());
> > INSERT INTO t VALUES (statement_timestamp());' test
> > INSERT 0 1
> 
> Uh, why should these be the same?

This gets into cases where a single statement generates more than one
parsenode, e.g. rules.  We want all the parse nodes to have the same
timestamp.

We had a long discussion that the statement time isn't really
meaningful/logical, so I went with code that said the statement arrival
time is the proper time to return, and be consistent.

-- 
  Bruce Momjian   http://candle.pha.pa.us
  SRA OSS, Inc.   http://www.sraoss.com

  + If your life is a hard drive, Christ can be your backup. +

---(end of broadcast)---
TIP 6: explain analyze is your friend


Re: [PATCHES] Additional current timestamp values

2006-03-20 Thread Neil Conway

Bruce Momjian wrote:

Peter Eisentraut wrote:
The most common complaint that I recall is that current_timestamp 
returns the transaction timestamp rather than the statement timestamp, 
which is what many expect.  How does your patch address that?


No, we believe the standard requires it.


My copy of SQL 200n has the following to say:

Annex C, paragraph 16:

"The time of evaluation of the CURRENT_DATE, CURRENT_TIME, and
CURRENT_TIMESTAMP functions during the execution of an
SQL-statement is implementation-dependent."

6.31, :

(1) The s CURRENT_DATE, CURRENT_TIME,
and CURRENT_TIMESTAMP respectively return the current date,
current time, and current timestamp; the time and timestamp values
are returned with time zone displacement equal to the current
default time zone displacement of the SQL-session. [...]

(2) Let S be an  that is not generally
contained in a . All s
that are contained in s that are generally
contained, without an intervening  whose subject
routines do not include an SQL function, either in S without an
intervening  or in an  contained in the  of a trigger
activated as a consequence of executing S, are effectively evaluated
simultaneously. The time of evaluation of a  during the execution of S and its activated triggers is
implementation-dependent.

-Neil


---(end of broadcast)---
TIP 3: Have you checked our extensive FAQ?

  http://www.postgresql.org/docs/faq


Re: [PATCHES] Additional current timestamp values

2006-03-20 Thread Neil Conway

Bruce Momjian wrote:

CURRENT_TIMESTAMP might not be the
transaction start time on other database systems.
For this reason, and for completeness,
transaction_timestamp is provided.


Well, transaction_timestamp() is even more unlikely to be the 
transaction start time on other database systems :) If the user wants 
non-standard syntax for getting the timestamp at which the current 
transaction began, we already have now().



One trick is that these should be the same:

test=> SELECT statement_timestamp(), transaction_timestamp();


Should they be? It seems quite reasonable to me that the DBMS begins a 
transaction internally (setting transaction_timestamp()), and then a 
short while later begins executing the statement submitted by the user, 
at which point statement_timestamp() is set.


Perhaps ensuring they are identical for single-statement transactions is 
the best behavior, I just don't think this is required behavior.



And these should be the same:

$ psql -c '
INSERT INTO t VALUES (statement_timestamp());
INSERT INTO t VALUES (statement_timestamp());' test
INSERT 0 1


Uh, why should these be the same?

-Neil

---(end of broadcast)---
TIP 6: explain analyze is your friend


Re: [PATCHES] Additional current timestamp values

2006-03-20 Thread Bruce Momjian
Peter Eisentraut wrote:
> Bruce Momjian wrote:
> > Rather than applying the above patch, I have implemented this TODO
> > with the attached patch:
> >
> > * Add transaction_timestamp(), statement_timestamp(),
> > clock_timestamp() functionality
> 
> The most common complaint that I recall is that current_timestamp 
> returns the transaction timestamp rather than the statement timestamp, 
> which is what many expect.  How does your patch address that?

No, we believe the standard requires it.

> Do we really need clock_timestamp?

Yes, because timeofday() returns a unix text string.  Some people do
want a wallclock current timestamp.

-- 
  Bruce Momjian   http://candle.pha.pa.us
  SRA OSS, Inc.   http://www.sraoss.com

  + If your life is a hard drive, Christ can be your backup. +

---(end of broadcast)---
TIP 6: explain analyze is your friend


Re: [PATCHES] Additional current timestamp values

2006-03-20 Thread Peter Eisentraut
Bruce Momjian wrote:
> Rather than applying the above patch, I have implemented this TODO
> with the attached patch:
>
>   * Add transaction_timestamp(), statement_timestamp(),
> clock_timestamp() functionality

The most common complaint that I recall is that current_timestamp 
returns the transaction timestamp rather than the statement timestamp, 
which is what many expect.  How does your patch address that?

Do we really need clock_timestamp?

-- 
Peter Eisentraut
http://developer.postgresql.org/~petere/

---(end of broadcast)---
TIP 3: Have you checked our extensive FAQ?

   http://www.postgresql.org/docs/faq


[PATCHES] Additional current timestamp values

2006-03-20 Thread Bruce Momjian
I hBrendan Jurd wrote:
> On 8/7/05, Brendan Jurd <[EMAIL PROTECTED]> wrote:
> > Hi all,
> > 
> > I propose to add an internal function gettime() that transparently
> > returns the current system time, as a timestamptz with maximum
> > precision.

Rather than applying the above patch, I have implemented this TODO with
the attached patch:

* Add transaction_timestamp(), statement_timestamp(), clock_timestamp()
  functionality

  Current CURRENT_TIMESTAMP returns the start time of the current
  transaction, and gettimeofday() returns the wallclock time. This will
  make time reporting more consistent and will allow reporting of
  the statement start time.

I questioned whether we need transaction_timestamp() because it is the
same as CURRENT_TIMESTAMP and now(), but added this to the docs:

CURRENT_TIMESTAMP might not be the
transaction start time on other database systems.
For this reason, and for completeness,
transaction_timestamp is provided.

The overhead of this patch is an additional gettimeofday() call for each
statement in a multi-statement transaction.  We already do a
gettimeofday() for each transaction, even single-statement transactions.
I see no way to avoid the additional function call.

One trick is that these should be the same:

test=> SELECT statement_timestamp(), transaction_timestamp();
  statement_timestamp  | transaction_timestamp
---+---
 2006-03-20 16:59:33.790335-05 | 2006-03-20 16:59:33.790335-05
(1 row)

and these should be different:

test=> BEGIN;
BEGIN
test=> select statement_timestamp(), transaction_timestamp();
  statement_timestamp  | transaction_timestamp
---+---
 2006-03-20 16:59:55.347467-05 | 2006-03-20 16:59:54.520446-05
(1 row)

And these should be the same:

$ psql -c '
INSERT INTO t VALUES (statement_timestamp());
INSERT INTO t VALUES (statement_timestamp());' test
INSERT 0 1
$ psql test
Welcome to psql 8.2devel, the PostgreSQL interactive terminal.

Type:  \copyright for distribution terms
   \h for help with SQL commands
   \? for help with psql commands
   \g or terminate with semicolon to execute query
   \q to quit

test=> SELECT * FROM t;
   x
---
 2006-03-20 17:06:02.057077-05
 2006-03-20 17:06:02.057077-05
(2 rows)

And they all work.  Is there a cleaner method than the one I have used?

I have also improved the documentation so it is clearer what value is
returned by each current data/time function.

-- 
  Bruce Momjian   http://candle.pha.pa.us
  SRA OSS, Inc.   http://www.sraoss.com

  + If your life is a hard drive, Christ can be your backup. +
Index: doc/src/sgml/func.sgml
===
RCS file: /cvsroot/pgsql/doc/src/sgml/func.sgml,v
retrieving revision 1.313
diff -c -c -r1.313 func.sgml
*** doc/src/sgml/func.sgml  10 Mar 2006 20:15:25 -  1.313
--- doc/src/sgml/func.sgml  17 Mar 2006 20:04:27 -
***
*** 5303,5308 
--- 5303,5317 
  now
 
 
+ transaction_timestamp
+
+
+ statement_timestamp
+
+
+ clock_timestamp
+
+
  timeofday
 
  
***
*** 5358,5364 
 
  
current_timestamp
  timestamp with time zone
! Date and time; see 
  
  
  
--- 5367,5373 
 
  
current_timestamp
  timestamp with time zone
! Date and time of start of current transaction; see 
  
  
  
***
*** 5474,5481 
 
  now()
  timestamp with time zone
! Current date and time (equivalent to
!  current_timestamp); see 
  
  
  
--- 5483,5518 
 
  now()
  timestamp with time zone
! Date and time of start of current transaction (equivalent to
!  CURRENT_TIMESTAMP); see 
! 
! 
! 
!
! 
!
! 
transaction_timestamp()
! timestamp with time zone
! Date and time of start of current transaction (equivalent to
!  CURRENT_TIMESTAMP); see 
! 
! 
! 
!
! 
!
! 
statement_timestamp()
! timestamp with time zone
! Date and time of start of current statement; see 
! 
! 
! 
!
! 
!
! 
clock_timestamp()
! timestamp with time zone
! Current date and time (changes during statement execution); 
see 
  
  
  
*