Re: Re: [PATCHES] Patch to support transactions with BLOBs for current CVS

Sorry, here is a clean version of the patch.

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?

---------------------------------------------------------------------------
Hello,

here is the patch attached which do check in each BLOB operation, if we are
in transaction, and raise an error otherwise. This will prevent such
mistakes.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Bruce Momjian <pgman@candle.pha.pa.us> 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

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: dyp@perchine.com
HomePage: http://www.perchine.com/dyp/
FidoNet: 2:5000/120.5
----------------------------------

Denis Perchine <dyp@perchine.com> 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

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: dyp@perchine.com
HomePage: http://www.perchine.com/dyp/
FidoNet: 2:5000/120.5
----------------------------------

Denis Perchine <dyp@perchine.com> 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