Обсуждение: Proposal: new large object API
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
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
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
> 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.
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
> 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
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
> > 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">
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
> 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