Re: [HACKERS] xlog location arithmetic

2012-04-14 Thread Fujii Masao
On Sat, Apr 14, 2012 at 5:30 AM, Robert Haas robertmh...@gmail.com wrote:
 On Mon, Mar 12, 2012 at 10:34 PM, Fujii Masao masao.fu...@gmail.com wrote:
 I can see a usecase for having a pg_size_pretty(numeric) as an option.
 Not necessarily a very big one, but a 0 one.

 +1.

 +1, too.

 I did some beautification of this patch.  I think the attached version
 is cleaner and easier to read.  Thoughts?

Looks good to me. Thanks for polishing the patch!

Regards,

-- 
Fujii Masao

-- 
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] xlog location arithmetic

2012-04-14 Thread Robert Haas
On Sat, Apr 14, 2012 at 7:25 AM, Fujii Masao masao.fu...@gmail.com wrote:
 On Sat, Apr 14, 2012 at 5:30 AM, Robert Haas robertmh...@gmail.com wrote:
 On Mon, Mar 12, 2012 at 10:34 PM, Fujii Masao masao.fu...@gmail.com wrote:
 I can see a usecase for having a pg_size_pretty(numeric) as an option.
 Not necessarily a very big one, but a 0 one.

 +1.

 +1, too.

 I did some beautification of this patch.  I think the attached version
 is cleaner and easier to read.  Thoughts?

 Looks good to me. Thanks for polishing the patch!

You're welcome.  Committed.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
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] xlog location arithmetic

2012-04-13 Thread Robert Haas
On Mon, Mar 12, 2012 at 10:34 PM, Fujii Masao masao.fu...@gmail.com wrote:
 I can see a usecase for having a pg_size_pretty(numeric) as an option.
 Not necessarily a very big one, but a 0 one.

 +1.

 +1, too.

I did some beautification of this patch.  I think the attached version
is cleaner and easier to read.  Thoughts?

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


sizepretty_v4.patch
Description: Binary data

-- 
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] xlog location arithmetic

2012-03-12 Thread Bruce Momjian
On Fri, Mar 09, 2012 at 03:04:23PM -0500, Tom Lane wrote:
 The main actual simplification would be in getting rid of the hole
 at the end of each 4GB worth of WAL, cf this bit in xlog_internal.h:
 
 /*
  * We break each logical log file (xlogid value) into segment files of the
  * size indicated by XLOG_SEG_SIZE.  One possible segment at the end of each
  * log file is wasted, to ensure that we don't have problems representing
  * last-byte-position-plus-1.
  */
 #define XLogSegSize   ((uint32) XLOG_SEG_SIZE)
 #define XLogSegsPerFile (((uint32) 0x) / XLogSegSize)
 #define XLogFileSize  (XLogSegsPerFile * XLogSegSize)
 
 If we can't get rid of that and have a continuous 64-bit WAL address
 space then it's unlikely we can actually simplify any logic.
 
 Now, doing that doesn't break the naming convention exactly; what it
 changes is that there will be WAL files numbered xxx (for some
 number of trailing-1-bits I'm too lazy to work out at the moment) where
 before there were not.  So the question really is how much external code
 there is that is aware of that specific noncontiguous numbering behavior
 and would be broken if things stopped being that way.

Our current WAL naming is hopelessly arcane, and we would certainly be
benfitting users to simplify it.  Is this a TODO?

-- 
  Bruce Momjian  br...@momjian.ushttp://momjian.us
  EnterpriseDB http://enterprisedb.com

  + It's impossible for everything to be true. +

-- 
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] xlog location arithmetic

2012-03-12 Thread Fujii Masao
On Sat, Mar 10, 2012 at 3:23 AM, Robert Haas robertmh...@gmail.com wrote:
 On Fri, Mar 9, 2012 at 12:38 PM, Magnus Hagander mag...@hagander.net wrote:
 On Fri, Mar 9, 2012 at 18:18, Tom Lane t...@sss.pgh.pa.us wrote:
 Magnus Hagander mag...@hagander.net writes:
 On Fri, Mar 9, 2012 at 15:37, Tom Lane t...@sss.pgh.pa.us wrote:
 Why would it be useful to use pg_size_pretty on xlog locations?
 -1 because of the large expense of bigint-numeric-whatever conversion
 that would be added to existing uses.

 Given the expense, perhaps we need to different (overloaded) functions 
 instead?

Agreed. Attached patch introduces the overloaded funtion
pg_size_pretty(numeric).

 That would be a workable solution, but I continue to not believe that
 this is useful enough to be worth the trouble.

 There's certainly some use to being able to prettify it. Wouldn't a
 pg_size_pretty(numeric) also be useful if you want to pg_size_() a
 sum() of something? Used on files it doesn't make too much sense,
 given how big those files have to be, but it can be used on other
 things as well...

 I can see a usecase for having a pg_size_pretty(numeric) as an option.
 Not necessarily a very big one, but a 0 one.

 +1.

+1, too.

Regards,

-- 
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center
*** a/doc/src/sgml/func.sgml
--- b/doc/src/sgml/func.sgml
***
*** 14989,14995  postgres=# SELECT * FROM pg_xlogfile_name_offset(pg_stop_backup());
/row
row
 entry
! literalfunctionpg_size_pretty(typebigint/type)/function/literal
  /entry
 entrytypetext/type/entry
 entryConverts a size in bytes into a human-readable format with size units/entry
--- 14989,14995 
/row
row
 entry
! literalfunctionpg_size_pretty(typebigint/type or typenumeric/type)/function/literal
  /entry
 entrytypetext/type/entry
 entryConverts a size in bytes into a human-readable format with size units/entry
*** a/src/backend/utils/adt/dbsize.c
--- b/src/backend/utils/adt/dbsize.c
***
*** 24,29 
--- 24,30 
  #include storage/fd.h
  #include utils/acl.h
  #include utils/builtins.h
+ #include utils/numeric.h
  #include utils/rel.h
  #include utils/relmapper.h
  #include utils/syscache.h
***
*** 550,555  pg_size_pretty(PG_FUNCTION_ARGS)
--- 551,652 
  	PG_RETURN_TEXT_P(cstring_to_text(buf));
  }
  
+ Datum
+ pg_size_pretty_numeric(PG_FUNCTION_ARGS)
+ {
+ 	Numeric		size = PG_GETARG_NUMERIC(0);
+ 	Numeric		limit, limit2;
+ 	char		*buf, *result;
+ 
+ 	limit = DatumGetNumeric(DirectFunctionCall1(int8_numeric, Int64GetDatum((int64) (10 * 1024;
+ 	limit2 = DatumGetNumeric(DirectFunctionCall1(int8_numeric, Int64GetDatum((int64) (10 * 1024 * 2 - 1;
+ 
+ 	if (DatumGetBool(DirectFunctionCall2(numeric_lt, NumericGetDatum(size), NumericGetDatum(limit
+ 	{
+ 		buf = DatumGetCString(DirectFunctionCall1(numeric_out, NumericGetDatum(size)));
+ 		result = palloc(strlen(buf) + 7);
+ 		strcpy(result, buf);
+ 		strcat(result,  bytes);
+ 	}
+ 	else
+ 	{
+ 		Numeric		arg2;
+ 
+ 		/* keep one extra bit for rounding */
+ 		/* size = 9 */
+ 		arg2 = DatumGetNumeric(DirectFunctionCall1(int8_numeric, Int64GetDatum((int64) pow(2, 9;
+ 		size = DatumGetNumeric(DirectFunctionCall2(numeric_div_trunc, NumericGetDatum(size), NumericGetDatum(arg2)));
+ 
+ 		if (DatumGetBool(DirectFunctionCall2(numeric_lt, NumericGetDatum(size), NumericGetDatum(limit2
+ 		{
+ 			/* size = (size + 1) / 2 */
+ 			size = DatumGetNumeric(DirectFunctionCall2(numeric_add, NumericGetDatum(size),
+ 	   DirectFunctionCall1(int8_numeric, Int64GetDatum(1;
+ 			size = DatumGetNumeric(DirectFunctionCall2(numeric_div_trunc, NumericGetDatum(size),
+ 	   DirectFunctionCall1(int8_numeric, Int64GetDatum(2;
+ 			buf = DatumGetCString(DirectFunctionCall1(numeric_out, NumericGetDatum(size)));
+ 			result = palloc(strlen(buf) + 4);
+ 			strcpy(result, buf);
+ 			strcat(result,  kB);
+ 		}
+ 		else
+ 		{
+ 			Numeric		arg3;
+ 
+ 			/* size = 10 */
+ 			arg3 = DatumGetNumeric(DirectFunctionCall1(int8_numeric, Int64GetDatum((int64) pow(2, 10;
+ 			size = DatumGetNumeric(DirectFunctionCall2(numeric_div_trunc, NumericGetDatum(size), NumericGetDatum(arg3)));
+ 
+ 			if (DatumGetBool(DirectFunctionCall2(numeric_lt, NumericGetDatum(size), NumericGetDatum(limit2
+ 			{
+ /* size = (size + 1) / 2 */
+ size = DatumGetNumeric(DirectFunctionCall2(numeric_add, NumericGetDatum(size),
+ 		   DirectFunctionCall1(int8_numeric, Int64GetDatum(1;
+ size = DatumGetNumeric(DirectFunctionCall2(numeric_div_trunc, NumericGetDatum(size),
+ 		   DirectFunctionCall1(int8_numeric, Int64GetDatum(2;
+ buf = DatumGetCString(DirectFunctionCall1(numeric_out, NumericGetDatum(size)));
+ result = palloc(strlen(buf) + 4);
+ strcpy(result, buf);
+ strcat(result,  MB);
+ 			}

Re: [HACKERS] xlog location arithmetic

2012-03-12 Thread Fujii Masao
On Sat, Mar 10, 2012 at 5:04 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 Robert Haas robertmh...@gmail.com writes:
 On Fri, Mar 9, 2012 at 2:34 PM, Robert Haas robertmh...@gmail.com wrote:
 On Fri, Mar 9, 2012 at 2:26 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Hm.  I think thousands is an overestimate, but yeah the logic could be
 greatly simplified.  However, I'm not sure we could avoid breaking the
 existing naming convention for WAL files.  How much do we care about
 that?

 Probably not very much, since WAL files aren't portable across major
 versions anyway.  But I don't see why you couldn't keep the naming
 convention - there's nothing to prevent you from converting a 64-bit
 integer back into two 32-bit integers if and where needed.

 On further reflection, this seems likely to break quite a few
 third-party tools.  Maybe it'd be worth it anyway, but it definitely
 seems like it would be worth going to at least some minor trouble to
 avoid it.

 The main actual simplification would be in getting rid of the hole
 at the end of each 4GB worth of WAL, cf this bit in xlog_internal.h:

 /*
  * We break each logical log file (xlogid value) into segment files of the
  * size indicated by XLOG_SEG_SIZE.  One possible segment at the end of each
  * log file is wasted, to ensure that we don't have problems representing
  * last-byte-position-plus-1.
  */
 #define XLogSegSize             ((uint32) XLOG_SEG_SIZE)
 #define XLogSegsPerFile (((uint32) 0x) / XLogSegSize)
 #define XLogFileSize    (XLogSegsPerFile * XLogSegSize)

 If we can't get rid of that and have a continuous 64-bit WAL address
 space then it's unlikely we can actually simplify any logic.

 Now, doing that doesn't break the naming convention exactly; what it
 changes is that there will be WAL files numbered xxx (for some
 number of trailing-1-bits I'm too lazy to work out at the moment) where
 before there were not.  So the question really is how much external code
 there is that is aware of that specific noncontiguous numbering behavior
 and would be broken if things stopped being that way.

A page header contains WAL location, so getting rid of hole seems to
break pg_upgrade. No? Unless pg_upgrade converts noncontinuous
location to continuous one, we still need to handle noncontinuous one
after upgrade.

Regards,

-- 
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

-- 
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] xlog location arithmetic

2012-03-12 Thread Tom Lane
Fujii Masao masao.fu...@gmail.com writes:
 On Sat, Mar 10, 2012 at 5:04 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 The main actual simplification would be in getting rid of the hole
 at the end of each 4GB worth of WAL, cf this bit in xlog_internal.h:
 If we can't get rid of that and have a continuous 64-bit WAL address
 space then it's unlikely we can actually simplify any logic.
 ...
 Now, doing that doesn't break the naming convention exactly; what it
 changes is that there will be WAL files numbered xxx (for some
 number of trailing-1-bits I'm too lazy to work out at the moment) where
 before there were not.  So the question really is how much external code
 there is that is aware of that specific noncontiguous numbering behavior
 and would be broken if things stopped being that way.

 A page header contains WAL location, so getting rid of hole seems to
 break pg_upgrade. No?

No, why would it do that?  The meaning and ordering of WAL addresses is
the same as before.  The only difference is that after the upgrade, the
system will stop skipping over 16MB of potentially usable WAL addresses
at the end of each subsequently-used 4GB of space.  The holes before
the switchover point are still holes, but that doesn't matter.

regards, tom lane

-- 
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] xlog location arithmetic

2012-03-12 Thread Fujii Masao
On Tue, Mar 13, 2012 at 12:03 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Fujii Masao masao.fu...@gmail.com writes:
 On Sat, Mar 10, 2012 at 5:04 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 The main actual simplification would be in getting rid of the hole
 at the end of each 4GB worth of WAL, cf this bit in xlog_internal.h:
 If we can't get rid of that and have a continuous 64-bit WAL address
 space then it's unlikely we can actually simplify any logic.
 ...
 Now, doing that doesn't break the naming convention exactly; what it
 changes is that there will be WAL files numbered xxx (for some
 number of trailing-1-bits I'm too lazy to work out at the moment) where
 before there were not.  So the question really is how much external code
 there is that is aware of that specific noncontiguous numbering behavior
 and would be broken if things stopped being that way.

 A page header contains WAL location, so getting rid of hole seems to
 break pg_upgrade. No?

 No, why would it do that?  The meaning and ordering of WAL addresses is
 the same as before.  The only difference is that after the upgrade, the
 system will stop skipping over 16MB of potentially usable WAL addresses
 at the end of each subsequently-used 4GB of space.  The holes before
 the switchover point are still holes, but that doesn't matter.

Oh, I see. You're right.

Regards,

-- 
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

-- 
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] xlog location arithmetic

2012-03-09 Thread Fujii Masao
On Sun, Mar 4, 2012 at 8:26 PM, Magnus Hagander mag...@hagander.net wrote:
 On Sun, Feb 26, 2012 at 00:53, Euler Taveira de Oliveira
 eu...@timbira.com wrote:
 On 25-02-2012 09:23, Magnus Hagander wrote:
 Do we even *need* the validate_xlog_location() function? If we just
 remove those calls, won't we still catch all the incorrectly formatted
 ones in the errors of the sscanf() calls? Or am I too deep into
 weekend-mode and missing something obvious?

 sscanf() is too fragile for input sanity check. Try
 pg_xlog_location_diff('12/3', '-10/0'), for example. I won't object removing
 that function if you protect xlog location input from silly users.

 Ah, good point. No, that's the reason I was missing :-)

 Patch applied, thanks!

Thanks for committing the patch!

Euler proposed one more patch upthread, which replaces pg_size_pretty(bigint)
with pg_size_pretty(numeric) so that pg_size_pretty(pg_xlog_location_diff())
succeeds. It's also worth committing this patch?
http://archives.postgresql.org/message-id/4f315f6c.8030...@timbira.com

Regards,

-- 
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

-- 
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] xlog location arithmetic

2012-03-09 Thread Tom Lane
Fujii Masao masao.fu...@gmail.com writes:
 Euler proposed one more patch upthread, which replaces pg_size_pretty(bigint)
 with pg_size_pretty(numeric) so that pg_size_pretty(pg_xlog_location_diff())
 succeeds. It's also worth committing this patch?

Why would it be useful to use pg_size_pretty on xlog locations?
-1 because of the large expense of bigint-numeric-whatever conversion
that would be added to existing uses.

regards, tom lane

-- 
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] xlog location arithmetic

2012-03-09 Thread Robert Haas
On Fri, Mar 9, 2012 at 9:37 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 Fujii Masao masao.fu...@gmail.com writes:
 Euler proposed one more patch upthread, which replaces pg_size_pretty(bigint)
 with pg_size_pretty(numeric) so that pg_size_pretty(pg_xlog_location_diff())
 succeeds. It's also worth committing this patch?

 Why would it be useful to use pg_size_pretty on xlog locations?
 -1 because of the large expense of bigint-numeric-whatever conversion
 that would be added to existing uses.

The point is that it would be useful to use it on the difference
between two xlog locations, but that is a numeric value, not int8,
because of signedness issues.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
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] xlog location arithmetic

2012-03-09 Thread Tom Lane
I wrote:
 Fujii Masao masao.fu...@gmail.com writes:
 Euler proposed one more patch upthread, which replaces pg_size_pretty(bigint)
 with pg_size_pretty(numeric) so that pg_size_pretty(pg_xlog_location_diff())
 succeeds. It's also worth committing this patch?

 Why would it be useful to use pg_size_pretty on xlog locations?
 -1 because of the large expense of bigint-numeric-whatever conversion
 that would be added to existing uses.

Actually ... now that I look at it, isn't it completely bogus to be
using numeric for the result of pg_xlog_location_diff?  There's no way
for the difference of two xlog locations to be anywhere near as wide as
64 bits.  That'd only be possible if XLogFileSize exceeded 1GB, which we
don't let it get anywhere near.

regards, tom lane

-- 
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] xlog location arithmetic

2012-03-09 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 On Fri, Mar 9, 2012 at 9:37 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 Why would it be useful to use pg_size_pretty on xlog locations?

 The point is that it would be useful to use it on the difference
 between two xlog locations,

Um, that is exactly the claim I was questioning.  Why is that useful?

 but that is a numeric value, not int8, because of signedness issues.

See my followup --- this statement appears factually incorrect,
whatever you may feel about the usefulness issue.

regards, tom lane

-- 
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] xlog location arithmetic

2012-03-09 Thread Robert Haas
On Fri, Mar 9, 2012 at 9:55 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 I wrote:
 Fujii Masao masao.fu...@gmail.com writes:
 Euler proposed one more patch upthread, which replaces 
 pg_size_pretty(bigint)
 with pg_size_pretty(numeric) so that pg_size_pretty(pg_xlog_location_diff())
 succeeds. It's also worth committing this patch?

 Why would it be useful to use pg_size_pretty on xlog locations?
 -1 because of the large expense of bigint-numeric-whatever conversion
 that would be added to existing uses.

 Actually ... now that I look at it, isn't it completely bogus to be
 using numeric for the result of pg_xlog_location_diff?  There's no way
 for the difference of two xlog locations to be anywhere near as wide as
 64 bits.  That'd only be possible if XLogFileSize exceeded 1GB, which we
 don't let it get anywhere near.

rhaas=# select pg_xlog_location_diff('/0', '0/0');
 pg_xlog_location_diff
---
  18374686475393433600
(1 row)

rhaas=# select pg_xlog_location_diff('/0', '0/0')::int8;
ERROR:  bigint out of range
STATEMENT:  select pg_xlog_location_diff('/0', '0/0')::int8;

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
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] xlog location arithmetic

2012-03-09 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 On Fri, Mar 9, 2012 at 9:55 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 Actually ... now that I look at it, isn't it completely bogus to be
 using numeric for the result of pg_xlog_location_diff?

 rhaas=# select pg_xlog_location_diff('/0', '0/0')::int8;
 ERROR:  bigint out of range

Oh ... I see my mistake.  I was looking at this:

/*
 * result = XLogFileSize * (xlogid1 - xlogid2) + xrecoff1 - xrecoff2
 */

and confusing XLogFileSize with XLogSegSize.  Not the best choice of
names.

regards, tom lane

-- 
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] xlog location arithmetic

2012-03-09 Thread Magnus Hagander
On Fri, Mar 9, 2012 at 16:37, Tom Lane t...@sss.pgh.pa.us wrote:
 Robert Haas robertmh...@gmail.com writes:
 On Fri, Mar 9, 2012 at 9:55 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 Actually ... now that I look at it, isn't it completely bogus to be
 using numeric for the result of pg_xlog_location_diff?

 rhaas=# select pg_xlog_location_diff('/0', '0/0')::int8;
 ERROR:  bigint out of range

 Oh ... I see my mistake.  I was looking at this:

        /*
         * result = XLogFileSize * (xlogid1 - xlogid2) + xrecoff1 - xrecoff2
         */

 and confusing XLogFileSize with XLogSegSize.  Not the best choice of
 names.

Yeah, the use of XLogFile to mean something other than, well a file in
the xlog, is greatly annoying.. I guess we could change it, but it
goes pretty deep in the system so it's not a small change...

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

-- 
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] xlog location arithmetic

2012-03-09 Thread Magnus Hagander
On Fri, Mar 9, 2012 at 15:37, Tom Lane t...@sss.pgh.pa.us wrote:
 Fujii Masao masao.fu...@gmail.com writes:
 Euler proposed one more patch upthread, which replaces pg_size_pretty(bigint)
 with pg_size_pretty(numeric) so that pg_size_pretty(pg_xlog_location_diff())
 succeeds. It's also worth committing this patch?

 Why would it be useful to use pg_size_pretty on xlog locations?
 -1 because of the large expense of bigint-numeric-whatever conversion
 that would be added to existing uses.

Given the expense, perhaps we need to different (overloaded) functions instead?

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

-- 
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] xlog location arithmetic

2012-03-09 Thread Tom Lane
Magnus Hagander mag...@hagander.net writes:
 On Fri, Mar 9, 2012 at 15:37, Tom Lane t...@sss.pgh.pa.us wrote:
 Why would it be useful to use pg_size_pretty on xlog locations?
 -1 because of the large expense of bigint-numeric-whatever conversion
 that would be added to existing uses.

 Given the expense, perhaps we need to different (overloaded) functions 
 instead?

That would be a workable solution, but I continue to not believe that
this is useful enough to be worth the trouble.

regards, tom lane

-- 
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] xlog location arithmetic

2012-03-09 Thread Magnus Hagander
On Fri, Mar 9, 2012 at 18:18, Tom Lane t...@sss.pgh.pa.us wrote:
 Magnus Hagander mag...@hagander.net writes:
 On Fri, Mar 9, 2012 at 15:37, Tom Lane t...@sss.pgh.pa.us wrote:
 Why would it be useful to use pg_size_pretty on xlog locations?
 -1 because of the large expense of bigint-numeric-whatever conversion
 that would be added to existing uses.

 Given the expense, perhaps we need to different (overloaded) functions 
 instead?

 That would be a workable solution, but I continue to not believe that
 this is useful enough to be worth the trouble.

There's certainly some use to being able to prettify it. Wouldn't a
pg_size_pretty(numeric) also be useful if you want to pg_size_() a
sum() of something? Used on files it doesn't make too much sense,
given how big those files have to be, but it can be used on other
things as well...

I can see a usecase for having a pg_size_pretty(numeric) as an option.
Not necessarily a very big one, but a 0 one.


-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

-- 
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] xlog location arithmetic

2012-03-09 Thread Robert Haas
On Fri, Mar 9, 2012 at 12:38 PM, Magnus Hagander mag...@hagander.net wrote:
 On Fri, Mar 9, 2012 at 18:18, Tom Lane t...@sss.pgh.pa.us wrote:
 Magnus Hagander mag...@hagander.net writes:
 On Fri, Mar 9, 2012 at 15:37, Tom Lane t...@sss.pgh.pa.us wrote:
 Why would it be useful to use pg_size_pretty on xlog locations?
 -1 because of the large expense of bigint-numeric-whatever conversion
 that would be added to existing uses.

 Given the expense, perhaps we need to different (overloaded) functions 
 instead?

 That would be a workable solution, but I continue to not believe that
 this is useful enough to be worth the trouble.

 There's certainly some use to being able to prettify it. Wouldn't a
 pg_size_pretty(numeric) also be useful if you want to pg_size_() a
 sum() of something? Used on files it doesn't make too much sense,
 given how big those files have to be, but it can be used on other
 things as well...

 I can see a usecase for having a pg_size_pretty(numeric) as an option.
 Not necessarily a very big one, but a 0 one.

+1.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
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] xlog location arithmetic

2012-03-09 Thread Peter Eisentraut
On fre, 2012-03-09 at 18:13 +0100, Magnus Hagander wrote:
  and confusing XLogFileSize with XLogSegSize.  Not the best choice of
  names.
 
 Yeah, the use of XLogFile to mean something other than, well a file in
 the xlog, is greatly annoying.. I guess we could change it, but it
 goes pretty deep in the system so it's not a small change...
 
The whole thing was built around the lack of 64 bit integers.  If we bit
the bullet and changed the whole thing to be just a single 64-bit
counter, we could probably delete thousands of lines of code.


-- 
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] xlog location arithmetic

2012-03-09 Thread Tom Lane
Peter Eisentraut pete...@gmx.net writes:
 Yeah, the use of XLogFile to mean something other than, well a file in
 the xlog, is greatly annoying.. I guess we could change it, but it
 goes pretty deep in the system so it's not a small change...

 The whole thing was built around the lack of 64 bit integers.  If we bit
 the bullet and changed the whole thing to be just a single 64-bit
 counter, we could probably delete thousands of lines of code.

Hm.  I think thousands is an overestimate, but yeah the logic could be
greatly simplified.  However, I'm not sure we could avoid breaking the
existing naming convention for WAL files.  How much do we care about
that?

regards, tom lane

-- 
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] xlog location arithmetic

2012-03-09 Thread Robert Haas
On Fri, Mar 9, 2012 at 2:26 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Peter Eisentraut pete...@gmx.net writes:
 Yeah, the use of XLogFile to mean something other than, well a file in
 the xlog, is greatly annoying.. I guess we could change it, but it
 goes pretty deep in the system so it's not a small change...

 The whole thing was built around the lack of 64 bit integers.  If we bit
 the bullet and changed the whole thing to be just a single 64-bit
 counter, we could probably delete thousands of lines of code.

 Hm.  I think thousands is an overestimate, but yeah the logic could be
 greatly simplified.  However, I'm not sure we could avoid breaking the
 existing naming convention for WAL files.  How much do we care about
 that?

Probably not very much, since WAL files aren't portable across major
versions anyway.  But I don't see why you couldn't keep the naming
convention - there's nothing to prevent you from converting a 64-bit
integer back into two 32-bit integers if and where needed.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
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] xlog location arithmetic

2012-03-09 Thread Kevin Grittner
Tom Lane t...@sss.pgh.pa.us wrote:
 Peter Eisentraut pete...@gmx.net writes:
 
 The whole thing was built around the lack of 64 bit integers.  If
 we bit the bullet and changed the whole thing to be just a single
 64-bit counter, we could probably delete thousands of lines of
 code.
 
 Hm.  I think thousands is an overestimate, but yeah the logic
 could be greatly simplified.  However, I'm not sure we could avoid
 breaking the existing naming convention for WAL files.  How much
 do we care about that?
 
We have a few scripts in our backup area that are based around the
current WAL file naming convention, so there would be some impact;
but I have to believe it would be pretty minor.  Most of the pain
would be related to the need to support both naming conventions for
some transition period.  If it simplifies the WAL-related logic, it
seems well worth it to me.  We just have to know it's coming and be
clear on what the new naming rules are.
 
-Kevin

-- 
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] xlog location arithmetic

2012-03-09 Thread Robert Haas
On Fri, Mar 9, 2012 at 2:34 PM, Robert Haas robertmh...@gmail.com wrote:
 On Fri, Mar 9, 2012 at 2:26 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Peter Eisentraut pete...@gmx.net writes:
 Yeah, the use of XLogFile to mean something other than, well a file in
 the xlog, is greatly annoying.. I guess we could change it, but it
 goes pretty deep in the system so it's not a small change...

 The whole thing was built around the lack of 64 bit integers.  If we bit
 the bullet and changed the whole thing to be just a single 64-bit
 counter, we could probably delete thousands of lines of code.

 Hm.  I think thousands is an overestimate, but yeah the logic could be
 greatly simplified.  However, I'm not sure we could avoid breaking the
 existing naming convention for WAL files.  How much do we care about
 that?

 Probably not very much, since WAL files aren't portable across major
 versions anyway.  But I don't see why you couldn't keep the naming
 convention - there's nothing to prevent you from converting a 64-bit
 integer back into two 32-bit integers if and where needed.

On further reflection, this seems likely to break quite a few
third-party tools.  Maybe it'd be worth it anyway, but it definitely
seems like it would be worth going to at least some minor trouble to
avoid it.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
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] xlog location arithmetic

2012-03-09 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 On Fri, Mar 9, 2012 at 2:34 PM, Robert Haas robertmh...@gmail.com wrote:
 On Fri, Mar 9, 2012 at 2:26 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Hm.  I think thousands is an overestimate, but yeah the logic could be
 greatly simplified.  However, I'm not sure we could avoid breaking the
 existing naming convention for WAL files.  How much do we care about
 that?

 Probably not very much, since WAL files aren't portable across major
 versions anyway.  But I don't see why you couldn't keep the naming
 convention - there's nothing to prevent you from converting a 64-bit
 integer back into two 32-bit integers if and where needed.

 On further reflection, this seems likely to break quite a few
 third-party tools.  Maybe it'd be worth it anyway, but it definitely
 seems like it would be worth going to at least some minor trouble to
 avoid it.

The main actual simplification would be in getting rid of the hole
at the end of each 4GB worth of WAL, cf this bit in xlog_internal.h:

/*
 * We break each logical log file (xlogid value) into segment files of the
 * size indicated by XLOG_SEG_SIZE.  One possible segment at the end of each
 * log file is wasted, to ensure that we don't have problems representing
 * last-byte-position-plus-1.
 */
#define XLogSegSize ((uint32) XLOG_SEG_SIZE)
#define XLogSegsPerFile (((uint32) 0x) / XLogSegSize)
#define XLogFileSize(XLogSegsPerFile * XLogSegSize)

If we can't get rid of that and have a continuous 64-bit WAL address
space then it's unlikely we can actually simplify any logic.

Now, doing that doesn't break the naming convention exactly; what it
changes is that there will be WAL files numbered xxx (for some
number of trailing-1-bits I'm too lazy to work out at the moment) where
before there were not.  So the question really is how much external code
there is that is aware of that specific noncontiguous numbering behavior
and would be broken if things stopped being that way.

regards, tom lane

-- 
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] xlog location arithmetic

2012-03-09 Thread Robert Haas
On Fri, Mar 9, 2012 at 3:04 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Robert Haas robertmh...@gmail.com writes:
 On Fri, Mar 9, 2012 at 2:34 PM, Robert Haas robertmh...@gmail.com wrote:
 On Fri, Mar 9, 2012 at 2:26 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Hm.  I think thousands is an overestimate, but yeah the logic could be
 greatly simplified.  However, I'm not sure we could avoid breaking the
 existing naming convention for WAL files.  How much do we care about
 that?

 Probably not very much, since WAL files aren't portable across major
 versions anyway.  But I don't see why you couldn't keep the naming
 convention - there's nothing to prevent you from converting a 64-bit
 integer back into two 32-bit integers if and where needed.

 On further reflection, this seems likely to break quite a few
 third-party tools.  Maybe it'd be worth it anyway, but it definitely
 seems like it would be worth going to at least some minor trouble to
 avoid it.

 The main actual simplification would be in getting rid of the hole
 at the end of each 4GB worth of WAL, cf this bit in xlog_internal.h:

 /*
  * We break each logical log file (xlogid value) into segment files of the
  * size indicated by XLOG_SEG_SIZE.  One possible segment at the end of each
  * log file is wasted, to ensure that we don't have problems representing
  * last-byte-position-plus-1.
  */
 #define XLogSegSize             ((uint32) XLOG_SEG_SIZE)
 #define XLogSegsPerFile (((uint32) 0x) / XLogSegSize)
 #define XLogFileSize    (XLogSegsPerFile * XLogSegSize)

 If we can't get rid of that and have a continuous 64-bit WAL address
 space then it's unlikely we can actually simplify any logic.

 Now, doing that doesn't break the naming convention exactly; what it
 changes is that there will be WAL files numbered xxx (for some
 number of trailing-1-bits I'm too lazy to work out at the moment) where
 before there were not.  So the question really is how much external code
 there is that is aware of that specific noncontiguous numbering behavior
 and would be broken if things stopped being that way.

I would expect that most things would NOT know about that particular
foible, and just be matching pathnames on an RE, which should be fine.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
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] xlog location arithmetic

2012-03-04 Thread Magnus Hagander
On Sun, Feb 26, 2012 at 00:53, Euler Taveira de Oliveira
eu...@timbira.com wrote:
 On 25-02-2012 09:23, Magnus Hagander wrote:
 Do we even *need* the validate_xlog_location() function? If we just
 remove those calls, won't we still catch all the incorrectly formatted
 ones in the errors of the sscanf() calls? Or am I too deep into
 weekend-mode and missing something obvious?

 sscanf() is too fragile for input sanity check. Try
 pg_xlog_location_diff('12/3', '-10/0'), for example. I won't object removing
 that function if you protect xlog location input from silly users.

Ah, good point. No, that's the reason I was missing :-)

Patch applied, thanks!

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

-- 
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] xlog location arithmetic

2012-03-04 Thread Magnus Hagander
On Tue, Feb 28, 2012 at 07:21, Fujii Masao masao.fu...@gmail.com wrote:
 On Sun, Feb 26, 2012 at 8:53 AM, Euler Taveira de Oliveira
 eu...@timbira.com wrote:
 On 25-02-2012 09:23, Magnus Hagander wrote:
 Do we even *need* the validate_xlog_location() function? If we just
 remove those calls, won't we still catch all the incorrectly formatted
 ones in the errors of the sscanf() calls? Or am I too deep into
 weekend-mode and missing something obvious?

 sscanf() is too fragile for input sanity check. Try
 pg_xlog_location_diff('12/3', '-10/0'), for example. I won't object removing
 that function if you protect xlog location input from silly users.

 After this patch will have been committed, it would be better to change
 pg_xlogfile_name() and pg_xlogfile_name_offset() so that they use
 the validate_xlog_location() function to validate the input.

And I've done this part as well.

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

-- 
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] xlog location arithmetic

2012-02-27 Thread Fujii Masao
On Sun, Feb 26, 2012 at 8:53 AM, Euler Taveira de Oliveira
eu...@timbira.com wrote:
 On 25-02-2012 09:23, Magnus Hagander wrote:
 Do we even *need* the validate_xlog_location() function? If we just
 remove those calls, won't we still catch all the incorrectly formatted
 ones in the errors of the sscanf() calls? Or am I too deep into
 weekend-mode and missing something obvious?

 sscanf() is too fragile for input sanity check. Try
 pg_xlog_location_diff('12/3', '-10/0'), for example. I won't object removing
 that function if you protect xlog location input from silly users.

After this patch will have been committed, it would be better to change
pg_xlogfile_name() and pg_xlogfile_name_offset() so that they use
the validate_xlog_location() function to validate the input.

Regards,

-- 
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

-- 
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] xlog location arithmetic

2012-02-25 Thread Magnus Hagander
On Fri, Feb 10, 2012 at 09:32, Fujii Masao masao.fu...@gmail.com wrote:
 On Fri, Feb 10, 2012 at 7:00 AM, Euler Taveira de Oliveira
 eu...@timbira.com wrote:
 On 08-02-2012 09:35, Fujii Masao wrote:

 Fujii, new patch attached. Thanks for your tests.

 Thanks for the new patch!

 But another problem happened. When I changed pg_proc.h so that the unused
 OID was assigned to pg_xlog_location_diff(), and executed the above again,
 I encountered the segmentation fault:

 I reproduced the problems in my old 32-bit laptop. I fixed it casting to
 int64. I also updated the duplicated OID.

 Yep, in the updated patch, I could confirm that the function works fine 
 without
 any error in my machine. The patch looks fine to me except the following minor
 comments:

I started working on this one to commit it, and came up with a few things more.

Do we even *need* the validate_xlog_location() function? If we just
remove those calls, won't we still catch all the incorrectly formatted
ones in the errors of the sscanf() calls? Or am I too deep into
weekend-mode and missing something obvious?

I've also removed tabs in the documentation, fixed the merge confllict
in pg_proc.h that happened during the wait, and fixed some indentation
(updated patch with these changes attached).

But I'm going to hold off committing it until someone confirms I'm not
caught too deeply in weekend-mode and am missing something obvious in
the comment above about validate_xlog_location.

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index e8e637b..4ae76e2 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -14454,11 +14454,15 @@ SELECT set_config('log_statement_stats', 'off', false);
indexterm
 primarypg_xlogfile_name_offset/primary
/indexterm
+   indexterm
+primarypg_xlog_location_diff/primary
+   /indexterm
 
para
 The functions shown in xref
 linkend=functions-admin-backup-table assist in making on-line backups.
-These functions cannot be executed during recovery.
+These functions cannot be executed during recovery (except
+functionpg_xlog_location_diff/function).
/para
 
table id=functions-admin-backup-table
@@ -14526,6 +14530,13 @@ SELECT set_config('log_statement_stats', 'off', false);
entrytypetext/, typeinteger//entry
entryConvert transaction log location string to file name and decimal byte offset within file/entry
   /row
+  row
+   entry
+literalfunctionpg_xlog_location_diff(parameterlocation/ typetext/, parameterlocation/ typetext/)/function/literal
+   /entry
+   entrytypenumeric//entry
+   entryCalculate the difference between two transaction log locations/entry
+  /row
  /tbody
 /tgroup
/table
@@ -14619,6 +14630,13 @@ postgres=# SELECT * FROM pg_xlogfile_name_offset(pg_stop_backup());
/para
 
para
+functionpg_xlog_location_diff/ calculates the difference in bytes
+between two transaction log locations. It can be used with
+structnamepg_stat_replication/structname or some functions shown in
+xref linkend=functions-admin-backup-table to get the replication lag.
+   /para
+
+   para
 For details about proper usage of these functions, see
 xref linkend=continuous-archiving.
/para
diff --git a/src/backend/access/transam/xlogfuncs.c b/src/backend/access/transam/xlogfuncs.c
index 2e10d4d..b8f8152 100644
--- a/src/backend/access/transam/xlogfuncs.c
+++ b/src/backend/access/transam/xlogfuncs.c
@@ -26,6 +26,7 @@
 #include replication/walreceiver.h
 #include storage/smgr.h
 #include utils/builtins.h
+#include utils/numeric.h
 #include utils/guc.h
 #include utils/timestamp.h
 
@@ -465,3 +466,87 @@ pg_is_in_recovery(PG_FUNCTION_ARGS)
 {
 	PG_RETURN_BOOL(RecoveryInProgress());
 }
+
+static void
+validate_xlog_location(char *str)
+{
+#define MAXLSNCOMPONENT		8
+
+	int			len1,
+len2;
+
+	len1 = strspn(str, 0123456789abcdefABCDEF);
+	if (len1  1 || len1  MAXLSNCOMPONENT || str[len1] != '/')
+		ereport(ERROR,
+(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
+ errmsg(invalid input syntax for transaction log location: \%s\, str)));
+
+	len2 = strspn(str + len1 + 1, 0123456789abcdefABCDEF);
+	if (len2  1 || len2  MAXLSNCOMPONENT || str[len1 + 1 + len2] != '\0')
+		ereport(ERROR,
+(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
+ errmsg(invalid input syntax for transaction log location: \%s\, str)));
+}
+
+/*
+ * Compute the difference in bytes between two WAL locations.
+ */
+Datum
+pg_xlog_location_diff(PG_FUNCTION_ARGS)
+{
+	text	   *location1 = PG_GETARG_TEXT_P(0);
+	text	   *location2 = PG_GETARG_TEXT_P(1);
+	char	   *str1,
+			   *str2;
+	XLogRecPtr	loc1,
+loc2;
+	Numeric		result;
+
+	/*
+	 * Read and parse input
+	 */
+	str1 = text_to_cstring(location1);
+	str2 = text_to_cstring(location2);
+
+	validate_xlog_location(str1);
+	validate_xlog_location(str2);
+
+	if 

Re: [HACKERS] xlog location arithmetic

2012-02-25 Thread Euler Taveira de Oliveira
On 25-02-2012 09:23, Magnus Hagander wrote:
 Do we even *need* the validate_xlog_location() function? If we just
 remove those calls, won't we still catch all the incorrectly formatted
 ones in the errors of the sscanf() calls? Or am I too deep into
 weekend-mode and missing something obvious?
 
sscanf() is too fragile for input sanity check. Try
pg_xlog_location_diff('12/3', '-10/0'), for example. I won't object removing
that function if you protect xlog location input from silly users.


-- 
   Euler Taveira de Oliveira - Timbira   http://www.timbira.com.br/
   PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento

-- 
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] xlog location arithmetic

2012-02-10 Thread Fujii Masao
On Fri, Feb 10, 2012 at 7:00 AM, Euler Taveira de Oliveira
eu...@timbira.com wrote:
 On 08-02-2012 09:35, Fujii Masao wrote:

 Fujii, new patch attached. Thanks for your tests.

Thanks for the new patch!

 But another problem happened. When I changed pg_proc.h so that the unused
 OID was assigned to pg_xlog_location_diff(), and executed the above again,
 I encountered the segmentation fault:

 I reproduced the problems in my old 32-bit laptop. I fixed it casting to
 int64. I also updated the duplicated OID.

Yep, in the updated patch, I could confirm that the function works fine without
any error in my machine. The patch looks fine to me except the following minor
comments:

In the document, it's better to explain clearly that the function subtracts the
second argument from the first.

-These functions cannot be executed during recovery.
+   These functions cannot be executed during recovery (except
+   functionpg_xlog_location_diff/function).

+   functionpg_xlog_location_diff/ calculates the difference in bytes
+   between two transaction log locations. It can be used with
+   structnamepg_stat_replication/structname or some functions shown in
+   xref linkend=functions-admin-backup-table to get the replication 
lag.

Very minor comment: you should use spaces rather than a tab to indent each line.

 Why OID needs to be reassigned?

 There isn't a compelling reason. It is just a way to say: hey, it is another
 function with the same old name.

 I'll not attach another version for pg_size_pretty because it is a matter of
 updating a duplicated OID.

Okay, I reviewed the previous patch again. That looks fine to me.

Regards,

-- 
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

-- 
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] xlog location arithmetic

2012-02-09 Thread Euler Taveira de Oliveira
On 08-02-2012 09:35, Fujii Masao wrote:

Fujii, new patch attached. Thanks for your tests.

 But another problem happened. When I changed pg_proc.h so that the unused
 OID was assigned to pg_xlog_location_diff(), and executed the above again,
 I encountered the segmentation fault:
 
I reproduced the problems in my old 32-bit laptop. I fixed it casting to
int64. I also updated the duplicated OID.

 Why OID needs to be reassigned?
 
There isn't a compelling reason. It is just a way to say: hey, it is another
function with the same old name.

I'll not attach another version for pg_size_pretty because it is a matter of
updating a duplicated OID.


-- 
   Euler Taveira de Oliveira - Timbira   http://www.timbira.com.br/
   PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 236a60a..826f002 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -14446,11 +14446,15 @@ SELECT set_config('log_statement_stats', 'off', false);
indexterm
 primarypg_xlogfile_name_offset/primary
/indexterm
+   indexterm
+primarypg_xlog_location_diff/primary
+   /indexterm
 
para
 The functions shown in xref
 linkend=functions-admin-backup-table assist in making on-line backups.
-These functions cannot be executed during recovery.
+	These functions cannot be executed during recovery (except
+	functionpg_xlog_location_diff/function).
/para
 
table id=functions-admin-backup-table
@@ -14518,6 +14522,13 @@ SELECT set_config('log_statement_stats', 'off', false);
entrytypetext/, typeinteger//entry
entryConvert transaction log location string to file name and decimal byte offset within file/entry
   /row
+  row
+   entry
+literalfunctionpg_xlog_location_diff(parameterlocation/ typetext/, parameterlocation/ typetext/)/function/literal
+/entry
+   entrytypenumeric//entry
+   entryCalculate the difference between two transaction log locations/entry
+  /row
  /tbody
 /tgroup
/table
@@ -14611,6 +14622,13 @@ postgres=# SELECT * FROM pg_xlogfile_name_offset(pg_stop_backup());
/para
 
para
+	functionpg_xlog_location_diff/ calculates the difference in bytes
+	between two transaction log locations. It can be used with
+	structnamepg_stat_replication/structname or some functions shown in
+	xref linkend=functions-admin-backup-table to get the replication lag.
+   /para
+
+   para
 For details about proper usage of these functions, see
 xref linkend=continuous-archiving.
/para
diff --git a/src/backend/access/transam/xlogfuncs.c b/src/backend/access/transam/xlogfuncs.c
index 2e10d4d..be7d388 100644
--- a/src/backend/access/transam/xlogfuncs.c
+++ b/src/backend/access/transam/xlogfuncs.c
@@ -26,6 +26,7 @@
 #include replication/walreceiver.h
 #include storage/smgr.h
 #include utils/builtins.h
+#include utils/numeric.h
 #include utils/guc.h
 #include utils/timestamp.h
 
@@ -465,3 +466,83 @@ pg_is_in_recovery(PG_FUNCTION_ARGS)
 {
 	PG_RETURN_BOOL(RecoveryInProgress());
 }
+
+static void
+validate_xlog_location(char *str)
+{
+#define	MAXLSNCOMPONENT		8
+
+	int	len1, len2;
+
+	len1 = strspn(str, 0123456789abcdefABCDEF);
+	if (len1  1 || len1  MAXLSNCOMPONENT || str[len1] != '/')
+		ereport(ERROR,
+(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
+			 errmsg(invalid input syntax for transaction log location: \%s\, str)));
+	len2 = strspn(str + len1 + 1, 0123456789abcdefABCDEF);
+	if (len2  1 || len2  MAXLSNCOMPONENT || str[len1 + 1 + len2] != '\0')
+		ereport(ERROR,
+(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
+			 errmsg(invalid input syntax for transaction log location: \%s\, str)));
+}
+
+/*
+ * Compute the difference in bytes between two WAL locations.
+ */
+Datum
+pg_xlog_location_diff(PG_FUNCTION_ARGS)
+{
+	text		*location1 = PG_GETARG_TEXT_P(0);
+	text		*location2 = PG_GETARG_TEXT_P(1);
+	char		*str1, *str2;
+	XLogRecPtr	loc1, loc2;
+	Numeric		result;
+
+	/*
+	 * Read and parse input
+	 */
+	str1 = text_to_cstring(location1);
+	str2 = text_to_cstring(location2);
+
+	validate_xlog_location(str1);
+	validate_xlog_location(str2);
+
+	if (sscanf(str1, %X/%X, loc1.xlogid, loc1.xrecoff) != 2)
+		ereport(ERROR,
+(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+			errmsg(could not parse transaction log location \%s\, str1)));
+	if (sscanf(str2, %X/%X, loc2.xlogid, loc2.xrecoff) != 2)
+		ereport(ERROR,
+(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+			errmsg(could not parse transaction log location \%s\, str2)));
+
+	/*
+	 * Sanity check
+	 */
+	if (loc1.xrecoff  XLogFileSize)
+		ereport(ERROR,
+(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+			errmsg(xrecoff \%X\ is out of valid range, 0..%X, loc1.xrecoff, XLogFileSize)));
+	if (loc2.xrecoff  XLogFileSize)
+		ereport(ERROR,
+(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+			errmsg(xrecoff \%X\ is out of valid range, 0..%X, loc2.xrecoff, XLogFileSize)));
+
+	/*
+	 * result = 

Re: [HACKERS] xlog location arithmetic

2012-02-08 Thread Fujii Masao
On Wed, Feb 8, 2012 at 2:29 AM, Euler Taveira de Oliveira
eu...@timbira.com wrote:
 On 26-01-2012 06:19, Fujii Masao wrote:

 Thanks for your review. Comments below.

 When I compiled the source with xlogdiff.patch, I got the following warnings.

 xlogfuncs.c:511:2: warning: format '%lX' expects argument of type
 'long unsigned int *', but argument 3 has type 'uint64 *' [-Wformat]

 What is your compiler? I'm using gcc 4.6.2. I refactored the patch so I'm now
 using XLogRecPtr and %X.

gcc version 4.6.1 (Ubuntu/Linaro 4.6.1-9ubuntu3)

$ uname -a
Linux hermes 3.0.0-15-generic #26-Ubuntu SMP Fri Jan 20 15:59:53 UTC
2012 i686 i686 i386 GNU/Linux

In the updated version of the patch, I got no warnings at the compile time.
But initdb failed because the OID which you assigned to pg_xlog_location_diff()
has already been used for other function. So you need to update pg_proc.h.

 postgres=# SELECT pg_xlog_location_diff('0/274', '0/274');
 ERROR:  xrecoff 274 is out of valid range, 0..A4A534C

 Ugh? I can't reproduce that. It seems to be related to long int used by the
 prior version.

Maybe.

But another problem happened. When I changed pg_proc.h so that the unused
OID was assigned to pg_xlog_location_diff(), and executed the above again,
I encountered the segmentation fault:

LOG:  server process (PID 14384) was terminated by signal 11: Segmentation fault
DETAIL:  Failed process was running: SELECT
pg_xlog_location_diff('0/274', '0/274');
LOG:  terminating any other active server processes

ISTM that the cause is that int8_numeric() is executed for uint32 value. We
should use int4_numeric(), instead?

 While the output was int8 I could use
 pg_size_pretty but now I couldn't. I attached another patch that implements
 pg_size_pretty(numeric).

 I realized that it collides with the pg_size_pretty(int8) if we don't specify
 a type. Hence, I decided to drop the pg_size_pretty(int8) in favor of
 pg_size_pretty(numeric). It is slower than the former but it is not a
 performance critical function.

I'm OK with this.

-DATA(insert OID = 2288 ( pg_size_prettyPGNSP PGUID 12 
1 0 0 0 f f
f t f v 1 0 25 20 _null_ _null_ _null_ _null_ pg_size_pretty _null_
_null_ _null_ ));
-DESCR(convert a long int to a human readable text using size units);
+DATA(insert OID = 3158 ( pg_size_prettyPGNSP PGUID 12 
1 0 0 0 f f
f t f v 1 0 25 1700 _null_ _null_ _null_ _null_ pg_size_pretty
_null_ _null_ _null_ ));
+DESCR(convert a numeric to a human readable text using size units);

Why OID needs to be reassigned?

Regards,

-- 
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

-- 
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] xlog location arithmetic

2012-02-07 Thread Euler Taveira de Oliveira
On 26-01-2012 06:19, Fujii Masao wrote:

Thanks for your review. Comments below.

 When I compiled the source with xlogdiff.patch, I got the following warnings.
 
 xlogfuncs.c:511:2: warning: format '%lX' expects argument of type
 'long unsigned int *', but argument 3 has type 'uint64 *' [-Wformat]
 
What is your compiler? I'm using gcc 4.6.2. I refactored the patch so I'm now
using XLogRecPtr and %X.

 postgres=# SELECT pg_xlog_location_diff('0/274', '0/274');
 ERROR:  xrecoff 274 is out of valid range, 0..A4A534C
 
Ugh? I can't reproduce that. It seems to be related to long int used by the
prior version.

 Since pg_xlog_location_diff() can be executed during recovery,
 the above needs to be updated.
 
Fixed.

 While the output was int8 I could use
 pg_size_pretty but now I couldn't. I attached another patch that implements
 pg_size_pretty(numeric).
 
I realized that it collides with the pg_size_pretty(int8) if we don't specify
a type. Hence, I decided to drop the pg_size_pretty(int8) in favor of
pg_size_pretty(numeric). It is slower than the former but it is not a
performance critical function.

 According to the above source code comment in pg_proc.h, ISTM
 pg_size_pretty() for numeric also needs to have its own DESCR().
 
Fixed.

 According to man strcat, the dest string must have enough space for
 the result.
 buf has enough space?
 
Ops. Fixed.


-- 
   Euler Taveira de Oliveira - Timbira   http://www.timbira.com.br/
   PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 236a60a..511a918 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -14942,7 +14942,7 @@ postgres=# SELECT * FROM pg_xlogfile_name_offset(pg_stop_backup());
   /row
   row
entry
-literalfunctionpg_size_pretty(typebigint/type)/function/literal
+literalfunctionpg_size_pretty(typenumeric/type)/function/literal
 /entry
entrytypetext/type/entry
entryConverts a size in bytes into a human-readable format with size units/entry
diff --git a/src/backend/utils/adt/dbsize.c b/src/backend/utils/adt/dbsize.c
index 26a8c01..d4a142b 100644
--- a/src/backend/utils/adt/dbsize.c
+++ b/src/backend/utils/adt/dbsize.c
@@ -24,6 +24,7 @@
 #include storage/fd.h
 #include utils/acl.h
 #include utils/builtins.h
+#include utils/numeric.h
 #include utils/rel.h
 #include utils/relmapper.h
 #include utils/syscache.h
@@ -506,48 +507,101 @@ pg_total_relation_size(PG_FUNCTION_ARGS)
 	PG_RETURN_INT64(size);
 }
 
-/*
- * formatting with size units
- */
 Datum
 pg_size_pretty(PG_FUNCTION_ARGS)
 {
-	int64		size = PG_GETARG_INT64(0);
-	char		buf[64];
-	int64		limit = 10 * 1024;
-	int64		limit2 = limit * 2 - 1;
+	Numeric		size = PG_GETARG_NUMERIC(0);
+	Numeric		limit, limit2;
+
+	char		*buf, *result;
 
-	if (size  limit)
-		snprintf(buf, sizeof(buf), INT64_FORMAT  bytes, size);
+	limit = DatumGetNumeric(DirectFunctionCall1(int8_numeric, Int64GetDatum((int64) (10 * 1024;
+	limit2 = DatumGetNumeric(DirectFunctionCall1(int8_numeric, Int64GetDatum((int64) (10 * 1024 * 2 - 1;
+
+	if (DatumGetBool(DirectFunctionCall2(numeric_lt, NumericGetDatum(size), NumericGetDatum(limit
+	{
+		buf = DatumGetCString(DirectFunctionCall1(numeric_out, NumericGetDatum(size)));
+		result = palloc(strlen(buf) + 7);
+		strcpy(result, buf);
+		strcat(result,  bytes);
+	}
 	else
 	{
-		size = 9;/* keep one extra bit for rounding */
-		if (size  limit2)
-			snprintf(buf, sizeof(buf), INT64_FORMAT  kB,
-	 (size + 1) / 2);
+		Numeric		arg2;
+
+		/* keep one extra bit for rounding */
+		/* size = 9 */
+		arg2 = DatumGetNumeric(DirectFunctionCall1(int8_numeric, Int64GetDatum((int64) pow(2, 9;
+		size = DatumGetNumeric(DirectFunctionCall2(numeric_div_trunc, NumericGetDatum(size), NumericGetDatum(arg2)));
+
+		if (DatumGetBool(DirectFunctionCall2(numeric_lt, NumericGetDatum(size), NumericGetDatum(limit2
+		{
+			/* size = (size + 1) / 2 */
+			size = DatumGetNumeric(DirectFunctionCall2(numeric_add, NumericGetDatum(size), 
+	DirectFunctionCall1(int8_numeric, Int64GetDatum(1;
+			size = DatumGetNumeric(DirectFunctionCall2(numeric_div_trunc, NumericGetDatum(size), 
+	DirectFunctionCall1(int8_numeric, Int64GetDatum(2;
+			buf = DatumGetCString(DirectFunctionCall1(numeric_out, NumericGetDatum(size)));
+			result = palloc(strlen(buf) + 4);
+			strcpy(result, buf);
+			strcat(result,  kB);
+		}
 		else
 		{
-			size = 10;
-			if (size  limit2)
-snprintf(buf, sizeof(buf), INT64_FORMAT  MB,
-		 (size + 1) / 2);
+			Numeric		arg3;
+
+			/* size = 10 */
+			arg3 = DatumGetNumeric(DirectFunctionCall1(int8_numeric, Int64GetDatum((int64) pow(2, 10;
+			size = DatumGetNumeric(DirectFunctionCall2(numeric_div_trunc, NumericGetDatum(size), NumericGetDatum(arg3)));
+
+			if (DatumGetBool(DirectFunctionCall2(numeric_lt, NumericGetDatum(size), NumericGetDatum(limit2
+			{
+/* size 

Re: [HACKERS] xlog location arithmetic

2012-01-26 Thread Fujii Masao
On Sun, Jan 22, 2012 at 1:13 AM, Euler Taveira de Oliveira
eu...@timbira.com wrote:
 On 23-12-2011 12:05, Tom Lane wrote:
 I too think a datatype is overkill, if we're only planning on providing
 one function.  Just emit the values as numeric and have done.

 Here it is. Output changed to numeric.

Thanks!

When I compiled the source with xlogdiff.patch, I got the following warnings.

xlogfuncs.c:511:2: warning: format '%lX' expects argument of type
'long unsigned int *', but argument 3 has type 'uint64 *' [-Wformat]
xlogfuncs.c:511:2: warning: format '%lX' expects argument of type
'long unsigned int *', but argument 4 has type 'uint64 *' [-Wformat]
xlogfuncs.c:515:2: warning: format '%lX' expects argument of type
'long unsigned int *', but argument 3 has type 'uint64 *' [-Wformat]
xlogfuncs.c:515:2: warning: format '%lX' expects argument of type
'long unsigned int *', but argument 4 has type 'uint64 *' [-Wformat]
xlogfuncs.c:524:3: warning: format '%lX' expects argument of type
'long unsigned int', but argument 2 has type 'uint64' [-Wformat]
xlogfuncs.c:528:3: warning: format '%lX' expects argument of type
'long unsigned int', but argument 2 has type 'uint64' [-Wformat]

When I tested the patch, I got the following error:

postgres=# SELECT pg_current_xlog_location();
 pg_current_xlog_location
--
 0/274
(1 row)

postgres=# SELECT pg_xlog_location_diff('0/274', '0/274');
ERROR:  xrecoff 274 is out of valid range, 0..A4A534C

In func.sgml
   para
The functions shown in xref
linkend=functions-admin-backup-table assist in making on-line backups.
These functions cannot be executed during recovery.
   /para

Since pg_xlog_location_diff() can be executed during recovery,
the above needs to be updated.

 While the output was int8 I could use
 pg_size_pretty but now I couldn't. I attached another patch that implements
 pg_size_pretty(numeric).

I agree it's necessary.

 * Note: every entry in pg_proc.h is expected to have a DESCR() comment,
 * except for functions that implement pg_operator.h operators and don't
 * have a good reason to be called directly rather than via the operator.

According to the above source code comment in pg_proc.h, ISTM
pg_size_pretty() for numeric also needs to have its own DESCR().

+   buf = DatumGetCString(DirectFunctionCall1(numeric_out,
NumericGetDatum(size)));
+   result = strcat(buf,  kB);

According to man strcat, the dest string must have enough space for
the result.
buf has enough space?

Regards,

-- 
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

-- 
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] xlog location arithmetic

2012-01-21 Thread Euler Taveira de Oliveira
On 23-12-2011 12:05, Tom Lane wrote:
 I too think a datatype is overkill, if we're only planning on providing
 one function.  Just emit the values as numeric and have done.
 
Here it is. Output changed to numeric. While the output was int8 I could use
pg_size_pretty but now I couldn't. I attached another patch that implements
pg_size_pretty(numeric).


-- 
   Euler Taveira de Oliveira - Timbira   http://www.timbira.com.br/
   PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 48631cc..04bc24d 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -14378,6 +14378,9 @@ SELECT set_config('log_statement_stats', 'off', false);
indexterm
 primarypg_xlogfile_name_offset/primary
/indexterm
+   indexterm
+primarypg_xlog_location_diff/primary
+   /indexterm
 
para
 The functions shown in xref
@@ -14450,6 +14453,13 @@ SELECT set_config('log_statement_stats', 'off', false);
entrytypetext/, typeinteger//entry
entryConvert transaction log location string to file name and decimal byte offset within file/entry
   /row
+  row
+   entry
+literalfunctionpg_xlog_location_diff(parameterlocation/ typetext/, parameterlocation/ typetext/)/function/literal
+/entry
+   entrytypenumeric//entry
+   entryCalculate the difference between two transaction log locations/entry
+  /row
  /tbody
 /tgroup
/table
@@ -14543,6 +14553,13 @@ postgres=# SELECT * FROM pg_xlogfile_name_offset(pg_stop_backup());
/para
 
para
+	functionpg_xlog_location_diff/ calculates the difference in bytes
+	between two transaction log locations. It can be used with
+	structnamepg_stat_replication/structname or some functions shown in
+	xref linkend=functions-admin-backup-table to get the replication lag.
+   /para
+
+   para
 For details about proper usage of these functions, see
 xref linkend=continuous-archiving.
/para
diff --git a/src/backend/access/transam/xlogfuncs.c b/src/backend/access/transam/xlogfuncs.c
index 2e10d4d..e03c5e8 100644
--- a/src/backend/access/transam/xlogfuncs.c
+++ b/src/backend/access/transam/xlogfuncs.c
@@ -26,6 +26,7 @@
 #include replication/walreceiver.h
 #include storage/smgr.h
 #include utils/builtins.h
+#include utils/numeric.h
 #include utils/guc.h
 #include utils/timestamp.h
 
@@ -465,3 +466,84 @@ pg_is_in_recovery(PG_FUNCTION_ARGS)
 {
 	PG_RETURN_BOOL(RecoveryInProgress());
 }
+
+static void
+validate_xlog_location(char *str)
+{
+#define	MAXLSNCOMPONENT		8
+
+	int	len1, len2;
+
+	len1 = strspn(str, 0123456789abcdefABCDEF);
+	if (len1  1 || len1  MAXLSNCOMPONENT || str[len1] != '/')
+		ereport(ERROR,
+(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
+			 errmsg(invalid input syntax for transaction log location: \%s\, str)));
+	len2 = strspn(str + len1 + 1, 0123456789abcdefABCDEF);
+	if (len2  1 || len2  MAXLSNCOMPONENT || str[len1 + 1 + len2] != '\0')
+		ereport(ERROR,
+(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
+			 errmsg(invalid input syntax for transaction log location: \%s\, str)));
+}
+
+/*
+ * Compute the difference in bytes between two WAL locations.
+ */
+Datum
+pg_xlog_location_diff(PG_FUNCTION_ARGS)
+{
+	text	*location1 = PG_GETARG_TEXT_P(0);
+	text	*location2 = PG_GETARG_TEXT_P(1);
+	char	*str1, *str2;
+	uint64	xlogid1, xrecoff1;
+	uint64	xlogid2, xrecoff2;
+	Numeric	result;
+
+	/*
+	 * Read and parse input
+	 */
+	str1 = text_to_cstring(location1);
+	str2 = text_to_cstring(location2);
+
+	validate_xlog_location(str1);
+	validate_xlog_location(str2);
+
+	if (sscanf(str1, %8lX/%8lX, xlogid1, xrecoff1) != 2)
+		ereport(ERROR,
+(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+			errmsg(could not parse transaction log location \%s\, str1)));
+	if (sscanf(str2, %8lX/%8lX, xlogid2, xrecoff2) != 2)
+		ereport(ERROR,
+(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+			errmsg(could not parse transaction log location \%s\, str2)));
+
+	/*
+	 * Sanity check
+	 */
+	if (xrecoff1  XLogFileSize)
+		ereport(ERROR,
+(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+			errmsg(xrecoff \%lX\ is out of valid range, 0..%X, xrecoff1, XLogFileSize)));
+	if (xrecoff2  XLogFileSize)
+		ereport(ERROR,
+(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+			errmsg(xrecoff \%lX\ is out of valid range, 0..%X, xrecoff2, XLogFileSize)));
+
+	/*
+	 * result = XLogFileSize * (xlogid1 - xlogid2) + xrecoff1 - xrecoff2
+	 */
+	result = DatumGetNumeric(DirectFunctionCall2(numeric_sub,
+			DirectFunctionCall1(int8_numeric, Int64GetDatum(xlogid1)),
+			DirectFunctionCall1(int8_numeric, Int64GetDatum(xlogid2;
+	result = DatumGetNumeric(DirectFunctionCall2(numeric_mul,
+			DirectFunctionCall1(int8_numeric, Int64GetDatum(XLogFileSize)),
+			NumericGetDatum(result)));
+	result = DatumGetNumeric(DirectFunctionCall2(numeric_add,
+			NumericGetDatum(result),
+			DirectFunctionCall1(int8_numeric, Int64GetDatum(xrecoff1;

Re: [HACKERS] xlog location arithmetic

2012-01-17 Thread Peter Geoghegan
On 20 December 2011 10:27, Magnus Hagander mag...@hagander.net wrote:
 Doing it in numeric should be perfectly fine. The only real reason to
 pick int8 over in this context would be performance, but it's not like
 this is something that's going to be called in really performance
 critical paths...

FYI, my group commit patch has a little macro, in the spirit of
XLByteAdvance, to get the delta between two LSNs in bytes as an
uint64.

-- 
Peter Geoghegan       http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training and Services

-- 
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] xlog location arithmetic

2012-01-14 Thread Fujii Masao
On Tue, Dec 6, 2011 at 1:19 PM, Euler Taveira de Oliveira
eu...@timbira.com wrote:
 Hi,

 A while ago when blogging about WAL [1], I noticed a function to deal with
 xlog location arithmetic is wanted. I remembered Depez [2] mentioning it and
 after some questions during trainings and conferences I decided to translate
 my shell script function in C.

 The attached patch implements the function pg_xlog_location_diff (bikeshed
 colors are welcome). It calculates the difference between two given
 transaction log locations. Now that we have pg_stat_replication view, it will
 be easy to get the lag just passing columns as parameters. Also, the
 monitoring tools could take advantage of it instead of relying on a fragile
 routine to get the lag.

 I noticed that pg_xlogfile_name* functions does not sanity check the xrecoff
 boundaries but that is material for another patch.


 [1] http://eulerto.blogspot.com/2011/11/understanding-wal-nomenclature.html
 [2]
 http://www.depesz.com/index.php/2011/01/24/waiting-for-9-1-pg_stat_replication/

I think that this function is very useful. Can you add the patch into
CommitFest 2012-1 ?

Regards,

-- 
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

-- 
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] xlog location arithmetic

2012-01-14 Thread Euler Taveira de Oliveira
On 14-01-2012 11:06, Fujii Masao wrote:
 I think that this function is very useful. Can you add the patch into
 CommitFest 2012-1 ?
 
Sure. But I must adjust the patch based on the thread comments (basically,
numeric output). I have a new patch but need to test it before submitting it.
I'll post this weekend.


-- 
   Euler Taveira de Oliveira - Timbira   http://www.timbira.com.br/
   PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento

-- 
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] xlog location arithmetic

2012-01-14 Thread Greg Smith

On 01/14/2012 09:12 AM, Euler Taveira de Oliveira wrote:

But I must adjust the patch based on the thread comments (basically,
numeric output). I have a new patch but need to test it before submitting it.
I'll post this weekend.


It's now at https://commitfest.postgresql.org/action/patch_view?id=776 
listed as waiting on you right now.  It's good to put patches into the 
CF application early.  Helps planning, and gives a safety net against 
all sorts of things.  We wouldn't want something this obviously useful 
to get kicked out if, for example, you lost your Internet connection 
over the weekend and then didn't technically qualify as having submitted 
it there before the deadline.  As someone who sweated today for two 
hours when my power at home was turned off to install a new circuit 
breaker, I'm feeling particularly paranoid right now about that sort of 
thing here.


The fact that you got some review feedback before the official CF start 
doesn't mean you can't be listed there right now.  In fact, those are 
things I like to see tracked.  Having links to all of the e-mail 
messages that were important turning points for a feature that changed 
during review is very helpful to reviewers and committers.  And the 
easiest way to keep up with that is to start as early as possible:  add 
it to the app right after the first patch submission.


--
Greg Smith   2ndQuadrant USg...@2ndquadrant.com   Baltimore, MD
PostgreSQL Training, Services, and 24x7 Support www.2ndQuadrant.com


--
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] xlog location arithmetic

2012-01-14 Thread Gurjeet Singh
On Sat, Jan 14, 2012 at 8:18 PM, Greg Smith g...@2ndquadrant.com wrote:

 On 01/14/2012 09:12 AM, Euler Taveira de Oliveira wrote:

 But I must adjust the patch based on the thread comments (basically,
 numeric output). I have a new patch but need to test it before submitting
 it.
 I'll post this weekend.


 It's now at 
 https://commitfest.postgresql.**org/action/patch_view?id=776https://commitfest.postgresql.org/action/patch_view?id=776listed
  as waiting on you right now.  It's good to put patches into the CF
 application early.  Helps planning, and gives a safety net against all
 sorts of things.  We wouldn't want something this obviously useful to get
 kicked out if, for example, you lost your Internet connection over the
 weekend and then didn't technically qualify as having submitted it there
 before the deadline.  As someone who sweated today for two hours when my
 power at home was turned off to install a new circuit breaker, I'm feeling
 particularly paranoid right now about that sort of thing here.
 he patch
 The fact that you got some review feedback before the official CF start
 doesn't mean you can't be listed there right now.  In fact, those are
 things I like to see tracked.  Having links to all of the e-mail messages
 that were important turning points for a feature that changed during review
 is very helpful to reviewers and committers.  And the easiest way to keep
 up with that is to start as early as possible:  add it to the app right
 after the first patch submission.


I agree.

So lets make it easy for the patch submitter to start the process. I
propose that we have a page in the CF application where people can
upload/attach the patch, and the app posts the patch to -hackers and uses
the post URL to create the CF entry.

Regards,
-- 
Gurjeet Singh
EnterpriseDB Corporation
The Enterprise PostgreSQL Company


Re: [HACKERS] xlog location arithmetic

2011-12-23 Thread Magnus Hagander
On Tue, Dec 20, 2011 at 14:08, Euler Taveira de Oliveira
eu...@timbira.com wrote:
 On 20-12-2011 07:27, Magnus Hagander wrote:
 On Tue, Dec 6, 2011 at 19:06, Robert Haas robertmh...@gmail.com wrote:
 On Tue, Dec 6, 2011 at 1:00 PM, Euler Taveira de Oliveira
 eu...@timbira.com wrote:
 On 06-12-2011 13:11, Robert Haas wrote:
 On Tue, Dec 6, 2011 at 5:14 AM, Magnus Hagander mag...@hagander.net 
 wrote:
 I've been considering similar things, as you can find in the archives,
 but what I was thinking of was converting the number to just a plain
 bigint, then letting the user apply whatever arithmetic wanted at the
 SQL level. I never got around to acutally coding it, though. It could
 easily be extracted from your patch of course - and I think that's a
 more flexible approach. Is there some advantage to your method that
 I'm missing?

 I went so far as to put together an lsn data type.  I didn't actually
 get all that far with it, which is why I haven't posted it sooner, but
 here's what I came up with.  It's missing indexing support and stuff,
 but that could be added if people like the approach.  It solves this
 problem by implementing -(lsn,lsn) = numeric (not int8, that can
 overflow since it is not unsigned), which allows an lsn = numeric
 conversion by just subtracting '0/0'::lsn.

 Interesting approach. I don't want to go that far. If so, you want to 
 change
 all of those functions that deal with LSNs and add some implicit conversion
 between text and lsn data types (for backward compatibility). As of int8, 
 I'm

 As long as you have the conversion, you don't really need to change
 them, do you? It might be nice in some ways, but this is still a
 pretty internal operation, so I don't see it as critical.

 For correctness, yes.

 At this point, my question is: do we want to support the lsn data type idea or
 a basic function that implements the difference between LSNs?

Personally I think a function is enough - it solves the only case that
I've actually seen. But a datatype would be a more complete solution,
of course - but it seems a bit of an overkill to me. Not really sure
which way we should go - I was hoping somebody else would comment as
well..

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

-- 
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] xlog location arithmetic

2011-12-23 Thread Tom Lane
Magnus Hagander mag...@hagander.net writes:
 At this point, my question is: do we want to support the lsn data type idea 
 or
 a basic function that implements the difference between LSNs?

 Personally I think a function is enough - it solves the only case that
 I've actually seen. But a datatype would be a more complete solution,
 of course - but it seems a bit of an overkill to me. Not really sure
 which way we should go - I was hoping somebody else would comment as
 well..

I too think a datatype is overkill, if we're only planning on providing
one function.  Just emit the values as numeric and have done.

regards, tom lane

-- 
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] xlog location arithmetic

2011-12-23 Thread Robert Haas
On Fri, Dec 23, 2011 at 10:05 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 I too think a datatype is overkill, if we're only planning on providing
 one function.

Are there any other functions we ought to provide?

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
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] xlog location arithmetic

2011-12-23 Thread Andrew Dunstan



On 12/23/2011 10:05 AM, Tom Lane wrote:

Magnus Hagandermag...@hagander.net  writes:

At this point, my question is: do we want to support the lsn data type idea or
a basic function that implements the difference between LSNs?

Personally I think a function is enough - it solves the only case that
I've actually seen. But a datatype would be a more complete solution,
of course - but it seems a bit of an overkill to me. Not really sure
which way we should go - I was hoping somebody else would comment as
well..

I too think a datatype is overkill, if we're only planning on providing
one function.  Just emit the values as numeric and have done.




+1.

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


Re: [HACKERS] xlog location arithmetic

2011-12-23 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 On Fri, Dec 23, 2011 at 10:05 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 I too think a datatype is overkill, if we're only planning on providing
 one function.

 Are there any other functions we ought to provide?

Even if there are several, what exact advantage does a datatype offer
over representing LSN values as numerics?  It seems to me to be adding
complication and extra code (I/O converters at least) for very little
gain.

regards, tom lane

-- 
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] xlog location arithmetic

2011-12-23 Thread Robert Haas
On Fri, Dec 23, 2011 at 10:18 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 Robert Haas robertmh...@gmail.com writes:
 On Fri, Dec 23, 2011 at 10:05 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 I too think a datatype is overkill, if we're only planning on providing
 one function.

 Are there any other functions we ought to provide?

 Even if there are several, what exact advantage does a datatype offer
 over representing LSN values as numerics?  It seems to me to be adding
 complication and extra code (I/O converters at least) for very little
 gain.

I guess I'm just constitutionally averse to labeling things as text
when they really aren't.  I do it all the time in Perl, of course, but
in PostgreSQL we have strong data typing, and it seems like we might
as well use it.

Also, we've occasionally talked (in the light of Pavan's single-pass
vacuum patch, for example) about needing to store LSNs in system
catalogs; and we're certainly not going to want to do that as text.
I'll admit that it's not 100% clear that anything like this will ever
happen, though, so maybe it's premature to worry about it.

I can see I'm in the minority on this one, though...

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
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] xlog location arithmetic

2011-12-23 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 On Fri, Dec 23, 2011 at 10:18 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 Even if there are several, what exact advantage does a datatype offer
 over representing LSN values as numerics?  It seems to me to be adding
 complication and extra code (I/O converters at least) for very little
 gain.

 I guess I'm just constitutionally averse to labeling things as text
 when they really aren't.

Er ... text?  I thought the proposal was to use numeric.

regards, tom lane

-- 
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] xlog location arithmetic

2011-12-23 Thread Robert Haas
On Fri, Dec 23, 2011 at 10:59 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 Robert Haas robertmh...@gmail.com writes:
 On Fri, Dec 23, 2011 at 10:18 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 Even if there are several, what exact advantage does a datatype offer
 over representing LSN values as numerics?  It seems to me to be adding
 complication and extra code (I/O converters at least) for very little
 gain.

 I guess I'm just constitutionally averse to labeling things as text
 when they really aren't.

 Er ... text?  I thought the proposal was to use numeric.

The proposal is to make a function that takes a text argument (which
is really an LSN, but we choose to represent it as text) and returns
numeric.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
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] xlog location arithmetic

2011-12-20 Thread Magnus Hagander
On Tue, Dec 6, 2011 at 19:06, Robert Haas robertmh...@gmail.com wrote:
 On Tue, Dec 6, 2011 at 1:00 PM, Euler Taveira de Oliveira
 eu...@timbira.com wrote:
 On 06-12-2011 13:11, Robert Haas wrote:
 On Tue, Dec 6, 2011 at 5:14 AM, Magnus Hagander mag...@hagander.net wrote:
 I've been considering similar things, as you can find in the archives,
 but what I was thinking of was converting the number to just a plain
 bigint, then letting the user apply whatever arithmetic wanted at the
 SQL level. I never got around to acutally coding it, though. It could
 easily be extracted from your patch of course - and I think that's a
 more flexible approach. Is there some advantage to your method that
 I'm missing?

 I went so far as to put together an lsn data type.  I didn't actually
 get all that far with it, which is why I haven't posted it sooner, but
 here's what I came up with.  It's missing indexing support and stuff,
 but that could be added if people like the approach.  It solves this
 problem by implementing -(lsn,lsn) = numeric (not int8, that can
 overflow since it is not unsigned), which allows an lsn = numeric
 conversion by just subtracting '0/0'::lsn.

 Interesting approach. I don't want to go that far. If so, you want to change
 all of those functions that deal with LSNs and add some implicit conversion
 between text and lsn data types (for backward compatibility). As of int8, I'm

As long as you have the conversion, you don't really need to change
them, do you? It might be nice in some ways, but this is still a
pretty internal operation, so I don't see it as critical.

 not aware of any modern plataform that int8 is not 64 bits. I'm not against
 numeric use; I'm just saying that int8 is sufficient.

 The point isn't that int8 might not be 64 bits - of course it has to
 be 64 bits; that's why it's called int8 i.e. 8 bytes.  The point is
 that a large enough LSN, represented as an int8, will come out as a
 negative values.  int8 can only represent 2^63 *non-negative* values,
 because one bit is reserved for sign.

Doing it in numeric should be perfectly fine. The only real reason to
pick int8 over in this context would be performance, but it's not like
this is something that's going to be called in really performance
critical paths...

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

-- 
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] xlog location arithmetic

2011-12-20 Thread Euler Taveira de Oliveira
On 20-12-2011 07:27, Magnus Hagander wrote:
 On Tue, Dec 6, 2011 at 19:06, Robert Haas robertmh...@gmail.com wrote:
 On Tue, Dec 6, 2011 at 1:00 PM, Euler Taveira de Oliveira
 eu...@timbira.com wrote:
 On 06-12-2011 13:11, Robert Haas wrote:
 On Tue, Dec 6, 2011 at 5:14 AM, Magnus Hagander mag...@hagander.net 
 wrote:
 I've been considering similar things, as you can find in the archives,
 but what I was thinking of was converting the number to just a plain
 bigint, then letting the user apply whatever arithmetic wanted at the
 SQL level. I never got around to acutally coding it, though. It could
 easily be extracted from your patch of course - and I think that's a
 more flexible approach. Is there some advantage to your method that
 I'm missing?

 I went so far as to put together an lsn data type.  I didn't actually
 get all that far with it, which is why I haven't posted it sooner, but
 here's what I came up with.  It's missing indexing support and stuff,
 but that could be added if people like the approach.  It solves this
 problem by implementing -(lsn,lsn) = numeric (not int8, that can
 overflow since it is not unsigned), which allows an lsn = numeric
 conversion by just subtracting '0/0'::lsn.

 Interesting approach. I don't want to go that far. If so, you want to change
 all of those functions that deal with LSNs and add some implicit conversion
 between text and lsn data types (for backward compatibility). As of int8, 
 I'm
 
 As long as you have the conversion, you don't really need to change
 them, do you? It might be nice in some ways, but this is still a
 pretty internal operation, so I don't see it as critical.
 
For correctness, yes.

At this point, my question is: do we want to support the lsn data type idea or
a basic function that implements the difference between LSNs?


-- 
   Euler Taveira de Oliveira - Timbira   http://www.timbira.com.br/
   PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento

-- 
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] xlog location arithmetic

2011-12-13 Thread Jim Nasby
On Dec 6, 2011, at 12:06 PM, Robert Haas wrote:
 On Tue, Dec 6, 2011 at 1:00 PM, Euler Taveira de Oliveira
 eu...@timbira.com wrote:
 On 06-12-2011 13:11, Robert Haas wrote:
 On Tue, Dec 6, 2011 at 5:14 AM, Magnus Hagander mag...@hagander.net wrote:
 I've been considering similar things, as you can find in the archives,
 but what I was thinking of was converting the number to just a plain
 bigint, then letting the user apply whatever arithmetic wanted at the
 SQL level. I never got around to acutally coding it, though. It could
 easily be extracted from your patch of course - and I think that's a
 more flexible approach. Is there some advantage to your method that
 I'm missing?
 
 I went so far as to put together an lsn data type.  I didn't actually
 get all that far with it, which is why I haven't posted it sooner, but
 here's what I came up with.  It's missing indexing support and stuff,
 but that could be added if people like the approach.  It solves this
 problem by implementing -(lsn,lsn) = numeric (not int8, that can
 overflow since it is not unsigned), which allows an lsn = numeric
 conversion by just subtracting '0/0'::lsn.
 
 Interesting approach. I don't want to go that far. If so, you want to change
 all of those functions that deal with LSNs and add some implicit conversion
 between text and lsn data types (for backward compatibility). As of int8, I'm
 not aware of any modern plataform that int8 is not 64 bits. I'm not against
 numeric use; I'm just saying that int8 is sufficient.
 
 The point isn't that int8 might not be 64 bits - of course it has to
 be 64 bits; that's why it's called int8 i.e. 8 bytes.  The point is
 that a large enough LSN, represented as an int8, will come out as a
 negative values.  int8 can only represent 2^63 *non-negative* values,
 because one bit is reserved for sign.

I've often wondered about adding uint2/4/8... I suspect it's actually pretty 
uncommon for people to put negative numbers into int fields, since one of their 
biggest uses seems to be surrogate keys.

I realize that this opens a can of worms with casting, but perhaps that can be 
kept under control by not doing any implicit casting between int and uint... 
that just means that we'd have to be smart about casting from unknown, but 
hopefully that's doable since we already have a similar concern with casting 
unknown to int2/4/8 vs numeric?
--
Jim C. Nasby, Database Architect   j...@nasby.net
512.569.9461 (cell) http://jim.nasby.net



-- 
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] xlog location arithmetic

2011-12-13 Thread Robert Haas
On Tue, Dec 13, 2011 at 12:48 PM, Jim Nasby j...@nasby.net wrote:
 I've often wondered about adding uint2/4/8... I suspect it's actually pretty 
 uncommon for people to put negative numbers into int fields, since one of 
 their biggest uses seems to be surrogate keys.

 I realize that this opens a can of worms with casting, but perhaps that can 
 be kept under control by not doing any implicit casting between int and 
 uint... that just means that we'd have to be smart about casting from 
 unknown, but hopefully that's doable since we already have a similar concern 
 with casting unknown to int2/4/8 vs numeric?

I've wondered about it too, but it seems like too large a can of worms
to open just to address this case.  Returning the value as numeric is
hardly a disaster; the user can always downcast to int8 if they really
want, and as long as it's  2^63 (which in practice it virtually
always will be) it will work.  It's not clear what the point of this
is since for typical values numeric is going to take up less storage
anyway (e.g. 101 is 7 bytes on disk as a numeric), not to mention
that it only requires 4-byte alignment rather than 8-byte alignment,
and probably no one does enough arithmetic with LSN values for any
speed penalty to matter even slightly, but it should work.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
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] xlog location arithmetic

2011-12-06 Thread Magnus Hagander
On Tue, Dec 6, 2011 at 05:19, Euler Taveira de Oliveira
eu...@timbira.com wrote:
 Hi,

 A while ago when blogging about WAL [1], I noticed a function to deal with
 xlog location arithmetic is wanted. I remembered Depez [2] mentioning it and
 after some questions during trainings and conferences I decided to translate
 my shell script function in C.

 The attached patch implements the function pg_xlog_location_diff (bikeshed
 colors are welcome). It calculates the difference between two given
 transaction log locations. Now that we have pg_stat_replication view, it will
 be easy to get the lag just passing columns as parameters. Also, the
 monitoring tools could take advantage of it instead of relying on a fragile
 routine to get the lag.

I've been considering similar things, as you can find in the archives,
but what I was thinking of was converting the number to just a plain
bigint, then letting the user apply whatever arithmetic wanted at the
SQL level. I never got around to acutally coding it, though. It could
easily be extracted from your patch of course - and I think that's a
more flexible approach. Is there some advantage to your method that
I'm missing?

Also, why do you use DirectFunctionCall to do the simple math, and not
just do the math right there in the function?

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

-- 
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] xlog location arithmetic

2011-12-06 Thread Robert Haas
On Tue, Dec 6, 2011 at 5:14 AM, Magnus Hagander mag...@hagander.net wrote:
 I've been considering similar things, as you can find in the archives,
 but what I was thinking of was converting the number to just a plain
 bigint, then letting the user apply whatever arithmetic wanted at the
 SQL level. I never got around to acutally coding it, though. It could
 easily be extracted from your patch of course - and I think that's a
 more flexible approach. Is there some advantage to your method that
 I'm missing?

I went so far as to put together an lsn data type.  I didn't actually
get all that far with it, which is why I haven't posted it sooner, but
here's what I came up with.  It's missing indexing support and stuff,
but that could be added if people like the approach.  It solves this
problem by implementing -(lsn,lsn) = numeric (not int8, that can
overflow since it is not unsigned), which allows an lsn = numeric
conversion by just subtracting '0/0'::lsn.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


lsn.patch
Description: Binary data

-- 
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] xlog location arithmetic

2011-12-06 Thread Euler Taveira de Oliveira
On 06-12-2011 07:14, Magnus Hagander wrote:
 On Tue, Dec 6, 2011 at 05:19, Euler Taveira de Oliveira
 eu...@timbira.com wrote:
 Hi,

 A while ago when blogging about WAL [1], I noticed a function to deal with
 xlog location arithmetic is wanted. I remembered Depez [2] mentioning it and
 after some questions during trainings and conferences I decided to translate
 my shell script function in C.

 The attached patch implements the function pg_xlog_location_diff (bikeshed
 colors are welcome). It calculates the difference between two given
 transaction log locations. Now that we have pg_stat_replication view, it will
 be easy to get the lag just passing columns as parameters. Also, the
 monitoring tools could take advantage of it instead of relying on a fragile
 routine to get the lag.
 
 I've been considering similar things, as you can find in the archives,
 but what I was thinking of was converting the number to just a plain
 bigint, then letting the user apply whatever arithmetic wanted at the
 SQL level. I never got around to acutally coding it, though. It could
 easily be extracted from your patch of course - and I think that's a
 more flexible approach. Is there some advantage to your method that
 I'm missing?
 
The only advantage is that you don't expose the arithmetic, e.g., user doesn't
need to know the xlog internals (like I described in a recent blog post). If
one day we consider changes in xlog arithmetic (for example, XLogFileSize), we
don't need to worry too much about external tools.

 Also, why do you use DirectFunctionCall to do the simple math, and not
 just do the math right there in the function?
 
I use it because I don't want to duplicate the overflow code.


-- 
   Euler Taveira de Oliveira - Timbira   http://www.timbira.com.br/
   PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento

-- 
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] xlog location arithmetic

2011-12-06 Thread Euler Taveira de Oliveira
On 06-12-2011 13:11, Robert Haas wrote:
 On Tue, Dec 6, 2011 at 5:14 AM, Magnus Hagander mag...@hagander.net wrote:
 I've been considering similar things, as you can find in the archives,
 but what I was thinking of was converting the number to just a plain
 bigint, then letting the user apply whatever arithmetic wanted at the
 SQL level. I never got around to acutally coding it, though. It could
 easily be extracted from your patch of course - and I think that's a
 more flexible approach. Is there some advantage to your method that
 I'm missing?
 
 I went so far as to put together an lsn data type.  I didn't actually
 get all that far with it, which is why I haven't posted it sooner, but
 here's what I came up with.  It's missing indexing support and stuff,
 but that could be added if people like the approach.  It solves this
 problem by implementing -(lsn,lsn) = numeric (not int8, that can
 overflow since it is not unsigned), which allows an lsn = numeric
 conversion by just subtracting '0/0'::lsn.
 
Interesting approach. I don't want to go that far. If so, you want to change
all of those functions that deal with LSNs and add some implicit conversion
between text and lsn data types (for backward compatibility). As of int8, I'm
not aware of any modern plataform that int8 is not 64 bits. I'm not against
numeric use; I'm just saying that int8 is sufficient.


-- 
   Euler Taveira de Oliveira - Timbira   http://www.timbira.com.br/
   PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento

-- 
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] xlog location arithmetic

2011-12-06 Thread Robert Haas
On Tue, Dec 6, 2011 at 1:00 PM, Euler Taveira de Oliveira
eu...@timbira.com wrote:
 On 06-12-2011 13:11, Robert Haas wrote:
 On Tue, Dec 6, 2011 at 5:14 AM, Magnus Hagander mag...@hagander.net wrote:
 I've been considering similar things, as you can find in the archives,
 but what I was thinking of was converting the number to just a plain
 bigint, then letting the user apply whatever arithmetic wanted at the
 SQL level. I never got around to acutally coding it, though. It could
 easily be extracted from your patch of course - and I think that's a
 more flexible approach. Is there some advantage to your method that
 I'm missing?

 I went so far as to put together an lsn data type.  I didn't actually
 get all that far with it, which is why I haven't posted it sooner, but
 here's what I came up with.  It's missing indexing support and stuff,
 but that could be added if people like the approach.  It solves this
 problem by implementing -(lsn,lsn) = numeric (not int8, that can
 overflow since it is not unsigned), which allows an lsn = numeric
 conversion by just subtracting '0/0'::lsn.

 Interesting approach. I don't want to go that far. If so, you want to change
 all of those functions that deal with LSNs and add some implicit conversion
 between text and lsn data types (for backward compatibility). As of int8, I'm
 not aware of any modern plataform that int8 is not 64 bits. I'm not against
 numeric use; I'm just saying that int8 is sufficient.

The point isn't that int8 might not be 64 bits - of course it has to
be 64 bits; that's why it's called int8 i.e. 8 bytes.  The point is
that a large enough LSN, represented as an int8, will come out as a
negative values.  int8 can only represent 2^63 *non-negative* values,
because one bit is reserved for sign.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers