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, Pennsylvania 19073Bruce 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, Pennsylvania 19073Bruce 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, Pennsylvania 19073Bruce 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, Pennsylvania 19073On 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, Pennsylvania 19073Bruce 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 a transaction
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 USERALTER 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, Pennsylvania 19073... 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, Pennsylvania 19073Bruce 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, Pennsylvania 19073Bruce 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, Pennsylvania 19073Bruce 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, Pennsylvania 19073Bruce 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, Pennsylvania 19073Tom Lane wrote:
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
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
Import Notes
Resolved by subject fallback
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
Attachments:
dblink-regress-fix2.1.patchtext/plain; name=dblink-regress-fix2.1.patchDownload
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
Import Notes
Resolved by subject fallback
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, Pennsylvania 19073