Thread: Proposal: new large object API

Proposal: new large object API

From
Tatsuo Ishii
Date:
I would like to propose new large object client side API for 8.4.

Currently we have:
  Oid lo_import(PGconn *conn, const char *filename);

But we do not have an API which imports a large object specifying the
object id. This is inconvenient and inconsistent since we already have
lo_create() and lo_open() which allow to specify the large object id.

So I propose to add new API:
  int lo_import_with_oid(PGconn *conn, Oid lobjId, const char *filename);

Another idea is changing the signature of lo_import:
  Oid lo_import(PGconn *conn, Oid lobjId, const char *filename);

which will be cleaner but break the backward compatibility.

Comments are welcome.
--
Tatsuo Ishii
SRA OSS, Inc. Japan


Re: Proposal: new large object API

From
Tatsuo Ishii
Date:
I have posted proposed patches to pgsql-patches.
--
Tatsuo Ishii
SRA OSS, Inc. Japan

> I would like to propose new large object client side API for 8.4.
> 
> Currently we have:
> 
>       Oid lo_import(PGconn *conn, const char *filename);
> 
> But we do not have an API which imports a large object specifying the
> object id. This is inconvenient and inconsistent since we already have
> lo_create() and lo_open() which allow to specify the large object id.
> 
> So I propose to add new API:
> 
>       int lo_import_with_oid(PGconn *conn, Oid lobjId, const char *filename);
> 
> Another idea is changing the signature of lo_import:
> 
>       Oid lo_import(PGconn *conn, Oid lobjId, const char *filename);
> 
> which will be cleaner but break the backward compatibility.
> 
> Comments are welcome.
> --
> Tatsuo Ishii
> 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: Proposal: new large object API

From
Tatsuo Ishii
Date:
lo_import_with_oid added.

Note that actually committed function signature is:

Oid lo_import_with_oid(PGconn *conn, const char *filename, Oid lobjId);
--
Tatsuo Ishii
SRA OSS, Inc. Japan

> I have posted proposed patches to pgsql-patches.
> --
> Tatsuo Ishii
> SRA OSS, Inc. Japan
> 
> > I would like to propose new large object client side API for 8.4.
> > 
> > Currently we have:
> > 
> >       Oid lo_import(PGconn *conn, const char *filename);
> > 
> > But we do not have an API which imports a large object specifying the
> > object id. This is inconvenient and inconsistent since we already have
> > lo_create() and lo_open() which allow to specify the large object id.
> > 
> > So I propose to add new API:
> > 
> >       int lo_import_with_oid(PGconn *conn, Oid lobjId, const char *filename);
> > 
> > Another idea is changing the signature of lo_import:
> > 
> >       Oid lo_import(PGconn *conn, Oid lobjId, const char *filename);
> > 
> > which will be cleaner but break the backward compatibility.
> > 
> > Comments are welcome.
> > --
> > Tatsuo Ishii
> > 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
> 
> -- 
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers


Re: Proposal: new large object API

From
Tatsuo Ishii
Date:
> lo_import_with_oid added.
> 
> Note that actually committed function signature is:
> 
> Oid lo_import_with_oid(PGconn *conn, const char *filename, Oid lobjId);
> --
> Tatsuo Ishii
> SRA OSS, Inc. Japan

It seems I forgot about the serer side lo_import. Included are the
patches to add new form of lo_import which accepts the large object id
as the second argument.

Comments, objection?
--
Tatsuo Ishii
SRA OSS, Inc. Japan
Index: src/include/catalog/pg_proc.h
===================================================================
RCS file: /cvsroot/pgsql/src/include/catalog/pg_proc.h,v
retrieving revision 1.482
diff -c -r1.482 pg_proc.h
*** src/include/catalog/pg_proc.h    1 Jan 2008 19:45:57 -0000    1.482
--- src/include/catalog/pg_proc.h    20 Mar 2008 05:58:38 -0000
***************
*** 1027,1032 ****
--- 1027,1034 ----  DATA(insert OID = 764 (  lo_import           PGNSP PGUID 12 1 0 f f t f v 1 26 "25" _null_ _null_
_null_   lo_import - _null_ _null_ )); DESCR("large object import");
 
+ DATA(insert OID = 767 (  lo_import           PGNSP PGUID 12 1 0 f f t f v 2 26 "25 26" _null_ _null_ _null_
lo_import- _null_ _null_ ));
 
+ DESCR("large object import"); DATA(insert OID = 765 (  lo_export           PGNSP PGUID 12 1 0 f f t f v 2 23 "26 25"
_null__null_ _null_ lo_export - _null_ _null_ )); DESCR("large object export"); 
 
Index: src/backend/libpq/be-fsstubs.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/libpq/be-fsstubs.c,v
retrieving revision 1.87
diff -c -r1.87 be-fsstubs.c
*** src/backend/libpq/be-fsstubs.c    1 Jan 2008 19:45:49 -0000    1.87
--- src/backend/libpq/be-fsstubs.c    20 Mar 2008 05:58:38 -0000
***************
*** 327,333 ****     char        fnamebuf[MAXPGPATH];     LargeObjectDesc *lobj;     Oid            lobjOid;
!  #ifndef ALLOW_DANGEROUS_LO_FUNCTIONS     if (!superuser())         ereport(ERROR,
--- 327,333 ----     char        fnamebuf[MAXPGPATH];     LargeObjectDesc *lobj;     Oid            lobjOid;
!      #ifndef ALLOW_DANGEROUS_LO_FUNCTIONS     if (!superuser())         ereport(ERROR,
***************
*** 336,341 ****
--- 336,346 ----                  errhint("Anyone can use the client-side lo_import() provided by libpq."))); #endif 
+     if (PG_NARGS() > 1)
+         lobjOid = PG_GETARG_OID(1);
+     else
+         lobjOid = InvalidOid;
+      CreateFSContext();      /*
***************
*** 356,362 ****     /*      * create an inversion object      */
!     lobjOid = inv_create(InvalidOid);      /*      * read in from the filesystem and write to the inversion object
--- 361,367 ----     /*      * create an inversion object      */
!     lobjOid = inv_create(lobjOid);      /*      * read in from the filesystem and write to the inversion object
Index: doc/src/sgml/lobj.sgml
===================================================================
RCS file: /cvsroot/pgsql/doc/src/sgml/lobj.sgml,v
retrieving revision 1.47
diff -c -r1.47 lobj.sgml
*** doc/src/sgml/lobj.sgml    19 Mar 2008 00:39:33 -0000    1.47
--- doc/src/sgml/lobj.sgml    20 Mar 2008 05:58:38 -0000
***************
*** 438,443 ****
--- 438,450 ----     The client-side functions can be used by any     <productname>PostgreSQL</productname> user.
</para>
+ 
+   <para>
+     As of 8.4, a different form of the server-side
+     <function>lo_import</function> added, which accepts the large
+     object id as the second argument. The usage of this form is same as the client
+     side function <function>lo_import_with_oid</function>.
+   </para> </sect1>  <sect1 id="lo-examplesect">
Index: src/test/regress/expected/opr_sanity.out
===================================================================
RCS file: /cvsroot/pgsql/src/test/regress/expected/opr_sanity.out,v
retrieving revision 1.79
diff -c -r1.79 opr_sanity.out
*** src/test/regress/expected/opr_sanity.out    27 Nov 2007 12:21:05 -0000    1.79
--- src/test/regress/expected/opr_sanity.out    20 Mar 2008 05:58:39 -0000
***************
*** 86,94 ****      p1.proretset != p2.proretset OR      p1.provolatile != p2.provolatile OR      p1.pronargs !=
p2.pronargs);
!  oid | proname | oid | proname 
! -----+---------+-----+---------
! (0 rows)  -- Look for uses of different type OIDs in the argument/result type fields -- for different aliases of the
samebuilt-in function.
 
--- 86,95 ----      p1.proretset != p2.proretset OR      p1.provolatile != p2.provolatile OR      p1.pronargs !=
p2.pronargs);
!  oid |  proname  | oid |  proname  
! -----+-----------+-----+-----------
!  764 | lo_import | 767 | lo_import
! (1 row)  -- Look for uses of different type OIDs in the argument/result type fields -- for different aliases of the
samebuilt-in function. 

Re: Proposal: new large object API

From
Tom Lane
Date:
Tatsuo Ishii <ishii@postgresql.org> writes:
> It seems I forgot about the serer side lo_import. Included are the
> patches to add new form of lo_import which accepts the large object id
> as the second argument.

> Comments, objection?

Breaking the type_sanity test is not acceptable.  Put in a second C
function.
        regards, tom lane


Re: Proposal: new large object API

From
Tatsuo Ishii
Date:
> Tatsuo Ishii <ishii@postgresql.org> writes:
> > It seems I forgot about the serer side lo_import. Included are the
> > patches to add new form of lo_import which accepts the large object id
> > as the second argument.
> 
> > Comments, objection?
> 
> Breaking the type_sanity test is not acceptable.  Put in a second C
> function.

Are you talking opr_sanity?

From what I read from opr_sanity.sql:

-- Look for uses of different type OIDs in the argument/result type fields
-- for different aliases of the same built-in function.
-- This indicates that the types are being presumed to be binary-equivalent,
-- or that the built-in function is prepared to deal with different types.
-- That's not wrong, necessarily, but we make lists of all the types being
-- so treated.  Note that the expected output of this part of the test will
-- need to be modified whenever new pairs of types are made binary-equivalent,
-- or when new polymorphic built-in functions are added!
-- Note: ignore aggregate functions here, since they all point to the same
-- dummy built-in function.

What is evil with a polymorphic function?
--
Tatsuo Ishii
SRA OSS, Inc. Japan


Re: Proposal: new large object API

From
Tom Lane
Date:
Tatsuo Ishii <ishii@postgresql.org> writes:
>> Breaking the type_sanity test is not acceptable.  Put in a second C
>> function.

> Are you talking opr_sanity?

Sorry, yes, too little caffeine ...

> What is evil with a polymorphic function?

(1) It's creating a false match --- your proposed entry in the opr_sanity
results has nothing at all to do with what the test is looking for.

(2) Refactoring to have two separate C functions will make the code
clearer, and not noticeably longer.
        regards, tom lane


Re: Proposal: new large object API

From
Tatsuo Ishii
Date:
> > What is evil with a polymorphic function?
> 
> (1) It's creating a false match --- your proposed entry in the opr_sanity
> results has nothing at all to do with what the test is looking for.
> 
> (2) Refactoring to have two separate C functions will make the code
> clearer, and not noticeably longer.

Ok, here is the revised patch.
--
Tatsuo Ishii
SRA OSS, Inc. Japan
Index: src/backend/libpq/be-fsstubs.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/libpq/be-fsstubs.c,v
retrieving revision 1.87
diff -c -r1.87 be-fsstubs.c
*** src/backend/libpq/be-fsstubs.c    1 Jan 2008 19:45:49 -0000    1.87
--- src/backend/libpq/be-fsstubs.c    21 Mar 2008 14:35:02 -0000
***************
*** 79,84 ****
--- 79,85 ----  static int    newLOfd(LargeObjectDesc *lobjCookie); static void deleteLOfd(int fd);
+ static Oid lo_import_internal(text *filename, Oid lobjOid);
/*****************************************************************************
***************
*** 320,333 **** lo_import(PG_FUNCTION_ARGS) {     text       *filename = PG_GETARG_TEXT_P(0);     File        fd;
int           nbytes,                 tmp;     char        buf[BUFSIZE];     char        fnamebuf[MAXPGPATH];
LargeObjectDesc*lobj;
 
!     Oid            lobjOid;
!  #ifndef ALLOW_DANGEROUS_LO_FUNCTIONS     if (!superuser())         ereport(ERROR,
--- 321,354 ---- lo_import(PG_FUNCTION_ARGS) {     text       *filename = PG_GETARG_TEXT_P(0);
+ 
+     PG_RETURN_OID(lo_import_internal(filename, InvalidOid));
+ }
+ 
+ /*
+  * lo_import_with_oid -
+  *      imports a file as an (inversion) large object specifying oid.
+  */
+ Datum
+ lo_import_with_oid(PG_FUNCTION_ARGS)
+ {
+     text       *filename = PG_GETARG_TEXT_P(0);
+     Oid           oid = PG_GETARG_OID(1);
+ 
+     PG_RETURN_OID(lo_import_internal(filename, oid));
+ }
+ 
+ static Oid
+ lo_import_internal(text *filename, Oid lobjOid)
+ {     File        fd;     int            nbytes,                 tmp;     char        buf[BUFSIZE];     char
fnamebuf[MAXPGPATH];    LargeObjectDesc *lobj;
 
!     Oid    oid;
!      #ifndef ALLOW_DANGEROUS_LO_FUNCTIONS     if (!superuser())         ereport(ERROR,
***************
*** 356,367 ****     /*      * create an inversion object      */
!     lobjOid = inv_create(InvalidOid);      /*      * read in from the filesystem and write to the inversion object
 */
 
!     lobj = inv_open(lobjOid, INV_WRITE, fscxt);      while ((nbytes = FileRead(fd, buf, BUFSIZE)) > 0)     {
--- 377,388 ----     /*      * create an inversion object      */
!     oid = inv_create(lobjOid);      /*      * read in from the filesystem and write to the inversion object      */
!     lobj = inv_open(oid, INV_WRITE, fscxt);      while ((nbytes = FileRead(fd, buf, BUFSIZE)) > 0)     {
***************
*** 378,384 ****     inv_close(lobj);     FileClose(fd); 
!     PG_RETURN_OID(lobjOid); }  /*
--- 399,405 ----     inv_close(lobj);     FileClose(fd); 
!     return oid; }  /*
Index: src/include/catalog/pg_proc.h
===================================================================
RCS file: /cvsroot/pgsql/src/include/catalog/pg_proc.h,v
retrieving revision 1.482
diff -c -r1.482 pg_proc.h
*** src/include/catalog/pg_proc.h    1 Jan 2008 19:45:57 -0000    1.482
--- src/include/catalog/pg_proc.h    21 Mar 2008 14:35:07 -0000
***************
*** 1027,1032 ****
--- 1027,1034 ----  DATA(insert OID = 764 (  lo_import           PGNSP PGUID 12 1 0 f f t f v 1 26 "25" _null_ _null_
_null_   lo_import - _null_ _null_ )); DESCR("large object import");
 
+ DATA(insert OID = 767 (  lo_import           PGNSP PGUID 12 1 0 f f t f v 2 26 "25 26" _null_ _null_ _null_
lo_import_with_oid- _null_ _null_ ));
 
+ DESCR("large object import"); DATA(insert OID = 765 (  lo_export           PGNSP PGUID 12 1 0 f f t f v 2 23 "26 25"
_null__null_ _null_ lo_export - _null_ _null_ )); DESCR("large object export"); 
 
Index: src/include/catalog/catversion.h
===================================================================
RCS file: /cvsroot/pgsql/src/include/catalog/catversion.h,v
retrieving revision 1.442
diff -c -r1.442 catversion.h
*** src/include/catalog/catversion.h    10 Mar 2008 13:53:35 -0000    1.442
--- src/include/catalog/catversion.h    21 Mar 2008 14:35:07 -0000
***************
*** 53,58 ****  */  /*                            yyyymmddN */
! #define CATALOG_VERSION_NO    200803101  #endif
--- 53,58 ----  */  /*                            yyyymmddN */
! #define CATALOG_VERSION_NO    200803211  #endif
Index: src/include/libpq/be-fsstubs.h
===================================================================
RCS file: /cvsroot/pgsql/src/include/libpq/be-fsstubs.h,v
retrieving revision 1.30
diff -c -r1.30 be-fsstubs.h
*** src/include/libpq/be-fsstubs.h    1 Jan 2008 19:45:58 -0000    1.30
--- src/include/libpq/be-fsstubs.h    21 Mar 2008 14:35:07 -0000
***************
*** 20,25 ****
--- 20,26 ----  * LO functions available via pg_proc entries  */ extern Datum lo_import(PG_FUNCTION_ARGS);
+ extern Datum lo_import_with_oid(PG_FUNCTION_ARGS); extern Datum lo_export(PG_FUNCTION_ARGS);  extern Datum
lo_creat(PG_FUNCTION_ARGS);
Index: doc/src/sgml/lobj.sgml
===================================================================
RCS file: /cvsroot/pgsql/doc/src/sgml/lobj.sgml,v
retrieving revision 1.47
diff -c -r1.47 lobj.sgml
*** doc/src/sgml/lobj.sgml    19 Mar 2008 00:39:33 -0000    1.47
--- doc/src/sgml/lobj.sgml    21 Mar 2008 14:35:07 -0000
***************
*** 438,443 ****
--- 438,450 ----     The client-side functions can be used by any     <productname>PostgreSQL</productname> user.
</para>
+ 
+   <para>
+     As of 8.4, a different form of the server-side
+     <function>lo_import</function> added, which accepts the large
+     object id as the second argument. The usage of this form is same as the client
+     side function <function>lo_import_with_oid</function>.
+   </para> </sect1>  <sect1 id="lo-examplesect">

Re: Proposal: new large object API

From
Tom Lane
Date:
Tatsuo Ishii <ishii@postgresql.org> writes:
> Ok, here is the revised patch.

This looks sane to me, but I'd suggest leaving out the mention of 8.4
in the docs.  Actually, I'm not sure you need a paragraph at all ---
just adding an example would be enough, I think.
 SELECT lo_unlink(173454);  -- deletes large object with OID 173454  INSERT INTO image (name, raster)     VALUES
('beautifulimage', lo_import('/etc/motd'));
 
+ 
+ INSERT INTO image (name, raster)  -- same as above, but specify OID to use
+     VALUES ('beautiful image', lo_import('/etc/motd', 68583));  SELECT lo_export(image.raster, '/tmp/motd') FROM
image    WHERE name = 'beautiful image'; </programlisting>
 

        regards, tom lane


Re: Proposal: new large object API

From
Tatsuo Ishii
Date:
> Tatsuo Ishii <ishii@postgresql.org> writes:
> > Ok, here is the revised patch.
> 
> This looks sane to me, but I'd suggest leaving out the mention of 8.4
> in the docs.  Actually, I'm not sure you need a paragraph at all ---
> just adding an example would be enough, I think.
> 
>   SELECT lo_unlink(173454);  -- deletes large object with OID 173454
>   
>   INSERT INTO image (name, raster)
>       VALUES ('beautiful image', lo_import('/etc/motd'));
> + 
> + INSERT INTO image (name, raster)  -- same as above, but specify OID to use
> +     VALUES ('beautiful image', lo_import('/etc/motd', 68583));
>   
>   SELECT lo_export(image.raster, '/tmp/motd') FROM image
>       WHERE name = 'beautiful image';
>   </programlisting>

Thanks for the comment. I have committed with your suggested doc
changing.
--
Tatsuo Ishii
SRA OSS, Inc. Japan