Re: [HACKERS] MERGE SQL Statement for PG11 - Mailing list pgsql-hackers

From Peter Geoghegan
Subject Re: [HACKERS] MERGE SQL Statement for PG11
Date
Msg-id CAH2-WzkJdBuxj9PO=2QaO9-3h3xGbQPZ34kJH=HukRekwM-GZg@mail.gmail.com
Whole thread Raw
In response to [HACKERS] MERGE SQL Statement for PG11  (Simon Riggs <simon@2ndquadrant.com>)
Responses Re: [HACKERS] MERGE SQL Statement for PG11  (Pavan Deolasee <pavan.deolasee@gmail.com>)
Re: [HACKERS] MERGE SQL Statement for PG11  (Robert Haas <robertmhaas@gmail.com>)
List pgsql-hackers
On Thu, Mar 22, 2018 at 11:42 AM, Pavan Deolasee
<pavan.deolasee@gmail.com> wrote:
> A slightly improved version attached.

You still need to remove this change:

> diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h
> index a4574cd533..dbfb5d2a1a 100644
> --- a/src/include/miscadmin.h
> +++ b/src/include/miscadmin.h
> @@ -444,5 +444,6 @@ extern bool has_rolreplication(Oid roleid);
>  /* in access/transam/xlog.c */
>  extern bool BackupInProgress(void);
>  extern void CancelBackup(void);
> +extern int64 GetXactWALBytes(void);

I see that we're still using two target RTEs in this latest revision,
v23e -- the new approach to partitioning, which I haven't had time to
study in more detail, has not produced a change there. This causes
weird effects, such as the following:

"""
pg@~[20658]=# create table foo(bar int4);
CREATE TABLE
pg@~[20658]=# merge into foo f using (select 1 col) dd on f.bar=dd.col
when matched then update set bar = f.barr + 1;
ERROR:  column f.barr does not exist
LINE 1: ...n f.bar=dd.col when matched then update set bar = f.barr + 1...
                                                             ^
HINT:  Perhaps you meant to reference the column "f.bar" or the column "f.bar".

"""

While I think this this particular HINT buglet is pretty harmless, I
continue to be concerned about the unintended consequences of having
multiple RTEs for MERGE's target table. Each RTE comes from a
different lookup path -- the first one goes through setTargetTable()'s
parserOpenTable() + addRangeTableEntryForRelation() calls. The second
one goes through transformFromClauseItem(), for the join, which
ultimately ends up calling transformTableEntry()/addRangeTableEntry().
Consider commit 5f173040, which fixed a privilege escalation security
bug around multiple name lookup. Could the approach taken by MERGE
here introduce a similar security issue?

Using GDB, I see two calls to RangeVarGetRelidExtended() when a simple
MERGE is executed. They both have identical relation arguments, that
look like this:

(gdb) p *relation
$4 = {
  type = T_RangeVar,
  catalogname = 0x0,
  schemaname = 0x0,
  relname = 0x5600ebdcafb0 "foo",
  inh = 1 '\001',
  relpersistence = 112 'p',
  alias = 0x5600ebdcb048,
  location = 11
}

This seems like something that needs to be explained, at a minimum.
Even if I'm completely wrong about there being a security hazard,
maybe the suggestion that there might be still gives you some idea of
what I mean about unintended consequences.

-- 
Peter Geoghegan


pgsql-hackers by date:

Previous
From: Michael Banck
Date:
Subject: Re: [PATCH] Verify Checksums during Basebackups
Next
From: Peter Geoghegan
Date:
Subject: Re: [HACKERS] MERGE SQL Statement for PG11