Hi,
On Sat, 13 Jul 2024 14:47:48 -0700
Noah Misch <noah@leadboat.com> wrote:
> On Fri, Jul 12, 2024 at 04:50:17PM -0700, Jeff Davis wrote:
> > On Fri, 2024-07-12 at 16:11 -0700, Noah Misch wrote:
> > > Since refresh->relation is a RangeVar, this departs from the standard
> > > against
> > > repeated name lookups, from CVE-2014-0062 (commit 5f17304).
> >
> > Interesting, thank you.
> >
> > I did a rough refactor and attached v3. Aside from cleanup issues, is
> > this what you had in mind?
>
> > +extern ObjectAddress RefreshMatViewByOid(Oid matviewOid, bool skipData, bool concurrent,
> > + const char *queryString, ParamListInfo params,
> > + QueryCompletion *qc);
> >
>
> Yes, that's an API design that avoids repeated name lookups.
Since this commit, matviews are no longer handled in ExecCreateTableAs, so the
following error message has not to consider materialized view cases, and can be made simple.
/* SELECT should never rewrite to more or less than one SELECT query */
if (list_length(rewritten) != 1)
elog(ERROR, "unexpected rewrite result for %s",
is_matview ? "CREATE MATERIALIZED VIEW" :
"CREATE TABLE AS SELECT");
RefreshMatViewByOid has REFRESH specific error messages in spite of its use
in CREATE MATERIALIZED VIEW, but these errors seem not to occur in CREATE MATERIALIZED
VIEW case, so I don't think it would be a problem.
Another my question is why RefreshMatViewByOid has a ParamListInfo parameter.
I don't understand why ExecRefreshMatView has one, either, because currently
materialized views may not be defined using bound parameters, which is checked
in transformCreateTableAsStmt, and the param argument is not used at all. It might
be unsafe to change the interface of ExecRefreshMatView since this is public for a
long time, but I don't think the new interface RefreshMatViewByOid has to have this
unused argument.
I attaehd patches for fixing them respectedly.
What do you think about it?
Regards,
Yugo Nagata
--
Yugo Nagata <nagata@sraoss.co.jp>