Yugo NAGATA <nagata@sraoss.co.jp> wrote:
> On Wed, 01 Feb 2023 12:45:57 +0100
> Antonin Houska <ah@cybertec.at> wrote:
>
> > While working on [1] I noticed that if RLS gets enabled, the COPY TO command
> > includes the contents of child table into the result, although the
> > documentation says it should not:
> >
> > "COPY TO can be used only with plain tables, not views, and does not
> > copy rows from child tables or child partitions. For example, COPY
> > table TO copies the same rows as SELECT * FROM ONLY table. The syntax
> > COPY (SELECT * FROM table) TO ... can be used to dump all of the rows
> > in an inheritance hierarchy, partitioned table, or view."
> >
> > A test case is attached (rls.sql) as well as fix proposal
> > (copy_rls_no_inh.diff).
>
> I think this is a bug because the current behaviour is different from
> the documentation.
>
> When RLS is enabled on a table in `COPY ... TO ...`, the query is converted
> to `COPY (SELECT * FROM ...) TO ...` to allow the rewriter to add in RLS
> clauses. This causes to dump the rows of child tables.
>
> The patch fixes this by setting "inh" of the table in the converted query
> to false. This seems reasonable and actually fixes the problem.
>
> However, I think we would want a comment on the added line.
A short comment added, see the new patch version.
> Also, the attached test should be placed in the regression test.
Hm, I'm not sure it's necessary. It would effectively test whether the 'inh'
field works, but if it didn't, many other tests would fail. I discovered the
bug by reading the code, so I wanted to demonstrate (also to myself) that it
causes incorrect behavior from user perspective. That was the purpose of the
test.
--
Antonin Houska
Web: https://www.cybertec-postgresql.com
From 3a4acb29901a7b53eb32767e30bdfce74b80df8b Mon Sep 17 00:00:00 2001
From: Antonin Houska <ah@cybertec.at>
Date: Thu, 2 Feb 2023 07:31:32 +0100
Subject: [PATCH] Make sure that COPY TO does not process child tables.
It's expected (and documented) that the child tables are not copied, however
the query generated for RLS didn't meet this expectation so far.
---
src/backend/commands/copy.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index e34f583ea7..539fb682c2 100644
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -250,6 +250,9 @@ DoCopy(ParseState *pstate, const CopyStmt *stmt,
pstrdup(RelationGetRelationName(rel)),
-1);
+ /* COPY TO should not process child tables. */
+ from->inh = false;
+
/* Build query */
select = makeNode(SelectStmt);
select->targetList = targetList;
--
2.31.1