Hi,
On Sat, 13 Jul 2024 14:47:48 -0700
Noah Misch <n...@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 <nag...@sraoss.co.jp>
>From 854d01bfbdece781bad46ab686b08763b8d16a95 Mon Sep 17 00:00:00 2001
From: Yugo Nagata <nag...@sraoss.co.jp>
Date: Fri, 26 Jul 2024 12:02:56 +0900
Subject: [PATCH 2/2] Remove ParamListInfo argument from RefreshMatViewByOid
---
src/backend/commands/createas.c | 2 +-
src/backend/commands/matview.c | 5 ++---
src/include/commands/matview.h | 3 +--
3 files changed, 4 insertions(+), 6 deletions(-)
diff --git a/src/backend/commands/createas.c b/src/backend/commands/createas.c
index a15a1e187e..880425da66 100644
--- a/src/backend/commands/createas.c
+++ b/src/backend/commands/createas.c
@@ -345,7 +345,7 @@ ExecCreateTableAs(ParseState *pstate, CreateTableAsStmt *stmt,
if (do_refresh)
{
RefreshMatViewByOid(address.objectId, false, false,
- pstate->p_sourcetext, NULL, qc);
+ pstate->p_sourcetext, qc);
if (qc)
qc->commandTag = CMDTAG_SELECT;
diff --git a/src/backend/commands/matview.c b/src/backend/commands/matview.c
index 6bb64be274..aba30caf25 100644
--- a/src/backend/commands/matview.c
+++ b/src/backend/commands/matview.c
@@ -136,7 +136,7 @@ ExecRefreshMatView(RefreshMatViewStmt *stmt, const char *queryString,
NULL);
return RefreshMatViewByOid(matviewOid, stmt->skipData, stmt->concurrent,
- queryString, params, qc);
+ queryString, qc);
}
/*
@@ -160,8 +160,7 @@ ExecRefreshMatView(RefreshMatViewStmt *stmt, const char *queryString,
*/
ObjectAddress
RefreshMatViewByOid(Oid matviewOid, bool skipData, bool concurrent,
- const char *queryString, ParamListInfo params,
- QueryCompletion *qc)
+ const char *queryString, QueryCompletion *qc)
{
Relation matviewRel;
RewriteRule *rule;
diff --git a/src/include/commands/matview.h b/src/include/commands/matview.h
index a226b2e68f..5ff80788fa 100644
--- a/src/include/commands/matview.h
+++ b/src/include/commands/matview.h
@@ -26,8 +26,7 @@ extern void SetMatViewPopulatedState(Relation relation, bool newstate);
extern ObjectAddress ExecRefreshMatView(RefreshMatViewStmt *stmt, const char *queryString,
ParamListInfo params, QueryCompletion *qc);
extern ObjectAddress RefreshMatViewByOid(Oid matviewOid, bool skipData, bool concurrent,
- const char *queryString, ParamListInfo params,
- QueryCompletion *qc);
+ const char *queryString, QueryCompletion *qc);
extern DestReceiver *CreateTransientRelDestReceiver(Oid transientoid);
--
2.34.1
>From 4d2ae5bdb9eaf227b5e3f1278ce965a7732f514a Mon Sep 17 00:00:00 2001
From: Yugo Nagata <nag...@sraoss.co.jp>
Date: Fri, 26 Jul 2024 11:58:22 +0900
Subject: [PATCH 1/2] Fix an error message in ExecCreateTableAs
---
src/backend/commands/createas.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/src/backend/commands/createas.c b/src/backend/commands/createas.c
index 2c8a93b6e5..a15a1e187e 100644
--- a/src/backend/commands/createas.c
+++ b/src/backend/commands/createas.c
@@ -292,9 +292,7 @@ ExecCreateTableAs(ParseState *pstate, CreateTableAsStmt *stmt,
/* 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");
+ elog(ERROR, "unexpected rewrite result for CREATE TABLE AS SELECT");
query = linitial_node(Query, rewritten);
Assert(query->commandType == CMD_SELECT);
--
2.34.1
