Question about savepoint level?

Started by Japin Liabout 3 years ago7 messages
#1Japin Li
japinli@hotmail.com

Hi, hackers

The TransactionStateData has savepointLevel field, however, I do not understand
what is savepoint level, it seems all savepoints have the same savepointLevel,
I want to know how the savepoint level changes.

--
Regrads,
Japin Li.
ChengDu WenWu Information Technology Co.,Ltd.

#2Japin Li
japinli@hotmail.com
In reply to: Japin Li (#1)
1 attachment(s)
Re: Question about savepoint level?

On Mon, 24 Oct 2022 at 12:19, Japin Li <japinli@hotmail.com> wrote:

Hi, hackers

The TransactionStateData has savepointLevel field, however, I do not understand
what is savepoint level, it seems all savepoints have the same savepointLevel,
I want to know how the savepoint level changes.

I try to remove the savepointLevel, and it seems harmless. Any thoughts?

--
Regrads,
Japin Li.
ChengDu WenWu Information Technology Co.,Ltd.

Attachments:

v1-0001-Remove-useless-savepoint-level.patchtext/x-patchDownload
From 1e5c015efc44bcf2bc93365e99740deb618eebfe Mon Sep 17 00:00:00 2001
From: Japin Li <japinli@hotmail.com>
Date: Mon, 24 Oct 2022 14:54:03 +0800
Subject: [PATCH v1] Remove useless savepoint level

---
 src/backend/access/transam/xact.c | 18 ------------------
 1 file changed, 18 deletions(-)

diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index fd5103a78e..e8a90a3a30 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -190,7 +190,6 @@ typedef struct TransactionStateData
 	FullTransactionId fullTransactionId;	/* my FullTransactionId */
 	SubTransactionId subTransactionId;	/* my subxact ID */
 	char	   *name;			/* savepoint name, if any */
-	int			savepointLevel; /* savepoint level */
 	TransState	state;			/* low-level state */
 	TBlockState blockState;		/* high-level state */
 	int			nestingLevel;	/* transaction nesting depth */
@@ -3234,12 +3233,10 @@ CommitTransactionCommand(void)
 		case TBLOCK_SUBRESTART:
 			{
 				char	   *name;
-				int			savepointLevel;
 
 				/* save name and keep Cleanup from freeing it */
 				name = s->name;
 				s->name = NULL;
-				savepointLevel = s->savepointLevel;
 
 				AbortSubTransaction();
 				CleanupSubTransaction();
@@ -3247,7 +3244,6 @@ CommitTransactionCommand(void)
 				DefineSavepoint(NULL);
 				s = CurrentTransactionState;	/* changed by push */
 				s->name = name;
-				s->savepointLevel = savepointLevel;
 
 				/* This is the same as TBLOCK_SUBBEGIN case */
 				AssertState(s->blockState == TBLOCK_SUBBEGIN);
@@ -3263,19 +3259,16 @@ CommitTransactionCommand(void)
 		case TBLOCK_SUBABORT_RESTART:
 			{
 				char	   *name;
-				int			savepointLevel;
 
 				/* save name and keep Cleanup from freeing it */
 				name = s->name;
 				s->name = NULL;
-				savepointLevel = s->savepointLevel;
 
 				CleanupSubTransaction();
 
 				DefineSavepoint(NULL);
 				s = CurrentTransactionState;	/* changed by push */
 				s->name = name;
-				s->savepointLevel = savepointLevel;
 
 				/* This is the same as TBLOCK_SUBBEGIN case */
 				AssertState(s->blockState == TBLOCK_SUBBEGIN);
@@ -4352,11 +4345,6 @@ ReleaseSavepoint(const char *name)
 				(errcode(ERRCODE_S_E_INVALID_SPECIFICATION),
 				 errmsg("savepoint \"%s\" does not exist", name)));
 
-	/* disallow crossing savepoint level boundaries */
-	if (target->savepointLevel != s->savepointLevel)
-		ereport(ERROR,
-				(errcode(ERRCODE_S_E_INVALID_SPECIFICATION),
-				 errmsg("savepoint \"%s\" does not exist within current savepoint level", name)));
 
 	/*
 	 * Mark "commit pending" all subtransactions up to the target
@@ -4461,11 +4449,6 @@ RollbackToSavepoint(const char *name)
 				(errcode(ERRCODE_S_E_INVALID_SPECIFICATION),
 				 errmsg("savepoint \"%s\" does not exist", name)));
 
-	/* disallow crossing savepoint level boundaries */
-	if (target->savepointLevel != s->savepointLevel)
-		ereport(ERROR,
-				(errcode(ERRCODE_S_E_INVALID_SPECIFICATION),
-				 errmsg("savepoint \"%s\" does not exist within current savepoint level", name)));
 
 	/*
 	 * Mark "abort pending" all subtransactions up to the target
@@ -5253,7 +5236,6 @@ PushTransaction(void)
 	s->parent = p;
 	s->nestingLevel = p->nestingLevel + 1;
 	s->gucNestLevel = NewGUCNestLevel();
-	s->savepointLevel = p->savepointLevel;
 	s->state = TRANS_DEFAULT;
 	s->blockState = TBLOCK_SUBBEGIN;
 	GetUserIdAndSecContext(&s->prevUser, &s->prevSecContext);
-- 
2.25.1

#3Richard Guo
guofenglinux@gmail.com
In reply to: Japin Li (#2)
Re: Question about savepoint level?

On Mon, Oct 24, 2022 at 3:00 PM Japin Li <japinli@hotmail.com> wrote:

On Mon, 24 Oct 2022 at 12:19, Japin Li <japinli@hotmail.com> wrote:

The TransactionStateData has savepointLevel field, however, I do not

understand

what is savepoint level, it seems all savepoints have the same

savepointLevel,

I want to know how the savepoint level changes.

I try to remove the savepointLevel, and it seems harmless. Any thoughts?

ISTM the savepointLevel always remains the same as what is in
TopTransactionStateData after looking at the codes. Now I also get
confused. Maybe what we want is nestingLevel?

Thanks
Richard

#4Alvaro Herrera
alvherre@alvh.no-ip.org
In reply to: Richard Guo (#3)
Re: Question about savepoint level?

On 2022-Oct-24, Richard Guo wrote:

On Mon, Oct 24, 2022 at 3:00 PM Japin Li <japinli@hotmail.com> wrote:

I try to remove the savepointLevel, and it seems harmless. Any thoughts?

ISTM the savepointLevel always remains the same as what is in
TopTransactionStateData after looking at the codes. Now I also get
confused. Maybe what we want is nestingLevel?

This has already been discussed:
/messages/by-id/1317297307-sup-7945@alvh.no-ip.org
Now that we have transaction-controlling procedures, I think the next
step is to add the SQL-standard feature that allows savepoint level
control for them, which would make the savepointLevel no longer dead
code.

--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
"You're _really_ hosed if the person doing the hiring doesn't understand
relational systems: you end up with a whole raft of programmers, none of
whom has had a Date with the clue stick." (Andrew Sullivan)

#5Japin Li
japinli@hotmail.com
In reply to: Alvaro Herrera (#4)
Re: Question about savepoint level?

On Mon, 24 Oct 2022 at 17:56, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:

This has already been discussed:
/messages/by-id/1317297307-sup-7945@alvh.no-ip.org

Sorry for my lazy search.

Now that we have transaction-controlling procedures, I think the next
step is to add the SQL-standard feature that allows savepoint level
control for them, which would make the savepointLevel no longer dead
code.

So the savepoint level is used for CREATE PROCEDURE ... OLD/NEW SAVEPOINT LEVEL
syntax [1]https://www.ibm.com/docs/en/db2/10.1.0?topic=statements-create-procedure-sql, right?

[1]: https://www.ibm.com/docs/en/db2/10.1.0?topic=statements-create-procedure-sql

--
Regrads,
Japin Li.
ChengDu WenWu Information Technology Co.,Ltd.

#6Alvaro Herrera
alvherre@alvh.no-ip.org
In reply to: Japin Li (#5)
Re: Question about savepoint level?

On 2022-Oct-24, Japin Li wrote:

On Mon, 24 Oct 2022 at 17:56, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:

Now that we have transaction-controlling procedures, I think the next
step is to add the SQL-standard feature that allows savepoint level
control for them, which would make the savepointLevel no longer dead
code.

So the savepoint level is used for CREATE PROCEDURE ... OLD/NEW SAVEPOINT LEVEL
syntax [1], right?

[1] https://www.ibm.com/docs/en/db2/10.1.0?topic=statements-create-procedure-sql

Yeah, that's what I understand. The default behavior is the current
behavior (OLD SAVEPOINT LEVEL). In a procedure that specifies NEW
SAVEPOINT LEVEL trying to rollback a savepoint that was defined before
the procedure was called is an error, which sounds a useful protection.

--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
"El sentido de las cosas no viene de las cosas, sino de
las inteligencias que las aplican a sus problemas diarios
en busca del progreso." (Ernesto Hernández-Novich)

#7Richard Guo
guofenglinux@gmail.com
In reply to: Alvaro Herrera (#4)
Re: Question about savepoint level?

On Mon, Oct 24, 2022 at 6:01 PM Alvaro Herrera <alvherre@alvh.no-ip.org>
wrote:

On 2022-Oct-24, Richard Guo wrote:

ISTM the savepointLevel always remains the same as what is in
TopTransactionStateData after looking at the codes. Now I also get
confused. Maybe what we want is nestingLevel?

This has already been discussed:
/messages/by-id/1317297307-sup-7945@alvh.no-ip.org
Now that we have transaction-controlling procedures, I think the next
step is to add the SQL-standard feature that allows savepoint level
control for them, which would make the savepointLevel no longer dead
code.

Now I see the context. Thanks for pointing that out.

Thanks
Richard