On Thu, Mar 22, 2018 at 11:42 AM, Pavan Deolasee
<[email protected]> 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
