Message-ID: <2e3ed175-8618-46a9-847b-b7e54597aa83@gmail.com>
Date: Wed, 26 Aug 2026 20:04:15 -0500
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
From: dbryan.green@gmail.com
To: pgsql-hackers@lists.postgresql.org
Subject: Re: COPY FROM with RLS
References: 
 <CACJufxFbmnoa5O-vL43DPTCGt6oagY4dXgKxy=rcD9-e9g0zEg@mail.gmail.com>
 <CACJufxFdt0+R6C5ZByAK3DpMEXbb=3Hc0XyRTPvbjpc9sPt=gQ@mail.gmail.com>
 <70d4ee07.f25.19f161ce6b2.Coremail.jiezhilove@126.com>
 <CACJufxHU3tetCM+Qymxpj7-OLP8b6x3HJAnARadSrEAt64MHkg@mail.gmail.com>
 <1266b6dc.6689.19fa801caa0.Coremail.jiezhilove@126.com>
In-Reply-To: <1266b6dc.6689.19fa801caa0.Coremail.jiezhilove@126.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/2e3ed175-8618-46a9-847b-b7e54597aa83%40gmail.com>
Precedence: bulk

On 7/28/26 4:15 AM, Yilin Zhang <jiezhilove@126.com> wrote:
> At 2026-07-13 16:13:58, "jian he" <jian.universality@gmail.com> wrote:
> > Hi.
> > While v2 has extensive failure test cases, it doesn't have a single
> > successful test case,
> > that's why this issue wasn't found.
> > The 2 issues you reported are fixed in the attached v3.
> >
> > Also, previously in v2:
> > +COPY r1 FROM STDIN WITH (DELIMITER ','); -- fail
> > +ERROR:  new row violates row-level security policy "p2" for table "r1"
> > +CONTEXT:  COPY r1, line 1: "4"
> >
> > it's better to remove the CONTEXT line.
> > because ExecWithCheckOptions has comments like:
> > /*
> > * For WITH CHECK OPTIONs coming from views, we might be
> > * able to provide the details on the row, depending on
> > * the permissions on the relation (that is, if the user
> > * could view it directly anyway).  For RLS violations, we
> > * don't include the data since we don't know if the user
> > * should be able to view the tuple as that depends on the
> > * USING policy.
> > */
> 
> 
> 
> Hi,
> I have reviewed the latest version of the patch, LGTM.
> 
> 
> Best regards,
> 
> --
> 
> Yilin Zhang
> 

The RLS enforcement in v3 looks right.  The WITH CHECK runs in the per-row
path for both the single and batched inserts, and row_security=off against a
forced policy errors the way INSERT does.  The patch has two other problems:
it can crash the backend, and it changes how COPY treats rules.

CopyFrom pushes the error-context callback under a guard and pops it without
one.  In master both are unconditional.  The patch wraps the push at
copyfrom.c:1211 in "if (queryDesc == NULL)" but leaves the pop at 1574,
"error_context_stack = errcallback.previous".  On the RLS path queryDesc is
set, so errcallback is never initialized, and the pop stores a garbage stack
value into error_context_stack.  The next ereport in CopyFrom walks it.  COPY
with ON_ERROR ignore and one bad row gets there through the "rows skipped"
NOTICE at 1580:

  CREATE ROLE alice LOGIN;
  CREATE TABLE t (id int, v text);
  ALTER TABLE t ENABLE ROW LEVEL SECURITY;
  CREATE POLICY p ON t USING (true) WITH CHECK (true);
  GRANT INSERT ON t TO alice;
  -- as alice, with a file whose second line has a non-integer id:
  COPY t FROM '...' WITH (ON_ERROR ignore);

That crashes a non-assert backend and takes the cluster into recovery, so any
role with INSERT on the table its policy applies to can crash the server.  The
same guard also drops the line-number context for every error on an RLS table,
not just policy violations.  Registering the callback unconditionally and
scoping relname_only to the ExecWithCheckOptions call balances the push and
pop and limits the suppression to the policy violation.

Separately, getting the WITH CHECK expressions from a full rewrite of a dummy
INSERT runs the target's ON INSERT rules on the RLS path, so the same COPY
behaves differently depending on whether the table has RLS enabled:

                        COPY into non-RLS table      COPY into RLS table
  DO ALSO               loads target, rule ignored   ERROR (unsupported)
  DO INSTEAD (uncond.)  loads the named table        loads the redirect target
  DO INSTEAD (cond.)    loads target, rule ignored   ERROR (unsupported)

The length check rejects the rewrites that fan out past one query, but a
single unconditional DO INSTEAD passes it, so COPY into an RLS table lands the
rows in the rule's redirect target rather than the named table.

The remaining point is cosmetic.  The query_string is only sourceText, but it
is built with raw %s and no quote-doubling, so it is malformed for a name like
foo"bar.  quote_qualified_identifier() would build it correctly.

bg