public inbox for [email protected]
help / color / mirror / Atom feedFrom: Alexander Korotkov <[email protected]>
To: Pavel Borisov <[email protected]>
Cc: Dmitry Koval <[email protected]>
Cc: Noah Misch <[email protected]>
Cc: [email protected]
Subject: Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
Date: Wed, 21 Aug 2024 15:40:44 +0300
Message-ID: <CAPpHfdst=eqJ8-n=yhSk=5Xk2QHLF90HWryHWgARuityB3=b7A@mail.gmail.com> (raw)
In-Reply-To: <CALT9ZEGTFdwrP+jO1tKxHHeGPM-enMm734MP0wCwJTQZDembYA@mail.gmail.com>
References: <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>
<[email protected]>
<[email protected]>
<CAPpHfdtYgQ8PUZMS043c8USeKcKh5isrAW0QQKMAxQacX-+nuA@mail.gmail.com>
<[email protected]>
<CAPpHfdu+iX5TP65Cyn6vUe8JKGtLmMA=sKKKgE7tB6G6Qk=18Q@mail.gmail.com>
<CALT9ZEHhVt-i1vGL_eCfNqAisTiC6VaM6U-fxP3kD306uPLEcw@mail.gmail.com>
<CAPpHfdsEtAWhpcPEaUky+gO20=rz0P03OskAS0c8teuTtHP9hQ@mail.gmail.com>
<CALT9ZEGTFdwrP+jO1tKxHHeGPM-enMm734MP0wCwJTQZDembYA@mail.gmail.com>
On Wed, Aug 21, 2024 at 3:06 PM Pavel Borisov <[email protected]> wrote:
> On Wed, 21 Aug 2024 at 15:55, Alexander Korotkov <[email protected]> wrote:
>>
>> Hi, Pavel!
>>
>> On Wed, Aug 21, 2024 at 1:48 PM Pavel Borisov <[email protected]> wrote:
>> > On Mon, 19 Aug 2024 at 02:24, Alexander Korotkov <[email protected]> wrote:
>> >>
>> >> On Sat, Aug 10, 2024 at 6:57 PM Dmitry Koval <[email protected]> wrote:
>> >> > > Probably
>> >> > > QueryCompletion struct fits this purpose best from the existing
>> >> > > parameters. Attached draft patch implements returning oid of newly
>> >> > > created relation as part of QueryCompletion. Thoughts?
>> >> >
>> >> > I agree, returning the oid of the newly created relation is the best way
>> >> > to solve the problem.
>> >> > (Excuse me, I won't have access to a laptop for the next week - and
>> >> > won't be able to look at the source code).
>> >>
>> >> Thank you for your feedback. Although, I decided QueryCompletion is
>> >> not a good place for this new field. It looks more appropriate to
>> >> place it to TableLikeClause, which already contains one relation oid
>> >> inside. The revised patch is attached.
>> >
>> >
>> > I've looked at the patch v2. Remembering the OID of a relation newly created with LIKE in TableLikeClause seems good to me.
>> > Check-world passes sucessfully.
>>
>> Thank you.
>>
>> > Shouldn't we also modify the TableLikeClause node in gram.y accordingly?
>>
>> On the one hand, makeNode() uses palloc0() and initializes all fields
>> with zero anyway. On the other hand, there is already assignment of
>> relationOid. So, yes I'll add assignment of newRelationOid for the
>> sake of uniformity.
>>
>> > For the comments:
>> > Put the Oid -> Store the OID
>>
>> > so caller might use it -> for the caller to use it.
>>
>> Accepted.
>>
>> > (Maybe also caller -> table create function)
>>
>> I'll prefer to leave it "caller" as more generic term, which could
>> also fit potential future usages.
>>
>> The revised patch is attached. I'm going to push it if no objections.
>
> Looked at v3
> All good except the patch has "Oid" and "OID" in two comments. I suppose "OID" is preferred elsewhere in the PG comments.
Correct, the same file contains "OID" multiple times. Revised version
is attached.
------
Regards,
Alexander Korotkov
Supabase
Attachments:
[application/octet-stream] v4-0001-Avoid-repeated-table-name-lookups-in-createPartit.patch (3.4K, ../CAPpHfdst=eqJ8-n=yhSk=5Xk2QHLF90HWryHWgARuityB3=b7A@mail.gmail.com/2-v4-0001-Avoid-repeated-table-name-lookups-in-createPartit.patch)
download | inline diff:
From cc3992a9262b126bc16a2e9f620c41aedcf99cba Mon Sep 17 00:00:00 2001
From: Alexander Korotkov <[email protected]>
Date: Sat, 10 Aug 2024 16:19:28 +0300
Subject: [PATCH v4] Avoid repeated table name lookups in
createPartitionTable()
Currently, createPartitionTable() opens newly created table using its name.
This approach is prone to privilege escalation attack, because we might end
up opening another table than we just created.
This commit address the issue above by opening newly created table by its
oid. It appears to be tricky to get a relation oid out of ProcessUtility().
We have to extend TableLikeClause with new newRelationOid field, which is
filled within ProcessUtility() to be further accessed by caller.
Security: CVE-2014-0062
Reported-by: Noah Misch
Discussion: https://postgr.es/m/20240808171351.a9.nmisch%40google.com
Reviewed-by: Dmitry Koval
---
src/backend/commands/tablecmds.c | 3 ++-
src/backend/parser/gram.y | 1 +
src/backend/tcop/utility.c | 6 ++++++
src/include/nodes/parsenodes.h | 1 +
4 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index dfba5f357b8..bcc2d9ae0b3 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -20381,6 +20381,7 @@ createPartitionTable(RangeVar *newPartName, Relation modelRel,
tlc->options = CREATE_TABLE_LIKE_ALL &
~(CREATE_TABLE_LIKE_INDEXES | CREATE_TABLE_LIKE_IDENTITY | CREATE_TABLE_LIKE_STATISTICS);
tlc->relationOid = InvalidOid;
+ tlc->newRelationOid = InvalidOid;
createStmt->tableElts = lappend(createStmt->tableElts, tlc);
/* Need to make a wrapper PlannedStmt. */
@@ -20404,7 +20405,7 @@ createPartitionTable(RangeVar *newPartName, Relation modelRel,
* Open the new partition with no lock, because we already have
* AccessExclusiveLock placed there after creation.
*/
- newRel = table_openrv(newPartName, NoLock);
+ newRel = table_open(tlc->newRelationOid, NoLock);
/*
* We intended to create the partition with the same persistence as the
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index c3f25582c38..b7d98eb9f02 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -4138,6 +4138,7 @@ TableLikeClause:
n->relation = $2;
n->options = $3;
n->relationOid = InvalidOid;
+ n->newRelationOid = InvalidOid;
$$ = (Node *) n;
}
;
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index b2ea8125c92..b385175e7a2 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -1225,6 +1225,12 @@ ProcessUtilitySlow(ParseState *pstate,
morestmts = expandTableLikeClause(table_rv, like);
stmts = list_concat(morestmts, stmts);
+
+ /*
+ * Store the OID of newly created relation to the
+ * TableLikeClause for the caller to use it.
+ */
+ like->newRelationOid = address.objectId;
}
else
{
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 85a62b538e5..577c4bfef76 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -754,6 +754,7 @@ typedef struct TableLikeClause
RangeVar *relation;
bits32 options; /* OR of TableLikeOption flags */
Oid relationOid; /* If table has been looked up, its OID */
+ Oid newRelationOid; /* OID of newly created table */
} TableLikeClause;
typedef enum TableLikeOption
--
2.39.3 (Apple Git-146)
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], [email protected]
Subject: Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
In-Reply-To: <CAPpHfdst=eqJ8-n=yhSk=5Xk2QHLF90HWryHWgARuityB3=b7A@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