Thread: autocommit vs TRUNCATE et al
There are a number of statements, such as TRUNCATE TABLE, that refuse to
run in a transaction block because they perform actions that can't be
rolled back later.
These statements currently do not look at autocommit, which means that
if autocommit is off, their tests will succeed ... but then a
transaction block is started anyway, defeating the point of the test.
We could fix these statements to fail if autocommit is off, which means
that you could not use them at all except by setting autocommit on.
Ugh.
Or we could fix them to force an autocommit. Which would mean that
these "dangerous" statements would become even more dangerous, since
that's exactly the behavior a person using autocommit-off would not
expect. Also ugh.
Anyone see a way out of this catch-22? If not, which is the least
bad alternative?
regards, tom lane
Tom Lane wrote: > There are a number of statements, such as TRUNCATE TABLE, that refuse to > run in a transaction block because they perform actions that can't be > rolled back later. > > These statements currently do not look at autocommit, which means that > if autocommit is off, their tests will succeed ... but then a > transaction block is started anyway, defeating the point of the test. > > We could fix these statements to fail if autocommit is off, which means > that you could not use them at all except by setting autocommit on. > Ugh. > > Or we could fix them to force an autocommit. Which would mean that > these "dangerous" statements would become even more dangerous, since > that's exactly the behavior a person using autocommit-off would not > expect. Also ugh. > > Anyone see a way out of this catch-22? If not, which is the least > bad alternative? > I think the "least bad" is the first option -- disallow TRUNCATE unless autocommit is on. With the second option, people would be caught by surprise at precisely the worst possible moment. Better to make them take the extra step. Joe
Tom Lane wrote: > There are a number of statements, such as TRUNCATE TABLE, that refuse to > run in a transaction block because they perform actions that can't be > rolled back later. > > These statements currently do not look at autocommit, which means that > if autocommit is off, their tests will succeed ... but then a > transaction block is started anyway, defeating the point of the test. > > We could fix these statements to fail if autocommit is off, which means > that you could not use them at all except by setting autocommit on. > Ugh. > > Or we could fix them to force an autocommit. Which would mean that > these "dangerous" statements would become even more dangerous, since > that's exactly the behavior a person using autocommit-off would not > expect. Also ugh. > > Anyone see a way out of this catch-22? If not, which is the least > bad alternative? Ugh, I see what you mean. With the old code, you had to do a BEGIN to start a multi-statement transaction, while with autocommit off, you are always in one. I don't think forcing them to turn on autocommit makes any sense; it isn't obvious, and it doesn't buy anything. I think we should just do an automatic COMMIT if it is the first statement of a transaction, and if not, throw the same error we used to throw. We are performing autocommit for SET at the start of a transaction now anyway, so it isn't totally strange to do it for TRUNCATE, etc. too. In fact, you can just put the xact commit check in the same place SET is handled in postgres.c. It isn't great, but it is clean. ;-) You could also throw a NOTICE mentioning it was committed as a separate transaction. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001+ If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania19073
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> ... I think we
> should just do an automatic COMMIT if it is the first statement of a
> transaction, and if not, throw the same error we used to throw. We are
> performing autocommit for SET at the start of a transaction now anyway,
> so it isn't totally strange to do it for TRUNCATE, etc. too. In fact,
> you can just put the xact commit check in the same place SET is handled
> in postgres.c. It isn't great, but it is clean. ;-)
Well, "clean" isn't the adjective I would use ;-), but this might be the
most useful approach. The analogy to SET hadn't occurred to me.
regards, tom lane
Tom Lane wrote: > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > ... I think we > > should just do an automatic COMMIT if it is the first statement of a > > transaction, and if not, throw the same error we used to throw. We are > > performing autocommit for SET at the start of a transaction now anyway, > > so it isn't totally strange to do it for TRUNCATE, etc. too. In fact, > > you can just put the xact commit check in the same place SET is handled > > in postgres.c. It isn't great, but it is clean. ;-) > > Well, "clean" isn't the adjective I would use ;-), but this might be the Clean in coding terms, _only_. > most useful approach. The analogy to SET hadn't occurred to me. Yea, the SET behavior appeared pretty queer to me, but now that I have used it, I am getting used to it. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001+ If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania19073
Bruce Momjian wrote:
>>most useful approach. The analogy to SET hadn't occurred to me.
>
>
> Yea, the SET behavior appeared pretty queer to me, but now that I have
> used it, I am getting used to it.
>
So does that mean:
set autocommit to off; begin; insert into foo values('a'); insert into bar values('b'); truncate table foobar;
will automatically commit the two inserts?
Joe
Joe Conway wrote:
> Bruce Momjian wrote:
> >>most useful approach. The analogy to SET hadn't occurred to me.
> >
> >
> > Yea, the SET behavior appeared pretty queer to me, but now that I have
> > used it, I am getting used to it.
> >
>
> So does that mean:
>
> set autocommit to off;
> begin;
> insert into foo values('a');
> insert into bar values('b');
> truncate table foobar;
>
> will automatically commit the two inserts?
No, the entire transaction will aborted because TRUNCATE has to be at
the start of a multi-statement transaction. This will also fail:
> set autocommit to off;
> begin;
> truncate table foobar;
> insert into foo values('a');
> insert into bar values('b');
but this will work:
> set autocommit to off;
> truncate table foobar;
> insert into foo values('a');
> insert into bar values('b');
In the last case, the TRUNCATE will happen, and the INSERTs will be in
their own multi-statement transaction. A SET in place of TRUNCATE will
behave the same way.
-- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610)
359-1001+ If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square,
Pennsylvania19073
Bruce Momjian wrote: > Joe Conway wrote: > >>Bruce Momjian wrote: >> >>>>most useful approach. The analogy to SET hadn't occurred to me. >>> >>> >>>Yea, the SET behavior appeared pretty queer to me, but now that I have >>>used it, I am getting used to it. <snip examples> > > In the last case, the TRUNCATE will happen, and the INSERTs will be in > their own multi-statement transaction. A SET in place of TRUNCATE will > behave the same way. > Hmmm. It does look strange. We ought to make this prominent in the release notes and docs. Joe
On Fri, 18 Oct 2002, Tom Lane wrote: > Anyone see a way out of this catch-22? If not, which is the least > bad alternative? Ultimately, fix TRUNCATE to be transaction safe. This is non-trivial, I know :-). Regardless, the first option seems the less of the two evils. Gavin
Gavin Sherry wrote: > On Fri, 18 Oct 2002, Tom Lane wrote: > > >>Anyone see a way out of this catch-22? If not, which is the least >>bad alternative? > > > Ultimately, fix TRUNCATE to be transaction safe. This is non-trivial, > I know :-). > > Regardless, the first option seems the less of the two evils. Even though TRUNCATE was modeled after Oracle's TRUNCATE and Oracle's TRUNCATE commits the running tx, truncates the relation, and starts a new tx, regardless of whether or not TRUNCATE is the first statement of the tx? Mike Mascari mascarm@mascari.com
Mike Mascari wrote: > Gavin Sherry wrote: > > On Fri, 18 Oct 2002, Tom Lane wrote: > > > > > >>Anyone see a way out of this catch-22? If not, which is the least > >>bad alternative? > > > > > > Ultimately, fix TRUNCATE to be transaction safe. This is non-trivial, > > I know :-). > > > > Regardless, the first option seems the less of the two evils. > > Even though TRUNCATE was modeled after Oracle's TRUNCATE and > Oracle's TRUNCATE commits the running tx, truncates the > relation, and starts a new tx, regardless of whether or not > TRUNCATE is the first statement of the tx? That seems just too harsh to me. I think we should impose some structure to it, though we will have compatibility issues. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001+ If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania19073
On Fri, 18 Oct 2002, Mike Mascari wrote: > Gavin Sherry wrote: > > On Fri, 18 Oct 2002, Tom Lane wrote: > > > > > >>Anyone see a way out of this catch-22? If not, which is the least > >>bad alternative? > > > > > > Ultimately, fix TRUNCATE to be transaction safe. This is non-trivial, > > I know :-). > > > > Regardless, the first option seems the less of the two evils. > > Even though TRUNCATE was modeled after Oracle's TRUNCATE and > Oracle's TRUNCATE commits the running tx, truncates the > relation, and starts a new tx, regardless of whether or not > TRUNCATE is the first statement of the tx? Why should we be *only* as good as Oracle? :-) Gavin
Gavin Sherry <swm@linuxworld.com.au> writes:
> On Fri, 18 Oct 2002, Tom Lane wrote:
>> Anyone see a way out of this catch-22? If not, which is the least
>> bad alternative?
> Ultimately, fix TRUNCATE to be transaction safe. This is non-trivial,
> I know :-).
I was about to say that the entire *point* of TRUNCATE is to be
transaction-unsafe ;-)
But on the other hand ... now that we have relation versioning (like
CLUSTER) it seems like TRUNCATE could generate new versions of the
relation and its indexes, without touching the originals. This would
make it transaction-safe, at the cost of not releasing the original
version's disk space till you commit. Seems like a good tradeoff to me.
It's not happening for 7.3, in any case, so we need a stopgap answer.
There are other examples --- CREATE/DROP DATABASE, for example ---
where we'd probably need an answer anyway; I doubt we'll ever make
those completely transaction-safe.
regards, tom lane
On Friday 18 October 2002 11:25 pm, Tom Lane wrote: > Gavin Sherry <swm@linuxworld.com.au> writes: > > On Fri, 18 Oct 2002, Tom Lane wrote: > >> Anyone see a way out of this catch-22? If not, which is the least > >> bad alternative? > > Ultimately, fix TRUNCATE to be transaction safe. This is non-trivial, > > I know :-). > I was about to say that the entire *point* of TRUNCATE is to be > transaction-unsafe ;-) I actually was considering using a transaction-safe TRUNCATE in an application involving daily imports of 170MB of data into a set of linked tables. Since the import takes a finite amount of time, it would be nice to have the previous data available while the new is being imported. And TRUNCATE is significantly faster than DELETE over 170MB of data. -- Lamar Owen WGCR Internet Radio 1 Peter 4:11
Added to TODO: * Make a transaction-safe TRUNCATE --------------------------------------------------------------------------- Lamar Owen wrote: > On Friday 18 October 2002 11:25 pm, Tom Lane wrote: > > Gavin Sherry <swm@linuxworld.com.au> writes: > > > On Fri, 18 Oct 2002, Tom Lane wrote: > > >> Anyone see a way out of this catch-22? If not, which is the least > > >> bad alternative? > > > > Ultimately, fix TRUNCATE to be transaction safe. This is non-trivial, > > > I know :-). > > > I was about to say that the entire *point* of TRUNCATE is to be > > transaction-unsafe ;-) > > I actually was considering using a transaction-safe TRUNCATE in an application > involving daily imports of 170MB of data into a set of linked tables. Since > the import takes a finite amount of time, it would be nice to have the > previous data available while the new is being imported. And TRUNCATE is > significantly faster than DELETE over 170MB of data. > -- > Lamar Owen > WGCR Internet Radio > 1 Peter 4:11 > > ---------------------------(end of broadcast)--------------------------- > TIP 3: if posting/reading through Usenet, please send an appropriate > subscribe-nomail command to majordomo@postgresql.org so that your > message can get through to the mailing list cleanly > -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001+ If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania19073
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> Tom Lane wrote:
>> There are a number of statements, such as TRUNCATE TABLE, that refuse to
>> run in a transaction block because they perform actions that can't be
>> rolled back later.
>>
>> These statements currently do not look at autocommit, which means that
>> if autocommit is off, their tests will succeed ... but then a
>> transaction block is started anyway, defeating the point of the test.
> ... I think we
> should just do an automatic COMMIT if it is the first statement of a
> transaction, and if not, throw the same error we used to throw. We are
> performing autocommit for SET at the start of a transaction now anyway,
> so it isn't totally strange to do it for TRUNCATE, etc. too.
There is another aspect of this, which is: what if one of these
statements is called by a user-defined function, say a plpgsql function
that does a TRUNCATE and then other stuff? If the function is called
by a SELECT that's not inside a transaction block, then its
IsTransactionBlock() test will succeed --- but the possibility remains
that the later actions of the function could cause an elog and
transaction rollback. Which is what we wanted to prevent.
We can go with the auto-COMMIT idea for statements that are invoked at
the outer interactive level, but that doesn't work for stuff invoked
inside a function. I think we need to forbid these statements inside
functions, too. We already have that for VACUUM, because of its
internal commits causing problems for functions, but we'll need to
extend it to all of them.
Just FYI, the statements involved are
VACUUM
TRUNCATE TABLE
CREATE/DROP DATABASE
REINDEX (all forms)
ALTER USER changing password
DROP USER
ALTER and DROP USER just issue NOTICEs rather than failing, which seems
pretty bogus in itself. The reason they are worried is that there's
no mechanism for rolling back updates of the flat password file.
I think we could fix that by arranging not to write the flat password
file at all until we are ready to commit the current transaction;
will take a look at it.
REINDEX perhaps could be treated as transaction-safe in the forms that
build a new index file rather than truncating. Will look at that, too.
Another place that is calling IsTransactionBlock is parser/analyze.c
while processing DECLARE CURSOR. I think this is pretty bogus for
several reasons:
1. There's no good reason to make DECLARE CURSOR outside a transaction block be an ERROR; at most it should be a
NOTICE.
2. Parse analysis is the wrong place anyway; it should be tested at execution time, methinks.
3. If the cursor is being declared and used inside a function, then it could be used successfully without being inside
atransaction block at all.
Point 3 makes me think we should just get rid of the test entirely.
Comments?
regards, tom lane
Tom Lane wrote: > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > Tom Lane wrote: > >> There are a number of statements, such as TRUNCATE TABLE, that refuse to > >> run in a transaction block because they perform actions that can't be > >> rolled back later. > >> > >> These statements currently do not look at autocommit, which means that > >> if autocommit is off, their tests will succeed ... but then a > >> transaction block is started anyway, defeating the point of the test. > > > ... I think we > > should just do an automatic COMMIT if it is the first statement of a > > transaction, and if not, throw the same error we used to throw. We are > > performing autocommit for SET at the start of a transaction now anyway, > > so it isn't totally strange to do it for TRUNCATE, etc. too. > > There is another aspect of this, which is: what if one of these > statements is called by a user-defined function, say a plpgsql function > that does a TRUNCATE and then other stuff? If the function is called > by a SELECT that's not inside a transaction block, then its > IsTransactionBlock() test will succeed --- but the possibility remains > that the later actions of the function could cause an elog and > transaction rollback. Which is what we wanted to prevent. > > We can go with the auto-COMMIT idea for statements that are invoked at > the outer interactive level, but that doesn't work for stuff invoked > inside a function. I think we need to forbid these statements inside > functions, too. We already have that for VACUUM, because of its > internal commits causing problems for functions, but we'll need to > extend it to all of them. > > Just FYI, the statements involved are > VACUUM > TRUNCATE TABLE > CREATE/DROP DATABASE > REINDEX (all forms) > ALTER USER changing password > DROP USER > > ALTER and DROP USER just issue NOTICEs rather than failing, which seems > pretty bogus in itself. The reason they are worried is that there's > no mechanism for rolling back updates of the flat password file. > I think we could fix that by arranging not to write the flat password > file at all until we are ready to commit the current transaction; > will take a look at it. Yes, I thought we had those secure, but I see now we don't. We only have interlocking so the file is reread at the proper time. > REINDEX perhaps could be treated as transaction-safe in the forms that > build a new index file rather than truncating. Will look at that, too. > > Another place that is calling IsTransactionBlock is parser/analyze.c > while processing DECLARE CURSOR. I think this is pretty bogus for > several reasons: > 1. There's no good reason to make DECLARE CURSOR outside a transaction > block be an ERROR; at most it should be a NOTICE. > 2. Parse analysis is the wrong place anyway; it should be tested > at execution time, methinks. > 3. If the cursor is being declared and used inside a function, then > it could be used successfully without being inside a transaction > block at all. > > Point 3 makes me think we should just get rid of the test entirely. > Comments? I thought the transaction test was in DECLARE so people didn't create cursors outside of transactions and then wonder why they didn't work. If it is going to fail, an ERROR seems more appropriate than a NOTICE. I can see it happening inside a function, yes. Another question related to this is the handling of SET/SHOW/RESET in functions. People should realize it isn't really the first command in the transaction so will be part of the transaction. The big issue is that SET has a fallback when it is not first in a transaciton, namely to be part of the transaction, while TRUNCATE doesn't have that fallback because it can't be rolled back. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001+ If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania19073
>>> ... I think we
>>> should just do an automatic COMMIT if it is the first statement of a
>>> transaction, and if not, throw the same error we used to throw. We are
>>> performing autocommit for SET at the start of a transaction now anyway,
>>> so it isn't totally strange to do it for TRUNCATE, etc. too.
>>
>> We can go with the auto-COMMIT idea for statements that are invoked at
>> the outer interactive level,
What I just committed uses your idea of auto-committing TRUNCATE et al,
but now that I review the thread I think that everyone else thought that
that was a dangerous idea. How do you feel about simply throwing an error
in autocommit-off mode, instead? (At least it's a localized change now)
regards, tom lane
Tom Lane wrote: > >>> ... I think we > >>> should just do an automatic COMMIT if it is the first statement of a > >>> transaction, and if not, throw the same error we used to throw. We are > >>> performing autocommit for SET at the start of a transaction now anyway, > >>> so it isn't totally strange to do it for TRUNCATE, etc. too. > >> > >> We can go with the auto-COMMIT idea for statements that are invoked at > >> the outer interactive level, > > What I just committed uses your idea of auto-committing TRUNCATE et al, > but now that I review the thread I think that everyone else thought that > that was a dangerous idea. How do you feel about simply throwing an error > in autocommit-off mode, instead? (At least it's a localized change now) Yes, I saw more votes to not allow it, as you said, but turning off autocommit just seemed really strange to me, because then they have to turn it on again to continue. It just seemed strange to tell them to set something to execute a command. Maybe we can throw a WARNING when autocommit is on. Would that make everyone happy? -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001+ If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania19073
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> Maybe we can throw a WARNING when autocommit is on. Would that make
> everyone happy?
I doubt it, because by the time you read the WARNING it's too late:
the statement's already committed.
regards, tom lane
Tom Lane wrote: > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > Maybe we can throw a WARNING when autocommit is on. Would that make > > everyone happy? > > I doubt it, because by the time you read the WARNING it's too late: > the statement's already committed. I assume the same limitation would hold for VACUUM, right, that you have to turn on autocommit mode to use it? -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001+ If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania19073
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> Tom Lane wrote:
>> Bruce Momjian <pgman@candle.pha.pa.us> writes:
>>> Maybe we can throw a WARNING when autocommit is on. Would that make
>>> everyone happy?
>>
>> I doubt it, because by the time you read the WARNING it's too late:
>> the statement's already committed.
> I assume the same limitation would hold for VACUUM, right, that you have
> to turn on autocommit mode to use it?
Yeah, it would, unless we wanted to throw in some additional hack to
distinguish VACUUM from the "more dangerous" cases.
regards, tom lane
Tom Lane wrote: > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > Tom Lane wrote: > >> Bruce Momjian <pgman@candle.pha.pa.us> writes: > >>> Maybe we can throw a WARNING when autocommit is on. Would that make > >>> everyone happy? > >> > >> I doubt it, because by the time you read the WARNING it's too late: > >> the statement's already committed. > > > I assume the same limitation would hold for VACUUM, right, that you have > > to turn on autocommit mode to use it? > > Yeah, it would, unless we wanted to throw in some additional hack to > distinguish VACUUM from the "more dangerous" cases. From my perspective, I think it would be consistent to disallow all transaction-unsafe commands and tell people they have to turn autocommit on to execute them, so it would be: SET autocommit TO 'on';VACUUM;SET autocommit TO 'off'; That is a pain, but it is probably the safest, as you explained. One particularly nasty problem is issuing a VACUUM or TRUNCATE in cases where you don't know the autocommit mode. You could set autocommit to 'on', and issue the command, but how do you know if you need to turn autocommit back off again? I suppose you have to conditionally test the autocommit value and reset it after the command if needed. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001+ If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania19073
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> One particularly nasty problem is issuing a VACUUM or TRUNCATE in cases
> where you don't know the autocommit mode. You could set autocommit to
> 'on', and issue the command, but how do you know if you need to turn
> autocommit back off again?
Perhaps RESET AUTOCOMMIT is a good enough answer?
regards, tom lane
Tom Lane wrote: > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > One particularly nasty problem is issuing a VACUUM or TRUNCATE in cases > > where you don't know the autocommit mode. You could set autocommit to > > 'on', and issue the command, but how do you know if you need to turn > > autocommit back off again? > > Perhaps RESET AUTOCOMMIT is a good enough answer? I was unclear on that. RESET sets it back to the postgresql.conf value, right? Do we know that the session didn't change it earlier in the script? That's where it gets tricky. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001+ If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania19073
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> Tom Lane wrote:
>> Perhaps RESET AUTOCOMMIT is a good enough answer?
> I was unclear on that. RESET sets it back to the postgresql.conf value,
> right? Do we know that the session didn't change it earlier in the
> script? That's where it gets tricky.
You're postulating a scenario in which some snippet of code doesn't know
what the surrounding script/application likes for AUTOCOMMIT, but does
know enough about the context to know that it's not inside a transaction
block already. That combination seems moderately implausible to me.
Anyone have an example where it'd really be useful?
regards, tom lane
Tom Lane wrote: > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > Tom Lane wrote: > >> Perhaps RESET AUTOCOMMIT is a good enough answer? > > > I was unclear on that. RESET sets it back to the postgresql.conf value, > > right? Do we know that the session didn't change it earlier in the > > script? That's where it gets tricky. > > You're postulating a scenario in which some snippet of code doesn't know > what the surrounding script/application likes for AUTOCOMMIT, but does > know enough about the context to know that it's not inside a transaction > block already. That combination seems moderately implausible to me. > Anyone have an example where it'd really be useful? Well, in most cases, if you don't know, you do BEGIN ... COMMIT, but in the case of VACUUM/TRUNCATE, you can't do that, so you need some smarts. It is a contrived example. I am just throwing it out for illumination. I often throw out scenarios at the edges to see if it strikes anyone as a problem. When it doesn't, we can move ahead. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001+ If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania19073
Tom Lane wrote:> We can go with the auto-COMMIT idea for statements that are invoked at> the outer interactive level, butthat doesn't work for stuff invoked> inside a function. I think we need to forbid these statements inside> functions,too. We already have that for VACUUM, because of its> internal commits causing problems for functions, but we'llneed to> extend it to all of them.>> Just FYI, the statements involved are> VACUUM> TRUNCATE TABLE> CREATE/DROP DATABASE I just noticed that this afternoon's changes cause dblink's regression test to fail due to: CREATE OR REPLACE FUNCTION conditional_drop() [...] IF FOUND THEN DROP DATABASE regression_slave; END IF; [...] ' LANGUAGE 'plpgsql'; SELECT conditional_drop(); I'm wondering what is the best alternative? Should we simply do a "DROP DATABASE regression_slave;" and have the expected results include the 'database "regression_slave" does not exist'? In this case there would be an expected failure whenever the regression test was run more than once. Joe
Joe Conway <mail@joeconway.com> writes:
> I just noticed that this afternoon's changes cause dblink's regression
> test to fail due to:
> CREATE OR REPLACE FUNCTION conditional_drop()
> [...]
> IF FOUND THEN
> DROP DATABASE regression_slave;
> END IF;
> [...]
> ' LANGUAGE 'plpgsql';
> SELECT conditional_drop();
> I'm wondering what is the best alternative?
Well, the *best* alternative would be to make CREATE/DROP DATABASE
transaction-safe ;-). I was speculating to myself earlier today about
how we might do that. It seems like it's not that far out of reach:
we could make smgr's list of files-to-remove-at-xact-commit-or-abort
include whole database subdirectories. But I'm not sure how that would
interact with upcoming features like tablespaces, so I don't want to
go off and implement it right now.
In the meantime, to tell you the truth, the cleanest way to handle the
dblink regression test would be to make it circularly connect to
database "regression". I know this seems cheesy, but as long as the
software under test doesn't know that it's a connection-to-self, seems
like the test is perfectly good. And it's surely easier to manage that
way.
regards, tom lane
Tom Lane wrote:
> In the meantime, to tell you the truth, the cleanest way to handle the
> dblink regression test would be to make it circularly connect to
> database "regression". I know this seems cheesy, but as long as the
> software under test doesn't know that it's a connection-to-self, seems
> like the test is perfectly good. And it's surely easier to manage that
> way.
OK, easy enough. Patch attached.
I also added "SET autocommit TO ''on'';" to the beginning of each dblink_exec
input statement because the "SET autocommit TO 'on';" at the top of the script
won't help for the connected database.
Joe
Index: contrib/dblink/expected/dblink.out
===================================================================
RCS file: /opt/src/cvs/pgsql-server/contrib/dblink/expected/dblink.out,v
retrieving revision 1.5
diff -c -r1.5 dblink.out
*** contrib/dblink/expected/dblink.out 21 Oct 2002 01:42:13 -0000 1.5
--- contrib/dblink/expected/dblink.out 22 Oct 2002 05:19:40 -0000
***************
*** 1,31 ****
--
! -- First, create a slave database and define the functions and test data
-- therein.
--
- -- This initial hackery is to allow successive runs without failures.
- --
- -- Adjust this setting to control where the objects get created.
- SET search_path = public;
- CREATE OR REPLACE FUNCTION conditional_drop()
- RETURNS text AS '
- DECLARE
- dbname text;
- BEGIN
- SELECT INTO dbname datname FROM pg_database WHERE datname = ''regression_slave'';
- IF FOUND THEN
- DROP DATABASE regression_slave;
- END IF;
- RETURN ''OK'';
- END;
- ' LANGUAGE 'plpgsql';
- SELECT conditional_drop();
- conditional_drop
- ------------------
- OK
- (1 row)
-
- CREATE DATABASE regression_slave;
- \connect regression_slave
-- Turn off echoing so that expected file does not depend on
-- contents of dblink.sql.
\set ECHO none
--- 1,9 ----
+ -- Adjust this setting to control where the objects get created.
+ SET search_path = public;
--
! -- Define the functions and test data
-- therein.
--
-- Turn off echoing so that expected file does not depend on
-- contents of dblink.sql.
\set ECHO none
***************
*** 81,96 ****
DELETE FROM foo WHERE f1 = '0' AND f2 = 'a'
(1 row)
- --
- -- Connect back to the regression database and define the functions.
- -- Turn off echoing so that expected file does not depend on
- -- contents of dblink.sql.
- --
- \connect regression
- \set ECHO none
-- regular old dblink
SELECT *
! FROM dblink('dbname=regression_slave','SELECT * FROM foo') AS t(a int, b text, c text[])
WHERE t.a > 7;
a | b | c
---+---+------------
--- 59,67 ----
DELETE FROM foo WHERE f1 = '0' AND f2 = 'a'
(1 row)
-- regular old dblink
SELECT *
! FROM dblink('dbname=regression','SELECT * FROM foo') AS t(a int, b text, c text[])
WHERE t.a > 7;
a | b | c
---+---+------------
***************
*** 104,110 ****
WHERE t.a > 7;
ERROR: dblink: no connection available
-- create a persistent connection
! SELECT dblink_connect('dbname=regression_slave');
dblink_connect
----------------
OK
--- 75,81 ----
WHERE t.a > 7;
ERROR: dblink: no connection available
-- create a persistent connection
! SELECT dblink_connect('dbname=regression');
dblink_connect
----------------
OK
***************
*** 182,195 ****
ERROR: dblink: no connection available
-- put more data into our slave table, first using arbitrary connection syntax
-- but truncate the actual return value so we can use diff to check for success
! SELECT substr(dblink_exec('dbname=regression_slave','SET autocommit TO ''on'';INSERT INTO foo VALUES
(10,''k'',''{"a10","b10","c10"}'')'),1,6);
substr
--------
INSERT
(1 row)
-- create a persistent connection
! SELECT dblink_connect('dbname=regression_slave');
dblink_connect
----------------
OK
--- 153,166 ----
ERROR: dblink: no connection available
-- put more data into our slave table, first using arbitrary connection syntax
-- but truncate the actual return value so we can use diff to check for success
! SELECT substr(dblink_exec('dbname=regression','SET autocommit TO ''on'';INSERT INTO foo
VALUES(10,''k'',''{"a10","b10","c10"}'')'),1,6);
substr
--------
INSERT
(1 row)
-- create a persistent connection
! SELECT dblink_connect('dbname=regression');
dblink_connect
----------------
OK
***************
*** 197,203 ****
-- put more data into our slave table, using persistent connection syntax
-- but truncate the actual return value so we can use diff to check for success
! SELECT substr(dblink_exec('INSERT INTO foo VALUES (11,''l'',''{"a11","b11","c11"}'')'),1,6);
substr
--------
INSERT
--- 168,174 ----
-- put more data into our slave table, using persistent connection syntax
-- but truncate the actual return value so we can use diff to check for success
! SELECT substr(dblink_exec('SET autocommit TO ''on'';INSERT INTO foo VALUES(11,''l'',''{"a11","b11","c11"}'')'),1,6);
substr
--------
INSERT
***************
*** 223,229 ****
(12 rows)
-- change some data
! SELECT dblink_exec('UPDATE foo SET f3[2] = ''b99'' WHERE f1 = 11');
dblink_exec
-------------
UPDATE 1
--- 194,200 ----
(12 rows)
-- change some data
! SELECT dblink_exec('SET autocommit TO ''on'';UPDATE foo SET f3[2] = ''b99'' WHERE f1 = 11');
dblink_exec
-------------
UPDATE 1
***************
*** 239,245 ****
(1 row)
-- delete some data
! SELECT dblink_exec('DELETE FROM foo WHERE f1 = 11');
dblink_exec
-------------
DELETE 1
--- 210,216 ----
(1 row)
-- delete some data
! SELECT dblink_exec('SET autocommit TO ''on'';DELETE FROM foo WHERE f1 = 11');
dblink_exec
-------------
DELETE 1
Index: contrib/dblink/sql/dblink.sql
===================================================================
RCS file: /opt/src/cvs/pgsql-server/contrib/dblink/sql/dblink.sql,v
retrieving revision 1.5
diff -c -r1.5 dblink.sql
*** contrib/dblink/sql/dblink.sql 21 Oct 2002 01:42:13 -0000 1.5
--- contrib/dblink/sql/dblink.sql 22 Oct 2002 05:19:19 -0000
***************
*** 1,30 ****
- --
- -- First, create a slave database and define the functions and test data
- -- therein.
- --
- -- This initial hackery is to allow successive runs without failures.
- --
-
-- Adjust this setting to control where the objects get created.
SET search_path = public;
! CREATE OR REPLACE FUNCTION conditional_drop()
! RETURNS text AS '
! DECLARE
! dbname text;
! BEGIN
! SELECT INTO dbname datname FROM pg_database WHERE datname = ''regression_slave'';
! IF FOUND THEN
! DROP DATABASE regression_slave;
! END IF;
! RETURN ''OK'';
! END;
! ' LANGUAGE 'plpgsql';
! SELECT conditional_drop();
!
! CREATE DATABASE regression_slave;
! \connect regression_slave
!
-- Turn off echoing so that expected file does not depend on
-- contents of dblink.sql.
\set ECHO none
--- 1,10 ----
-- Adjust this setting to control where the objects get created.
SET search_path = public;
! --
! -- Define the functions and test data
! -- therein.
! --
-- Turn off echoing so that expected file does not depend on
-- contents of dblink.sql.
\set ECHO none
***************
*** 64,83 ****
-- build a delete statement based on a local tuple,
SELECT dblink_build_sql_delete('foo','1 2',2,'{"0", "a"}');
- --
- -- Connect back to the regression database and define the functions.
- -- Turn off echoing so that expected file does not depend on
- -- contents of dblink.sql.
- --
- \connect regression
- \set ECHO none
- SET autocommit TO 'on';
- \i dblink.sql
- \set ECHO all
-
-- regular old dblink
SELECT *
! FROM dblink('dbname=regression_slave','SELECT * FROM foo') AS t(a int, b text, c text[])
WHERE t.a > 7;
-- should generate "no connection available" error
--- 44,52 ----
-- build a delete statement based on a local tuple,
SELECT dblink_build_sql_delete('foo','1 2',2,'{"0", "a"}');
-- regular old dblink
SELECT *
! FROM dblink('dbname=regression','SELECT * FROM foo') AS t(a int, b text, c text[])
WHERE t.a > 7;
-- should generate "no connection available" error
***************
*** 86,92 ****
WHERE t.a > 7;
-- create a persistent connection
! SELECT dblink_connect('dbname=regression_slave');
-- use the persistent connection
SELECT *
--- 55,61 ----
WHERE t.a > 7;
-- create a persistent connection
! SELECT dblink_connect('dbname=regression');
-- use the persistent connection
SELECT *
***************
*** 124,144 ****
-- put more data into our slave table, first using arbitrary connection syntax
-- but truncate the actual return value so we can use diff to check for success
! SELECT substr(dblink_exec('dbname=regression_slave','SET autocommit TO ''on'';INSERT INTO foo
VALUES(10,''k'',''{"a10","b10","c10"}'')'),1,6);
-- create a persistent connection
! SELECT dblink_connect('dbname=regression_slave');
-- put more data into our slave table, using persistent connection syntax
-- but truncate the actual return value so we can use diff to check for success
! SELECT substr(dblink_exec('INSERT INTO foo VALUES(11,''l'',''{"a11","b11","c11"}'')'),1,6);
-- let's see it
SELECT *
FROM dblink('SELECT * FROM foo') AS t(a int, b text, c text[]);
-- change some data
! SELECT dblink_exec('UPDATE foo SET f3[2] = ''b99'' WHERE f1 = 11');
-- let's see it
SELECT *
--- 93,113 ----
-- put more data into our slave table, first using arbitrary connection syntax
-- but truncate the actual return value so we can use diff to check for success
! SELECT substr(dblink_exec('dbname=regression','SET autocommit TO ''on'';INSERT INTO foo
VALUES(10,''k'',''{"a10","b10","c10"}'')'),1,6);
-- create a persistent connection
! SELECT dblink_connect('dbname=regression');
-- put more data into our slave table, using persistent connection syntax
-- but truncate the actual return value so we can use diff to check for success
! SELECT substr(dblink_exec('SET autocommit TO ''on'';INSERT INTO foo VALUES(11,''l'',''{"a11","b11","c11"}'')'),1,6);
-- let's see it
SELECT *
FROM dblink('SELECT * FROM foo') AS t(a int, b text, c text[]);
-- change some data
! SELECT dblink_exec('SET autocommit TO ''on'';UPDATE foo SET f3[2] = ''b99'' WHERE f1 = 11');
-- let's see it
SELECT *
***************
*** 146,152 ****
WHERE a = 11;
-- delete some data
! SELECT dblink_exec('DELETE FROM foo WHERE f1 = 11');
-- let's see it
SELECT *
--- 115,121 ----
WHERE a = 11;
-- delete some data
! SELECT dblink_exec('SET autocommit TO ''on'';DELETE FROM foo WHERE f1 = 11');
-- let's see it
SELECT *
> What I just committed uses your idea of auto-committing TRUNCATE et al, > but now that I review the thread I think that everyone else thought that > that was a dangerous idea. How do you feel about simply throwing an error > in autocommit-off mode, instead? (At least it's a localized > change now) Well, if I can throw in another opinion, I think what you did is perfect. It will make Oracle users happy too. Only very shrewd applications would commit previous changes with a "truncate" statement, and those will learn to issue a commit before truncate. I don't like the solutions involving "set autocommit ...". Andreas
Tom Lane wrote: > Joe Conway <mail@joeconway.com> writes: > > I just noticed that this afternoon's changes cause dblink's regression > > test to fail due to: > > > CREATE OR REPLACE FUNCTION conditional_drop() > > [...] > > IF FOUND THEN > > DROP DATABASE regression_slave; > > END IF; > > [...] > > ' LANGUAGE 'plpgsql'; > > SELECT conditional_drop(); > > > I'm wondering what is the best alternative? > > Well, the *best* alternative would be to make CREATE/DROP DATABASE > transaction-safe ;-). I was speculating to myself earlier today about > how we might do that. It seems like it's not that far out of reach: > we could make smgr's list of files-to-remove-at-xact-commit-or-abort > include whole database subdirectories. But I'm not sure how that would > interact with upcoming features like tablespaces, so I don't want to > go off and implement it right now. FYI, the MSWin port in 7.4 will have C versions of 'cp' and 'rm -r', so those can be used to hook into the smgr layer for all platforms. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001+ If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania19073