public inbox for [email protected]  
help / color / mirror / Atom feed
[PATCH v23 05/15] Add Incremental View Maintenance support to pg_dump
4+ messages / 2 participants
[nested] [flat]

* [PATCH v23 05/15] Add Incremental View Maintenance support to pg_dump
@ 2020-11-11 08:01 Yugo Nagata <[email protected]>
  0 siblings, 0 replies; 4+ messages in thread

From: Yugo Nagata @ 2020-11-11 08:01 UTC (permalink / raw)

Support CREATE INCREMENTAL MATERIALIZED VIEW syntax.
---
 src/bin/pg_dump/pg_dump.c        | 42 ++++++++++++++++++++++++--------
 src/bin/pg_dump/pg_dump.h        |  1 +
 src/bin/pg_dump/t/002_pg_dump.pl | 15 ++++++++++++
 3 files changed, 48 insertions(+), 10 deletions(-)

diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 90ac445bcd..17e0a2193d 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -6268,6 +6268,7 @@ getTables(Archive *fout, int *numTables)
 	int			i_ispartition;
 	int			i_partbound;
 	int			i_amname;
+	int			i_isivm;
 
 	/*
 	 * Find all the tables and table-like objects.
@@ -6299,6 +6300,7 @@ getTables(Archive *fout, int *numTables)
 		char	   *ispartition = "false";
 		char	   *partbound = "NULL";
 		char	   *relhasoids = "c.relhasoids";
+		char	   *isivm = "false";
 
 		PQExpBuffer acl_subquery = createPQExpBuffer();
 		PQExpBuffer racl_subquery = createPQExpBuffer();
@@ -6326,6 +6328,10 @@ getTables(Archive *fout, int *numTables)
 		if (fout->remoteVersion >= 120000)
 			relhasoids = "'f'::bool";
 
+		/* The information about incremental view maintenance */
+		if (fout->remoteVersion >= 150000)
+			isivm = "c.relisivm";
+
 		/*
 		 * Left join to pick up dependency info linking sequences to their
 		 * owning column, if any (note this dependency is AUTO as of 8.2)
@@ -6384,7 +6390,8 @@ getTables(Archive *fout, int *numTables)
 						  "AS changed_acl, "
 						  "%s AS partkeydef, "
 						  "%s AS ispartition, "
-						  "%s AS partbound "
+						  "%s AS partbound, "
+						  "%s AS isivm "
 						  "FROM pg_class c "
 						  "LEFT JOIN pg_depend d ON "
 						  "(c.relkind = '%c' AND "
@@ -6413,6 +6420,7 @@ getTables(Archive *fout, int *numTables)
 						  partkeydef,
 						  ispartition,
 						  partbound,
+						  isivm,
 						  RELKIND_SEQUENCE,
 						  RELKIND_PARTITIONED_TABLE,
 						  RELKIND_RELATION, RELKIND_SEQUENCE,
@@ -6466,7 +6474,8 @@ getTables(Archive *fout, int *numTables)
 						  "NULL AS changed_acl, "
 						  "NULL AS partkeydef, "
 						  "false AS ispartition, "
-						  "NULL AS partbound "
+						  "NULL AS partbound, "
+						  "false AS isivm "
 						  "FROM pg_class c "
 						  "LEFT JOIN pg_depend d ON "
 						  "(c.relkind = '%c' AND "
@@ -6519,7 +6528,8 @@ getTables(Archive *fout, int *numTables)
 						  "NULL AS changed_acl, "
 						  "NULL AS partkeydef, "
 						  "false AS ispartition, "
-						  "NULL AS partbound "
+						  "NULL AS partbound, "
+						  "false AS isivm "
 						  "FROM pg_class c "
 						  "LEFT JOIN pg_depend d ON "
 						  "(c.relkind = '%c' AND "
@@ -6572,7 +6582,8 @@ getTables(Archive *fout, int *numTables)
 						  "NULL AS changed_acl, "
 						  "NULL AS partkeydef, "
 						  "false AS ispartition, "
-						  "NULL AS partbound "
+						  "NULL AS partbound, "
+						  "false AS isivm "
 						  "FROM pg_class c "
 						  "LEFT JOIN pg_depend d ON "
 						  "(c.relkind = '%c' AND "
@@ -6623,7 +6634,8 @@ getTables(Archive *fout, int *numTables)
 						  "NULL AS changed_acl, "
 						  "NULL AS partkeydef, "
 						  "false AS ispartition, "
-						  "NULL AS partbound "
+						  "NULL AS partbound, "
+						  "false AS isivm "
 						  "FROM pg_class c "
 						  "LEFT JOIN pg_depend d ON "
 						  "(c.relkind = '%c' AND "
@@ -6672,7 +6684,8 @@ getTables(Archive *fout, int *numTables)
 						  "NULL AS changed_acl, "
 						  "NULL AS partkeydef, "
 						  "false AS ispartition, "
-						  "NULL AS partbound "
+						  "NULL AS partbound, "
+						  "false AS isivm "
 						  "FROM pg_class c "
 						  "LEFT JOIN pg_depend d ON "
 						  "(c.relkind = '%c' AND "
@@ -6720,7 +6733,8 @@ getTables(Archive *fout, int *numTables)
 						  "NULL AS changed_acl, "
 						  "NULL AS partkeydef, "
 						  "false AS ispartition, "
-						  "NULL AS partbound "
+						  "NULL AS partbound, "
+						  "false AS isivm "
 						  "FROM pg_class c "
 						  "LEFT JOIN pg_depend d ON "
 						  "(c.relkind = '%c' AND "
@@ -6768,7 +6782,8 @@ getTables(Archive *fout, int *numTables)
 						  "NULL AS changed_acl, "
 						  "NULL AS partkeydef, "
 						  "false AS ispartition, "
-						  "NULL AS partbound "
+						  "NULL AS partbound, "
+						  "false AS isivm "
 						  "FROM pg_class c "
 						  "LEFT JOIN pg_depend d ON "
 						  "(c.relkind = '%c' AND "
@@ -6815,7 +6830,8 @@ getTables(Archive *fout, int *numTables)
 						  "NULL AS changed_acl, "
 						  "NULL AS partkeydef, "
 						  "false AS ispartition, "
-						  "NULL AS partbound "
+						  "NULL AS partbound, "
+						  "false AS isivm "
 						  "FROM pg_class c "
 						  "LEFT JOIN pg_depend d ON "
 						  "(c.relkind = '%c' AND "
@@ -6887,6 +6903,7 @@ getTables(Archive *fout, int *numTables)
 	i_ispartition = PQfnumber(res, "ispartition");
 	i_partbound = PQfnumber(res, "partbound");
 	i_amname = PQfnumber(res, "amname");
+	i_isivm = PQfnumber(res, "isivm");
 
 	if (dopt->lockWaitTimeout)
 	{
@@ -7000,6 +7017,9 @@ getTables(Archive *fout, int *numTables)
 		/* foreign server */
 		tblinfo[i].foreign_server = atooid(PQgetvalue(res, i, i_foreignserver));
 
+		/* Incremental view maintenance information */
+		tblinfo[i].isivm = (strcmp(PQgetvalue(res, i, i_isivm), "t") == 0);
+
 		/*
 		 * Read-lock target tables to make sure they aren't DROPPED or altered
 		 * in schema before we get around to dumping them.
@@ -15990,9 +16010,11 @@ dumpTableSchema(Archive *fout, const TableInfo *tbinfo)
 			binary_upgrade_set_pg_class_oids(fout, q,
 											 tbinfo->dobj.catId.oid, false);
 
-		appendPQExpBuffer(q, "CREATE %s%s %s",
+		appendPQExpBuffer(q, "CREATE %s%s%s %s",
 						  tbinfo->relpersistence == RELPERSISTENCE_UNLOGGED ?
 						  "UNLOGGED " : "",
+						  tbinfo->relkind == RELKIND_MATVIEW && tbinfo->isivm ?
+						  "INCREMENTAL " : "",
 						  reltypename,
 						  qualrelname);
 
diff --git a/src/bin/pg_dump/pg_dump.h b/src/bin/pg_dump/pg_dump.h
index f5e170e0db..ae8f24bb78 100644
--- a/src/bin/pg_dump/pg_dump.h
+++ b/src/bin/pg_dump/pg_dump.h
@@ -298,6 +298,7 @@ typedef struct _tableInfo
 	bool		dummy_view;		/* view's real definition must be postponed */
 	bool		postponed_def;	/* matview must be postponed into post-data */
 	bool		ispartition;	/* is table a partition? */
+	bool		isivm;			/* is incrementally maintainable materialized view? */
 
 	/*
 	 * These fields are computed only if we decide the table is interesting
diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl
index c5d8915be8..9d1776a506 100644
--- a/src/bin/pg_dump/t/002_pg_dump.pl
+++ b/src/bin/pg_dump/t/002_pg_dump.pl
@@ -2160,6 +2160,21 @@ my %tests = (
 		  { exclude_dump_test_schema => 1, no_toast_compression => 1, },
 	},
 
+	'CREATE MATERIALIZED VIEW matview_ivm' => {
+		create_order => 21,
+		create_sql   => 'CREATE INCREMENTAL MATERIALIZED VIEW dump_test.matview_ivm (col1) AS
+					   SELECT col1 FROM dump_test.test_table;',
+		regexp => qr/^
+			\QCREATE INCREMENTAL MATERIALIZED VIEW dump_test.matview_ivm AS\E
+			\n\s+\QSELECT test_table.col1\E
+			\n\s+\QFROM dump_test.test_table\E
+			\n\s+\QWITH NO DATA;\E
+			/xm,
+		like =>
+		  { %full_runs, %dump_test_schema_runs, section_pre_data => 1, },
+		unlike => { exclude_dump_test_schema => 1, },
+	},
+
 	'CREATE POLICY p1 ON test_table' => {
 		create_order => 22,
 		create_sql   => 'CREATE POLICY p1 ON dump_test.test_table
-- 
2.17.1


--Multipart=_Mon__2_Aug_2021_15_28_34_+0900_wlHCjIpnD/FrGAKu
Content-Type: text/x-diff;
 name="v23-0006-Add-Incremental-View-Maintenance-support-to-psql.patch"
Content-Disposition: attachment;
 filename="v23-0006-Add-Incremental-View-Maintenance-support-to-psql.patch"
Content-Transfer-Encoding: 7bit



^ permalink  raw  reply  [nested|flat] 4+ messages in thread

* Re: "variable not found in subplan target list"
@ 2023-03-28 12:56 Tom Lane <[email protected]>
  2023-03-28 13:17 ` Re: "variable not found in subplan target list" Tom Lane <[email protected]>
  0 siblings, 1 reply; 4+ messages in thread

From: Tom Lane @ 2023-03-28 12:56 UTC (permalink / raw)
  To: Alvaro Herrera <[email protected]>; +Cc: Pg Hackers <[email protected]>

Alvaro Herrera <[email protected]> writes:
> I have to run now so can't dissect it, but while running sqlsmith on the
> SQL/JSON patch after Justin's report, I got $SUBJECT in this query:

I reduced this down to

MERGE INTO public.target_parted as target_0
    USING public.itest1 as ref_0
    ON target_0.b = ref_0.a
    WHEN NOT MATCHED
       THEN INSERT VALUES (42, 13);

The critical moving part seems to just be that the MERGE target
is a partitioned table ... but surely somebody tested that before?

			regards, tom lane






^ permalink  raw  reply  [nested|flat] 4+ messages in thread

* Re: "variable not found in subplan target list"
  2023-03-28 12:56 Re: "variable not found in subplan target list" Tom Lane <[email protected]>
@ 2023-03-28 13:17 ` Tom Lane <[email protected]>
  2023-03-28 14:46   ` Re: "variable not found in subplan target list" Tom Lane <[email protected]>
  0 siblings, 1 reply; 4+ messages in thread

From: Tom Lane @ 2023-03-28 13:17 UTC (permalink / raw)
  To: Alvaro Herrera <[email protected]>; +Cc: Pg Hackers <[email protected]>

I wrote:
> I reduced this down to

> MERGE INTO public.target_parted as target_0
>     USING public.itest1 as ref_0
>     ON target_0.b = ref_0.a
>     WHEN NOT MATCHED
>        THEN INSERT VALUES (42, 13);

> The critical moving part seems to just be that the MERGE target
> is a partitioned table ... but surely somebody tested that before?

Oh, it's not just any partitioned table:

regression=# \d+ target_parted
                                Partitioned table "public.target_parted"
 Column |  Type   | Collation | Nullable | Default | Storage | Compression | Stats target | Description 
--------+---------+-----------+----------+---------+---------+-------------+--------------+-------------
 a      | integer |           |          |         | plain   |             |              | 
 b      | integer |           |          |         | plain   |             |              | 
Partition key: LIST (a)
Number of partitions: 0

The planner is reducing the scan of target_parted to
a dummy scan, as is reasonable, but it forgets to
provide ctid as an output from that scan; then the
parent join node is unhappy because it does have
a ctid output.  So it looks like the problem is some
shortcut we take while creating the dummy scan.

I suppose that without the planner bug, this'd fail at
runtime for lack of a partition to put (42,13) into.
Because of that, the case isn't really interesting
for production, which may explain the lack of reports.

			regards, tom lane






^ permalink  raw  reply  [nested|flat] 4+ messages in thread

* Re: "variable not found in subplan target list"
  2023-03-28 12:56 Re: "variable not found in subplan target list" Tom Lane <[email protected]>
  2023-03-28 13:17 ` Re: "variable not found in subplan target list" Tom Lane <[email protected]>
@ 2023-03-28 14:46   ` Tom Lane <[email protected]>
  0 siblings, 0 replies; 4+ messages in thread

From: Tom Lane @ 2023-03-28 14:46 UTC (permalink / raw)
  To: Alvaro Herrera <[email protected]>; +Cc: Pg Hackers <[email protected]>

I wrote:
> The planner is reducing the scan of target_parted to
> a dummy scan, as is reasonable, but it forgets to
> provide ctid as an output from that scan; then the
> parent join node is unhappy because it does have
> a ctid output.  So it looks like the problem is some
> shortcut we take while creating the dummy scan.

Oh, actually the problem is in distribute_row_identity_vars,
which is supposed to handle this case, but it thinks it doesn't
have to back-fill the rel's reltarget.  Wrong.  Now that I see
the problem, I wonder if we can't reproduce a similar symptom
without MERGE, which would mean that v14 has the issue too.

The attached seems to fix it, but I'm going to look for a
non-MERGE test case before pushing.

			regards, tom lane



Attachments:

  [text/x-diff] fix-missing-ctid-variable.patch (1.6K, ../../[email protected]/2-fix-missing-ctid-variable.patch)
  download | inline diff:
diff --git a/src/backend/optimizer/util/appendinfo.c b/src/backend/optimizer/util/appendinfo.c
index 9d377385f1..c1b1557570 100644
--- a/src/backend/optimizer/util/appendinfo.c
+++ b/src/backend/optimizer/util/appendinfo.c
@@ -21,6 +21,7 @@
 #include "nodes/nodeFuncs.h"
 #include "optimizer/appendinfo.h"
 #include "optimizer/pathnode.h"
+#include "optimizer/planmain.h"
 #include "parser/parsetree.h"
 #include "utils/lsyscache.h"
 #include "utils/rel.h"
@@ -994,9 +995,10 @@ distribute_row_identity_vars(PlannerInfo *root)
 	 * certainly process no rows.  Handle this edge case by re-opening the top
 	 * result relation and adding the row identity columns it would have used,
 	 * as preprocess_targetlist() would have done if it weren't marked "inh".
-	 * (This is a bit ugly, but it seems better to confine the ugliness and
-	 * extra cycles to this unusual corner case.)  We needn't worry about
-	 * fixing the rel's reltarget, as that won't affect the finished plan.
+	 * Then re-run build_base_rel_tlists() to ensure that the added columns
+	 * get propagated to the relation's reltarget.  (This is a bit ugly, but
+	 * it seems better to confine the ugliness and extra cycles to this
+	 * unusual corner case.)
 	 */
 	if (root->row_identity_vars == NIL)
 	{
@@ -1006,6 +1008,8 @@ distribute_row_identity_vars(PlannerInfo *root)
 		add_row_identity_columns(root, result_relation,
 								 target_rte, target_relation);
 		table_close(target_relation, NoLock);
+		build_base_rel_tlists(root, root->processed_tlist);
+		/* There are no ROWID_VAR Vars in this case, so we're done. */
 		return;
 	}
 


^ permalink  raw  reply  [nested|flat] 4+ messages in thread


end of thread, other threads:[~2023-03-28 14:46 UTC | newest]

Thread overview: 4+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-11-11 08:01 [PATCH v23 05/15] Add Incremental View Maintenance support to pg_dump Yugo Nagata <[email protected]>
2023-03-28 12:56 Re: "variable not found in subplan target list" Tom Lane <[email protected]>
2023-03-28 13:17 ` Re: "variable not found in subplan target list" Tom Lane <[email protected]>
2023-03-28 14:46   ` Re: "variable not found in subplan target list" Tom Lane <[email protected]>

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox