Re: [HACKERS] 64-bit API for large object

2012-10-07 Thread Tatsuo Ishii
Amit,

 Today when I tried to build the latest code on my windows m/c, I got few 
 errors from the checkin of this patch.
 
 lo_hton64 (due to -- unint32_t)
  .\src\interfaces\libpq\fe-lobj.c(1049) : error C2065: 'uint32_t' : 
 undeclared identifier
 inv_seek (due to   -- MAX_LARGE_OBJECT_SIZE)
 \src\backend\storage\large_object\inv_api.c(389) : error C2065: 'LOBLKSIZELL' 
 : undeclared identifier
 inv_read ((due to   -- MAX_LARGE_OBJECT_SIZE))
 \src\backend\storage\large_object\inv_api.c(441) : error C2065: 'LOBLKSIZELL' 
 : undeclared identifier

Thanks for the report. Can you please try included patch?
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese: http://www.sraoss.co.jp
diff --git a/src/include/storage/large_object.h b/src/include/storage/large_object.h
new file mode 100644
index 52f01c6..715f0c3
*** a/src/include/storage/large_object.h
--- b/src/include/storage/large_object.h
*** typedef struct LargeObjectDesc
*** 65,71 
  /*
   * Maximum byte length for each large object
  */
! #define MAX_LARGE_OBJECT_SIZE	INT64CONST(INT_MAX * LOBLKSIZE)
  
  /*
   * Function definitions...
--- 65,71 
  /*
   * Maximum byte length for each large object
  */
! #define MAX_LARGE_OBJECT_SIZE	((int64)INT_MAX * LOBLKSIZE)
  
  /*
   * Function definitions...
diff --git a/src/interfaces/libpq/fe-lobj.c b/src/interfaces/libpq/fe-lobj.c
new file mode 100644
index fb17ac8..022cfec
*** a/src/interfaces/libpq/fe-lobj.c
--- b/src/interfaces/libpq/fe-lobj.c
*** static pg_int64
*** 1046,1058 
  lo_hton64(pg_int64 host64)
  {
  	pg_int64 	result;
! 	uint32_t	h32, l32;
  
  	/* High order half first, since we're doing MSB-first */
! 	h32 = (uint32_t) (host64  32);
  
  	/* Now the low order half */
! 	l32 = (uint32_t) (host64  0x);
  
  	result = htonl(l32);
  	result = 32;
--- 1046,1058 
  lo_hton64(pg_int64 host64)
  {
  	pg_int64 	result;
! 	uint32	h32, l32;
  
  	/* High order half first, since we're doing MSB-first */
! 	h32 = (uint32) (host64  32);
  
  	/* Now the low order half */
! 	l32 = (uint32) (host64  0x);
  
  	result = htonl(l32);
  	result = 32;
*** static pg_int64
*** 1069,1078 
  lo_ntoh64(pg_int64 net64)
  {
  	pg_int64 	result;
! 	uint32_t	h32, l32;
  
! 	l32 = (uint32_t) (net64  32);
! 	h32 = (uint32_t) (net64  0x);
  
  	result = ntohl(h32);
  	result = 32;
--- 1069,1078 
  lo_ntoh64(pg_int64 net64)
  {
  	pg_int64 	result;
! 	uint32	h32, l32;
  
! 	l32 = (uint32) (net64  32);
! 	h32 = (uint32) (net64  0x);
  
  	result = ntohl(h32);
  	result = 32;

-- 
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] 64-bit API for large object

2012-10-07 Thread Amit Kapila
 On Sunday, October 07, 2012 1:25 PM Tatsuo Ishii wrote:
 Amit,
 
  Today when I tried to build the latest code on my windows m/c, I got
 few errors from the checkin of this patch.
 
  lo_hton64 (due to -- unint32_t)
   .\src\interfaces\libpq\fe-lobj.c(1049) : error C2065: 'uint32_t' :
 undeclared identifier
  inv_seek (due to   -- MAX_LARGE_OBJECT_SIZE)
  \src\backend\storage\large_object\inv_api.c(389) : error C2065:
 'LOBLKSIZELL' : undeclared identifier
  inv_read ((due to   -- MAX_LARGE_OBJECT_SIZE))
  \src\backend\storage\large_object\inv_api.c(441) : error C2065:
 'LOBLKSIZELL' : undeclared identifier
 
 Thanks for the report. Can you please try included patch?

Above errors are not coming after the changes in attached patch.

With Regards,
Amit Kapila.



-- 
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] 64-bit API for large object

2012-10-06 Thread Tatsuo Ishii
Ok, committed with minor editings(fix header comments in testlo64.c).
Thank you Kaigai-san for review!
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese: http://www.sraoss.co.jp

 As a committer, I have looked into the patch and it seems it's good to
 commit. However I want to make a small enhancement in the
 documentation part:
 
 1) lo_open section needs to mention about new 64bit APIs. Also it
should include description about lo_truncate, but this is not 64bit
APIs author's fault since it should had been there when lo_truncate
was added.
 
 2) Add mention that 64bit APIs are only available in PostgreSQL 9.3 or
later and if the API is requested against older version of servers
it will fail.
 
 If there's no objection, I would like commit attached patches.
 --
 Tatsuo Ishii
 SRA OSS, Inc. Japan
 English: http://www.sraoss.co.jp/index_en.php
 Japanese: http://www.sraoss.co.jp
 
 Hi Anzai-san,
 
 The latest patch is fair enough for me, so let me hand over its reviewing
 for comitters.
 
 Thanks,
 
 2012/10/1 Nozomi Anzai an...@sraoss.co.jp:
 Here is 64-bit API for large object version 3 patch.

 I checked this patch. It looks good, but here are still some points to be
 discussed.

 * I have a question. What is the meaning of INT64_IS_BUSTED?
   It seems to me a marker to indicate a platform without 64bit support.
   However, the commit 901be0fad4034c9cf8a3588fd6cf2ece82e4b8ce
   says as follows:
   | Remove all the special-case code for INT64_IS_BUSTED, per decision that
   | we're not going to support that anymore.

 Removed INT64_IS_BUSTED.


 * At inv_seek(), it seems to me it checks offset correctness with wrong 
 way,
   as follows:
 |  case SEEK_SET:
 |  if (offset  0)
 |  elog(ERROR, invalid seek offset:  INT64_FORMAT, 
 offset);
 |  obj_desc-offset = offset;
 |  break;
   It is a right assumption, if large object size would be restricted to 
 2GB.
   But the largest positive int64 is larger than expected limitation.
   So, it seems to me it should be compared with (INT_MAX * PAGE_SIZE)
   instead.

 Fixed.


 * At inv_write(), it definitely needs a check to prevent data-write upper 
 4TB.
   In case when obj_desc-offset is a bit below 4TB, an additional 1GB write
   will break head of the large object because of pageno overflow.

 Added a such check.


 * Please also add checks on inv_read() to prevent LargeObjectDesc-offset
   unexpectedly overflows 4TB boundary.

 Added a such check.


 * At inv_truncate(), variable off is re-defined to int64. Is it really 
 needed
   change? All its usage is to store the result of len % LOBLKSIZE.

 Fixed and back to int32.


 Thanks,

 2012/9/24 Nozomi Anzai an...@sraoss.co.jp:
  Here is 64-bit API for large object version 2 patch.
 
  I checked this patch. It can be applied onto the latest master branch
  without any problems. My comments are below.
 
  2012/9/11 Tatsuo Ishii is...@postgresql.org:
   Ok, here is the patch to implement 64-bit API for large object, to
   allow to use up to 4TB large objects(or 16TB if BLCKSZ changed to
   32KB). The patch is based on Jeremy Drake's patch posted on September
   23, 2005
   (http://archives.postgresql.org/pgsql-hackers/2005-09/msg01026.php)
   and reasonably updated/edited to adopt PostgreSQL 9.3 by Nozomi Anzai
   for the backend part and Yugo Nagata for the rest(including
   documentation patch).
  
   Here are changes made in the patch:
  
   1) Frontend lo_* libpq functions(fe-lobj.c)(Yugo Nagata)
  
   lo_initialize() gathers backend 64-bit large object handling
   function's oid, namely lo_lseek64, lo_tell64, lo_truncate64.
  
   If client calls lo_*64 functions and backend does not support them,
   lo_*64 functions return error to caller. There might be an argument
   since calls to lo_*64 functions can automatically be redirected to
   32-bit older API. I don't know this is worth the trouble though.
  
  I think it should definitely return an error code when user tries to
  use lo_*64 functions towards the backend v9.2 or older, because
  fallback to 32bit API can raise unexpected errors if application
  intends to seek the area over than 2GB.
 
   Currently lo_initialize() throws an error if one of oids are not
   available. I doubt we do the same way for 64-bit functions since this
   will make 9.3 libpq unable to access large objects stored in pre-9.2
   PostgreSQL servers.
  
  It seems to me the situation to split the case of pre-9.2 and post-9.3
  using a condition of conn-sversion = 90300.
 
  Fixed so, and tested it by deleteing the lo_tell64's row from pg_proc.
 
 
   To pass 64-bit integer to PQfn, PQArgBlock is used like this: int *ptr
   is a pointer to 64-bit integer and actual data is placed somewhere
   else. There might be other way: add new member to union u to store
   64-bit integer:
  
   typedef struct
   {
   int len;
   int   

Re: [HACKERS] 64-bit API for large object

2012-10-06 Thread Amit kapila

On Sunday, October 07, 2012 5:42 AM Tatsuo Ishii wrote:

Ok, committed with minor editings(fix header comments in testlo64.c).
Thank you Kaigai-san for review!

Hello Tatsuo Ishii San,

Today when I tried to build the latest code on my windows m/c, I got few errors 
from the checkin of this patch.

lo_hton64 (due to -- unint32_t)
 .\src\interfaces\libpq\fe-lobj.c(1049) : error C2065: 'uint32_t' : undeclared 
identifier
inv_seek (due to   -- MAX_LARGE_OBJECT_SIZE)
\src\backend\storage\large_object\inv_api.c(389) : error C2065: 'LOBLKSIZELL' : 
undeclared identifier
inv_read ((due to   -- MAX_LARGE_OBJECT_SIZE))
\src\backend\storage\large_object\inv_api.c(441) : error C2065: 'LOBLKSIZELL' : 
undeclared identifier


It may be some settings problem of my m/c if it is okay on some other windows 
m/c. 


With Regards,
Amit Kapila. 

 As a committer, I have looked into the patch and it seems it's good to
 commit. However I want to make a small enhancement in the
 documentation part:

 1) lo_open section needs to mention about new 64bit APIs. Also it
should include description about lo_truncate, but this is not 64bit
APIs author's fault since it should had been there when lo_truncate
was added.

 2) Add mention that 64bit APIs are only available in PostgreSQL 9.3 or
later and if the API is requested against older version of servers
it will fail.

 If there's no objection, I would like commit attached patches.
 --
 Tatsuo Ishii
 SRA OSS, Inc. Japan
 English: http://www.sraoss.co.jp/index_en.php
 Japanese: http://www.sraoss.co.jp

 Hi Anzai-san,

 The latest patch is fair enough for me, so let me hand over its reviewing
 for comitters.

 Thanks,

 2012/10/1 Nozomi Anzai an...@sraoss.co.jp:
 Here is 64-bit API for large object version 3 patch.

 I checked this patch. It looks good, but here are still some points to be
 discussed.

 * I have a question. What is the meaning of INT64_IS_BUSTED?
   It seems to me a marker to indicate a platform without 64bit support.
   However, the commit 901be0fad4034c9cf8a3588fd6cf2ece82e4b8ce
   says as follows:
   | Remove all the special-case code for INT64_IS_BUSTED, per decision that
   | we're not going to support that anymore.

 Removed INT64_IS_BUSTED.


 * At inv_seek(), it seems to me it checks offset correctness with wrong 
 way,
   as follows:
 |  case SEEK_SET:
 |  if (offset  0)
 |  elog(ERROR, invalid seek offset:  INT64_FORMAT, 
 offset);
 |  obj_desc-offset = offset;
 |  break;
   It is a right assumption, if large object size would be restricted to 
 2GB.
   But the largest positive int64 is larger than expected limitation.
   So, it seems to me it should be compared with (INT_MAX * PAGE_SIZE)
   instead.

 Fixed.


 * At inv_write(), it definitely needs a check to prevent data-write upper 
 4TB.
   In case when obj_desc-offset is a bit below 4TB, an additional 1GB write
   will break head of the large object because of pageno overflow.

 Added a such check.


 * Please also add checks on inv_read() to prevent LargeObjectDesc-offset
   unexpectedly overflows 4TB boundary.

 Added a such check.


 * At inv_truncate(), variable off is re-defined to int64. Is it really 
 needed
   change? All its usage is to store the result of len % LOBLKSIZE.

 Fixed and back to int32.


 Thanks,

 2012/9/24 Nozomi Anzai an...@sraoss.co.jp:
  Here is 64-bit API for large object version 2 patch.
 
  I checked this patch. It can be applied onto the latest master branch
  without any problems. My comments are below.
 
  2012/9/11 Tatsuo Ishii is...@postgresql.org:
   Ok, here is the patch to implement 64-bit API for large object, to
   allow to use up to 4TB large objects(or 16TB if BLCKSZ changed to
   32KB). The patch is based on Jeremy Drake's patch posted on September
   23, 2005
   (http://archives.postgresql.org/pgsql-hackers/2005-09/msg01026.php)
   and reasonably updated/edited to adopt PostgreSQL 9.3 by Nozomi Anzai
   for the backend part and Yugo Nagata for the rest(including
   documentation patch).
  
   Here are changes made in the patch:
  
   1) Frontend lo_* libpq functions(fe-lobj.c)(Yugo Nagata)
  
   lo_initialize() gathers backend 64-bit large object handling
   function's oid, namely lo_lseek64, lo_tell64, lo_truncate64.
  
   If client calls lo_*64 functions and backend does not support them,
   lo_*64 functions return error to caller. There might be an argument
   since calls to lo_*64 functions can automatically be redirected to
   32-bit older API. I don't know this is worth the trouble though.
  
  I think it should definitely return an error code when user tries to
  use lo_*64 functions towards the backend v9.2 or older, because
  fallback to 32bit API can raise unexpected errors if application
  intends to seek the area over than 2GB.
 
   Currently lo_initialize() throws an error if one of oids are not
   available. I doubt we do the same way for 64-bit functions since this
   

Re: [HACKERS] 64-bit API for large object

2012-10-05 Thread Kohei KaiGai
Hi Anzai-san,

The latest patch is fair enough for me, so let me hand over its reviewing
for comitters.

Thanks,

2012/10/1 Nozomi Anzai an...@sraoss.co.jp:
 Here is 64-bit API for large object version 3 patch.

 I checked this patch. It looks good, but here are still some points to be
 discussed.

 * I have a question. What is the meaning of INT64_IS_BUSTED?
   It seems to me a marker to indicate a platform without 64bit support.
   However, the commit 901be0fad4034c9cf8a3588fd6cf2ece82e4b8ce
   says as follows:
   | Remove all the special-case code for INT64_IS_BUSTED, per decision that
   | we're not going to support that anymore.

 Removed INT64_IS_BUSTED.


 * At inv_seek(), it seems to me it checks offset correctness with wrong way,
   as follows:
 |  case SEEK_SET:
 |  if (offset  0)
 |  elog(ERROR, invalid seek offset:  INT64_FORMAT, offset);
 |  obj_desc-offset = offset;
 |  break;
   It is a right assumption, if large object size would be restricted to 2GB.
   But the largest positive int64 is larger than expected limitation.
   So, it seems to me it should be compared with (INT_MAX * PAGE_SIZE)
   instead.

 Fixed.


 * At inv_write(), it definitely needs a check to prevent data-write upper 
 4TB.
   In case when obj_desc-offset is a bit below 4TB, an additional 1GB write
   will break head of the large object because of pageno overflow.

 Added a such check.


 * Please also add checks on inv_read() to prevent LargeObjectDesc-offset
   unexpectedly overflows 4TB boundary.

 Added a such check.


 * At inv_truncate(), variable off is re-defined to int64. Is it really 
 needed
   change? All its usage is to store the result of len % LOBLKSIZE.

 Fixed and back to int32.


 Thanks,

 2012/9/24 Nozomi Anzai an...@sraoss.co.jp:
  Here is 64-bit API for large object version 2 patch.
 
  I checked this patch. It can be applied onto the latest master branch
  without any problems. My comments are below.
 
  2012/9/11 Tatsuo Ishii is...@postgresql.org:
   Ok, here is the patch to implement 64-bit API for large object, to
   allow to use up to 4TB large objects(or 16TB if BLCKSZ changed to
   32KB). The patch is based on Jeremy Drake's patch posted on September
   23, 2005
   (http://archives.postgresql.org/pgsql-hackers/2005-09/msg01026.php)
   and reasonably updated/edited to adopt PostgreSQL 9.3 by Nozomi Anzai
   for the backend part and Yugo Nagata for the rest(including
   documentation patch).
  
   Here are changes made in the patch:
  
   1) Frontend lo_* libpq functions(fe-lobj.c)(Yugo Nagata)
  
   lo_initialize() gathers backend 64-bit large object handling
   function's oid, namely lo_lseek64, lo_tell64, lo_truncate64.
  
   If client calls lo_*64 functions and backend does not support them,
   lo_*64 functions return error to caller. There might be an argument
   since calls to lo_*64 functions can automatically be redirected to
   32-bit older API. I don't know this is worth the trouble though.
  
  I think it should definitely return an error code when user tries to
  use lo_*64 functions towards the backend v9.2 or older, because
  fallback to 32bit API can raise unexpected errors if application
  intends to seek the area over than 2GB.
 
   Currently lo_initialize() throws an error if one of oids are not
   available. I doubt we do the same way for 64-bit functions since this
   will make 9.3 libpq unable to access large objects stored in pre-9.2
   PostgreSQL servers.
  
  It seems to me the situation to split the case of pre-9.2 and post-9.3
  using a condition of conn-sversion = 90300.
 
  Fixed so, and tested it by deleteing the lo_tell64's row from pg_proc.
 
 
   To pass 64-bit integer to PQfn, PQArgBlock is used like this: int *ptr
   is a pointer to 64-bit integer and actual data is placed somewhere
   else. There might be other way: add new member to union u to store
   64-bit integer:
  
   typedef struct
   {
   int len;
   int isint;
   union
   {
   int*ptr;/* can't use 
   void (dec compiler barfs)   */
   int integer;
   int64   bigint; /* 64-bit integer */
   }   u;
   } PQArgBlock;
  
   I'm a little bit worried about this way because PQArgBlock is a public
   interface.
  
  I'm inclined to add a new field for the union; that seems to me straight
  forward approach.
  For example, the manner in lo_seek64() seems to me confusable.
  It set 1 on isint field even though pointer is delivered actually.
 
  +   argv[1].isint = 1;
  +   argv[1].len = 8;
  +   argv[1].u.ptr = (int *) len;
 
  Your proposal was not adopted per discussion.
 
 
   Also we add new type pg_int64:
  
   #ifndef NO_PG_INT64
   #define HAVE_PG_INT64 1
   typedef long long int pg_int64;
   #endif
  
   in 

Re: [HACKERS] 64-bit API for large object

2012-10-05 Thread Tatsuo Ishii
As a committer, I have looked into the patch and it seems it's good to
commit. However I want to make a small enhancement in the
documentation part:

1) lo_open section needs to mention about new 64bit APIs. Also it
   should include description about lo_truncate, but this is not 64bit
   APIs author's fault since it should had been there when lo_truncate
   was added.

2) Add mention that 64bit APIs are only available in PostgreSQL 9.3 or
   later and if the API is requested against older version of servers
   it will fail.

If there's no objection, I would like commit attached patches.
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese: http://www.sraoss.co.jp

 Hi Anzai-san,
 
 The latest patch is fair enough for me, so let me hand over its reviewing
 for comitters.
 
 Thanks,
 
 2012/10/1 Nozomi Anzai an...@sraoss.co.jp:
 Here is 64-bit API for large object version 3 patch.

 I checked this patch. It looks good, but here are still some points to be
 discussed.

 * I have a question. What is the meaning of INT64_IS_BUSTED?
   It seems to me a marker to indicate a platform without 64bit support.
   However, the commit 901be0fad4034c9cf8a3588fd6cf2ece82e4b8ce
   says as follows:
   | Remove all the special-case code for INT64_IS_BUSTED, per decision that
   | we're not going to support that anymore.

 Removed INT64_IS_BUSTED.


 * At inv_seek(), it seems to me it checks offset correctness with wrong way,
   as follows:
 |  case SEEK_SET:
 |  if (offset  0)
 |  elog(ERROR, invalid seek offset:  INT64_FORMAT, 
 offset);
 |  obj_desc-offset = offset;
 |  break;
   It is a right assumption, if large object size would be restricted to 2GB.
   But the largest positive int64 is larger than expected limitation.
   So, it seems to me it should be compared with (INT_MAX * PAGE_SIZE)
   instead.

 Fixed.


 * At inv_write(), it definitely needs a check to prevent data-write upper 
 4TB.
   In case when obj_desc-offset is a bit below 4TB, an additional 1GB write
   will break head of the large object because of pageno overflow.

 Added a such check.


 * Please also add checks on inv_read() to prevent LargeObjectDesc-offset
   unexpectedly overflows 4TB boundary.

 Added a such check.


 * At inv_truncate(), variable off is re-defined to int64. Is it really 
 needed
   change? All its usage is to store the result of len % LOBLKSIZE.

 Fixed and back to int32.


 Thanks,

 2012/9/24 Nozomi Anzai an...@sraoss.co.jp:
  Here is 64-bit API for large object version 2 patch.
 
  I checked this patch. It can be applied onto the latest master branch
  without any problems. My comments are below.
 
  2012/9/11 Tatsuo Ishii is...@postgresql.org:
   Ok, here is the patch to implement 64-bit API for large object, to
   allow to use up to 4TB large objects(or 16TB if BLCKSZ changed to
   32KB). The patch is based on Jeremy Drake's patch posted on September
   23, 2005
   (http://archives.postgresql.org/pgsql-hackers/2005-09/msg01026.php)
   and reasonably updated/edited to adopt PostgreSQL 9.3 by Nozomi Anzai
   for the backend part and Yugo Nagata for the rest(including
   documentation patch).
  
   Here are changes made in the patch:
  
   1) Frontend lo_* libpq functions(fe-lobj.c)(Yugo Nagata)
  
   lo_initialize() gathers backend 64-bit large object handling
   function's oid, namely lo_lseek64, lo_tell64, lo_truncate64.
  
   If client calls lo_*64 functions and backend does not support them,
   lo_*64 functions return error to caller. There might be an argument
   since calls to lo_*64 functions can automatically be redirected to
   32-bit older API. I don't know this is worth the trouble though.
  
  I think it should definitely return an error code when user tries to
  use lo_*64 functions towards the backend v9.2 or older, because
  fallback to 32bit API can raise unexpected errors if application
  intends to seek the area over than 2GB.
 
   Currently lo_initialize() throws an error if one of oids are not
   available. I doubt we do the same way for 64-bit functions since this
   will make 9.3 libpq unable to access large objects stored in pre-9.2
   PostgreSQL servers.
  
  It seems to me the situation to split the case of pre-9.2 and post-9.3
  using a condition of conn-sversion = 90300.
 
  Fixed so, and tested it by deleteing the lo_tell64's row from pg_proc.
 
 
   To pass 64-bit integer to PQfn, PQArgBlock is used like this: int *ptr
   is a pointer to 64-bit integer and actual data is placed somewhere
   else. There might be other way: add new member to union u to store
   64-bit integer:
  
   typedef struct
   {
   int len;
   int isint;
   union
   {
   int*ptr;/* can't use 
   void (dec compiler barfs)   */
   int integer;
   int64  

Re: [HACKERS] 64-bit API for large object

2012-10-01 Thread Nozomi Anzai
Here is 64-bit API for large object version 3 patch.

 I checked this patch. It looks good, but here are still some points to be
 discussed.
 
 * I have a question. What is the meaning of INT64_IS_BUSTED?
   It seems to me a marker to indicate a platform without 64bit support.
   However, the commit 901be0fad4034c9cf8a3588fd6cf2ece82e4b8ce
   says as follows:
   | Remove all the special-case code for INT64_IS_BUSTED, per decision that
   | we're not going to support that anymore.

Removed INT64_IS_BUSTED.


 * At inv_seek(), it seems to me it checks offset correctness with wrong way,
   as follows:
 |  case SEEK_SET:
 |  if (offset  0)
 |  elog(ERROR, invalid seek offset:  INT64_FORMAT, offset);
 |  obj_desc-offset = offset;
 |  break;
   It is a right assumption, if large object size would be restricted to 2GB.
   But the largest positive int64 is larger than expected limitation.
   So, it seems to me it should be compared with (INT_MAX * PAGE_SIZE)
   instead.

Fixed.


 * At inv_write(), it definitely needs a check to prevent data-write upper 4TB.
   In case when obj_desc-offset is a bit below 4TB, an additional 1GB write
   will break head of the large object because of pageno overflow.

Added a such check.


 * Please also add checks on inv_read() to prevent LargeObjectDesc-offset
   unexpectedly overflows 4TB boundary.

Added a such check.


 * At inv_truncate(), variable off is re-defined to int64. Is it really 
 needed
   change? All its usage is to store the result of len % LOBLKSIZE.

Fixed and back to int32.


 Thanks,
 
 2012/9/24 Nozomi Anzai an...@sraoss.co.jp:
  Here is 64-bit API for large object version 2 patch.
 
  I checked this patch. It can be applied onto the latest master branch
  without any problems. My comments are below.
 
  2012/9/11 Tatsuo Ishii is...@postgresql.org:
   Ok, here is the patch to implement 64-bit API for large object, to
   allow to use up to 4TB large objects(or 16TB if BLCKSZ changed to
   32KB). The patch is based on Jeremy Drake's patch posted on September
   23, 2005
   (http://archives.postgresql.org/pgsql-hackers/2005-09/msg01026.php)
   and reasonably updated/edited to adopt PostgreSQL 9.3 by Nozomi Anzai
   for the backend part and Yugo Nagata for the rest(including
   documentation patch).
  
   Here are changes made in the patch:
  
   1) Frontend lo_* libpq functions(fe-lobj.c)(Yugo Nagata)
  
   lo_initialize() gathers backend 64-bit large object handling
   function's oid, namely lo_lseek64, lo_tell64, lo_truncate64.
  
   If client calls lo_*64 functions and backend does not support them,
   lo_*64 functions return error to caller. There might be an argument
   since calls to lo_*64 functions can automatically be redirected to
   32-bit older API. I don't know this is worth the trouble though.
  
  I think it should definitely return an error code when user tries to
  use lo_*64 functions towards the backend v9.2 or older, because
  fallback to 32bit API can raise unexpected errors if application
  intends to seek the area over than 2GB.
 
   Currently lo_initialize() throws an error if one of oids are not
   available. I doubt we do the same way for 64-bit functions since this
   will make 9.3 libpq unable to access large objects stored in pre-9.2
   PostgreSQL servers.
  
  It seems to me the situation to split the case of pre-9.2 and post-9.3
  using a condition of conn-sversion = 90300.
 
  Fixed so, and tested it by deleteing the lo_tell64's row from pg_proc.
 
 
   To pass 64-bit integer to PQfn, PQArgBlock is used like this: int *ptr
   is a pointer to 64-bit integer and actual data is placed somewhere
   else. There might be other way: add new member to union u to store
   64-bit integer:
  
   typedef struct
   {
   int len;
   int isint;
   union
   {
   int*ptr;/* can't use 
   void (dec compiler barfs)   */
   int integer;
   int64   bigint; /* 64-bit integer */
   }   u;
   } PQArgBlock;
  
   I'm a little bit worried about this way because PQArgBlock is a public
   interface.
  
  I'm inclined to add a new field for the union; that seems to me straight
  forward approach.
  For example, the manner in lo_seek64() seems to me confusable.
  It set 1 on isint field even though pointer is delivered actually.
 
  +   argv[1].isint = 1;
  +   argv[1].len = 8;
  +   argv[1].u.ptr = (int *) len;
 
  Your proposal was not adopted per discussion.
 
 
   Also we add new type pg_int64:
  
   #ifndef NO_PG_INT64
   #define HAVE_PG_INT64 1
   typedef long long int pg_int64;
   #endif
  
   in postgres_ext.h per suggestion from Tom Lane:
   http://archives.postgresql.org/pgsql-hackers/2005-09/msg01062.php
  
  I'm uncertain about context of this discussion.
 
  Does 

Re: [HACKERS] 64-bit API for large object

2012-10-01 Thread Peter Eisentraut
On 9/28/12 10:35 AM, Alvaro Herrera wrote:
 Now there is one more problem in this area which is that the patch
 defined a new type pg_int64 for frontend code (postgres_ext.h).  This
 seems a bad idea to me.  We already have int64 defined in c.h.  Should
 we expose int64 to postgres_ext.h somehow?  Should we use standard-
 mandated int64_t instead?  One way would be to have a new configure
 check for int64_t, and if that type doesn't exist, then just don't
 provide the 64 bit functionality to frontend.

Or create a new type like pg_lo_off_t.



-- 
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] 64-bit API for large object

2012-09-30 Thread Kohei KaiGai
2012/9/30 Tatsuo Ishii is...@postgresql.org:
 * At inv_seek(), it seems to me it checks offset correctness with wrong way,
   as follows:
 |  case SEEK_SET:
 |  if (offset  0)
 |  elog(ERROR, invalid seek offset:  INT64_FORMAT, offset);
 |  obj_desc-offset = offset;
 |  break;
   It is a right assumption, if large object size would be restricted to 2GB.
   But the largest positive int64 is larger than expected limitation.
   So, it seems to me it should be compared with (INT_MAX * PAGE_SIZE)
   instead.

 Point taken. However, checking offset  0 seems to be still valid
 because it is possible to pass minus offset to inv_seek(), no?  Also I
 think upper limit for seek position should be defined as (INT_MAX *
 LOBLKSIZE), rather than (INT_MAX * PAGE_SIZE). Probably (INT_MAX *
 LOBLKSIZE) should be defined in pg_largeobject.h as:

 /*
  * Maximum byte length for each large object
 */
 #define MAX_LARGE_OBJECT_SIZE   INT64CONST(INT_MAX * LOBLKSIZE)

 Then the checking offset in inv_seek() will be:

 case SEEK_SET:
 if (offset  0 || offset = MAX_LARGE_OBJECT_SIZE)
 elog(ERROR, invalid seek offset:  
 INT64_FORMAT, offset);
 obj_desc-offset = offset;
 break;
 case SEEK_CUR:
 if ((offset + obj_desc-offset)  0 ||
(offset + obj_desc-offset) = 
 MAX_LARGE_OBJECT_SIZE)
 elog(ERROR, invalid seek offset:  
 INT64_FORMAT, offset);
 obj_desc-offset += offset;
 break;
 case SEEK_END:
 {
 int64   pos = inv_getsize(obj_desc) + 
 offset;

 if (pos  0 || pos = MAX_LARGE_OBJECT_SIZE)
 elog(ERROR, invalid seek offset:  
 INT64_FORMAT, offset);
 obj_desc-offset = pos;
 }

 What do you think?

Yes, it is exactly what I expected. Indeed, it is still need a check to prevent
negative offset here.

Thanks,
-- 
KaiGai Kohei kai...@kaigai.gr.jp


-- 
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] 64-bit API for large object

2012-09-29 Thread Tatsuo Ishii
 Excerpts from Kohei KaiGai's message of jue sep 27 01:01:18 -0300 2012:
 
 * I have a question. What is the meaning of INT64_IS_BUSTED?
   It seems to me a marker to indicate a platform without 64bit support.
   However, the commit 901be0fad4034c9cf8a3588fd6cf2ece82e4b8ce
   says as follows:
   | Remove all the special-case code for INT64_IS_BUSTED, per decision that
   | we're not going to support that anymore.
 
 Yeah, I think we should just get rid of those bits.  I don't remember
 seeing *any* complaint when INT64_IS_BUSTED was removed, which means
 nobody was using that code anyway.

Ok.

 Now there is one more problem in this area which is that the patch
 defined a new type pg_int64 for frontend code (postgres_ext.h).  This
 seems a bad idea to me.  We already have int64 defined in c.h.  Should
 we expose int64 to postgres_ext.h somehow?  Should we use standard-
 mandated int64_t instead?  One way would be to have a new configure
 check for int64_t, and if that type doesn't exist, then just don't
 provide the 64 bit functionality to frontend.

This has been already explained in upthread:
http://archives.postgresql.org/pgsql-hackers/2012-09/msg00447.php
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese: http://www.sraoss.co.jp


-- 
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] 64-bit API for large object

2012-09-29 Thread Tatsuo Ishii
Kaiai-san,

Thank you for review.

 I checked this patch. It looks good, but here are still some points to be
 discussed.
 
 * I have a question. What is the meaning of INT64_IS_BUSTED?
   It seems to me a marker to indicate a platform without 64bit support.
   However, the commit 901be0fad4034c9cf8a3588fd6cf2ece82e4b8ce
   says as follows:
   | Remove all the special-case code for INT64_IS_BUSTED, per decision that
   | we're not going to support that anymore.

Agreed.

 * At inv_seek(), it seems to me it checks offset correctness with wrong way,
   as follows:
 |  case SEEK_SET:
 |  if (offset  0)
 |  elog(ERROR, invalid seek offset:  INT64_FORMAT, offset);
 |  obj_desc-offset = offset;
 |  break;
   It is a right assumption, if large object size would be restricted to 2GB.
   But the largest positive int64 is larger than expected limitation.
   So, it seems to me it should be compared with (INT_MAX * PAGE_SIZE)
   instead.

Point taken. However, checking offset  0 seems to be still valid
because it is possible to pass minus offset to inv_seek(), no?  Also I
think upper limit for seek position should be defined as (INT_MAX *
LOBLKSIZE), rather than (INT_MAX * PAGE_SIZE). Probably (INT_MAX *
LOBLKSIZE) should be defined in pg_largeobject.h as:

/*
 * Maximum byte length for each large object
*/
#define MAX_LARGE_OBJECT_SIZE   INT64CONST(INT_MAX * LOBLKSIZE)

Then the checking offset in inv_seek() will be:

case SEEK_SET:
if (offset  0 || offset = MAX_LARGE_OBJECT_SIZE)
elog(ERROR, invalid seek offset:  
INT64_FORMAT, offset);
obj_desc-offset = offset;
break;
case SEEK_CUR:
if ((offset + obj_desc-offset)  0 ||
   (offset + obj_desc-offset) = MAX_LARGE_OBJECT_SIZE)
elog(ERROR, invalid seek offset:  
INT64_FORMAT, offset);
obj_desc-offset += offset;
break;
case SEEK_END:
{
int64   pos = inv_getsize(obj_desc) + 
offset;

if (pos  0 || pos = MAX_LARGE_OBJECT_SIZE)
elog(ERROR, invalid seek offset:  
INT64_FORMAT, offset);
obj_desc-offset = pos;
}

What do you think?

 * At inv_write(), it definitely needs a check to prevent data-write upper 4TB.
   In case when obj_desc-offset is a bit below 4TB, an additional 1GB write
   will break head of the large object because of pageno overflow.

Ok. I will add checking:

if ((nbytes + obj_desc-offset)  MAX_LARGE_OBJECT_SIZE)
elog(ERROR, invalid write request size: %d, nbytes);

 * Please also add checks on inv_read() to prevent LargeObjectDesc-offset
   unexpectedly overflows 4TB boundary.

Ok. I will add checking:

if ((nbytes + obj_desc-offset)  MAX_LARGE_OBJECT_SIZE)
elog(ERROR, invalid read request size: %d, nbytes);

 * At inv_truncate(), variable off is re-defined to int64. Is it really 
 needed
   change? All its usage is to store the result of len % LOBLKSIZE.

Your point is correct. Back to int32.
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese: http://www.sraoss.co.jp

 Thanks,
 
 2012/9/24 Nozomi Anzai an...@sraoss.co.jp:
 Here is 64-bit API for large object version 2 patch.

 I checked this patch. It can be applied onto the latest master branch
 without any problems. My comments are below.

 2012/9/11 Tatsuo Ishii is...@postgresql.org:
  Ok, here is the patch to implement 64-bit API for large object, to
  allow to use up to 4TB large objects(or 16TB if BLCKSZ changed to
  32KB). The patch is based on Jeremy Drake's patch posted on September
  23, 2005
  (http://archives.postgresql.org/pgsql-hackers/2005-09/msg01026.php)
  and reasonably updated/edited to adopt PostgreSQL 9.3 by Nozomi Anzai
  for the backend part and Yugo Nagata for the rest(including
  documentation patch).
 
  Here are changes made in the patch:
 
  1) Frontend lo_* libpq functions(fe-lobj.c)(Yugo Nagata)
 
  lo_initialize() gathers backend 64-bit large object handling
  function's oid, namely lo_lseek64, lo_tell64, lo_truncate64.
 
  If client calls lo_*64 functions and backend does not support them,
  lo_*64 functions return error to caller. There might be an argument
  since calls to lo_*64 functions can automatically be redirected to
  32-bit older API. I don't know this is worth the trouble though.
 
 I think it should definitely return an error code when user tries to
 use lo_*64 functions towards the backend v9.2 or older, because
 fallback to 32bit API can raise unexpected errors if application
 intends to seek the area over than 2GB.

  Currently lo_initialize() throws 

Re: [HACKERS] 64-bit API for large object

2012-09-28 Thread Alvaro Herrera
Excerpts from Kohei KaiGai's message of jue sep 27 01:01:18 -0300 2012:

 * I have a question. What is the meaning of INT64_IS_BUSTED?
   It seems to me a marker to indicate a platform without 64bit support.
   However, the commit 901be0fad4034c9cf8a3588fd6cf2ece82e4b8ce
   says as follows:
   | Remove all the special-case code for INT64_IS_BUSTED, per decision that
   | we're not going to support that anymore.

Yeah, I think we should just get rid of those bits.  I don't remember
seeing *any* complaint when INT64_IS_BUSTED was removed, which means
nobody was using that code anyway.

Now there is one more problem in this area which is that the patch
defined a new type pg_int64 for frontend code (postgres_ext.h).  This
seems a bad idea to me.  We already have int64 defined in c.h.  Should
we expose int64 to postgres_ext.h somehow?  Should we use standard-
mandated int64_t instead?  One way would be to have a new configure
check for int64_t, and if that type doesn't exist, then just don't
provide the 64 bit functionality to frontend.

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training  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] 64-bit API for large object

2012-09-26 Thread Kohei KaiGai
I checked this patch. It looks good, but here are still some points to be
discussed.

* I have a question. What is the meaning of INT64_IS_BUSTED?
  It seems to me a marker to indicate a platform without 64bit support.
  However, the commit 901be0fad4034c9cf8a3588fd6cf2ece82e4b8ce
  says as follows:
  | Remove all the special-case code for INT64_IS_BUSTED, per decision that
  | we're not going to support that anymore.

* At inv_seek(), it seems to me it checks offset correctness with wrong way,
  as follows:
|  case SEEK_SET:
|  if (offset  0)
|  elog(ERROR, invalid seek offset:  INT64_FORMAT, offset);
|  obj_desc-offset = offset;
|  break;
  It is a right assumption, if large object size would be restricted to 2GB.
  But the largest positive int64 is larger than expected limitation.
  So, it seems to me it should be compared with (INT_MAX * PAGE_SIZE)
  instead.

* At inv_write(), it definitely needs a check to prevent data-write upper 4TB.
  In case when obj_desc-offset is a bit below 4TB, an additional 1GB write
  will break head of the large object because of pageno overflow.

* Please also add checks on inv_read() to prevent LargeObjectDesc-offset
  unexpectedly overflows 4TB boundary.

* At inv_truncate(), variable off is re-defined to int64. Is it really needed
  change? All its usage is to store the result of len % LOBLKSIZE.

Thanks,

2012/9/24 Nozomi Anzai an...@sraoss.co.jp:
 Here is 64-bit API for large object version 2 patch.

 I checked this patch. It can be applied onto the latest master branch
 without any problems. My comments are below.

 2012/9/11 Tatsuo Ishii is...@postgresql.org:
  Ok, here is the patch to implement 64-bit API for large object, to
  allow to use up to 4TB large objects(or 16TB if BLCKSZ changed to
  32KB). The patch is based on Jeremy Drake's patch posted on September
  23, 2005
  (http://archives.postgresql.org/pgsql-hackers/2005-09/msg01026.php)
  and reasonably updated/edited to adopt PostgreSQL 9.3 by Nozomi Anzai
  for the backend part and Yugo Nagata for the rest(including
  documentation patch).
 
  Here are changes made in the patch:
 
  1) Frontend lo_* libpq functions(fe-lobj.c)(Yugo Nagata)
 
  lo_initialize() gathers backend 64-bit large object handling
  function's oid, namely lo_lseek64, lo_tell64, lo_truncate64.
 
  If client calls lo_*64 functions and backend does not support them,
  lo_*64 functions return error to caller. There might be an argument
  since calls to lo_*64 functions can automatically be redirected to
  32-bit older API. I don't know this is worth the trouble though.
 
 I think it should definitely return an error code when user tries to
 use lo_*64 functions towards the backend v9.2 or older, because
 fallback to 32bit API can raise unexpected errors if application
 intends to seek the area over than 2GB.

  Currently lo_initialize() throws an error if one of oids are not
  available. I doubt we do the same way for 64-bit functions since this
  will make 9.3 libpq unable to access large objects stored in pre-9.2
  PostgreSQL servers.
 
 It seems to me the situation to split the case of pre-9.2 and post-9.3
 using a condition of conn-sversion = 90300.

 Fixed so, and tested it by deleteing the lo_tell64's row from pg_proc.


  To pass 64-bit integer to PQfn, PQArgBlock is used like this: int *ptr
  is a pointer to 64-bit integer and actual data is placed somewhere
  else. There might be other way: add new member to union u to store
  64-bit integer:
 
  typedef struct
  {
  int len;
  int isint;
  union
  {
  int*ptr;/* can't use void 
  (dec compiler barfs)   */
  int integer;
  int64   bigint; /* 64-bit integer */
  }   u;
  } PQArgBlock;
 
  I'm a little bit worried about this way because PQArgBlock is a public
  interface.
 
 I'm inclined to add a new field for the union; that seems to me straight
 forward approach.
 For example, the manner in lo_seek64() seems to me confusable.
 It set 1 on isint field even though pointer is delivered actually.

 +   argv[1].isint = 1;
 +   argv[1].len = 8;
 +   argv[1].u.ptr = (int *) len;

 Your proposal was not adopted per discussion.


  Also we add new type pg_int64:
 
  #ifndef NO_PG_INT64
  #define HAVE_PG_INT64 1
  typedef long long int pg_int64;
  #endif
 
  in postgres_ext.h per suggestion from Tom Lane:
  http://archives.postgresql.org/pgsql-hackers/2005-09/msg01062.php
 
 I'm uncertain about context of this discussion.

 Does it make matter if we include stdint.h and use int64_t instead
 of the self defined data type?

 Your proposal was not adopted per discussion.
 Per discussion, endiannness translation was moved to fe-lobj.c.


  2) Backend lo_* functions (be-fsstubs.c)(Nozomi Anzai)
 
  Add 

Re: [HACKERS] 64-bit API for large object

2012-09-24 Thread Nozomi Anzai
Here is 64-bit API for large object version 2 patch.

 I checked this patch. It can be applied onto the latest master branch
 without any problems. My comments are below.
 
 2012/9/11 Tatsuo Ishii is...@postgresql.org:
  Ok, here is the patch to implement 64-bit API for large object, to
  allow to use up to 4TB large objects(or 16TB if BLCKSZ changed to
  32KB). The patch is based on Jeremy Drake's patch posted on September
  23, 2005
  (http://archives.postgresql.org/pgsql-hackers/2005-09/msg01026.php)
  and reasonably updated/edited to adopt PostgreSQL 9.3 by Nozomi Anzai
  for the backend part and Yugo Nagata for the rest(including
  documentation patch).
 
  Here are changes made in the patch:
 
  1) Frontend lo_* libpq functions(fe-lobj.c)(Yugo Nagata)
 
  lo_initialize() gathers backend 64-bit large object handling
  function's oid, namely lo_lseek64, lo_tell64, lo_truncate64.
 
  If client calls lo_*64 functions and backend does not support them,
  lo_*64 functions return error to caller. There might be an argument
  since calls to lo_*64 functions can automatically be redirected to
  32-bit older API. I don't know this is worth the trouble though.
 
 I think it should definitely return an error code when user tries to
 use lo_*64 functions towards the backend v9.2 or older, because
 fallback to 32bit API can raise unexpected errors if application
 intends to seek the area over than 2GB.

  Currently lo_initialize() throws an error if one of oids are not
  available. I doubt we do the same way for 64-bit functions since this
  will make 9.3 libpq unable to access large objects stored in pre-9.2
  PostgreSQL servers.
 
 It seems to me the situation to split the case of pre-9.2 and post-9.3
 using a condition of conn-sversion = 90300.

Fixed so, and tested it by deleteing the lo_tell64's row from pg_proc.


  To pass 64-bit integer to PQfn, PQArgBlock is used like this: int *ptr
  is a pointer to 64-bit integer and actual data is placed somewhere
  else. There might be other way: add new member to union u to store
  64-bit integer:
 
  typedef struct
  {
  int len;
  int isint;
  union
  {
  int*ptr;/* can't use void 
  (dec compiler barfs)   */
  int integer;
  int64   bigint; /* 64-bit integer */
  }   u;
  } PQArgBlock;
 
  I'm a little bit worried about this way because PQArgBlock is a public
  interface.
 
 I'm inclined to add a new field for the union; that seems to me straight
 forward approach.
 For example, the manner in lo_seek64() seems to me confusable.
 It set 1 on isint field even though pointer is delivered actually.
 
 +   argv[1].isint = 1;
 +   argv[1].len = 8;
 +   argv[1].u.ptr = (int *) len;

Your proposal was not adopted per discussion.


  Also we add new type pg_int64:
 
  #ifndef NO_PG_INT64
  #define HAVE_PG_INT64 1
  typedef long long int pg_int64;
  #endif
 
  in postgres_ext.h per suggestion from Tom Lane:
  http://archives.postgresql.org/pgsql-hackers/2005-09/msg01062.php
 
 I'm uncertain about context of this discussion.
 
 Does it make matter if we include stdint.h and use int64_t instead
 of the self defined data type?

Your proposal was not adopted per discussion.
Per discussion, endiannness translation was moved to fe-lobj.c.


  2) Backend lo_* functions (be-fsstubs.c)(Nozomi Anzai)
 
  Add lo_lseek64, lo_tell64, lo_truncate64 so that they can handle
  64-bit seek position and data length. loread64 and lowrite64 are not
  added because if a program tries to read/write more than 2GB at once,
  it would be a sign that the program need to be re-designed anyway.
 
 I think it is a reasonable.
 
  3) Backend inv_api.c functions(Nozomi Anzai)
 
  No need to add new functions. Just extend them to handle 64-bit data.
 
  BTW , what will happen if older 32-bit libpq accesses large objects
  over 2GB?
 
  lo_read and lo_write: they can read or write lobjs using 32-bit API as
  long as requested read/write data length is smaller than 2GB. So I
  think we can safely allow them to access over 2GB lobjs.
 
  lo_lseek: again as long as requested offset is smaller than 2GB, there
  would be no problem.
 
  lo_tell:if current seek position is beyond 2GB, returns an error.
 
 Even though iteration of lo_lseek() may move the offset to 4TB, it also
 makes unavailable to use lo_tell() to obtain the current offset, so I think
 it is reasonable behavior.
 
 However, error code is not an appropriate one.
 
 +   if (INT_MAX  offset)
 +   {
 +   ereport(ERROR,
 +   (errcode(ERRCODE_UNDEFINED_OBJECT),
 +errmsg(invalid large-object
 descriptor: %d, fd)));
 +   PG_RETURN_INT32(-1);
 +   }
 
 According to the manpage of lseek(2)
 EOVERFLOW
 The resulting file 

Re: [HACKERS] 64-bit API for large object

2012-09-22 Thread Kohei KaiGai
2012/9/22 Tatsuo Ishii is...@postgresql.org:
 Tom, Kaigai,

 Kohei KaiGai kai...@kaigai.gr.jp writes:
 Tom, could you give us a suggestion which manner is better approach; whether
 the PQfn should have responsibility for endian translation of 
 64bit-integer, or
 callers (lo_tell64 or lo_seek64)?

 Adding anything inside pqFunctionCall is useless, unless we were to add
 an int64 variant to PQArgBlock, which isn't a good idea because it will
 be an ABI break.  The functions in fe-lobj.c have to set up the int64
 value as if it were pass-by-reference, which means dealing with
 endianness concerns there.

 I just want to make sure you guy's point.

 We do not modify pqFunctionCall. That means PQfn does not accept
 PQArgBlock.isint != 0 and PQArgBlock.len == 8 case. If a PQfn caller
 wants to send 64-bit integer, it should set PQArgBlock.isint = 0 and
 PQArgBlock.len = 8 and set data pass-by-reference. Endianness should
 be taken care by the PQfn caller. Also we do not modify fe-misc.c
 because there's no point to add pqPutint64/pqGetint64(they are called
 from pqFunctionCall in the patch).

Yes, it is exactly what I suggested.

Thanks,
-- 
KaiGai Kohei kai...@kaigai.gr.jp


-- 
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] 64-bit API for large object

2012-09-22 Thread Tatsuo Ishii
 2012/9/22 Tatsuo Ishii is...@postgresql.org:
 Tom, Kaigai,

 Kohei KaiGai kai...@kaigai.gr.jp writes:
 Tom, could you give us a suggestion which manner is better approach; 
 whether
 the PQfn should have responsibility for endian translation of 
 64bit-integer, or
 callers (lo_tell64 or lo_seek64)?

 Adding anything inside pqFunctionCall is useless, unless we were to add
 an int64 variant to PQArgBlock, which isn't a good idea because it will
 be an ABI break.  The functions in fe-lobj.c have to set up the int64
 value as if it were pass-by-reference, which means dealing with
 endianness concerns there.

 I just want to make sure you guy's point.

 We do not modify pqFunctionCall. That means PQfn does not accept
 PQArgBlock.isint != 0 and PQArgBlock.len == 8 case. If a PQfn caller
 wants to send 64-bit integer, it should set PQArgBlock.isint = 0 and
 PQArgBlock.len = 8 and set data pass-by-reference. Endianness should
 be taken care by the PQfn caller. Also we do not modify fe-misc.c
 because there's no point to add pqPutint64/pqGetint64(they are called
 from pqFunctionCall in the patch).

 Yes, it is exactly what I suggested.

Thanks for the confirmation!
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese: http://www.sraoss.co.jp


-- 
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] 64-bit API for large object

2012-09-21 Thread Kohei KaiGai
 I think Tom's point is, there are tons of applications which define
 their own int64_t (at least in 2005).
 Also pg_config.h has:

 #define HAVE_STDINT_H   1

 and this suggests that PostgreSQL adopts to platforms which does not
 have stdint.h. If so, we need to take care of such platforms anyway.

OK, it makes me clear. It might be helpful a source code comment
to remain why we used self defined datatype here.

2012/9/21 Tom Lane t...@sss.pgh.pa.us:
 Tatsuo Ishii is...@postgresql.org writes:
 To pass 64-bit integer to PQfn, PQArgBlock is used like this: int *ptr
 is a pointer to 64-bit integer and actual data is placed somewhere
 else.

 Yeah, I think we have to do it like that.  Changing the size of
 PQArgBlock would be a libpq ABI break, which IMO is sufficiently painful
 to kill this whole proposal.  Much better a little localized ugliness
 in fe-lobj.c.

Hmm, I see. Please deliver the 64bit integer argument as reference,
and don't forget endian translations here.

Thanks,
-- 
KaiGai Kohei kai...@kaigai.gr.jp


-- 
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] 64-bit API for large object

2012-09-21 Thread Yugo Nagata

  Currently lo_initialize() throws an error if one of oids are not
  available. I doubt we do the same way for 64-bit functions since this
  will make 9.3 libpq unable to access large objects stored in pre-9.2
  PostgreSQL servers.
 
 It seems to me the situation to split the case of pre-9.2 and post-9.3
 using a condition of conn-sversion = 90300.
 

Agreed. I'll fix it like that.

  4) src/test/examples/testlo64.c added for 64-bit API example(Yugo Nagata)
 
  Comments and suggestions are welcome.
 
 miscellaneous comments are below.
 
 Regression test is helpful. Even though no need to try to create 4TB large
 object, it is helpful to write some chunks around the design boundary.
 Could you add some test cases that writes some chunks around 4TB offset.

Agreed. I'll do that.

 
 Thanks,
 -- 
 KaiGai Kohei kai...@kaigai.gr.jp
 
 
 -- 
 Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
 To make changes to your subscription:
 http://www.postgresql.org/mailpref/pgsql-hackers


-- 
Yugo Nagata nag...@sraoss.co.jp


-- 
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] 64-bit API for large object

2012-09-21 Thread Tatsuo Ishii
 I think Tom's point is, there are tons of applications which define
 their own int64_t (at least in 2005).
 Also pg_config.h has:

 #define HAVE_STDINT_H   1

 and this suggests that PostgreSQL adopts to platforms which does not
 have stdint.h. If so, we need to take care of such platforms anyway.

 OK, it makes me clear. It might be helpful a source code comment
 to remain why we used self defined datatype here.

Ok.

 2012/9/21 Tom Lane t...@sss.pgh.pa.us:
 Tatsuo Ishii is...@postgresql.org writes:
 To pass 64-bit integer to PQfn, PQArgBlock is used like this: int *ptr
 is a pointer to 64-bit integer and actual data is placed somewhere
 else.

 Yeah, I think we have to do it like that.  Changing the size of
 PQArgBlock would be a libpq ABI break, which IMO is sufficiently painful
 to kill this whole proposal.  Much better a little localized ugliness
 in fe-lobj.c.

 Hmm, I see. Please deliver the 64bit integer argument as reference,
 and don't forget endian translations here.

I thought pgPutInt64() takes care of endianness. No?
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese: http://www.sraoss.co.jp


-- 
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] 64-bit API for large object

2012-09-21 Thread Kohei KaiGai
2012/9/21 Tatsuo Ishii is...@postgresql.org:
 I think Tom's point is, there are tons of applications which define
 their own int64_t (at least in 2005).
 Also pg_config.h has:

 #define HAVE_STDINT_H   1

 and this suggests that PostgreSQL adopts to platforms which does not
 have stdint.h. If so, we need to take care of such platforms anyway.

 OK, it makes me clear. It might be helpful a source code comment
 to remain why we used self defined datatype here.

 Ok.

 2012/9/21 Tom Lane t...@sss.pgh.pa.us:
 Tatsuo Ishii is...@postgresql.org writes:
 To pass 64-bit integer to PQfn, PQArgBlock is used like this: int *ptr
 is a pointer to 64-bit integer and actual data is placed somewhere
 else.

 Yeah, I think we have to do it like that.  Changing the size of
 PQArgBlock would be a libpq ABI break, which IMO is sufficiently painful
 to kill this whole proposal.  Much better a little localized ugliness
 in fe-lobj.c.

 Hmm, I see. Please deliver the 64bit integer argument as reference,
 and don't forget endian translations here.

 I thought pgPutInt64() takes care of endianness. No?

It works inside of the PGfn(), when isint = 1 towards pointer data type.
In my sense, it is a bit problem specific solution.

So, I'd like to see other person's opinion here.

Thanks,
-- 
KaiGai Kohei kai...@kaigai.gr.jp


-- 
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] 64-bit API for large object

2012-09-21 Thread Tatsuo Ishii
 Hmm, I see. Please deliver the 64bit integer argument as reference,
 and don't forget endian translations here.

 I thought pgPutInt64() takes care of endianness. No?

 It works inside of the PGfn(), when isint = 1 towards pointer data type.
 In my sense, it is a bit problem specific solution.
 
 So, I'd like to see other person's opinion here.

I think we cannot change this because we want to keep the counter part
backend side function pq_getmsgint64() as it is (the function is not
part of the patch).
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese: http://www.sraoss.co.jp


-- 
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] 64-bit API for large object

2012-09-21 Thread Kohei KaiGai
2012/9/21 Tatsuo Ishii is...@postgresql.org:
 Hmm, I see. Please deliver the 64bit integer argument as reference,
 and don't forget endian translations here.

 I thought pgPutInt64() takes care of endianness. No?

 It works inside of the PGfn(), when isint = 1 towards pointer data type.
 In my sense, it is a bit problem specific solution.

 So, I'd like to see other person's opinion here.

 I think we cannot change this because we want to keep the counter part
 backend side function pq_getmsgint64() as it is (the function is not
 part of the patch).

My opinion is lo_lseek64() and lo_tell64() should handle endian translation
prior and next to PQfn() invocation; to avoid the int64 specific case-handling
inside of PQfn() that can be called by other applications.

Am I missing something?

Thanks,
-- 
KaiGai Kohei kai...@kaigai.gr.jp


-- 
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] 64-bit API for large object

2012-09-21 Thread Tatsuo Ishii
 I thought pgPutInt64() takes care of endianness. No?

 It works inside of the PGfn(), when isint = 1 towards pointer data type.
 In my sense, it is a bit problem specific solution.

 So, I'd like to see other person's opinion here.

 I think we cannot change this because we want to keep the counter part
 backend side function pq_getmsgint64() as it is (the function is not
 part of the patch).

 My opinion is lo_lseek64() and lo_tell64() should handle endian translation
 prior and next to PQfn() invocation; to avoid the int64 specific case-handling
 inside of PQfn() that can be called by other applications.
 
 Am I missing something?

So what do you want to do with pq_getmsgint64()? It exactly does the
same thing as pqPutInt64(), just in opposit direction. Do you want to
change pq_getmsgint64()? Or add new function in backend?
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese: http://www.sraoss.co.jp


-- 
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] 64-bit API for large object

2012-09-21 Thread Kohei KaiGai
2012/9/21 Tatsuo Ishii is...@postgresql.org:
 I thought pgPutInt64() takes care of endianness. No?

 It works inside of the PGfn(), when isint = 1 towards pointer data type.
 In my sense, it is a bit problem specific solution.

 So, I'd like to see other person's opinion here.

 I think we cannot change this because we want to keep the counter part
 backend side function pq_getmsgint64() as it is (the function is not
 part of the patch).

 My opinion is lo_lseek64() and lo_tell64() should handle endian translation
 prior and next to PQfn() invocation; to avoid the int64 specific 
 case-handling
 inside of PQfn() that can be called by other applications.

 Am I missing something?

 So what do you want to do with pq_getmsgint64()? It exactly does the
 same thing as pqPutInt64(), just in opposit direction. Do you want to
 change pq_getmsgint64()? Or add new function in backend?

My preference is nothing are changed both pg_getmsgint64() of the backend
and routines under PQfn() of the libpq. Isn't it unavailable to deliver int64-
value after the endian translation on the caller side?

Thanks,
-- 
KaiGai Kohei kai...@kaigai.gr.jp


-- 
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] 64-bit API for large object

2012-09-21 Thread Tatsuo Ishii
 2012/9/21 Tatsuo Ishii is...@postgresql.org:
 I thought pgPutInt64() takes care of endianness. No?

 It works inside of the PGfn(), when isint = 1 towards pointer data type.
 In my sense, it is a bit problem specific solution.

 So, I'd like to see other person's opinion here.

 I think we cannot change this because we want to keep the counter part
 backend side function pq_getmsgint64() as it is (the function is not
 part of the patch).

 My opinion is lo_lseek64() and lo_tell64() should handle endian translation
 prior and next to PQfn() invocation; to avoid the int64 specific 
 case-handling
 inside of PQfn() that can be called by other applications.

 Am I missing something?

 So what do you want to do with pq_getmsgint64()? It exactly does the
 same thing as pqPutInt64(), just in opposit direction. Do you want to
 change pq_getmsgint64()? Or add new function in backend?

 My preference is nothing are changed both pg_getmsgint64() of the backend
 and routines under PQfn() of the libpq. Isn't it unavailable to deliver int64-
 value after the endian translation on the caller side?

I am confused.

 My opinion is lo_lseek64() and lo_tell64() should handle endian translation
 prior and next to PQfn() invocation; to avoid the int64 specific 
 case-handling
 inside of PQfn() that can be called by other applications.

Why do we need this? If PQArgBlock.isint != 0, it treats input data as
integer anyway. So I don't see any use case other than int64 specific
case-handling if isint != 0 and len == 8. If you have other use case
for isint != 0 and len == 8, please show it.
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese: http://www.sraoss.co.jp


-- 
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] 64-bit API for large object

2012-09-21 Thread Tom Lane
Kohei KaiGai kai...@kaigai.gr.jp writes:
 My preference is nothing are changed both pg_getmsgint64() of the backend
 and routines under PQfn() of the libpq. Isn't it unavailable to deliver int64-
 value after the endian translation on the caller side?

Right.  If we had to change anything on the backend side, it would mean
we had a wire protocol change, which is even less acceptable than a
libpq ABI change.

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] 64-bit API for large object

2012-09-21 Thread Tatsuo Ishii
 Kohei KaiGai kai...@kaigai.gr.jp writes:
 My preference is nothing are changed both pg_getmsgint64() of the backend
 and routines under PQfn() of the libpq. Isn't it unavailable to deliver 
 int64-
 value after the endian translation on the caller side?
 
 Right.  If we had to change anything on the backend side, it would mean
 we had a wire protocol change, which is even less acceptable than a
 libpq ABI change.

The patch does not touch pg_getmsgint64() and I don't think we are not
going have a wire protocol change.
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese: http://www.sraoss.co.jp


-- 
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] 64-bit API for large object

2012-09-21 Thread Kohei KaiGai
2012/9/21 Tatsuo Ishii is...@postgresql.org:
 Kohei KaiGai kai...@kaigai.gr.jp writes:
 My preference is nothing are changed both pg_getmsgint64() of the backend
 and routines under PQfn() of the libpq. Isn't it unavailable to deliver 
 int64-
 value after the endian translation on the caller side?

 Right.  If we had to change anything on the backend side, it would mean
 we had a wire protocol change, which is even less acceptable than a
 libpq ABI change.

 The patch does not touch pg_getmsgint64() and I don't think we are not
 going have a wire protocol change.

It's also uncertain what portion does Tom said right for...

What I pointed out is this patch adds a special case handling on pqFunctionCall3
of libpq to fetch 64bit-integer from PQArgBlock-u.ptr and adjust endian orders.
It is never the topic on backend side.

It is not a technical problem, but I feel a bit strange coding style.
So, I don't want to against it so much.

Tom, could you give us a suggestion which manner is better approach; whether
the PQfn should have responsibility for endian translation of 64bit-integer, or
callers (lo_tell64 or lo_seek64)?

Thanks,
-- 
KaiGai Kohei kai...@kaigai.gr.jp


-- 
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] 64-bit API for large object

2012-09-21 Thread Tom Lane
Kohei KaiGai kai...@kaigai.gr.jp writes:
 Tom, could you give us a suggestion which manner is better approach; whether
 the PQfn should have responsibility for endian translation of 64bit-integer, 
 or
 callers (lo_tell64 or lo_seek64)?

Adding anything inside pqFunctionCall is useless, unless we were to add
an int64 variant to PQArgBlock, which isn't a good idea because it will
be an ABI break.  The functions in fe-lobj.c have to set up the int64
value as if it were pass-by-reference, which means dealing with
endianness concerns there.

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] 64-bit API for large object

2012-09-21 Thread Tatsuo Ishii
Tom, Kaigai,

 Kohei KaiGai kai...@kaigai.gr.jp writes:
 Tom, could you give us a suggestion which manner is better approach; whether
 the PQfn should have responsibility for endian translation of 64bit-integer, 
 or
 callers (lo_tell64 or lo_seek64)?
 
 Adding anything inside pqFunctionCall is useless, unless we were to add
 an int64 variant to PQArgBlock, which isn't a good idea because it will
 be an ABI break.  The functions in fe-lobj.c have to set up the int64
 value as if it were pass-by-reference, which means dealing with
 endianness concerns there.

I just want to make sure you guy's point.

We do not modify pqFunctionCall. That means PQfn does not accept
PQArgBlock.isint != 0 and PQArgBlock.len == 8 case. If a PQfn caller
wants to send 64-bit integer, it should set PQArgBlock.isint = 0 and
PQArgBlock.len = 8 and set data pass-by-reference. Endianness should
be taken care by the PQfn caller. Also we do not modify fe-misc.c
because there's no point to add pqPutint64/pqGetint64(they are called
from pqFunctionCall in the patch).
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese: http://www.sraoss.co.jp


-- 
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] 64-bit API for large object

2012-09-21 Thread Tatsuo Ishii
 Tom, Kaigai,
 
 Kohei KaiGai kai...@kaigai.gr.jp writes:
 Tom, could you give us a suggestion which manner is better approach; whether
 the PQfn should have responsibility for endian translation of 
 64bit-integer, or
 callers (lo_tell64 or lo_seek64)?
 
 Adding anything inside pqFunctionCall is useless, unless we were to add
 an int64 variant to PQArgBlock, which isn't a good idea because it will
 be an ABI break.  The functions in fe-lobj.c have to set up the int64
 value as if it were pass-by-reference, which means dealing with
 endianness concerns there.
 
 I just want to make sure you guy's point.
 
 We do not modify pqFunctionCall. That means PQfn does not accept
 PQArgBlock.isint != 0 and PQArgBlock.len == 8 case. If a PQfn caller
 wants to send 64-bit integer, it should set PQArgBlock.isint = 0 and
 PQArgBlock.len = 8 and set data pass-by-reference. Endianness should
 be taken care by the PQfn caller. Also we do not modify fe-misc.c
 because there's no point to add pqPutint64/pqGetint64(they are called
 from pqFunctionCall in the patch).

Oops. There's no such a function pqGetint64 in the patch. 64-bit int
case is taken care inside pqGetint.
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese: http://www.sraoss.co.jp


-- 
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] 64-bit API for large object

2012-09-20 Thread Nozomi Anzai
  3) Backend inv_api.c functions(Nozomi Anzai)
 
  No need to add new functions. Just extend them to handle 64-bit data.
 
  BTW , what will happen if older 32-bit libpq accesses large objects
  over 2GB?
 
  lo_read and lo_write: they can read or write lobjs using 32-bit API as
  long as requested read/write data length is smaller than 2GB. So I
  think we can safely allow them to access over 2GB lobjs.
 
  lo_lseek: again as long as requested offset is smaller than 2GB, there
  would be no problem.
 
  lo_tell:if current seek position is beyond 2GB, returns an error.
 
 Even though iteration of lo_lseek() may move the offset to 4TB, it also
 makes unavailable to use lo_tell() to obtain the current offset, so I think
 it is reasonable behavior.
 
 However, error code is not an appropriate one.
 
 +   if (INT_MAX  offset)
 +   {
 +   ereport(ERROR,
 +   (errcode(ERRCODE_UNDEFINED_OBJECT),
 +errmsg(invalid large-object
 descriptor: %d, fd)));
 +   PG_RETURN_INT32(-1);
 +   }
 
 According to the manpage of lseek(2)
 EOVERFLOW
 The resulting file offset cannot be represented in an off_t.
 
 Please add a new error code such as ERRCODE_BLOB_OFFSET_OVERFLOW.

Agreed.

-- 
Nozomi Anzai
SRA OSS, Inc. Japan


-- 
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] 64-bit API for large object

2012-09-20 Thread Tatsuo Ishii
 To pass 64-bit integer to PQfn, PQArgBlock is used like this: int *ptr
 is a pointer to 64-bit integer and actual data is placed somewhere
 else. There might be other way: add new member to union u to store
 64-bit integer:

 typedef struct
 {
 int len;
 int isint;
 union
 {
 int*ptr;/* can't use void 
 (dec compiler barfs)   */
 int integer;
 int64   bigint; /* 64-bit integer */
 }   u;
 } PQArgBlock;

 I'm a little bit worried about this way because PQArgBlock is a public
 interface.

 I'm inclined to add a new field for the union; that seems to me straight
 forward approach.
 For example, the manner in lo_seek64() seems to me confusable.
 It set 1 on isint field even though pointer is delivered actually.
 
 +   argv[1].isint = 1;
 +   argv[1].len = 8;
 +   argv[1].u.ptr = (int *) len;

I have to admit that this is confusing. However I'm worring about
changing sizeof(PQArgBlock) from compatibility's point of view. Maybe
I'm just a paranoia though.

 Also we add new type pg_int64:

 #ifndef NO_PG_INT64
 #define HAVE_PG_INT64 1
 typedef long long int pg_int64;
 #endif

 in postgres_ext.h per suggestion from Tom Lane:
 http://archives.postgresql.org/pgsql-hackers/2005-09/msg01062.php

 I'm uncertain about context of this discussion.
 
 Does it make matter if we include stdint.h and use int64_t instead
 of the self defined data type?

I think Tom's point is, there are tons of applications which define
their own int64_t (at least in 2005).
Also pg_config.h has:

#define HAVE_STDINT_H   1

and this suggests that PostgreSQL adopts to platforms which does not
have stdint.h. If so, we need to take care of such platforms anyway.
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese: http://www.sraoss.co.jp


-- 
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] 64-bit API for large object

2012-09-20 Thread Tom Lane
Tatsuo Ishii is...@postgresql.org writes:
 To pass 64-bit integer to PQfn, PQArgBlock is used like this: int *ptr
 is a pointer to 64-bit integer and actual data is placed somewhere
 else.

Yeah, I think we have to do it like that.  Changing the size of
PQArgBlock would be a libpq ABI break, which IMO is sufficiently painful
to kill this whole proposal.  Much better a little localized ugliness
in fe-lobj.c.

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] 64-bit API for large object

2012-09-19 Thread Kohei KaiGai
I checked this patch. It can be applied onto the latest master branch
without any problems. My comments are below.

2012/9/11 Tatsuo Ishii is...@postgresql.org:
 Ok, here is the patch to implement 64-bit API for large object, to
 allow to use up to 4TB large objects(or 16TB if BLCKSZ changed to
 32KB). The patch is based on Jeremy Drake's patch posted on September
 23, 2005
 (http://archives.postgresql.org/pgsql-hackers/2005-09/msg01026.php)
 and reasonably updated/edited to adopt PostgreSQL 9.3 by Nozomi Anzai
 for the backend part and Yugo Nagata for the rest(including
 documentation patch).

 Here are changes made in the patch:

 1) Frontend lo_* libpq functions(fe-lobj.c)(Yugo Nagata)

 lo_initialize() gathers backend 64-bit large object handling
 function's oid, namely lo_lseek64, lo_tell64, lo_truncate64.

 If client calls lo_*64 functions and backend does not support them,
 lo_*64 functions return error to caller. There might be an argument
 since calls to lo_*64 functions can automatically be redirected to
 32-bit older API. I don't know this is worth the trouble though.

I think it should definitely return an error code when user tries to
use lo_*64 functions towards the backend v9.2 or older, because
fallback to 32bit API can raise unexpected errors if application
intends to seek the area over than 2GB.

 Currently lo_initialize() throws an error if one of oids are not
 available. I doubt we do the same way for 64-bit functions since this
 will make 9.3 libpq unable to access large objects stored in pre-9.2
 PostgreSQL servers.

It seems to me the situation to split the case of pre-9.2 and post-9.3
using a condition of conn-sversion = 90300.

 To pass 64-bit integer to PQfn, PQArgBlock is used like this: int *ptr
 is a pointer to 64-bit integer and actual data is placed somewhere
 else. There might be other way: add new member to union u to store
 64-bit integer:

 typedef struct
 {
 int len;
 int isint;
 union
 {
 int*ptr;/* can't use void 
 (dec compiler barfs)   */
 int integer;
 int64   bigint; /* 64-bit integer */
 }   u;
 } PQArgBlock;

 I'm a little bit worried about this way because PQArgBlock is a public
 interface.

I'm inclined to add a new field for the union; that seems to me straight
forward approach.
For example, the manner in lo_seek64() seems to me confusable.
It set 1 on isint field even though pointer is delivered actually.

+   argv[1].isint = 1;
+   argv[1].len = 8;
+   argv[1].u.ptr = (int *) len;

 Also we add new type pg_int64:

 #ifndef NO_PG_INT64
 #define HAVE_PG_INT64 1
 typedef long long int pg_int64;
 #endif

 in postgres_ext.h per suggestion from Tom Lane:
 http://archives.postgresql.org/pgsql-hackers/2005-09/msg01062.php

I'm uncertain about context of this discussion.

Does it make matter if we include stdint.h and use int64_t instead
of the self defined data type?

 2) Backend lo_* functions (be-fsstubs.c)(Nozomi Anzai)

 Add lo_lseek64, lo_tell64, lo_truncate64 so that they can handle
 64-bit seek position and data length. loread64 and lowrite64 are not
 added because if a program tries to read/write more than 2GB at once,
 it would be a sign that the program need to be re-designed anyway.

I think it is a reasonable.

 3) Backend inv_api.c functions(Nozomi Anzai)

 No need to add new functions. Just extend them to handle 64-bit data.

 BTW , what will happen if older 32-bit libpq accesses large objects
 over 2GB?

 lo_read and lo_write: they can read or write lobjs using 32-bit API as
 long as requested read/write data length is smaller than 2GB. So I
 think we can safely allow them to access over 2GB lobjs.

 lo_lseek: again as long as requested offset is smaller than 2GB, there
 would be no problem.

 lo_tell:if current seek position is beyond 2GB, returns an error.

Even though iteration of lo_lseek() may move the offset to 4TB, it also
makes unavailable to use lo_tell() to obtain the current offset, so I think
it is reasonable behavior.

However, error code is not an appropriate one.

+   if (INT_MAX  offset)
+   {
+   ereport(ERROR,
+   (errcode(ERRCODE_UNDEFINED_OBJECT),
+errmsg(invalid large-object
descriptor: %d, fd)));
+   PG_RETURN_INT32(-1);
+   }

According to the manpage of lseek(2)
EOVERFLOW
The resulting file offset cannot be represented in an off_t.

Please add a new error code such as ERRCODE_BLOB_OFFSET_OVERFLOW.

 4) src/test/examples/testlo64.c added for 64-bit API example(Yugo Nagata)

 Comments and suggestions are welcome.

miscellaneous comments are below.

Regression test is helpful. Even though no need to try to create 4TB large
object, it is helpful to write some chunks around the design boundary.
Could 

Re: [HACKERS] 64-bit API for large object

2012-09-10 Thread Tatsuo Ishii
Ok, here is the patch to implement 64-bit API for large object, to
allow to use up to 4TB large objects(or 16TB if BLCKSZ changed to
32KB). The patch is based on Jeremy Drake's patch posted on September
23, 2005
(http://archives.postgresql.org/pgsql-hackers/2005-09/msg01026.php)
and reasonably updated/edited to adopt PostgreSQL 9.3 by Nozomi Anzai
for the backend part and Yugo Nagata for the rest(including
documentation patch).

Here are changes made in the patch:

1) Frontend lo_* libpq functions(fe-lobj.c)(Yugo Nagata)

lo_initialize() gathers backend 64-bit large object handling
function's oid, namely lo_lseek64, lo_tell64, lo_truncate64.

If client calls lo_*64 functions and backend does not support them,
lo_*64 functions return error to caller. There might be an argument
since calls to lo_*64 functions can automatically be redirected to
32-bit older API. I don't know this is worth the trouble though.

Currently lo_initialize() throws an error if one of oids are not
available. I doubt we do the same way for 64-bit functions since this
will make 9.3 libpq unable to access large objects stored in pre-9.2
PostgreSQL servers.

To pass 64-bit integer to PQfn, PQArgBlock is used like this: int *ptr
is a pointer to 64-bit integer and actual data is placed somewhere
else. There might be other way: add new member to union u to store
64-bit integer:

typedef struct
{
int len;
int isint;
union
{
int*ptr;/* can't use void (dec 
compiler barfs)   */
int integer;
int64   bigint; /* 64-bit integer */
}   u;
} PQArgBlock;

I'm a little bit worried about this way because PQArgBlock is a public
interface.

Also we add new type pg_int64:

#ifndef NO_PG_INT64
#define HAVE_PG_INT64 1
typedef long long int pg_int64;
#endif

in postgres_ext.h per suggestion from Tom Lane:
http://archives.postgresql.org/pgsql-hackers/2005-09/msg01062.php

2) Backend lo_* functions (be-fsstubs.c)(Nozomi Anzai)

Add lo_lseek64, lo_tell64, lo_truncate64 so that they can handle
64-bit seek position and data length. loread64 and lowrite64 are not
added because if a program tries to read/write more than 2GB at once,
it would be a sign that the program need to be re-designed anyway.

3) Backend inv_api.c functions(Nozomi Anzai)

No need to add new functions. Just extend them to handle 64-bit data.

BTW , what will happen if older 32-bit libpq accesses large objects
over 2GB?

lo_read and lo_write: they can read or write lobjs using 32-bit API as
long as requested read/write data length is smaller than 2GB. So I
think we can safely allow them to access over 2GB lobjs.

lo_lseek: again as long as requested offset is smaller than 2GB, there
would be no problem.

lo_tell:if current seek position is beyond 2GB, returns an error.

4) src/test/examples/testlo64.c added for 64-bit API example(Yugo Nagata)

Comments and suggestions are welcome.
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese: http://www.sraoss.co.jp


lobj64.patch.gz
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] 64-bit API for large object

2012-08-28 Thread Tatsuo Ishii
Correct me if I am wrong.

After expanding large object API to 64-bit, the max size of a large
object will be 8TB(assuming 8KB default BLKSZ).

large object max size = pageno(int32) * LOBLKSIZE
  = (2^32-1) * (BLCKSZ / 4)
  = (2^32-1) * (8192/4)
  = 8TB

I just want to confirm my calculation is correct.
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese: http://www.sraoss.co.jp


-- 
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] 64-bit API for large object

2012-08-28 Thread Tom Lane
Tatsuo Ishii is...@postgresql.org writes:
 Correct me if I am wrong.
 After expanding large object API to 64-bit, the max size of a large
 object will be 8TB(assuming 8KB default BLKSZ).

 large object max size = pageno(int32) * LOBLKSIZE
 = (2^32-1) * (BLCKSZ / 4)
 = (2^32-1) * (8192/4)
 = 8TB

 I just want to confirm my calculation is correct.

pg_largeobject.pageno is a signed int, so I don't think we can let it go
past 2^31-1, so half that.

We could buy back the other bit if we redefined the column as oid
instead of int4 (to make it unsigned), but I think that would create
fairly considerable risk of confusion between the loid and pageno
columns (loid already being oid).  I'd just as soon not go there,
at least not till we start seeing actual field complaints about
4TB being paltry ;-)

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] 64-bit API for large object

2012-08-28 Thread Tatsuo Ishii
 pg_largeobject.pageno is a signed int, so I don't think we can let it go
 past 2^31-1, so half that.
 
 We could buy back the other bit if we redefined the column as oid
 instead of int4 (to make it unsigned), but I think that would create
 fairly considerable risk of confusion between the loid and pageno
 columns (loid already being oid).  I'd just as soon not go there,
 at least not till we start seeing actual field complaints about
 4TB being paltry ;-)

Agreed. 4TB should be enough.
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese: http://www.sraoss.co.jp


-- 
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] 64-bit API for large object

2012-08-28 Thread Robert Haas
On Tue, Aug 28, 2012 at 10:51 PM, Tatsuo Ishii is...@postgresql.org wrote:
 pg_largeobject.pageno is a signed int, so I don't think we can let it go
 past 2^31-1, so half that.

 We could buy back the other bit if we redefined the column as oid
 instead of int4 (to make it unsigned), but I think that would create
 fairly considerable risk of confusion between the loid and pageno
 columns (loid already being oid).  I'd just as soon not go there,
 at least not till we start seeing actual field complaints about
 4TB being paltry ;-)

 Agreed. 4TB should be enough.

...for anybody!

-- 
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] 64-bit API for large object

2012-08-26 Thread Tatsuo Ishii
 Hi,
 
 I found this in the TODO list:
 
   Add API for 64-bit large object access 
 
 If this is a still valid TODO item and nobody is working on this, I
 would like to work in this.

Here are the list of functions think we need to change.

1) Frontend lo_* libpq functions(fe-lobj.c)

lo_initialize() need to get backend 64-bit large object handling
function's oid, namely lo_lseek64, lo_tell64, lo_truncate64, loread64,
lowrite64(explained later). If they are not available, use older
32-bit backend functions.

BTW, currently lo_initialize() throws an error if one of oids are not
avilable. I doubt we do the same way for 64-bit functions since this
will make 9.3 libpq unable to access large objects stored in pre-9.2
PostgreSQL servers.

2) Bakend lo_* functions (be-fsstubs.c)

Add lo_lseek64, lo_tell64, lo_truncate64, loread64 and lowrite64 so
that they can handle 64-bit seek position and data lenghth.

3) Backend inv_api.c functions

No need to add new functions. Just extend them to handle 64-bit data.

BTW , what will happen if older 32-bit libpq accesses large objects
over 2GB?

lo_read and lo_write: they can read or write lobjs using 32-bit API as
long as requested read/write data length is smaller than 2GB. So I
think we can safely allow them to access over 2GB lobjs.

lo_lseek: again as long as requested offset is smaller than 2GB, there
would be no problem.

lo_tell:if current seek position is beyond 2GB, returns an error.

Comments, suggestions?
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese: http://www.sraoss.co.jp


-- 
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] 64-bit API for large object

2012-08-26 Thread Tom Lane
Tatsuo Ishii is...@postgresql.org writes:
 Here are the list of functions think we need to change.

 1) Frontend lo_* libpq functions(fe-lobj.c)

 lo_initialize() need to get backend 64-bit large object handling
 function's oid, namely lo_lseek64, lo_tell64, lo_truncate64, loread64,
 lowrite64(explained later). If they are not available, use older
 32-bit backend functions.

I don't particularly see a need for loread64 or lowrite64.  Who's going
to be reading or writing more than 2GB at once?  If someone tries,
they'd be well advised to reconsider their code design anyway.

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] 64-bit API for large object

2012-08-26 Thread Tatsuo Ishii
 1) Frontend lo_* libpq functions(fe-lobj.c)
 
 lo_initialize() need to get backend 64-bit large object handling
 function's oid, namely lo_lseek64, lo_tell64, lo_truncate64, loread64,
 lowrite64(explained later). If they are not available, use older
 32-bit backend functions.
 
 I don't particularly see a need for loread64 or lowrite64.  Who's going
 to be reading or writing more than 2GB at once?  If someone tries,
 they'd be well advised to reconsider their code design anyway.

Ok, loread64 and lowrite64 will not be added.
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese: http://www.sraoss.co.jp


-- 
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] 64-bit API for large object

2012-08-22 Thread Peter Eisentraut
On Wed, 2012-08-22 at 01:14 -0400, Tom Lane wrote:
 Peter Eisentraut pete...@gmx.net writes:
  On Wed, 2012-08-22 at 07:27 +0900, Tatsuo Ishii wrote:
  I found this in the TODO list:
  Add API for 64-bit large object access 
  If this is a still valid TODO item and nobody is working on this, I
  would like to work in this.
 
  Large objects are limited to 2 GB in size, so a 64-bit API doesn't sound
  very useful to me at the moment.
 
 Not entirely.  pg_largeobject.pageno is int32, but that's still 2G pages
 not bytes, so there's three or so orders of magnitude that could be
 gotten by expanding the client-side API before we'd have to change the
 server's on-disk representation.

Well then a 64-bit API would be very useful.  Go for it.  :-)




-- 
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] 64-bit API for large object

2012-08-22 Thread Tatsuo Ishii
 On Wed, 2012-08-22 at 01:14 -0400, Tom Lane wrote:
 Peter Eisentraut pete...@gmx.net writes:
  On Wed, 2012-08-22 at 07:27 +0900, Tatsuo Ishii wrote:
  I found this in the TODO list:
  Add API for 64-bit large object access 
  If this is a still valid TODO item and nobody is working on this, I
  would like to work in this.
 
  Large objects are limited to 2 GB in size, so a 64-bit API doesn't sound
  very useful to me at the moment.
 
 Not entirely.  pg_largeobject.pageno is int32, but that's still 2G pages
 not bytes, so there's three or so orders of magnitude that could be
 gotten by expanding the client-side API before we'd have to change the
 server's on-disk representation.
 
 Well then a 64-bit API would be very useful.  Go for it.  :-)

Ok, I will do it.
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese: http://www.sraoss.co.jp


-- 
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] 64-bit API for large object

2012-08-21 Thread Peter Eisentraut
On Wed, 2012-08-22 at 07:27 +0900, Tatsuo Ishii wrote:
 I found this in the TODO list:
 
   Add API for 64-bit large object access 
 
 If this is a still valid TODO item and nobody is working on this, I
 would like to work in this.

Large objects are limited to 2 GB in size, so a 64-bit API doesn't sound
very useful to me at the moment.



-- 
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] 64-bit API for large object

2012-08-21 Thread Tom Lane
Peter Eisentraut pete...@gmx.net writes:
 On Wed, 2012-08-22 at 07:27 +0900, Tatsuo Ishii wrote:
 I found this in the TODO list:
 Add API for 64-bit large object access 
 If this is a still valid TODO item and nobody is working on this, I
 would like to work in this.

 Large objects are limited to 2 GB in size, so a 64-bit API doesn't sound
 very useful to me at the moment.

Not entirely.  pg_largeobject.pageno is int32, but that's still 2G pages
not bytes, so there's three or so orders of magnitude that could be
gotten by expanding the client-side API before we'd have to change the
server's on-disk representation.

There might well be some local variables in the server's largeobject
code that would need to be widened, but that's the easiest part of the
job.

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] 64-bit API for large object

2012-08-21 Thread Tatsuo Ishii
 Large objects are limited to 2 GB in size, so a 64-bit API doesn't sound
 very useful to me at the moment.
 
 Not entirely.  pg_largeobject.pageno is int32, but that's still 2G pages
 not bytes, so there's three or so orders of magnitude that could be
 gotten by expanding the client-side API before we'd have to change the
 server's on-disk representation.

Right. You have already explained that in this:
http://archives.postgresql.org/pgsql-hackers/2010-09/msg01888.php

 There might well be some local variables in the server's largeobject
 code that would need to be widened, but that's the easiest part of the
 job.
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese: http://www.sraoss.co.jp


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