Re: [HACKERS] Re: [PATCHES] Patch to support transactions with BLOBs for current CVS
Denis Perchine <[EMAIL PROTECTED]> writes: >> lo_import and lo_export always execute in a transaction, just like any >> other backend operation. There is no need to force them to be done in >> a transaction block. If you're not clear about this, perhaps you need >> to review the difference between transactions and transaction blocks. > Hmmm... Where can I read about it? At least which source/header? Try src/backend/access/transam/xact.c. The point is that you need a transaction block only if you need to combine multiple SQL commands into a single transaction. A standalone command or function call is still done inside a transaction. regards, tom lane
Re: [HACKERS] Re: [PATCHES] Patch to support transactions with BLOBs for current CVS
> > First of all it will not break lo_creat, lo_unlink for sure. > > lo_creat depends on inv_create followed by inv_close; your patch > proposed to disable both of those outside transaction blocks. > lo_unlink depends on inv_drop, which ditto. Your patch therefore > restricts lo_creat and lo_unlink to be done inside transaction blocks, > which is a new and completely unnecessary restriction that will > doubtless break many existing applications. OK.As I already said we can remove checks from inv_create/inv_drop. They are not needed there. > > But I do not see any reasons why we not put lo_import, and lo_export in > > TX. At least this will prevent other backends from reading partially > > imported BLOBs... > > lo_import and lo_export always execute in a transaction, just like any > other backend operation. There is no need to force them to be done in > a transaction block. If you're not clear about this, perhaps you need > to review the difference between transactions and transaction blocks. Hmmm... Where can I read about it? At least which source/header? -- Sincerely Yours, Denis Perchine -- E-Mail: [EMAIL PROTECTED] HomePage: http://www.perchine.com/dyp/ FidoNet: 2:5000/120.5 --
Re: [HACKERS] Re: [PATCHES] Patch to support transactions with BLOBs for current CVS
Denis Perchine <[EMAIL PROTECTED]> writes: > First of all it will not break lo_creat, lo_unlink for sure. lo_creat depends on inv_create followed by inv_close; your patch proposed to disable both of those outside transaction blocks. lo_unlink depends on inv_drop, which ditto. Your patch therefore restricts lo_creat and lo_unlink to be done inside transaction blocks, which is a new and completely unnecessary restriction that will doubtless break many existing applications. > But I do not see any reasons why we not put lo_import, and lo_export in TX. > At least this will prevent other backends from reading partially imported > BLOBs... lo_import and lo_export always execute in a transaction, just like any other backend operation. There is no need to force them to be done in a transaction block. If you're not clear about this, perhaps you need to review the difference between transactions and transaction blocks. regards, tom lane
Re: [HACKERS] Re: [PATCHES] Patch to support transactions with BLOBs for current CVS
> > On Saturday 20 January 2001 10:05, you wrote: > > > I just wanted to confirm that this patch was applied. > > > > Yes, it is. But the following patch is not applied. But I sure that it is > > neccessary, otherwise we will get really strange errors (see discussion > > in the thread). > > > > http://www.postgresql.org/mhonarc/pgsql-patches/2000-11/msg00013.html > > Can people comment on the following patch that Dennis says is needed? > It prevents BLOB operations outside transactions. Dennis, can you > explain why BLOB operations have to be done inside transactions? If you forget to put BLOB in TX, you will get errors like 'lo_read: invalid large obj descriptor (0)'. The problem is that in be-fsstubs.c in lo_commit all descriptors are removed. And if you did not opened TX, it will be commited after each function call. And for the next call there will be no such fd in the tables. Tom later wrote: > I object strongly. As given, this would break lo_creat, lo_unlink, > lo_import, and lo_export --- none of which need to be in a transaction > block --- not to mention possibly causing gratuitous failures during > lo_commit. First of all it will not break lo_creat, lo_unlink for sure. But we can remove checks from inv_create, and inv_drop. They are not important. At least there will be no strange errors issued. I do not know why do you think there will be any problems with lo_commit. I can not find such reasons. I can not say anything about lo_import/lo_export, as I do not know why they are not inside TX themselves. I am not sure, maybe Tom is right, and we should fix be-fsstubs.c instead. But I do not see any reasons why we not put lo_import, and lo_export in TX. At least this will prevent other backends from reading partially imported BLOBs... -- Sincerely Yours, Denis Perchine -- E-Mail: [EMAIL PROTECTED] HomePage: http://www.perchine.com/dyp/ FidoNet: 2:5000/120.5 --
Re: [HACKERS] Re: [PATCHES] Patch to support transactions with BLOBs for current CVS
Bruce Momjian <[EMAIL PROTECTED]> writes: > Can people comment on the following patch that Dennis says is needed? I object strongly. As given, this would break lo_creat, lo_unlink, lo_import, and lo_export --- none of which need to be in a transaction block --- not to mention possibly causing gratuitous failures during lo_commit. I'm not convinced that we need such a check at all; I don't see anything especially wrong with the existing behavior. But if we do want it, this is the wrong abstraction level. be-fsstubs.c is the place to do it, and only in the routines that take or return an open-LO descriptor. regards, tom lane
Re: [HACKERS] Re: [PATCHES] Patch to support transactions with BLOBs for current CVS
Hi, > OK, Denis, can you run the regression tests with your patch and see what > is going on? > > > Bruce Momjian writes: > > > Applied. Thanks. I know it is a pain to generate a new patch against > > > the release. > > > > Regression tests opr_sanity and sanity_check are now failing. This was due to change in template1. Here is regression.diff attached. And also there's test.patch attached which will fix this. -- Sincerely Yours, Denis Perchine -- E-Mail: [EMAIL PROTECTED] HomePage: http://www.perchine.com/dyp/ FidoNet: 2:5000/120.5 -- *** ./expected/opr_sanity.out Mon Aug 21 11:48:57 2000 --- ./results/opr_sanity.out Sun Oct 22 13:16:40 2000 *** *** 482,489 (p2.pronargs = 1 AND p1.aggbasetype = 0))); oid | aggname | oid | proname ---+-+-+- ! 16984 | max | 768 | int4larger ! 16998 | min | 769 | int4smaller (2 rows) -- Cross-check finalfn (if present) against its entry in pg_proc. --- 482,489 (p2.pronargs = 1 AND p1.aggbasetype = 0))); oid | aggname | oid | proname ---+-+-+- ! 16996 | max | 768 | int4larger ! 17010 | min | 769 | int4smaller (2 rows) -- Cross-check finalfn (if present) against its entry in pg_proc. == *** ./expected/sanity_check.out Thu Jul 6 06:02:37 2000 --- ./results/sanity_check.out Sun Oct 22 13:16:55 2000 *** *** 40,45 --- 40,46 pg_index| t pg_inherits | t pg_language | t + pg_largeobject | t pg_listener | t pg_opclass | t pg_operator | t *** *** 54,58 shighway| t tenk1 | t tenk2 | t ! (44 rows) --- 55,59 shighway| t tenk1 | t tenk2 | t ! (45 rows) == Index: opr_sanity.out === RCS file: /home/projects/pgsql/cvsroot/pgsql/src/test/regress/expected/opr_sanity.out,v retrieving revision 1.20 diff -u -r1.20 opr_sanity.out --- opr_sanity.out 2000/08/21 04:48:57 1.20 +++ opr_sanity.out 2000/10/22 06:19:58 @@ -482,8 +482,8 @@ (p2.pronargs = 1 AND p1.aggbasetype = 0))); oid | aggname | oid | proname ---+-+-+- - 16984 | max | 768 | int4larger - 16998 | min | 769 | int4smaller + 16996 | max | 768 | int4larger + 17010 | min | 769 | int4smaller (2 rows) -- Cross-check finalfn (if present) against its entry in pg_proc. Index: sanity_check.out === RCS file: /home/projects/pgsql/cvsroot/pgsql/src/test/regress/expected/sanity_check.out,v retrieving revision 1.12 diff -u -r1.12 sanity_check.out --- sanity_check.out 2000/07/05 23:02:37 1.12 +++ sanity_check.out 2000/10/22 06:19:58 @@ -40,6 +40,7 @@ pg_index| t pg_inherits | t pg_language | t + pg_largeobject | t pg_listener | t pg_opclass | t pg_operator | t @@ -54,5 +55,5 @@ shighway| t tenk1 | t tenk2 | t -(44 rows) +(45 rows)
