public inbox for [email protected]
help / color / mirror / Atom feedFrom: Tender Wang <[email protected]>
To: Alexander Korotkov <[email protected]>
To: Dmitry Koval <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
Date: Mon, 8 Apr 2024 18:43:51 +0800
Message-ID: <CAHewXNkGMPU50QG7V6Q60JGFORfo8LfYO1_GCkCa0VWbmB-fEw@mail.gmail.com> (raw)
In-Reply-To: <CAPpHfdtj7YsPaASoVPN+N3H4_Ct+kQw8QY1d_9u7FPnbghkicw@mail.gmail.com>
References: <[email protected]>
<171085360143.2046436.7217841141682511557.pgcf@coridan.postgresql.org>
<171085858750.2046434.12246066729700037354.pgcf@coridan.postgresql.org>
<[email protected]>
<CAPpHfdsVQwnZPxaQPwcu1g_01F0d-o=ECRcf14X3CmavJB6vSQ@mail.gmail.com>
<[email protected]>
<CAPpHfdsCCV8DOv0Ovu4dTSnuWXiEz_AjuW-J6TPj++tcfEGvRw@mail.gmail.com>
<[email protected]>
<CAPpHfduNYkMVuUSxZjD6u4tZG6aKdhgQcmhr-K9OQfeg56MNww@mail.gmail.com>
<[email protected]>
<CAPpHfdtj7YsPaASoVPN+N3H4_Ct+kQw8QY1d_9u7FPnbghkicw@mail.gmail.com>
Hi all,
I went through the MERGE/SPLIT partition codes today, thanks for the
works. I found some grammar errors:
i. in error messages(Users can see this grammar errors, not friendly).
ii. in codes comments
Alexander Korotkov <[email protected]> 于2024年4月7日周日 06:23写道：
> Hi, Dmitry!
>
> On Fri, Apr 5, 2024 at 4:00 PM Dmitry Koval <[email protected]>
> wrote:
> > > I've revised the patchset.
> >
> > Thanks for the corrections (especially ddl.sgml).
> > Could you also look at a small optimization for the MERGE PARTITIONS
> > command (in a separate file
> > v31-0003-Additional-patch-for-ALTER-TABLE-.-MERGE-PARTITI.patch, I wrote
> > about it in an email 2024-03-31 00:56:50)?
> >
> > Files v31-0001-*.patch, v31-0002-*.patch are the same as
> > v30-0001-*.patch, v30-0002-*.patch (after rebasing because patch stopped
> > applying due to changes in upstream).
>
> I've pushed 0001 and 0002. I didn't push 0003 for the following reasons.
> 1) This doesn't keep functionality equivalent to 0001. With 0003, the
> merged partition will inherit indexes, constraints, and so on from the
> one of merging partitions.
> 2) This is not necessarily an optimization. Without 0003 indexes on
> the merged partition are created after moving the rows in
> attachPartitionTable(). With 0003 we merge data into the existing
> partition which saves its indexes. That might cause a significant
> performance loss because mass inserts into indexes may be much slower
> than building indexes from scratch.
> I think both aspects need to be carefully considered. Even if we
> accept them, this needs to be documented. I think now it's too late
> for both of these. So, this should wait for v18.
>
> ------
> Regards,
> Alexander Korotkov
>
>
>
--
Tender Wang
OpenPie: https://en.openpie.com/
Attachments:
[application/octet-stream] 0001-Fix-some-grammer-errors-from-error-messages-and-code.patch (6.3K, ../CAHewXNkGMPU50QG7V6Q60JGFORfo8LfYO1_GCkCa0VWbmB-fEw@mail.gmail.com/3-0001-Fix-some-grammer-errors-from-error-messages-and-code.patch)
download | inline diff:
From 2b0fff52002df8e4dc35c7b50d2ffa9e302b50d7 Mon Sep 17 00:00:00 2001
From: "tender.wang" <[email protected]>
Date: Mon, 8 Apr 2024 18:35:05 +0800
Subject: [PATCH] Fix some grammer errors from error messages and codes
comments.
---
src/backend/commands/tablecmds.c | 18 +++++++++---------
src/backend/parser/parse_utilcmd.c | 2 +-
src/backend/partitioning/partbounds.c | 6 +++---
src/include/nodes/parsenodes.h | 2 +-
4 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 582890a302..2b0b594d75 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -19086,7 +19086,7 @@ QueuePartitionConstraintValidation(List **wqueue, Relation scanrel,
}
/*
- * attachPartitionTable: attach new partition to partitioned table
+ * attachPartitionTable: attach a new partition to the partitioned table
*
* wqueue: the ALTER TABLE work queue; can be NULL when not running as part
* of an ALTER TABLE sequence.
@@ -20879,10 +20879,10 @@ GetAttributeStorage(Oid atttypid, const char *storagemode)
*/
typedef struct SplitPartitionContext
{
- ExprState *partqualstate; /* expression for check slot for partition
+ ExprState *partqualstate; /* expression for checking slot for partition
* (NULL for DEFAULT partition) */
BulkInsertState bistate; /* state of bulk inserts for partition */
- TupleTableSlot *dstslot; /* slot for insert row into partition */
+ TupleTableSlot *dstslot; /* slot for inserting row into partition */
Relation partRel; /* relation for partition */
} SplitPartitionContext;
@@ -21129,7 +21129,7 @@ moveSplitTableRows(Relation rel, Relation splitRel, List *partlist, List *newPar
}
/*
- * createPartitionTable: create table for new partition with given name
+ * createPartitionTable: create table for a new partition with given name
* (newPartName) like table (modelRelName)
*
* Emulates command: CREATE TABLE <newPartName> (LIKE <modelRelName>
@@ -21206,7 +21206,7 @@ ATExecSplitPartition(List **wqueue, AlteredTableInfo *tab, Relation rel,
/*
* We are going to detach and remove this partition: need to use exclusive
- * lock for prevent DML-queries to the partition.
+ * lock for preventing DML-queries to the partition.
*/
splitRel = table_openrv(cmd->name, AccessExclusiveLock);
@@ -21256,13 +21256,13 @@ ATExecSplitPartition(List **wqueue, AlteredTableInfo *tab, Relation rel,
/*
* If new partition has the same name as split partition then we should
- * rename split partition for reuse name.
+ * rename split partition for reusing name.
*/
if (isSameName)
{
/*
* We must bump the command counter to make the split partition tuple
- * visible for rename.
+ * visible for renaming.
*/
CommandCounterIncrement();
/* Rename partition. */
@@ -21271,7 +21271,7 @@ ATExecSplitPartition(List **wqueue, AlteredTableInfo *tab, Relation rel,
/*
* We must bump the command counter to make the split partition tuple
- * visible after rename.
+ * visible after renaming.
*/
CommandCounterIncrement();
}
@@ -21446,7 +21446,7 @@ ATExecMergePartitions(List **wqueue, AlteredTableInfo *tab, Relation rel,
/*
* We are going to detach and remove this partition: need to use
- * exclusive lock for prevent DML-queries to the partition.
+ * exclusive lock for preventing DML-queries to the partition.
*/
mergingPartition = table_openrv(name, AccessExclusiveLock);
diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c
index 88a4a41186..9e3e14087f 100644
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -3804,7 +3804,7 @@ transformAlterTableStmt(Oid relid, AlterTableStmt *stmt,
if (list_length(partcmd->partlist) < 2)
ereport(ERROR,
(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
- errmsg("list of new partitions should contains at least two items")));
+ errmsg("list of new partitions should contain at least two items")));
if (cmd->subtype == AT_SplitPartition)
transformPartitionCmdForSplit(&cxt, partcmd);
diff --git a/src/backend/partitioning/partbounds.c b/src/backend/partitioning/partbounds.c
index c36e26ba4b..83df89e9b1 100644
--- a/src/backend/partitioning/partbounds.c
+++ b/src/backend/partitioning/partbounds.c
@@ -4986,7 +4986,7 @@ satisfies_hash_partition(PG_FUNCTION_ARGS)
* This is a helper function for check_partitions_for_split() and
* calculate_partition_bound_for_merge().
* This function compares upper bound of first_bound and lower bound of
- * second_bound. These bounds should be equals except case
+ * second_bound. These bounds should be equal except case
* "defaultPart == true" (this means that one of split partitions is DEFAULT).
* In this case upper bound of first_bound can be less than lower bound of
* second_bound because space between of these bounds will be included in
@@ -5262,7 +5262,7 @@ check_partition_bounds_for_split_range(Relation parent,
errmsg("%s bound of partition \"%s\" is %s %s bound of split partition",
first ? "lower" : "upper",
relname,
- defaultPart ? (first ? "less than" : "greater than") : "not equals to",
+ defaultPart ? (first ? "less than" : "greater than") : "not equal to",
first ? "lower" : "upper"),
parser_errposition(pstate, datum->location)));
}
@@ -5483,7 +5483,7 @@ check_parent_values_in_new_partitions(Relation parent,
Const *notFoundVal;
if (!searchNull)
- /* Make Const for get string representation of not found value. */
+ /* Make Const for getting string representation of not found value. */
notFoundVal = makeConst(key->parttypid[0],
key->parttypmod[0],
key->parttypcoll[0],
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 01fa1a6c2e..32df680a9a 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -956,7 +956,7 @@ typedef struct PartitionCmd
NodeTag type;
RangeVar *name; /* name of partition to attach/detach */
PartitionBoundSpec *bound; /* FOR VALUES, if attaching */
- List *partlist; /* list of partitions, for SPLIT PARTITION
+ List *partlist; /* list of partitions, for MERGE/SPLIT PARTITION
* command */
bool concurrent;
} PartitionCmd;
--
2.25.1
view thread (20+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected]
Subject: Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
In-Reply-To: <CAHewXNkGMPU50QG7V6Q60JGFORfo8LfYO1_GCkCa0VWbmB-fEw@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox