Re: [HACKERS] [PATCH] Remove unused argument in btree_xlog_split
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
Re: [HACKERS] [PATCH] Remove unused argument in btree_xlog_split
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]. [1] https://commitfest.postgresql.org/14/1097/ -- Best regards, Aleksander Alekseev signature.asc Description: PGP signature
Re: [HACKERS] [PATCH] Remove unused argument in btree_xlog_split
On Thu, Apr 6, 2017 at 8:21 AM, 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. 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
Re: [HACKERS] [PATCH] Remove unused argument in btree_xlog_split
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
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 */
signature.asc
Description: PGP signature
Re: [HACKERS] [PATCH] Remove unused argument in btree_xlog_split
On Fri, Mar 31, 2017 at 10:10 AM, Aleksander Alekseev 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
[HACKERS] [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
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);
signature.asc
Description: PGP signature
