Re: Feature: temporary materialized views
On 3/15/19 3:19 AM, Mitar wrote: On Thu, Mar 14, 2019 at 7:56 AM Andreas Karlsson wrote: Yeah, your patch is sadly stuck behind the refactoring, and the refactoring proved to be harder to do than I initially thought. The different code paths for executing CREATE MATERIALIZED VIEW are so different that it is hard to find a good common interface. So there is unfortunately little you can do here other than wait for me or someone else to do the refactoring as I cannot see your patch getting accepted without keeping the existing restrictions on side effects for CREATE MATERIALIZED VIEW. Sounds good. I will wait. This patch has been marked as Returned with Feedback since it is not clear when the refactoring it depends on will be done. You can submit to a future commitfest when you are able to produce a new patch. Regards, -- -David da...@pgmasters.net
Re: Feature: temporary materialized views
Hi! On Thu, Mar 14, 2019 at 7:56 AM Andreas Karlsson wrote: > Yeah, your patch is sadly stuck behind the refactoring, and the > refactoring proved to be harder to do than I initially thought. The > different code paths for executing CREATE MATERIALIZED VIEW are so > different that it is hard to find a good common interface. > > So there is unfortunately little you can do here other than wait for me > or someone else to do the refactoring as I cannot see your patch getting > accepted without keeping the existing restrictions on side effects for > CREATE MATERIALIZED VIEW. Sounds good. I will wait. Thanks. Mitar -- http://mitar.tnode.com/ https://twitter.com/mitar_m
Re: Feature: temporary materialized views
On 3/14/19 9:13 AM, Mitar wrote:> I just want to make sure if I understand correctly. But my initial proposal/patch is currently waiting first for all patches for the refactoring to happen, which are done by amazing Andreas? This sounds good to me and I see a lot of progress/work has been done and I am OK with waiting. Please ping me explicitly if there will be anything I am expected to do at any point in time. And just to make sure, these current patches are doing just refactoring but are not also introducing temporary materialized views yet? Or is that also done in patches made by Andreas? Yeah, your patch is sadly stuck behind the refactoring, and the refactoring proved to be harder to do than I initially thought. The different code paths for executing CREATE MATERIALIZED VIEW are so different that it is hard to find a good common interface. So there is unfortunately little you can do here other than wait for me or someone else to do the refactoring as I cannot see your patch getting accepted without keeping the existing restrictions on side effects for CREATE MATERIALIZED VIEW. Andreas
Re: Feature: temporary materialized views
Hi! I just want to make sure if I understand correctly. But my initial proposal/patch is currently waiting first for all patches for the refactoring to happen, which are done by amazing Andreas? This sounds good to me and I see a lot of progress/work has been done and I am OK with waiting. Please ping me explicitly if there will be anything I am expected to do at any point in time. And just to make sure, these current patches are doing just refactoring but are not also introducing temporary materialized views yet? Or is that also done in patches made by Andreas? Mitar -- http://mitar.tnode.com/ https://twitter.com/mitar_m
Re: Feature: temporary materialized views
On 3/8/19 2:38 AM, Michael Paquier wrote: On Thu, Mar 07, 2019 at 10:45:04AM +0200, David Steele wrote: I think a new patch is required here so I have marked this Waiting on Author. cfbot is certainly not happy and anyone trying to review is going to have hard time trying to determine what to review. I would recommend to mark this patch as returned with feedback as we already know that we need to rethink a bit harder the way relations are created in CTAS, not to mention that the case of EXPLAIN CTAS IF NOT EXISTS is not correctly handled. This requires more than three of work which is what remains until the end of this CF, so v12 is not a sane target. Agreed. Even if I could find the time to write a patch for this there is no way it would make it into v12. Andreas
Re: Feature: temporary materialized views
On 3/8/19 3:38 AM, Michael Paquier wrote: On Thu, Mar 07, 2019 at 10:45:04AM +0200, David Steele wrote: I think a new patch is required here so I have marked this Waiting on Author. cfbot is certainly not happy and anyone trying to review is going to have hard time trying to determine what to review. I would recommend to mark this patch as returned with feedback as we already know that we need to rethink a bit harder the way relations are created in CTAS, not to mention that the case of EXPLAIN CTAS IF NOT EXISTS is not correctly handled. This requires more than three of work which is what remains until the end of this CF, so v12 is not a sane target. OK, I will do that on March 13th if there are no arguments to the contrary. Regards, -- -David da...@pgmasters.net
Re: Re: Feature: temporary materialized views
On Thu, Mar 07, 2019 at 10:45:04AM +0200, David Steele wrote: > I think a new patch is required here so I have marked this Waiting on > Author. cfbot is certainly not happy and anyone trying to review is going > to have hard time trying to determine what to review. I would recommend to mark this patch as returned with feedback as we already know that we need to rethink a bit harder the way relations are created in CTAS, not to mention that the case of EXPLAIN CTAS IF NOT EXISTS is not correctly handled. This requires more than three of work which is what remains until the end of this CF, so v12 is not a sane target. -- Michael signature.asc Description: PGP signature
Re: Re: Feature: temporary materialized views
On 2/7/19 2:23 AM, Michael Paquier wrote: On Wed, Feb 06, 2019 at 05:05:56PM +0100, Andreas Karlsson wrote: On 2/6/19 10:18 AM, Michael Paquier wrote: Attached is a patch to do that and close the gap. With that, we will be able to check for inconsistencies better when working on the follow-up patches. What do you think? I approve. I was when testing this stuff that I found the IF NOT EXISTS issue. Thanks, I have committed those extra tests to close the gap. I think a new patch is required here so I have marked this Waiting on Author. cfbot is certainly not happy and anyone trying to review is going to have hard time trying to determine what to review. Regards, -- -David da...@pgmasters.net
Re: Feature: temporary materialized views
On Wed, Feb 06, 2019 at 05:05:56PM +0100, Andreas Karlsson wrote: > On 2/6/19 10:18 AM, Michael Paquier wrote: >> Attached is a patch to do that and close the gap. With that, we will >> be able to check for inconsistencies better when working on the >> follow-up patches. What do you think? > > I approve. I was when testing this stuff that I found the IF NOT EXISTS > issue. Thanks, I have committed those extra tests to close the gap. -- Michael signature.asc Description: PGP signature
Re: Feature: temporary materialized views
On 2/6/19 10:18 AM, Michael Paquier wrote: Attached is a patch to do that and close the gap. With that, we will be able to check for inconsistencies better when working on the follow-up patches. What do you think? I approve. I was when testing this stuff that I found the IF NOT EXISTS issue. Andreas
Re: Feature: temporary materialized views
On Tue, Feb 05, 2019 at 06:56:00PM +0100, Andreas Karlsson wrote: > I guess that I could fix that for the second case as soon as I understand > how much of the portal stuff can be skipped in ExecuteQuery(). But I am not > sure what we should do with EXPLAIN ANALYZE ... NO DATA. It feels like a > contraction to me. Should we just raise an error? Or should we try to > preserve the current behavior where you see something like the > below? This grammar is documented, so it seems to me that it would be just annoying for users relying on it to throw an error for a pattern that simply worked, particularly if a driver layer is using it. The issue this outlines is that we have a gap in the tests for a subset of the grammar, which is not a good thing. If I put Assert(!into->skipData) at the beginning of intorel_startup() then the main regression test suite is able to pass, both on HEAD and with your patch. There is one test for CTAS EXECUTE in prepare.sql, so let's add a pattern with WITH NO DATA there for the first pattern. Adding a second test with EXPLAIN SELECT INTO into select_into.sql also looks like a good thing. Attached is a patch to do that and close the gap. With that, we will be able to check for inconsistencies better when working on the follow-up patches. What do you think? -- Michael diff --git a/src/test/regress/expected/prepare.out b/src/test/regress/expected/prepare.out index d07c0cc9c9..717732300d 100644 --- a/src/test/regress/expected/prepare.out +++ b/src/test/regress/expected/prepare.out @@ -145,6 +145,13 @@ SELECT * FROM q5_prep_results; 9961 |2058 | 1 |1 | 1 | 1 | 61 | 961 |1961 | 4961 | 9961 | 122 | 123 | DT | EBDAAA | xx (16 rows) +CREATE TEMPORARY TABLE q5_prep_nodata AS EXECUTE q5(200, 'DT') +WITH NO DATA; +SELECT * FROM q5_prep_nodata; + unique1 | unique2 | two | four | ten | twenty | hundred | thousand | twothousand | fivethous | tenthous | odd | even | stringu1 | stringu2 | string4 +-+-+-+--+-++-+--+-+---+--+-+--+--+--+- +(0 rows) + -- unknown or unspecified parameter types: should succeed PREPARE q6 AS SELECT * FROM tenk1 WHERE unique1 = $1 AND stringu1 = $2; diff --git a/src/test/regress/expected/select_into.out b/src/test/regress/expected/select_into.out index 942f975e95..f373fae679 100644 --- a/src/test/regress/expected/select_into.out +++ b/src/test/regress/expected/select_into.out @@ -112,14 +112,15 @@ SELECT * FROM created_table; 4567890123456789 | -4567890123456789 (5 rows) --- Try EXPLAIN ANALYZE SELECT INTO, but hide the output since it won't --- be stable. +-- Try EXPLAIN ANALYZE SELECT INTO and EXPLAIN ANALYZE CREATE TABLE AS +-- WITH NO DATA, but hide the outputs since they won't be stable. DO $$ BEGIN EXECUTE 'EXPLAIN ANALYZE SELECT * INTO TABLE easi FROM int8_tbl'; + EXECUTE 'EXPLAIN ANALYZE CREATE TABLE easi2 AS SELECT * FROM int8_tbl WITH NO DATA'; END$$; DROP TABLE created_table; -DROP TABLE easi; +DROP TABLE easi, easi2; -- -- Disallowed uses of SELECT ... INTO. All should fail -- diff --git a/src/test/regress/sql/prepare.sql b/src/test/regress/sql/prepare.sql index 7fe8c8d7f5..985d0f05c9 100644 --- a/src/test/regress/sql/prepare.sql +++ b/src/test/regress/sql/prepare.sql @@ -61,6 +61,9 @@ PREPARE q5(int, text) AS ORDER BY unique1; CREATE TEMPORARY TABLE q5_prep_results AS EXECUTE q5(200, 'DT'); SELECT * FROM q5_prep_results; +CREATE TEMPORARY TABLE q5_prep_nodata AS EXECUTE q5(200, 'DT') +WITH NO DATA; +SELECT * FROM q5_prep_nodata; -- unknown or unspecified parameter types: should succeed PREPARE q6 AS diff --git a/src/test/regress/sql/select_into.sql b/src/test/regress/sql/select_into.sql index 62eddeed9d..a708fef0ea 100644 --- a/src/test/regress/sql/select_into.sql +++ b/src/test/regress/sql/select_into.sql @@ -85,15 +85,16 @@ SELECT make_table(); SELECT * FROM created_table; --- Try EXPLAIN ANALYZE SELECT INTO, but hide the output since it won't --- be stable. +-- Try EXPLAIN ANALYZE SELECT INTO and EXPLAIN ANALYZE CREATE TABLE AS +-- WITH NO DATA, but hide the outputs since they won't be stable. DO $$ BEGIN EXECUTE 'EXPLAIN ANALYZE SELECT * INTO TABLE easi FROM int8_tbl'; + EXECUTE 'EXPLAIN ANALYZE CREATE TABLE easi2 AS SELECT * FROM int8_tbl WITH NO DATA'; END$$; DROP TABLE created_table; -DROP TABLE easi; +DROP TABLE easi, easi2; -- -- Disallowed uses of SELECT ... INTO. All should fail signature.asc Description: PGP signature
Re: Feature: temporary materialized views
On 2/5/19 6:56 PM, Andreas Karlsson wrote: On 2/5/19 12:36 PM, Michael Paquier wrote:> - skipData is visibly always false. > We may want to keep skipData to have an assertion at the beginning of > inforel_startup for sanity purposes though. This is not true in this version of the patch. The following two cases would crash if we add such an assertion: EXPLAIN ANALYZE CREATE TABLE foo AS SELECT 1 WITH NO DATA; and PREPARE s AS SELECT 1; CREATE TABLE bar AS EXECUTE s WITH NO DATA; since they both still run the setup and tear down steps of the executor. I guess that I could fix that for the second case as soon as I understand how much of the portal stuff can be skipped in ExecuteQuery(). But I am not sure what we should do with EXPLAIN ANALYZE ... NO DATA. It feels like a contraction to me. Should we just raise an error? Or should we try to preserve the current behavior where you see something like the below? In general I do not like how EXPLAIN CREATE TABLE AS uses a separate code path than CREATE TABLE AS, because we get weird but mostly harmless edge cases like the below and that I do not think that EXPLAIN ANALYZE CREATE MATERIALIZED VIEW sets the security context properly. I am not sure if any of this is worth fixing, but it certainly contributed to why I thought that it was hard to design a good API. postgres=# EXPLAIN ANALYZE CREATE TABLE IF NOT EXISTS bar AS SELECT 1; QUERY PLAN Result (cost=0.00..0.01 rows=1 width=4) (actual time=0.001..0.002 rows=1 loops=1) Planning Time: 0.030 ms Execution Time: 12.245 ms (3 rows) Time: 18.223 ms postgres=# EXPLAIN ANALYZE CREATE TABLE IF NOT EXISTS bar AS SELECT 1; ERROR: relation "bar" already exists Time: 2.129 ms Andreas
Re: Feature: temporary materialized views
On 2/5/19 12:36 PM, Michael Paquier wrote:> - skipData is visibly always false. > We may want to keep skipData to have an assertion at the beginning of > inforel_startup for sanity purposes though. This is not true in this version of the patch. The following two cases would crash if we add such an assertion: EXPLAIN ANALYZE CREATE TABLE foo AS SELECT 1 WITH NO DATA; and PREPARE s AS SELECT 1; CREATE TABLE bar AS EXECUTE s WITH NO DATA; since they both still run the setup and tear down steps of the executor. I guess that I could fix that for the second case as soon as I understand how much of the portal stuff can be skipped in ExecuteQuery(). But I am not sure what we should do with EXPLAIN ANALYZE ... NO DATA. It feels like a contraction to me. Should we just raise an error? Or should we try to preserve the current behavior where you see something like the below? QUERY PLAN --- Result (cost=0.00..0.01 rows=1 width=4) (never executed) Planning Time: 0.040 ms Execution Time: 0.002 ms (3 rows) > 2) DefineIntoRelForDestReceiver is just a wrapper for > create_ctas_nodata, so we had better just merge both of them and > expose directly the routine creating the relation definition, so the > new interface is a bit awkward. Agreed, the API is awakward as it is now but it was the least awkward one I managed to design. But I think if we fix the issue above then it might be possible to create a less awkward API. > 3) The part about the regression diff is well... Expected... We may > want a comment about that. We could consider as well adding a > regression test inspired from REINDEX SCHEMA to show that the CTAS is > created before the data is actually filled in. Yeah, that sounds like a good idea. Andreas
Re: Feature: temporary materialized views
Hi Andreas, On Tue, Feb 05, 2019 at 12:59:12PM +0900, Michael Paquier wrote: > Now... You have on this thread all the audience which already worked > on 874fe3a. And I am just looking at this patch, evaluating the > behavior change this is introducing. Still I would recommend a > separate thread as others may want to comment on that particular > point. So I have read through your patch, and there are a couple of things which I think we could simplify more. Here are my notes: 1) We could remove the into clause from DR_intorel, which is used for two things: - Determine the relkind of the relation created. However the relation gets created before entering in the executor, and we already know its OID, so we also know its relkind. - skipData is visibly always false. We may want to keep skipData to have an assertion at the beginning of inforel_startup for sanity purposes though. 2) DefineIntoRelForDestReceiver is just a wrapper for create_ctas_nodata, so we had better just merge both of them and expose directly the routine creating the relation definition, so the new interface is a bit awkward. 3) The part about the regression diff is well... Expected... We may want a comment about that. We could consider as well adding a regression test inspired from REINDEX SCHEMA to show that the CTAS is created before the data is actually filled in. -- Michael signature.asc Description: PGP signature
Re: Feature: temporary materialized views
On Mon, Feb 04, 2019 at 04:10:09PM +0100, Andreas Karlsson wrote: > Should I submit it as a separate CF entry or is it easiest if my refactoring > and Mi Tar's feature are reviewed together? The refactoring patch is talking about changing the way objects are created within a CTAS, which is quite different from what is proposed on this thread, so in order to attract the correct audience a separate thread with another CF entry seems more appropriate. Now... You have on this thread all the audience which already worked on 874fe3a. And I am just looking at this patch, evaluating the behavior change this is introducing. Still I would recommend a separate thread as others may want to comment on that particular point. -- Michael signature.asc Description: PGP signature
Re: Feature: temporary materialized views
On 2/4/19 7:09 AM, Michael Paquier wrote: On Tue, Jan 22, 2019 at 03:10:17AM +0100, Andreas Karlsson wrote: On 1/21/19 3:31 AM, Andreas Karlsson wrote: Here is a a stab at refactoring this so the object creation does not happen in a callback. Rebased my patch on top of Andres's pluggable storage patches. Plus some minor style changes. Taking a note to look at this refactoring bit, which is different from the temp matview part. Moved to next CF for now. Should I submit it as a separate CF entry or is it easiest if my refactoring and Mi Tar's feature are reviewed together? Andreas
Re: Feature: temporary materialized views
On Tue, Jan 22, 2019 at 03:10:17AM +0100, Andreas Karlsson wrote: > On 1/21/19 3:31 AM, Andreas Karlsson wrote: > > Here is a a stab at refactoring this so the object creation does not > > happen in a callback. > > Rebased my patch on top of Andres's pluggable storage patches. Plus some > minor style changes. Taking a note to look at this refactoring bit, which is different from the temp matview part. Moved to next CF for now. -- Michael signature.asc Description: PGP signature
Re: Feature: temporary materialized views
On 1/21/19 3:31 AM, Andreas Karlsson wrote:
Here is a a stab at refactoring this so the object creation does not
happen in a callback.
Rebased my patch on top of Andres's pluggable storage patches. Plus some
minor style changes.
Andreas
diff --git a/src/backend/commands/createas.c b/src/backend/commands/createas.c
index 2bc8f928ea..7ef3794e08 100644
--- a/src/backend/commands/createas.c
+++ b/src/backend/commands/createas.c
@@ -55,16 +55,15 @@ typedef struct
{
DestReceiver pub; /* publicly-known function pointers */
IntoClause *into; /* target relation specification */
+ ObjectAddress reladdr; /* address of rel, for intorel_startup */
/* These fields are filled by intorel_startup: */
Relation rel; /* relation to write to */
- ObjectAddress reladdr; /* address of rel, for ExecCreateTableAs */
CommandId output_cid; /* cmin to insert in output tuples */
int hi_options; /* heap_insert performance options */
BulkInsertState bistate; /* bulk insert state */
} DR_intorel;
-/* utility functions for CTAS definition creation */
-static ObjectAddress create_ctas_internal(List *attrList, IntoClause *into);
+/* utility function for CTAS definition creation */
static ObjectAddress create_ctas_nodata(List *tlist, IntoClause *into);
/* DestReceiver routines for collecting data */
@@ -75,16 +74,18 @@ static void intorel_destroy(DestReceiver *self);
/*
- * create_ctas_internal
+ * create_ctas_nodata
*
- * Internal utility used for the creation of the definition of a relation
- * created via CREATE TABLE AS or a materialized view. Caller needs to
- * provide a list of attributes (ColumnDef nodes).
+ * Create CTAS or materialized view without the data, starting from the
+ * targetlist of the SELECT or view definition.
*/
static ObjectAddress
-create_ctas_internal(List *attrList, IntoClause *into)
+create_ctas_nodata(List *tlist, IntoClause *into)
{
- CreateStmt *create = makeNode(CreateStmt);
+ List *attrList;
+ ListCell *t,
+ *lc;
+ CreateStmt *create;
bool is_matview;
char relkind;
Datum toast_options;
@@ -96,71 +97,6 @@ create_ctas_internal(List *attrList, IntoClause *into)
relkind = is_matview ? RELKIND_MATVIEW : RELKIND_RELATION;
/*
- * Create the target relation by faking up a CREATE TABLE parsetree and
- * passing it to DefineRelation.
- */
- create->relation = into->rel;
- create->tableElts = attrList;
- create->inhRelations = NIL;
- create->ofTypename = NULL;
- create->constraints = NIL;
- create->options = into->options;
- create->oncommit = into->onCommit;
- create->tablespacename = into->tableSpaceName;
- create->if_not_exists = false;
-
- /*
- * Create the relation. (This will error out if there's an existing view,
- * so we don't need more code to complain if "replace" is false.)
- */
- intoRelationAddr = DefineRelation(create, relkind, InvalidOid, NULL, NULL);
-
- /*
- * If necessary, create a TOAST table for the target table. Note that
- * NewRelationCreateToastTable ends with CommandCounterIncrement(), so
- * that the TOAST table will be visible for insertion.
- */
- CommandCounterIncrement();
-
- /* parse and validate reloptions for the toast table */
- toast_options = transformRelOptions((Datum) 0,
- create->options,
- "toast",
- validnsps,
- true, false);
-
- (void) heap_reloptions(RELKIND_TOASTVALUE, toast_options, true);
-
- NewRelationCreateToastTable(intoRelationAddr.objectId, toast_options);
-
- /* Create the "view" part of a materialized view. */
- if (is_matview)
- {
- /* StoreViewQuery scribbles on tree, so make a copy */
- Query *query = (Query *) copyObject(into->viewQuery);
-
- StoreViewQuery(intoRelationAddr.objectId, query, false);
- CommandCounterIncrement();
- }
-
- return intoRelationAddr;
-}
-
-
-/*
- * create_ctas_nodata
- *
- * Create CTAS or materialized view when WITH NO DATA is used, starting from
- * the targetlist of the SELECT or view definition.
- */
-static ObjectAddress
-create_ctas_nodata(List *tlist, IntoClause *into)
-{
- List *attrList;
- ListCell *t,
- *lc;
-
- /*
* Build list of ColumnDefs from non-junk elements of the tlist. If a
* column name list was specified in CREATE TABLE AS, override the column
* names in the query. (Too few column names are OK, too many are not.)
@@ -213,8 +149,56 @@ create_ctas_nodata(List *tlist, IntoClause *into)
(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("too many column names were specified")));
- /* Create the relation definition using the ColumnDef list */
- return create_ctas_internal(attrList, into);
+ /*
+ * Create the target relation by faking up a CREATE TABLE parsetree and
+ * passing it to DefineRelation.
+ */
+ create = makeNode(CreateStmt);
+ create->relation = into->rel;
+ create->tableElts = attrList;
+ create->inhRelations = NIL;
+ create->ofTypename = NULL;
+ create->constraints = NIL;
+ create->options = into->options;
+ create->oncommit = into->onCommit;
+
Re: Feature: temporary materialized views
On 1/18/19 8:32 PM, Mitar wrote:
On Fri, Jan 18, 2019 at 7:18 AM Andreas Karlsson wrote:
These rules are usually pretty easy to add. Just take a look in
src/bin/psql/tab-complete.c to see how it is usually done.
Thanks. I have added the auto-complete and attached a new patch.
Hm, I do not think we should complete UNLOGGED MATERIALIZED VIEW even
though it is valid syntax. If you try to create one you will just get an
error. I am leaning towards removing the existing completion for this,
because I do not see the point of completing to useless but technically
valid syntax.
This is the one I think we should probably remove:
else if (TailMatches("CREATE", "UNLOGGED"))
COMPLETE_WITH("TABLE", "MATERIALIZED VIEW");
I might take a stab at refactoring this myself this weekend. Hopefully
it is not too involved.
That would be great! I can afterwards update the patch accordingly.
I have submitted a first shot at this. Let's see what others think of my
patch.
Andreas
Re: Feature: temporary materialized views
On 1/17/19 8:31 PM, Tom Lane wrote:
Creating the view object inside the rStartup callback is itself pretty
much of a kluge; you'd expect that to happen earlier. I think the
reason it was done that way was it was easier to find out the view's
column set there, but I'm sure we can find another way --- doing the
object creation more like a regular view does it is the obvious approach.
Here is a a stab at refactoring this so the object creation does not
happen in a callback. I am not that fond of the new API, but given how
different all the various callers of CreateIntoRelDestReceiver() are I
had no better idea.
The idea behind the patch is to always create the empty
table/materialized view before executing the query and do it in one more
unified code path, and then later populate it unless NO DATA was
specified. I feel this makes the code easier to follow.
This patch introduces a small behavioral change, as can be seen from the
diff in the test suite, where since the object creation is moved earlier
the CTAS query snapshot will for example see the newly created table.
The new behavior seems more correct to me, but maybe I am missing some
unintended consequences.
Andreas
diff --git a/src/backend/commands/createas.c b/src/backend/commands/createas.c
index 5947996d67..6ee96628cf 100644
--- a/src/backend/commands/createas.c
+++ b/src/backend/commands/createas.c
@@ -55,16 +55,15 @@ typedef struct
{
DestReceiver pub; /* publicly-known function pointers */
IntoClause *into; /* target relation specification */
+ ObjectAddress reladdr; /* address of rel, for intorel_startup */
/* These fields are filled by intorel_startup: */
Relation rel; /* relation to write to */
- ObjectAddress reladdr; /* address of rel, for ExecCreateTableAs */
CommandId output_cid; /* cmin to insert in output tuples */
int hi_options; /* heap_insert performance options */
BulkInsertState bistate; /* bulk insert state */
} DR_intorel;
-/* utility functions for CTAS definition creation */
-static ObjectAddress create_ctas_internal(List *attrList, IntoClause *into);
+/* utility function for CTAS definition creation */
static ObjectAddress create_ctas_nodata(List *tlist, IntoClause *into);
/* DestReceiver routines for collecting data */
@@ -75,16 +74,18 @@ static void intorel_destroy(DestReceiver *self);
/*
- * create_ctas_internal
+ * create_ctas_nodata
*
- * Internal utility used for the creation of the definition of a relation
- * created via CREATE TABLE AS or a materialized view. Caller needs to
- * provide a list of attributes (ColumnDef nodes).
+ * Create CTAS or materialized view without the data, starting from the
+ * targetlist of the SELECT or view definition.
*/
static ObjectAddress
-create_ctas_internal(List *attrList, IntoClause *into)
+create_ctas_nodata(List *tlist, IntoClause *into)
{
- CreateStmt *create = makeNode(CreateStmt);
+ List *attrList;
+ ListCell *t,
+ *lc;
+ CreateStmt *create;
bool is_matview;
char relkind;
Datum toast_options;
@@ -96,71 +97,6 @@ create_ctas_internal(List *attrList, IntoClause *into)
relkind = is_matview ? RELKIND_MATVIEW : RELKIND_RELATION;
/*
- * Create the target relation by faking up a CREATE TABLE parsetree and
- * passing it to DefineRelation.
- */
- create->relation = into->rel;
- create->tableElts = attrList;
- create->inhRelations = NIL;
- create->ofTypename = NULL;
- create->constraints = NIL;
- create->options = into->options;
- create->oncommit = into->onCommit;
- create->tablespacename = into->tableSpaceName;
- create->if_not_exists = false;
-
- /*
- * Create the relation. (This will error out if there's an existing view,
- * so we don't need more code to complain if "replace" is false.)
- */
- intoRelationAddr = DefineRelation(create, relkind, InvalidOid, NULL, NULL);
-
- /*
- * If necessary, create a TOAST table for the target table. Note that
- * NewRelationCreateToastTable ends with CommandCounterIncrement(), so
- * that the TOAST table will be visible for insertion.
- */
- CommandCounterIncrement();
-
- /* parse and validate reloptions for the toast table */
- toast_options = transformRelOptions((Datum) 0,
- create->options,
- "toast",
- validnsps,
- true, false);
-
- (void) heap_reloptions(RELKIND_TOASTVALUE, toast_options, true);
-
- NewRelationCreateToastTable(intoRelationAddr.objectId, toast_options);
-
- /* Create the "view" part of a materialized view. */
- if (is_matview)
- {
- /* StoreViewQuery scribbles on tree, so make a copy */
- Query *query = (Query *) copyObject(into->viewQuery);
-
- StoreViewQuery(intoRelationAddr.objectId, query, false);
- CommandCounterIncrement();
- }
-
- return intoRelationAddr;
-}
-
-
-/*
- * create_ctas_nodata
- *
- * Create CTAS or materialized view when WITH NO DATA is used, starting from
- * the targetlist of the SELECT or view definition.
- */
-static ObjectAddress
-create_ctas_nodata(List *tlist,
Re: Feature: temporary materialized views
Hi!
On Fri, Jan 18, 2019 at 7:18 AM Andreas Karlsson wrote:
> These rules are usually pretty easy to add. Just take a look in
> src/bin/psql/tab-complete.c to see how it is usually done.
Thanks. I have added the auto-complete and attached a new patch.
> I might take a stab at refactoring this myself this weekend. Hopefully
> it is not too involved.
That would be great! I can afterwards update the patch accordingly.
Mitar
--
http://mitar.tnode.com/
https://twitter.com/mitar_m
diff --git a/doc/src/sgml/ref/create_materialized_view.sgml b/doc/src/sgml/ref/create_materialized_view.sgml
index 7f31ab4d26..dd5ed8e3d0 100644
--- a/doc/src/sgml/ref/create_materialized_view.sgml
+++ b/doc/src/sgml/ref/create_materialized_view.sgml
@@ -21,7 +21,7 @@ PostgreSQL documentation
-CREATE MATERIALIZED VIEW [ IF NOT EXISTS ] table_name
+CREATE [ TEMP | TEMPORARY ] MATERIALIZED VIEW [ IF NOT EXISTS ] table_name
[ (column_name [, ...] ) ]
[ WITH ( storage_parameter [= value] [, ... ] ) ]
[ TABLESPACE tablespace_name ]
@@ -53,6 +53,26 @@ CREATE MATERIALIZED VIEW [ IF NOT EXISTS ] table_name
Parameters
+
+TEMPORARY or TEMP
+
+
+ If specified, the materialized view is created as a temporary materialized view.
+ Temporary materialized views are automatically dropped at the end of the
+ current session. Existing
+ permanent relations with the same name are not visible to the
+ current session while the temporary materialized view exists, unless they are
+ referenced with schema-qualified names.
+
+
+
+ If any of the tables referenced by the materialized view are temporary,
+ the materialized view is created as a temporary materialized view (whether
+ TEMPORARY is specified or not).
+
+
+
+
IF NOT EXISTS
diff --git a/src/backend/commands/createas.c b/src/backend/commands/createas.c
index 7185432763..c30f76b343 100644
--- a/src/backend/commands/createas.c
+++ b/src/backend/commands/createas.c
@@ -40,6 +40,7 @@
#include "nodes/makefuncs.h"
#include "nodes/nodeFuncs.h"
#include "parser/parse_clause.h"
+#include "parser/parse_relation.h"
#include "rewrite/rewriteHandler.h"
#include "storage/smgr.h"
#include "tcop/tcopprot.h"
@@ -85,6 +86,7 @@ create_ctas_internal(List *attrList, IntoClause *into)
{
CreateStmt *create = makeNode(CreateStmt);
bool is_matview;
+ RangeVar *relation;
char relkind;
Datum toast_options;
static char *validnsps[] = HEAP_RELOPT_NAMESPACES;
@@ -94,11 +96,27 @@ create_ctas_internal(List *attrList, IntoClause *into)
is_matview = (into->viewQuery != NULL);
relkind = is_matview ? RELKIND_MATVIEW : RELKIND_RELATION;
+ /*
+ * If the user didn't explicitly ask for a temporary MV, check whether
+ * we need one implicitly. We allow TEMP to be inserted automatically as
+ * long as the CREATE command is consistent with that --- no explicit
+ * schema name.
+ */
+ relation = copyObject(into->rel); /* don't corrupt original command */
+ if (is_matview && relation->relpersistence == RELPERSISTENCE_PERMANENT
+ && isQueryUsingTempRelation((Query *) into->viewQuery))
+ {
+ relation->relpersistence = RELPERSISTENCE_TEMP;
+ ereport(NOTICE,
+(errmsg("materialized view \"%s\" will be a temporary materialized view",
+ relation->relname)));
+ }
+
/*
* Create the target relation by faking up a CREATE TABLE parsetree and
* passing it to DefineRelation.
*/
- create->relation = into->rel;
+ create->relation = relation;
create->tableElts = attrList;
create->inhRelations = NIL;
create->ofTypename = NULL;
@@ -278,17 +296,12 @@ ExecCreateTableAs(CreateTableAsStmt *stmt, const char *queryString,
Assert(query->commandType == CMD_SELECT);
/*
- * For materialized views, lock down security-restricted operations and
- * arrange to make GUC variable changes local to this command. This is
- * not necessary for security, but this keeps the behavior similar to
- * REFRESH MATERIALIZED VIEW. Otherwise, one could create a materialized
- * view not possible to refresh.
+ * For materialized views, arrange to make GUC variable changes local
+ * to this command.
*/
if (is_matview)
{
GetUserIdAndSecContext(_userid, _sec_context);
- SetUserIdAndSecContext(save_userid,
- save_sec_context | SECURITY_RESTRICTED_OPERATION);
save_nestlevel = NewGUCNestLevel();
}
diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c
index 5ff6964d51..c855a0750d 100644
--- a/src/backend/parser/analyze.c
+++ b/src/backend/parser/analyze.c
@@ -2551,16 +2551,6 @@ transformCreateTableAsStmt(ParseState *pstate, CreateTableAsStmt *stmt)
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("materialized views must not use data-modifying statements in WITH")));
- /*
- * Check whether any temporary database objects are used in the
- * creation query. It would be hard to refresh data or incrementally
- * maintain it if
Re: Feature: temporary materialized views
On 1/18/19 2:53 AM, Mitar wrote:> On Thu, Jan 17, 2019 at 2:40 PM Andreas Karlsson wrote: I did some functional testing today and everything seems to work as expected other than that the tab completion for psql seems to be missing. Thanks. I can add those as soon as I figure how. :-) These rules are usually pretty easy to add. Just take a look in src/bin/psql/tab-complete.c to see how it is usually done. So what are next steps here besides tab autocompletion? It is OK to remove that security check? If I understand correctly, there are some general refactoring of code Tom is proposing, but I am not sure if I am able to do that/understand that. No, I do not think it is ok to remove the check without a compelling argument for why the usability we gain from this check is not worth it. Additionally I agree with Tom that the way the code is written currently is confusing so this refactoring would most likely be a win even without your patch. I might take a stab at refactoring this myself this weekend. Hopefully it is not too involved. Andreas
Re: Feature: temporary materialized views
Hi! On Thu, Jan 17, 2019 at 2:40 PM Andreas Karlsson wrote: > I did some functional testing today and everything seems to work as > expected other than that the tab completion for psql seems to be missing. Thanks. I can add those as soon as I figure how. :-) So what are next steps here besides tab autocompletion? It is OK to remove that security check? If I understand correctly, there are some general refactoring of code Tom is proposing, but I am not sure if I am able to do that/understand that. Mitar -- http://mitar.tnode.com/ https://twitter.com/mitar_m
Re: Feature: temporary materialized views
Hi! On Thu, Jan 17, 2019 at 9:53 AM Andreas Karlsson wrote: > > What is the stumbling block to just leaving that alone? > > I think the issue Mitar ran into is that the temporary materialized view > is created in the rStartup callback of the receiver which happens after > SECURITY_RESTRICTED_OPERATION is set in ExecCreateTableAs(), so the > creation of the view itself is denied. Yes, the error without that change is: ERROR: cannot create temporary table within security-restricted operation Mitar -- http://mitar.tnode.com/ https://twitter.com/mitar_m
Re: Feature: temporary materialized views
On 1/11/19 8:47 PM, Mitar wrote: Thanks for doing the review! I did some functional testing today and everything seems to work as expected other than that the tab completion for psql seems to be missing. Andreas
Re: Feature: temporary materialized views
Andreas Karlsson writes: > On 1/17/19 4:57 PM, Tom Lane wrote: >> What is the stumbling block to just leaving that alone? > I think the issue Mitar ran into is that the temporary materialized view > is created in the rStartup callback of the receiver which happens after > SECURITY_RESTRICTED_OPERATION is set in ExecCreateTableAs(), so the > creation of the view itself is denied. Hm. > From a cursory glance it looks like it would be possible to move the > setting of SECURITY_RESTRICTED_OPERATION to inside the rStartup > callabck, other than that the code for resetting the security context > might get a bit ugly. Do you see any flaws with that solution? Don't think that works: the point here is to restrict what can happen during planning/execution of the view query, so letting planning and query startup happen first is no good. Creating the view object inside the rStartup callback is itself pretty much of a kluge; you'd expect that to happen earlier. I think the reason it was done that way was it was easier to find out the view's column set there, but I'm sure we can find another way --- doing the object creation more like a regular view does it is the obvious approach. regards, tom lane
Re: Feature: temporary materialized views
On 1/17/19 4:57 PM, Tom Lane wrote: Andreas Karlsson writes: On 1/11/19 8:47 PM, Mitar wrote: Is it really ok to just remove SECURITY_RESTRICTED_OPERATION from ExecCreateTableAs()? The comment there said that this is not really necessary for security: "This is not necessary for security, but this keeps the behavior similar to REFRESH MATERIALIZED VIEW. Otherwise, one could create a materialized view not possible to refresh." Hm, I am still not convinced just removing it is a good idea. Sure, it is not a security issue but usability is also important. Indeed. I don't buy the argument that this should work differently for temp views. The fact that they're only accessible in the current session is no excuse for that: security considerations still matter, because you can have different privilege contexts within a single session (consider SECURITY DEFINER functions etc). What is the stumbling block to just leaving that alone? I think the issue Mitar ran into is that the temporary materialized view is created in the rStartup callback of the receiver which happens after SECURITY_RESTRICTED_OPERATION is set in ExecCreateTableAs(), so the creation of the view itself is denied. From a cursory glance it looks like it would be possible to move the setting of SECURITY_RESTRICTED_OPERATION to inside the rStartup callabck, other than that the code for resetting the security context might get a bit ugly. Do you see any flaws with that solution? Andreas
Re: Feature: temporary materialized views
Andreas Karlsson writes: > On 1/11/19 8:47 PM, Mitar wrote: >>> Is it really ok to just remove SECURITY_RESTRICTED_OPERATION from >>> ExecCreateTableAs()? >> The comment there said that this is not really necessary for security: >> "This is not necessary for security, but this keeps the behavior >> similar to REFRESH MATERIALIZED VIEW. Otherwise, one could create a >> materialized view not possible to refresh." > Hm, I am still not convinced just removing it is a good idea. Sure, it > is not a security issue but usability is also important. Indeed. I don't buy the argument that this should work differently for temp views. The fact that they're only accessible in the current session is no excuse for that: security considerations still matter, because you can have different privilege contexts within a single session (consider SECURITY DEFINER functions etc). What is the stumbling block to just leaving that alone? regards, tom lane
Re: Feature: temporary materialized views
On 1/11/19 8:47 PM, Mitar wrote: In create_ctas_internal() why do you copy the relation even when you do not modify it? I was modelling this after code in view.c [1]. I can move copy into the "if". Makes sense. Is it really ok to just remove SECURITY_RESTRICTED_OPERATION from ExecCreateTableAs()? I feel it is there for a good reason and that we preferably want to reduce the duration of SECURITY_RESTRICTED_OPERATION to only include when we actually execute the query. The comment there said that this is not really necessary for security: "This is not necessary for security, but this keeps the behavior similar to REFRESH MATERIALIZED VIEW. Otherwise, one could create a materialized view not possible to refresh." Based on my experimentation, this is required to be able to use temporary materialized views, but it does mean one has to pay attention from where one can refresh. For example, you cannot refresh from outside of the current session, because temporary object is not available there. I have not seen any other example where refresh would not be possible. This is why I felt comfortable removing this. Also, no test failed after removing this. Hm, I am still not convinced just removing it is a good idea. Sure, it is not a security issue but usability is also important. The question is how much this worsens usability and how much extra work it would be to keep the restriction. Btw, if we are going to remove SECURITY_RESTRICTED_OPERATION we should remove more code. There is no reason to save and reset the bitmask if we do not alter it. Andreas
Re: Feature: temporary materialized views
Hi! On Fri, Jan 11, 2019 at 8:51 AM Andreas Karlsson wrote: > Her is quick initial review. I will do more testing later. Thanks for doing the review! > In create_ctas_internal() why do you copy the relation even when you do > not modify it? I was modelling this after code in view.c [1]. I can move copy into the "if". > Is it really ok to just remove SECURITY_RESTRICTED_OPERATION from > ExecCreateTableAs()? I feel it is there for a good reason and that we > preferably want to reduce the duration of SECURITY_RESTRICTED_OPERATION > to only include when we actually execute the query. The comment there said that this is not really necessary for security: "This is not necessary for security, but this keeps the behavior similar to REFRESH MATERIALIZED VIEW. Otherwise, one could create a materialized view not possible to refresh." Based on my experimentation, this is required to be able to use temporary materialized views, but it does mean one has to pay attention from where one can refresh. For example, you cannot refresh from outside of the current session, because temporary object is not available there. I have not seen any other example where refresh would not be possible. This is why I felt comfortable removing this. Also, no test failed after removing this. [1] https://github.com/postgres/postgres/blob/master/src/backend/commands/view.c#L554 Mitar -- http://mitar.tnode.com/ https://twitter.com/mitar_m
Re: Feature: temporary materialized views
On 12/28/18 8:48 AM, Mitar wrote:> One more version of the patch with more deterministic tests. Her is quick initial review. I will do more testing later. It applies builds and passes the tests. The feature seems useful and also improves consistency, if we have temporary tables and temporary views there should logically also be temporary materialized tables. As for you leaving out ON COMMIT I feel that it is ok since of the existing options only really DROP makes any sense (you cannot truncate materialized views) and since temporary views do not have any ON COMMIT support. = Comments on the code The changes to the code are small and look mostly correct. In create_ctas_internal() why do you copy the relation even when you do not modify it? Is it really ok to just remove SECURITY_RESTRICTED_OPERATION from ExecCreateTableAs()? I feel it is there for a good reason and that we preferably want to reduce the duration of SECURITY_RESTRICTED_OPERATION to only include when we actually execute the query. Andreas
Re: Feature: temporary materialized views
Hi!
One more version of the patch with more deterministic tests.
Mitar
On Thu, Dec 27, 2018 at 10:35 AM Mitar wrote:
>
> Hi!
>
> Thanks, I did it.
>
> I am attaching a new version of the patch with few more lines added to tests.
>
> I noticed that there is no good summary of the latest patch, so let me
> make it here:
>
> So the latest version of the patch adds an option for "temporary"
> materialized views. Such materialized views are automatically deleted
> at the end of the session. Moreover, it also modifies the materialized
> view creation logic so that now if any of the source relations are
> temporary, the final materialized view is temporary as well. This now
> makes materialized views more aligned with regular views.
>
> Tests test that this really works, that refreshing of such views work,
> and that refreshing can also work from a trigger.
>
>
> Mitar
>
> On Thu, Dec 27, 2018 at 5:15 AM Alvaro Herrera
> wrote:
> >
> > On 2018-Dec-27, Mitar wrote:
> >
> > > Hi!
> > >
> > > I made a new version of the patch. I added tests and changes to the
> > > docs and made sure various other aspects of this change for as well. I
> > > think this now makes temporary materialized views fully implemented
> > > and that in my view patch is complete. If there is anything else to
> > > add, please let me know, I do not yet have much experience
> > > contributing here. What are next steps? Do I just wait for it to be
> > > included into Commitfest? Do I add it there myself?
> >
> > Yes, please add it yourself to the commitfest.
> >
> > --
> > Álvaro Herrerahttps://www.2ndQuadrant.com/
> > PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>
>
>
> --
> http://mitar.tnode.com/
> https://twitter.com/mitar_m
--
http://mitar.tnode.com/
https://twitter.com/mitar_m
diff --git a/doc/src/sgml/ref/create_materialized_view.sgml b/doc/src/sgml/ref/create_materialized_view.sgml
index 7f31ab4d26..dd5ed8e3d0 100644
--- a/doc/src/sgml/ref/create_materialized_view.sgml
+++ b/doc/src/sgml/ref/create_materialized_view.sgml
@@ -21,7 +21,7 @@ PostgreSQL documentation
-CREATE MATERIALIZED VIEW [ IF NOT EXISTS ] table_name
+CREATE [ TEMP | TEMPORARY ] MATERIALIZED VIEW [ IF NOT EXISTS ] table_name
[ (column_name [, ...] ) ]
[ WITH ( storage_parameter [= value] [, ... ] ) ]
[ TABLESPACE tablespace_name ]
@@ -53,6 +53,26 @@ CREATE MATERIALIZED VIEW [ IF NOT EXISTS ] table_name
Parameters
+
+TEMPORARY or TEMP
+
+
+ If specified, the materialized view is created as a temporary materialized view.
+ Temporary materialized views are automatically dropped at the end of the
+ current session. Existing
+ permanent relations with the same name are not visible to the
+ current session while the temporary materialized view exists, unless they are
+ referenced with schema-qualified names.
+
+
+
+ If any of the tables referenced by the materialized view are temporary,
+ the materialized view is created as a temporary materialized view (whether
+ TEMPORARY is specified or not).
+
+
+
+
IF NOT EXISTS
diff --git a/src/backend/commands/createas.c b/src/backend/commands/createas.c
index d01b258b65..706b6a23e2 100644
--- a/src/backend/commands/createas.c
+++ b/src/backend/commands/createas.c
@@ -40,6 +40,7 @@
#include "nodes/makefuncs.h"
#include "nodes/nodeFuncs.h"
#include "parser/parse_clause.h"
+#include "parser/parse_relation.h"
#include "rewrite/rewriteHandler.h"
#include "storage/smgr.h"
#include "tcop/tcopprot.h"
@@ -85,6 +86,7 @@ create_ctas_internal(List *attrList, IntoClause *into)
{
CreateStmt *create = makeNode(CreateStmt);
bool is_matview;
+ RangeVar *relation;
char relkind;
Datum toast_options;
static char *validnsps[] = HEAP_RELOPT_NAMESPACES;
@@ -94,11 +96,27 @@ create_ctas_internal(List *attrList, IntoClause *into)
is_matview = (into->viewQuery != NULL);
relkind = is_matview ? RELKIND_MATVIEW : RELKIND_RELATION;
+ /*
+ * If the user didn't explicitly ask for a temporary MV, check whether
+ * we need one implicitly. We allow TEMP to be inserted automatically as
+ * long as the CREATE command is consistent with that --- no explicit
+ * schema name.
+ */
+ relation = copyObject(into->rel); /* don't corrupt original command */
+ if (is_matview && relation->relpersistence == RELPERSISTENCE_PERMANENT
+ && isQueryUsingTempRelation((Query *) into->viewQuery))
+ {
+ relation->relpersistence = RELPERSISTENCE_TEMP;
+ ereport(NOTICE,
+(errmsg("materialized view \"%s\" will be a temporary materialized view",
+ relation->relname)));
+ }
+
/*
* Create the target relation by faking up a CREATE TABLE parsetree and
* passing it to DefineRelation.
*/
- create->relation = into->rel;
+ create->relation = relation;
create->tableElts = attrList;
create->inhRelations = NIL;
create->ofTypename = NULL;
@@ -278,17 +296,12 @@
Re: Feature: temporary materialized views
Hi!
Thanks, I did it.
I am attaching a new version of the patch with few more lines added to tests.
I noticed that there is no good summary of the latest patch, so let me
make it here:
So the latest version of the patch adds an option for "temporary"
materialized views. Such materialized views are automatically deleted
at the end of the session. Moreover, it also modifies the materialized
view creation logic so that now if any of the source relations are
temporary, the final materialized view is temporary as well. This now
makes materialized views more aligned with regular views.
Tests test that this really works, that refreshing of such views work,
and that refreshing can also work from a trigger.
Mitar
On Thu, Dec 27, 2018 at 5:15 AM Alvaro Herrera wrote:
>
> On 2018-Dec-27, Mitar wrote:
>
> > Hi!
> >
> > I made a new version of the patch. I added tests and changes to the
> > docs and made sure various other aspects of this change for as well. I
> > think this now makes temporary materialized views fully implemented
> > and that in my view patch is complete. If there is anything else to
> > add, please let me know, I do not yet have much experience
> > contributing here. What are next steps? Do I just wait for it to be
> > included into Commitfest? Do I add it there myself?
>
> Yes, please add it yourself to the commitfest.
>
> --
> Álvaro Herrerahttps://www.2ndQuadrant.com/
> PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
http://mitar.tnode.com/
https://twitter.com/mitar_m
diff --git a/doc/src/sgml/ref/create_materialized_view.sgml b/doc/src/sgml/ref/create_materialized_view.sgml
index 7f31ab4d26..dd5ed8e3d0 100644
--- a/doc/src/sgml/ref/create_materialized_view.sgml
+++ b/doc/src/sgml/ref/create_materialized_view.sgml
@@ -21,7 +21,7 @@ PostgreSQL documentation
-CREATE MATERIALIZED VIEW [ IF NOT EXISTS ] table_name
+CREATE [ TEMP | TEMPORARY ] MATERIALIZED VIEW [ IF NOT EXISTS ] table_name
[ (column_name [, ...] ) ]
[ WITH ( storage_parameter [= value] [, ... ] ) ]
[ TABLESPACE tablespace_name ]
@@ -53,6 +53,26 @@ CREATE MATERIALIZED VIEW [ IF NOT EXISTS ] table_name
Parameters
+
+TEMPORARY or TEMP
+
+
+ If specified, the materialized view is created as a temporary materialized view.
+ Temporary materialized views are automatically dropped at the end of the
+ current session. Existing
+ permanent relations with the same name are not visible to the
+ current session while the temporary materialized view exists, unless they are
+ referenced with schema-qualified names.
+
+
+
+ If any of the tables referenced by the materialized view are temporary,
+ the materialized view is created as a temporary materialized view (whether
+ TEMPORARY is specified or not).
+
+
+
+
IF NOT EXISTS
diff --git a/src/backend/commands/createas.c b/src/backend/commands/createas.c
index d01b258b65..706b6a23e2 100644
--- a/src/backend/commands/createas.c
+++ b/src/backend/commands/createas.c
@@ -40,6 +40,7 @@
#include "nodes/makefuncs.h"
#include "nodes/nodeFuncs.h"
#include "parser/parse_clause.h"
+#include "parser/parse_relation.h"
#include "rewrite/rewriteHandler.h"
#include "storage/smgr.h"
#include "tcop/tcopprot.h"
@@ -85,6 +86,7 @@ create_ctas_internal(List *attrList, IntoClause *into)
{
CreateStmt *create = makeNode(CreateStmt);
bool is_matview;
+ RangeVar *relation;
char relkind;
Datum toast_options;
static char *validnsps[] = HEAP_RELOPT_NAMESPACES;
@@ -94,11 +96,27 @@ create_ctas_internal(List *attrList, IntoClause *into)
is_matview = (into->viewQuery != NULL);
relkind = is_matview ? RELKIND_MATVIEW : RELKIND_RELATION;
+ /*
+ * If the user didn't explicitly ask for a temporary MV, check whether
+ * we need one implicitly. We allow TEMP to be inserted automatically as
+ * long as the CREATE command is consistent with that --- no explicit
+ * schema name.
+ */
+ relation = copyObject(into->rel); /* don't corrupt original command */
+ if (is_matview && relation->relpersistence == RELPERSISTENCE_PERMANENT
+ && isQueryUsingTempRelation((Query *) into->viewQuery))
+ {
+ relation->relpersistence = RELPERSISTENCE_TEMP;
+ ereport(NOTICE,
+(errmsg("materialized view \"%s\" will be a temporary materialized view",
+ relation->relname)));
+ }
+
/*
* Create the target relation by faking up a CREATE TABLE parsetree and
* passing it to DefineRelation.
*/
- create->relation = into->rel;
+ create->relation = relation;
create->tableElts = attrList;
create->inhRelations = NIL;
create->ofTypename = NULL;
@@ -278,17 +296,12 @@ ExecCreateTableAs(CreateTableAsStmt *stmt, const char *queryString,
Assert(query->commandType == CMD_SELECT);
/*
- * For materialized views, lock down security-restricted operations and
- * arrange to make GUC variable changes local to this command. This is
- * not
Re: Feature: temporary materialized views
On 2018-Dec-27, Mitar wrote: > Hi! > > I made a new version of the patch. I added tests and changes to the > docs and made sure various other aspects of this change for as well. I > think this now makes temporary materialized views fully implemented > and that in my view patch is complete. If there is anything else to > add, please let me know, I do not yet have much experience > contributing here. What are next steps? Do I just wait for it to be > included into Commitfest? Do I add it there myself? Yes, please add it yourself to the commitfest. -- Álvaro Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: Feature: temporary materialized views
Hi!
I made a new version of the patch. I added tests and changes to the
docs and made sure various other aspects of this change for as well. I
think this now makes temporary materialized views fully implemented
and that in my view patch is complete. If there is anything else to
add, please let me know, I do not yet have much experience
contributing here. What are next steps? Do I just wait for it to be
included into Commitfest? Do I add it there myself?
Mitar
On Wed, Dec 26, 2018 at 9:00 AM Alvaro Herrera wrote:
>
> On 2018-Dec-25, Mitar wrote:
>
> > Sometimes materialized views are used to cache a complex query on
> > which a client works. But after client disconnects, the materialized
> > view could be deleted. Regular VIEWs and TABLEs both have support for
> > temporary versions which get automatically dropped at the end of the
> > session. It seems it is easy to add the same thing for materialized
> > views as well. See attached PoC patch.
>
> I think MVs that are dropped at session end are a sensible feature. I
> probably wouldn't go as far as allowing ON COMMIT actions, though, so
> this much effort is the right amount.
>
> I think if you really want to do this you should just use OptTemp, and
> delete OptNoLog. Of course, you need to add tests and patch the docs.
>
> --
> Álvaro Herrerahttps://www.2ndQuadrant.com/
> PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
http://mitar.tnode.com/
https://twitter.com/mitar_m
diff --git a/doc/src/sgml/ref/create_materialized_view.sgml b/doc/src/sgml/ref/create_materialized_view.sgml
index 7f31ab4d26..dd5ed8e3d0 100644
--- a/doc/src/sgml/ref/create_materialized_view.sgml
+++ b/doc/src/sgml/ref/create_materialized_view.sgml
@@ -21,7 +21,7 @@ PostgreSQL documentation
-CREATE MATERIALIZED VIEW [ IF NOT EXISTS ] table_name
+CREATE [ TEMP | TEMPORARY ] MATERIALIZED VIEW [ IF NOT EXISTS ] table_name
[ (column_name [, ...] ) ]
[ WITH ( storage_parameter [= value] [, ... ] ) ]
[ TABLESPACE tablespace_name ]
@@ -53,6 +53,26 @@ CREATE MATERIALIZED VIEW [ IF NOT EXISTS ] table_name
Parameters
+
+TEMPORARY or TEMP
+
+
+ If specified, the materialized view is created as a temporary materialized view.
+ Temporary materialized views are automatically dropped at the end of the
+ current session. Existing
+ permanent relations with the same name are not visible to the
+ current session while the temporary materialized view exists, unless they are
+ referenced with schema-qualified names.
+
+
+
+ If any of the tables referenced by the materialized view are temporary,
+ the materialized view is created as a temporary materialized view (whether
+ TEMPORARY is specified or not).
+
+
+
+
IF NOT EXISTS
diff --git a/src/backend/commands/createas.c b/src/backend/commands/createas.c
index d01b258b65..706b6a23e2 100644
--- a/src/backend/commands/createas.c
+++ b/src/backend/commands/createas.c
@@ -40,6 +40,7 @@
#include "nodes/makefuncs.h"
#include "nodes/nodeFuncs.h"
#include "parser/parse_clause.h"
+#include "parser/parse_relation.h"
#include "rewrite/rewriteHandler.h"
#include "storage/smgr.h"
#include "tcop/tcopprot.h"
@@ -85,6 +86,7 @@ create_ctas_internal(List *attrList, IntoClause *into)
{
CreateStmt *create = makeNode(CreateStmt);
bool is_matview;
+ RangeVar *relation;
char relkind;
Datum toast_options;
static char *validnsps[] = HEAP_RELOPT_NAMESPACES;
@@ -94,11 +96,27 @@ create_ctas_internal(List *attrList, IntoClause *into)
is_matview = (into->viewQuery != NULL);
relkind = is_matview ? RELKIND_MATVIEW : RELKIND_RELATION;
+ /*
+ * If the user didn't explicitly ask for a temporary MV, check whether
+ * we need one implicitly. We allow TEMP to be inserted automatically as
+ * long as the CREATE command is consistent with that --- no explicit
+ * schema name.
+ */
+ relation = copyObject(into->rel); /* don't corrupt original command */
+ if (is_matview && relation->relpersistence == RELPERSISTENCE_PERMANENT
+ && isQueryUsingTempRelation((Query *) into->viewQuery))
+ {
+ relation->relpersistence = RELPERSISTENCE_TEMP;
+ ereport(NOTICE,
+(errmsg("materialized view \"%s\" will be a temporary materialized view",
+ relation->relname)));
+ }
+
/*
* Create the target relation by faking up a CREATE TABLE parsetree and
* passing it to DefineRelation.
*/
- create->relation = into->rel;
+ create->relation = relation;
create->tableElts = attrList;
create->inhRelations = NIL;
create->ofTypename = NULL;
@@ -278,17 +296,12 @@ ExecCreateTableAs(CreateTableAsStmt *stmt, const char *queryString,
Assert(query->commandType == CMD_SELECT);
/*
- * For materialized views, lock down security-restricted operations and
- * arrange to make GUC variable changes local to this command. This is
- * not necessary for security, but this keeps the behavior
Re: Feature: temporary materialized views
On 2018-Dec-26, Mitar wrote: > OptTemp seems to have a misleading warning in some cases when it is > not used on tables though: > > "GLOBAL is deprecated in temporary table creation" > > Should we change this language to something else? "GLOBAL is > deprecated in temporary object creation"? Based on grammar it seems to > be used for tables, views, sequences, and soon materialized views. I'd just leave those messages alone. -- Álvaro Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: Feature: temporary materialized views
st 26. 12. 2018 v 18:20 odesílatel Mitar napsal: > Hi! > > On Wed, Dec 26, 2018 at 9:00 AM Alvaro Herrera > wrote: > > I think MVs that are dropped at session end are a sensible feature. > > Thanks. > > > I probably wouldn't go as far as allowing ON COMMIT actions, though > > I agree. I do not see much usefulness for it. The only use case I can > think of would be to support REFRESH as an ON COMMIT action. That > would be maybe useful in the MV setting. After every transaction in my > session, REFRESH this materialized view. > > But personally I do not have an use case for that, so I will leave it > to somebody else. :-) > > > I think if you really want to do this you should just use OptTemp, and > > delete OptNoLog. > > Sounds good. > > OptTemp seems to have a misleading warning in some cases when it is > not used on tables though: > > "GLOBAL is deprecated in temporary table creation" > > Should we change this language to something else? "GLOBAL is > deprecated in temporary object creation"? Based on grammar it seems to > be used for tables, views, sequences, and soon materialized views. > This message is wrong - probably better "GLOBAL temporary tables are not supported" Regards Pavel > > > Of course, you need to add tests and patch the docs. > > Sure. > > [1] https://www.postgresql.org/message-id/29165.1545842105%40sss.pgh.pa.us > > > Mitar > > -- > http://mitar.tnode.com/ > https://twitter.com/mitar_m > >
Re: Feature: temporary materialized views
Hi! On Wed, Dec 26, 2018 at 9:00 AM Alvaro Herrera wrote: > I think MVs that are dropped at session end are a sensible feature. Thanks. > I probably wouldn't go as far as allowing ON COMMIT actions, though I agree. I do not see much usefulness for it. The only use case I can think of would be to support REFRESH as an ON COMMIT action. That would be maybe useful in the MV setting. After every transaction in my session, REFRESH this materialized view. But personally I do not have an use case for that, so I will leave it to somebody else. :-) > I think if you really want to do this you should just use OptTemp, and > delete OptNoLog. Sounds good. OptTemp seems to have a misleading warning in some cases when it is not used on tables though: "GLOBAL is deprecated in temporary table creation" Should we change this language to something else? "GLOBAL is deprecated in temporary object creation"? Based on grammar it seems to be used for tables, views, sequences, and soon materialized views. > Of course, you need to add tests and patch the docs. Sure. [1] https://www.postgresql.org/message-id/29165.1545842105%40sss.pgh.pa.us Mitar -- http://mitar.tnode.com/ https://twitter.com/mitar_m
Re: Feature: temporary materialized views
On 2018-Dec-25, Mitar wrote: > Sometimes materialized views are used to cache a complex query on > which a client works. But after client disconnects, the materialized > view could be deleted. Regular VIEWs and TABLEs both have support for > temporary versions which get automatically dropped at the end of the > session. It seems it is easy to add the same thing for materialized > views as well. See attached PoC patch. I think MVs that are dropped at session end are a sensible feature. I probably wouldn't go as far as allowing ON COMMIT actions, though, so this much effort is the right amount. I think if you really want to do this you should just use OptTemp, and delete OptNoLog. Of course, you need to add tests and patch the docs. -- Álvaro Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Feature: temporary materialized views
Hi!
Sometimes materialized views are used to cache a complex query on
which a client works. But after client disconnects, the materialized
view could be deleted. Regular VIEWs and TABLEs both have support for
temporary versions which get automatically dropped at the end of the
session. It seems it is easy to add the same thing for materialized
views as well. See attached PoC patch.
Mitar
--
http://mitar.tnode.com/
https://twitter.com/mitar_m
diff --git a/src/backend/commands/createas.c b/src/backend/commands/createas.c
index d01b258b65..996fe8f53d 100644
--- a/src/backend/commands/createas.c
+++ b/src/backend/commands/createas.c
@@ -278,17 +278,12 @@ ExecCreateTableAs(CreateTableAsStmt *stmt, const char *queryString,
Assert(query->commandType == CMD_SELECT);
/*
- * For materialized views, lock down security-restricted operations and
- * arrange to make GUC variable changes local to this command. This is
- * not necessary for security, but this keeps the behavior similar to
- * REFRESH MATERIALIZED VIEW. Otherwise, one could create a materialized
- * view not possible to refresh.
+ * For materialized views, arrange to make GUC variable changes local
+ * to this command.
*/
if (is_matview)
{
GetUserIdAndSecContext(_userid, _sec_context);
- SetUserIdAndSecContext(save_userid,
- save_sec_context | SECURITY_RESTRICTED_OPERATION);
save_nestlevel = NewGUCNestLevel();
}
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 2c2208ffb7..e59e53b154 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -420,7 +420,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
%type opt_trusted opt_restart_seqs
%type OptTemp
-%type OptNoLog
+%type OptTempNoLog
%type OnCommitOption
%type for_locking_strength
@@ -4054,7 +4054,7 @@ opt_with_data:
*/
CreateMatViewStmt:
- CREATE OptNoLog MATERIALIZED VIEW create_mv_target AS SelectStmt opt_with_data
+ CREATE OptTempNoLog MATERIALIZED VIEW create_mv_target AS SelectStmt opt_with_data
{
CreateTableAsStmt *ctas = makeNode(CreateTableAsStmt);
ctas->query = $7;
@@ -4067,7 +4067,7 @@ CreateMatViewStmt:
$5->skipData = !($8);
$$ = (Node *) ctas;
}
- | CREATE OptNoLog MATERIALIZED VIEW IF_P NOT EXISTS create_mv_target AS SelectStmt opt_with_data
+ | CREATE OptTempNoLog MATERIALIZED VIEW IF_P NOT EXISTS create_mv_target AS SelectStmt opt_with_data
{
CreateTableAsStmt *ctas = makeNode(CreateTableAsStmt);
ctas->query = $10;
@@ -4096,8 +4096,11 @@ create_mv_target:
}
;
-OptNoLog: UNLOGGED { $$ = RELPERSISTENCE_UNLOGGED; }
- | /*EMPTY*/ { $$ = RELPERSISTENCE_PERMANENT; }
+OptTempNoLog:
+ TEMPORARY { $$ = RELPERSISTENCE_TEMP; }
+ | TEMP { $$ = RELPERSISTENCE_TEMP; }
+ | UNLOGGED { $$ = RELPERSISTENCE_UNLOGGED; }
+ | /*EMPTY*/ { $$ = RELPERSISTENCE_PERMANENT; }
;
