This is an automated email from the ASF dual-hosted git repository. yjhjstz pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/cloudberry.git
commit f2245856fdcbba5a637b76b59470642615f05566 Author: Chris Hajas <[email protected]> AuthorDate: Mon Oct 10 15:40:47 2022 -0700 Fix dependency bug with minirepro and materialized views (#14223) Previously, when using minirepro on a query that accessed materialized views, the object that the materialized view used/accessed were not dumped with the minirepro. This is because queries themselves treat materialized views as tables, not views. However, when enumerating the dependent objects for dumping the relevant DDL, we need to treat materialized views as regular views. This commit sets a flag when dumping from gp_dump_query_oids so that materialized views are treated as regular views when parsing the query. --- src/backend/rewrite/rewriteHandler.c | 6 +++++- src/backend/utils/adt/gp_dump_oids.c | 11 ++++++----- src/include/nodes/parsenodes.h | 2 ++ src/include/utils/rel.h | 3 +-- src/test/regress/expected/gp_dump_query_oids.out | 17 +++++++++++++++++ src/test/regress/sql/gp_dump_query_oids.sql | 9 +++++++++ 6 files changed, 40 insertions(+), 8 deletions(-) diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c index 7ba5f9a56b..27e65a0b1d 100644 --- a/src/backend/rewrite/rewriteHandler.c +++ b/src/backend/rewrite/rewriteHandler.c @@ -2092,8 +2092,12 @@ fireRIRrules(Query *parsetree, List *activeRIRs) * expanded as if they were regular views, if they are not scannable. * In that case this test would need to be postponed till after we've * opened the rel, so that we could check its state. + * + * In the minirepro utility in GPDB, we use the expandMatViews flag + * to treat materialized views as regular views when dumping the + * DDL in order to dump dependent objects */ - if (rte->relkind == RELKIND_MATVIEW) + if (rte->relkind == RELKIND_MATVIEW && !parsetree->expandMatViews) continue; /* diff --git a/src/backend/utils/adt/gp_dump_oids.c b/src/backend/utils/adt/gp_dump_oids.c index 806c88e464..36fe718c28 100644 --- a/src/backend/utils/adt/gp_dump_oids.c +++ b/src/backend/utils/adt/gp_dump_oids.c @@ -18,6 +18,7 @@ #include "tcop/tcopprot.h" #include "optimizer/optimizer.h" #include "optimizer/planmain.h" +#include "parser/analyze.h" #include "utils/builtins.h" #include "utils/fmgroids.h" #include "utils/hsearch.h" @@ -120,11 +121,11 @@ gp_dump_query_oids(PG_FUNCTION_ARGS) RawStmt *parsetree = lfirst_node(RawStmt, lc); List *queryTree_sublist; - queryTree_sublist = pg_analyze_and_rewrite(parsetree, - sqlText, - NULL, - 0, - NULL); + + Query *query = parse_analyze(parsetree, sqlText, NULL, 0, NULL); + query->expandMatViews = true; + queryTree_sublist = pg_rewrite_query(query); + flat_query_list = list_concat(flat_query_list, list_copy(queryTree_sublist)); } diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index 2a3b3316c2..7a2118bad6 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -239,6 +239,8 @@ typedef struct Query */ int stmt_location; /* start location, or -1 if unknown */ int stmt_len; /* length in bytes; 0 means "rest of string" */ + + bool expandMatViews; /* force expansion of materialized views during rewrite to treat as views */ } Query; /**************************************************************************** diff --git a/src/include/utils/rel.h b/src/include/utils/rel.h index da05f580f8..08dc4a86fb 100644 --- a/src/include/utils/rel.h +++ b/src/include/utils/rel.h @@ -429,8 +429,7 @@ typedef struct ViewOptions * eval of argument! */ #define RelationIsSecurityView(relation) \ - (AssertMacro(relation->rd_rel->relkind == RELKIND_VIEW), \ - (relation)->rd_options ? \ + ((relation)->rd_options ? \ ((ViewOptions *) (relation)->rd_options)->security_barrier : false) /* diff --git a/src/test/regress/expected/gp_dump_query_oids.out b/src/test/regress/expected/gp_dump_query_oids.out index 0a58a93133..e5e98006b1 100644 --- a/src/test/regress/expected/gp_dump_query_oids.out +++ b/src/test/regress/expected/gp_dump_query_oids.out @@ -1,17 +1,28 @@ -- gp_dump_query_oids() doesn't list built-in functions, so we need a UDF to test with. CREATE FUNCTION dumptestfunc(t text) RETURNS integer AS $$ SELECT 123 $$ LANGUAGE SQL; CREATE FUNCTION dumptestfunc2(t text) RETURNS integer AS $$ SELECT 123 $$ LANGUAGE SQL; +create table base(a int); +NOTICE: Table doesn't have 'DISTRIBUTED BY' clause -- Using column named 'a' as the Greenplum Database data distribution key for this table. +HINT: The 'DISTRIBUTED BY' clause determines the distribution of data. Make sure column(s) chosen are the optimal data distribution key to minimize skew. +create materialized view base_mv as select a from base; +NOTICE: Table doesn't have 'DISTRIBUTED BY' clause. Creating a NULL policy entry. -- The function returns OIDs. We need to replace them with something reproducable. CREATE FUNCTION sanitize_output(t text) RETURNS text AS $$ declare dumptestfunc_oid oid; dumptestfunc2_oid oid; + base_oid oid; + base_mv_oid oid; begin dumptestfunc_oid = 'dumptestfunc'::regproc::oid; dumptestfunc2_oid = 'dumptestfunc2'::regproc::oid; + base_oid = 'base'::regclass::oid; + base_mv_oid = 'base_mv'::regclass::oid; t := replace(t, dumptestfunc_oid::text, '<dumptestfunc>'); t := replace(t, dumptestfunc2_oid::text, '<dumptestfunc2>'); + t := replace(t, base_oid::text, '<base_table>'); + t := replace(t, base_mv_oid::text, '<base_mv>'); RETURN t; end; @@ -127,6 +138,12 @@ SELECT array['pg_class'::regclass::oid::text] <@ (string_to_array((SELECT info- t (1 row) +SELECT sanitize_output(gp_dump_query_oids('SELECT * FROM base_mv')); + sanitize_output +----------------------------------------------------- + {"relids": "<base_mv>,<base_table>", "funcids": ""} +(1 row) + DROP TABLE foo; DROP TABLE cctable; DROP TABLE ctable; diff --git a/src/test/regress/sql/gp_dump_query_oids.sql b/src/test/regress/sql/gp_dump_query_oids.sql index dfd78ba55c..5a0152e21c 100644 --- a/src/test/regress/sql/gp_dump_query_oids.sql +++ b/src/test/regress/sql/gp_dump_query_oids.sql @@ -1,18 +1,26 @@ -- gp_dump_query_oids() doesn't list built-in functions, so we need a UDF to test with. CREATE FUNCTION dumptestfunc(t text) RETURNS integer AS $$ SELECT 123 $$ LANGUAGE SQL; CREATE FUNCTION dumptestfunc2(t text) RETURNS integer AS $$ SELECT 123 $$ LANGUAGE SQL; +create table base(a int); +create materialized view base_mv as select a from base; -- The function returns OIDs. We need to replace them with something reproducable. CREATE FUNCTION sanitize_output(t text) RETURNS text AS $$ declare dumptestfunc_oid oid; dumptestfunc2_oid oid; + base_oid oid; + base_mv_oid oid; begin dumptestfunc_oid = 'dumptestfunc'::regproc::oid; dumptestfunc2_oid = 'dumptestfunc2'::regproc::oid; + base_oid = 'base'::regclass::oid; + base_mv_oid = 'base_mv'::regclass::oid; t := replace(t, dumptestfunc_oid::text, '<dumptestfunc>'); t := replace(t, dumptestfunc2_oid::text, '<dumptestfunc2>'); + t := replace(t, base_oid::text, '<base_table>'); + t := replace(t, base_mv_oid::text, '<base_mv>'); RETURN t; end; @@ -74,6 +82,7 @@ SELECT array['ptable'::regclass::oid::text, 'ctable'::regclass::oid::text, 'cctable'::regclass::oid::text] <@ (string_to_array((SELECT info->>'relids' FROM minirepro_partition_test WHERE id = 2),',')); SELECT array['pg_class'::regclass::oid::text] <@ (string_to_array((SELECT info->>'relids' FROM minirepro_partition_test WHERE id = 3),',')); +SELECT sanitize_output(gp_dump_query_oids('SELECT * FROM base_mv')); DROP TABLE foo; DROP TABLE cctable; DROP TABLE ctable; --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
