Re: [HACKERS] Proposal: new large object API
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 - 1.87
--- src/backend/libpq/be-fsstubs.c 21 Mar 2008 14:35:02 -
***
*** 79,84
--- 79,85
static intnewLOfd(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);
Filefd;
int nbytes,
tmp;
charbuf[BUFSIZE];
charfnamebuf[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);
+ Oidoid = PG_GETARG_OID(1);
+
+ PG_RETURN_OID(lo_import_internal(filename, oid));
+ }
+
+ static Oid
+ lo_import_internal(text *filename, Oid lobjOid)
+ {
Filefd;
int nbytes,
tmp;
charbuf[BUFSIZE];
charfnamebuf[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 - 1.482
--- src/include/catalog/pg_proc.h 21 Mar 2008 14:35:07 -
***
*** 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.h10 Mar 2008 13:53:35 - 1.442
--- src/include/catalog/catversion.h21 Mar 2008 14:35:07 -
***
*** 53,58
*/
/*mmddN */
! #define CATALOG_VERSION_NO200803101
#endif
--- 53,58
*/
/*mmddN */
! #define CATALOG_VERSION_NO200803211
#endif
Index: src/include/libpq/be-fsstubs.h
Re: [HACKERS] Proposal: new large object API
Tatsuo Ishii [EMAIL PROTECTED] 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
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] Proposal: new large object API
Tatsuo Ishii [EMAIL PROTECTED] 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
--
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] Proposal: new large object API
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 - 1.482 --- src/include/catalog/pg_proc.h 20 Mar 2008 05:58:38 - *** *** 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 - 1.87 --- src/backend/libpq/be-fsstubs.c 20 Mar 2008 05:58:38 - *** *** 327,333 charfnamebuf[MAXPGPATH]; LargeObjectDesc *lobj; Oid lobjOid; ! #ifndef ALLOW_DANGEROUS_LO_FUNCTIONS if (!superuser()) ereport(ERROR, --- 327,333 charfnamebuf[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 - 1.47 --- doc/src/sgml/lobj.sgml 20 Mar 2008 05:58:38 - *** *** 438,443 --- 438,450 The client-side functions can be used by any productnamePostgreSQL/productname user. /para + + para + As of 8.4, a different form of the server-side + functionlo_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 functionlo_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.out27 Nov 2007 12:21:05 - 1.79 --- src/test/regress/expected/opr_sanity.out20 Mar 2008 05:58:39 - *** *** 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 same built-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 same built-in function. -- Sent via pgsql-hackers mailing list
Re: [HACKERS] Proposal: new large object API
Tatsuo Ishii [EMAIL PROTECTED] 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 -- 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] Proposal: new large object API
Tatsuo Ishii [EMAIL PROTECTED] 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 -- 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] Proposal: new large object API
Tatsuo Ishii [EMAIL PROTECTED] 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 -- 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] Proposal: new large object API
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 -- 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] Proposal: new large object API
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
