[PATCH] Remove unused argument in btree_xlog_split
Hi,
Turned out that there is an unused argument `isroot` in
`btree_xlog_split` procedure. Suggested patch fixes it.
This issue was discovered by Anastasia Lubennikova, coding is done by me.
--
Best regards,
Aleksander Alekseev
Attachments:
btree_xlog_split_refactoring.patchtext/x-diff; charset=us-asciiDownload
diff --git a/src/backend/access/nbtree/nbtxlog.c b/src/backend/access/nbtree/nbtxlog.c
index ac60db0d49..2db8f13cae 100644
--- a/src/backend/access/nbtree/nbtxlog.c
+++ b/src/backend/access/nbtree/nbtxlog.c
@@ -193,7 +193,7 @@ btree_xlog_insert(bool isleaf, bool ismeta, XLogReaderState *record)
}
static void
-btree_xlog_split(bool onleft, bool isroot, XLogReaderState *record)
+btree_xlog_split(bool onleft, XLogReaderState *record)
{
XLogRecPtr lsn = record->EndRecPtr;
xl_btree_split *xlrec = (xl_btree_split *) XLogRecGetData(record);
@@ -996,16 +996,12 @@ btree_redo(XLogReaderState *record)
btree_xlog_insert(false, true, record);
break;
case XLOG_BTREE_SPLIT_L:
- btree_xlog_split(true, false, record);
- break;
- case XLOG_BTREE_SPLIT_R:
- btree_xlog_split(false, false, record);
- break;
case XLOG_BTREE_SPLIT_L_ROOT:
- btree_xlog_split(true, true, record);
+ btree_xlog_split(true, record);
break;
+ case XLOG_BTREE_SPLIT_R:
case XLOG_BTREE_SPLIT_R_ROOT:
- btree_xlog_split(false, true, record);
+ btree_xlog_split(false, record);
break;
case XLOG_BTREE_VACUUM:
btree_xlog_vacuum(record);
On Fri, Mar 31, 2017 at 10:10 AM, Aleksander Alekseev
<a.alekseev@postgrespro.ru> wrote:
Turned out that there is an unused argument `isroot` in
`btree_xlog_split` procedure. Suggested patch fixes it.This issue was discovered by Anastasia Lubennikova, coding is done by me.
Hmm. I don't see anything wrong with that, particularly, but it seems
we also don't need the distinction between XLOG_BTREE_SPLIT_L and
XLOG_BTREE_SPLIT_L_ROOT or likewise between XLOG_BTREE_SPLIT_R and
XLOG_BTREE_SPLIT_R_ROOT -- in which case I think this patch should go
a little further and do all of that together.
It looks like this is fallout from Heikki's 2014 commit
40dae7ec537c5619fc93ad602c62f37be786d161, which removed
log_incomplete_split().
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Hi Robert,
Hmm. I don't see anything wrong with that, particularly, but it seems
we also don't need the distinction between XLOG_BTREE_SPLIT_L and
XLOG_BTREE_SPLIT_L_ROOT or likewise between XLOG_BTREE_SPLIT_R and
XLOG_BTREE_SPLIT_R_ROOT -- in which case I think this patch should go
a little further and do all of that together.
Thank you for sharing your thoughts on this patch. Here is a new
version.
--
Best regards,
Aleksander Alekseev
Attachments:
btree_xlog_split_refactoring_v2.patchtext/x-diff; charset=us-asciiDownload
diff --git a/src/backend/access/nbtree/nbtinsert.c b/src/backend/access/nbtree/nbtinsert.c
index 6dca8109fd..4de47de890 100644
--- a/src/backend/access/nbtree/nbtinsert.c
+++ b/src/backend/access/nbtree/nbtinsert.c
@@ -980,7 +980,6 @@ _bt_split(Relation rel, Buffer buf, Buffer cbuf, OffsetNumber firstright,
rightoff;
OffsetNumber maxoff;
OffsetNumber i;
- bool isroot;
bool isleaf;
/* Acquire a new page to split into */
@@ -1019,7 +1018,6 @@ _bt_split(Relation rel, Buffer buf, Buffer cbuf, OffsetNumber firstright,
lopaque = (BTPageOpaque) PageGetSpecialPointer(leftpage);
ropaque = (BTPageOpaque) PageGetSpecialPointer(rightpage);
- isroot = P_ISROOT(oopaque);
isleaf = P_ISLEAF(oopaque);
/* if we're splitting this page, it won't be the root when we're done */
@@ -1330,11 +1328,7 @@ _bt_split(Relation rel, Buffer buf, Buffer cbuf, OffsetNumber firstright,
(char *) rightpage + ((PageHeader) rightpage)->pd_upper,
((PageHeader) rightpage)->pd_special - ((PageHeader) rightpage)->pd_upper);
- if (isroot)
- xlinfo = newitemonleft ? XLOG_BTREE_SPLIT_L_ROOT : XLOG_BTREE_SPLIT_R_ROOT;
- else
- xlinfo = newitemonleft ? XLOG_BTREE_SPLIT_L : XLOG_BTREE_SPLIT_R;
-
+ xlinfo = newitemonleft ? XLOG_BTREE_SPLIT_L : XLOG_BTREE_SPLIT_R;
recptr = XLogInsert(RM_BTREE_ID, xlinfo);
PageSetLSN(origpage, recptr);
diff --git a/src/backend/access/nbtree/nbtxlog.c b/src/backend/access/nbtree/nbtxlog.c
index ac60db0d49..3610c7c7e0 100644
--- a/src/backend/access/nbtree/nbtxlog.c
+++ b/src/backend/access/nbtree/nbtxlog.c
@@ -193,7 +193,7 @@ btree_xlog_insert(bool isleaf, bool ismeta, XLogReaderState *record)
}
static void
-btree_xlog_split(bool onleft, bool isroot, XLogReaderState *record)
+btree_xlog_split(bool onleft, XLogReaderState *record)
{
XLogRecPtr lsn = record->EndRecPtr;
xl_btree_split *xlrec = (xl_btree_split *) XLogRecGetData(record);
@@ -996,16 +996,10 @@ btree_redo(XLogReaderState *record)
btree_xlog_insert(false, true, record);
break;
case XLOG_BTREE_SPLIT_L:
- btree_xlog_split(true, false, record);
+ btree_xlog_split(true, record);
break;
case XLOG_BTREE_SPLIT_R:
- btree_xlog_split(false, false, record);
- break;
- case XLOG_BTREE_SPLIT_L_ROOT:
- btree_xlog_split(true, true, record);
- break;
- case XLOG_BTREE_SPLIT_R_ROOT:
- btree_xlog_split(false, true, record);
+ btree_xlog_split(false, record);
break;
case XLOG_BTREE_VACUUM:
btree_xlog_vacuum(record);
diff --git a/src/backend/access/rmgrdesc/nbtdesc.c b/src/backend/access/rmgrdesc/nbtdesc.c
index fbde9d6555..00cd6e2379 100644
--- a/src/backend/access/rmgrdesc/nbtdesc.c
+++ b/src/backend/access/rmgrdesc/nbtdesc.c
@@ -35,8 +35,6 @@ btree_desc(StringInfo buf, XLogReaderState *record)
}
case XLOG_BTREE_SPLIT_L:
case XLOG_BTREE_SPLIT_R:
- case XLOG_BTREE_SPLIT_L_ROOT:
- case XLOG_BTREE_SPLIT_R_ROOT:
{
xl_btree_split *xlrec = (xl_btree_split *) rec;
@@ -121,12 +119,6 @@ btree_identify(uint8 info)
case XLOG_BTREE_SPLIT_R:
id = "SPLIT_R";
break;
- case XLOG_BTREE_SPLIT_L_ROOT:
- id = "SPLIT_L_ROOT";
- break;
- case XLOG_BTREE_SPLIT_R_ROOT:
- id = "SPLIT_R_ROOT";
- break;
case XLOG_BTREE_VACUUM:
id = "VACUUM";
break;
diff --git a/src/include/access/nbtxlog.h b/src/include/access/nbtxlog.h
index d6a3085923..3cc9734b2b 100644
--- a/src/include/access/nbtxlog.h
+++ b/src/include/access/nbtxlog.h
@@ -28,8 +28,6 @@
#define XLOG_BTREE_INSERT_META 0x20 /* same, plus update metapage */
#define XLOG_BTREE_SPLIT_L 0x30 /* add index tuple with split */
#define XLOG_BTREE_SPLIT_R 0x40 /* as above, new item on right */
-#define XLOG_BTREE_SPLIT_L_ROOT 0x50 /* add tuple with split of root */
-#define XLOG_BTREE_SPLIT_R_ROOT 0x60 /* as above, new item on right */
#define XLOG_BTREE_DELETE 0x70 /* delete leaf index tuples for a page */
#define XLOG_BTREE_UNLINK_PAGE 0x80 /* delete a half-dead page */
#define XLOG_BTREE_UNLINK_PAGE_META 0x90 /* same, and update metapage */
On Thu, Apr 6, 2017 at 8:21 AM, Aleksander Alekseev
<a.alekseev@postgrespro.ru> wrote:
Hi Robert,
Hmm. I don't see anything wrong with that, particularly, but it seems
we also don't need the distinction between XLOG_BTREE_SPLIT_L and
XLOG_BTREE_SPLIT_L_ROOT or likewise between XLOG_BTREE_SPLIT_R and
XLOG_BTREE_SPLIT_R_ROOT -- in which case I think this patch should go
a little further and do all of that together.Thank you for sharing your thoughts on this patch. Here is a new
version.
Thanks. Please add this to the next CommitFest, as there seems to be
no urgency (and some risk) in committing it right before feature
freeze.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Hi Robert,
Thanks. Please add this to the next CommitFest, as there seems to be
no urgency (and some risk) in committing it right before feature
freeze.
Sure. Already done [1]https://commitfest.postgresql.org/14/1097/.
[1]: https://commitfest.postgresql.org/14/1097/
--
Best regards,
Aleksander Alekseev
On 04/06/2017 03:21 PM, Aleksander Alekseev wrote:
Hi Robert,
Hmm. I don't see anything wrong with that, particularly, but it seems
we also don't need the distinction between XLOG_BTREE_SPLIT_L and
XLOG_BTREE_SPLIT_L_ROOT or likewise between XLOG_BTREE_SPLIT_R and
XLOG_BTREE_SPLIT_R_ROOT -- in which case I think this patch should go
a little further and do all of that together.Thank you for sharing your thoughts on this patch. Here is a new
version.
Committed, thanks.
- Heikki
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers