Allow to create unlogged IMMVs #1

Jamal-B · 2024-07-17T11:43:35Z

Description

In some cases (e.g. for very large IMMVs containing volatile data), and provided you are well aware of the risks or drawbacks involved, it may be useful to create an IMMV without writing to PostgreSQL Write-Ahead Logs.

PostgreSQL unlogged tables drawbacks are multiple (not replicated, not backuped, ...) but the main one to keep in mind is that they are automatically truncated after a crash or unclean shutdown. If these conditions meet your requirements, this, as with simple unlogged tables, allows to improve write performance, reduce vacuum impact, and produce fewer WALs (thus reducing network usage, backup size and restoration time).

I hesitated to propose this PR because allowing unlogged IMMVs goes a bit against the fact that PostgreSQL does not allow the creation of unlogged materialized views. But since this feature (based on the fact that IMMVs are, under the hood, classical PostgreSQL tables feeded by triggers) is very useful in our case, I thought that it could also be the same for some other users of this superb extension that is pg_ivm.

Please note that I am not a C developer, and I would obviously be more than happy to consider your comments related to the code... or, of course, the feature itself.

Additionally, I organized the commits based on the project's git history, particularly the Prepare 1.10 commit, without being sure that the PR, if accepted, would merit a version bump. I'll let you judge what's best to do.

Tests

IMMVs are created according to the defined persistence

my_database=# \df create_immv
/*
                                   List of functions
   Schema   |    Name     | Result data type |        Argument data types        | Type 
------------+-------------+------------------+-----------------------------------+------
 pg_catalog | create_immv | bigint           | text, text, boolean DEFAULT false | func
*/

select create_immv('test_immv_1', 'select 1');
select create_immv('test_immv_2', 'select 1', false);
select create_immv('test_immv_3', 'select 1', true);

select relname, relpersistence
from   pg_class
where  relname ~ 'test_immv_';

/*
   relname   | relpersistence 
-------------+----------------
 test_immv_1 | p
 test_immv_2 | p
 test_immv_3 | u
*/

IMMVs persistence is preserved after refresh_immv() using with_data = true

select refresh_immv('test_immv_1', true);
select refresh_immv('test_immv_2', true);
select refresh_immv('test_immv_3', true);

select relname, relpersistence
from   pg_class
where  relname ~ 'test_immv_';

/*
   relname   | relpersistence 
-------------+----------------
 test_immv_1 | p
 test_immv_2 | p
 test_immv_3 | u
*/

IMMVs persistence is preserved after refresh_immv() using with_data = false

select refresh_immv('test_immv_1', false);
select refresh_immv('test_immv_2', false);
select refresh_immv('test_immv_3', false);

select relname, relpersistence
from   pg_class
where  relname ~ 'test_immv_';

/*
   relname   | relpersistence 
-------------+----------------
 test_immv_1 | p
 test_immv_2 | p
 test_immv_3 | u
*/

pg_ivm--1.0.sql

desbouis · 2024-07-17T13:26:23Z

il y a aussi une typo là

pg_ivm/createas.c

Line 113 in 89a36db

* We use this always true flag to imitate ExecCreaetTableAs(9

Jamal-B · 2024-07-17T13:45:38Z

il y a aussi une typo là

pg_ivm/createas.c

Line 113 in 89a36db

* We use this always true flag to imitate ExecCreaetTableAs(9

Merci ! Corrigée et squashée ! Mais je crois que je vais m'arrêter là ... je veux empêcher maître Capello de passer sur tous les fichiers du projet pour corriger leurs anciennes typos 🙂

createas.c

coox · 2024-07-17T15:52:17Z

pg_ivm--1.0.sql

@@ -17,7 +17,7 @@ SELECT pg_catalog.pg_extension_config_dump('pg_catalog.pg_ivm_immv', '');

 -- functions

-CREATE FUNCTION create_immv(text, text)
+CREATE FUNCTION create_immv(text, text, boolean default true)


Je pense que ce fichier est immuable !

La logique de migration voudrait que ta fonction avec la signature modifiée soit entièrement redéclarée dans un fichier pg_ivm--1.8--1.9.sql, avec :

CREATE OR REPLACE FUNCTION create_immv(text, text, boolean default true)

ah mais oui...
Je n'avais pas fait attention, mais ces fichiers correspondent effectivement aux releases de pg_ivm...
Il est aussi à référencer dans https://github.com/mediapart/pg_ivm/blob/main/Makefile#L17

Bien vu @coox !!! ... et j'ai honte de ne pas y avoir fait attention car, comme je vous en parlais sur Mattermost, un ou deux jours avant d'ouvrir la PR (que j'avais en fait préparée depuis assez longtemps), j'avais aussi identifié le côté immuable de ces fichiers en cherchant le fameux event trigger de nettoyage de la table pg_catalog.pg_ivm_immv qui avait été remplacé dans la version 1.3 par des object_access_hook ayant nécessité cette suppression.

coox · 2024-07-17T15:54:24Z

README.md

@@ -84,12 +84,18 @@ When `pg_ivm` is installed, the following objects are created.

 Use `create_immv` function to create IMMV.
 ```
-create_immv(immv_name text, view_definition text) RETURNS bigint
+create_immv(immv_name text, view_definition text [, is_immv_logged bool]) RETURNS bigint


C'est drôle parce qu'en tant qu'utilisateur de pg_ivm, et connaissant ton intention, je me serais attendu à une fonction munie de la logique inverse (car reprenant la terminologie PostgreSQL).

Et aussi, je pense que c'est important de référencer dans les docs la valeur par défaut.

Ça donnerait quelque chose comme :

create_immv(immv_name text, view_definition text [, unlogged bool DEFAULT false]) RETURNS bigint

C'est drôle parce qu'en tant qu'utilisateur de pg_ivm, et connaissant ton intention, je me serais attendu à une fonction munie de la logique inverse (car reprenant la terminologie PostgreSQL).

Et aussi, je pense que c'est important de référencer dans les docs la valeur par défaut.

Ça donnerait quelque chose comme :

create_immv(immv_name text, view_definition text [, unlogged bool DEFAULT false]) RETURNS bigint

Très bonne remarque de l'amiral !!!
Pour l’historique, cette version contraire à la logique et sans la valeur par défaut dans la doc est due à ce que j'expliquais à David dans ce commentaire. Et ayant cru dans un premier temps à tord que je devais ajouter la nouvelle fonction à côté de la précédente (et non à la place), et estimant qu'elle était spécifiquement là pour la création d'IMMVs unlogged, ça m'avait paru naturel que true soit synonyme de "je veux que mon IMMV soit unlogged" (même si en effet ça se discute). Mais c'est vrai qu'au moment de ne garder qu'une seule fonction avec le troisième paramètre non obligatoire, j'aurais dû avoir la rigueur de revoir ça (valeur par défaut à false + l'indiquer dans la doc).
Merci pour ta vigilance 👍

coox · 2024-07-17T15:55:37Z

Outre mes deux commentaires ci-dessus, je trouve le reste nickel, les docs sont excellentes, et je suis globalement hyper heureux que cette PR existe (…bientôt dans le repo officiel 😉) 🎉

desbouis

je retire mon ✔️ pour le coup de la release ;-)

Jamal-B · 2024-07-19T16:00:47Z

Outre mes deux commentaires ci-dessus, je trouve le reste nickel, les docs sont excellentes, et je suis globalement hyper heureux que cette PR existe (…bientôt dans le repo officiel 😉) 🎉

Merci 😄

Jamal-B · 2024-07-19T16:02:38Z

je retire mon ✔️ pour le coup de la release ;-)

J'ai poussé un fixup qui prend en compte vos remarques et j'ai mis à jour la description de la PR avec les tests refaits avec la nouvelle version (le comportement du false devient celui du true et réciproquement).

coox

🔥

desbouis · 2024-07-19T16:23:16Z

Je pense qu'il faut faire un commit à part qui se nomme Prepare 1.9 en y mettant tout ce qui touche au bump de version, en s'insirant de 9736c8c

COil

Quelques NIP de typos à appliquer ou pas (rien de méchant).

COil · 2024-07-22T07:38:22Z

README.md

 ```
 `create_immv` defines a new IMMV of a query. A table of the name `immv_name` is created and a query specified by `view_definition` is executed and used to populate the IMMV. The query is stored in `pg_ivm_immv`, so that it can be refreshed later upon incremental view maintenance. `create_immv` returns the number of rows in the created IMMV.

 When an IMMV is created, some triggers are automatically created so that the view's contents are immediately updated when its base tables are modified. In addition, a unique index is created on the IMMV automatically if possible.  If the view definition query has a GROUP BY clause, a unique index is created on the columns of GROUP BY expressions. Also, if the view has DISTINCT clause, a unique index is created on all columns in the target list. Otherwise, if the IMMV contains all primary key attributes of its base tables in the target list, a unique index is created on these attributes.  In other cases, no index is created.

-#### refresh_imm
+In some cases (e.g. for very large IMMVs containing volatile data), and only if you really know what you are doing, it may be useful to create an IMMV without writing to the [PostgreSQL Write-Ahead Logs](https://www.postgresql.org/docs/current/wal-intro.html) (the underlying table is created with the `UNLOGGED` parameter). Unlogged tables allow to improve write performance, reduce vacuum impact and produce less WALs (thus reducing network usage, backup size and restoration time).


allow to improve > improve

NIP :
less > fewer

Celles-ci sont les miennes 😄 Corrigées ! Merci !

COil · 2024-07-22T07:40:33Z

README.md

@@ -261,7 +267,7 @@ If some base tables have row level security policy, rows that are not visible to

 IVM is effective when we want to keep an IMMV up-to-date and small fraction of a base table is modified infrequently.  Due to the overhead of immediate maintenance, IVM is not effective when a base table is modified frequently.  Also, when a large part of a base table is modified or large data is inserted into a base table, IVM is not effective and the cost of maintenance can be larger than refresh from scratch.

-In such situation, we can use `refesh_immv` function with `with_data = false` to disable immediate maintenance before modifying a base table. After a base table modification, call `refresh_immv`with `with_data = true` to refresh the view data and enable immediate maintenance.
+In such situation, we can use `refresh_immv` function with `with_data = false` to disable immediate maintenance before modifying a base table. After a base table modification, call `refresh_immv`with `with_data = true` to refresh the view data and enable immediate maintenance.


In such situation > In such a situation
we can use refresh_immv > we can use the refresh_immv

Merci pour la remarque mais je préfère laisser tel quel car ce ne sont pas des erreurs importantes/gênantes/structurantes et qu'elles ne proviennent pas de mes commits. Dans le 1er commit j'ai bien corrigé des typos qui étaient déjà dans le projet, mais uniquement car elles concernaient des noms de fonctions.

Dit autrement, je ne voudrais pas trop distraire le(s) mainteneur(s) avec autre chose que le but principal de cette PR, dont je ne suis pas du tout sûr qu'elle soit acceptée 🙂

COil · 2024-07-22T07:41:24Z

createas.c

 	 * aiming to make it easier to follow up the original code.
 	 */
 	const bool	is_ivm = true;

 	/* must be a CREATE MATERIALIZED VIEW statement */
 	Assert(is_matview);

+	/*
+	 * For volatile data, it can be useful not to write to WALs.
+	 * SEE THE POSTGRESQL DOCUMENTATION FOR DRAWBACKS (not replicated or included in physical backups, truncated in case of crash, ...):


of crash > of a crash

Idem, prise en compte dans le dernier fixup.

Jamal-B · 2024-07-22T07:53:19Z

Je pense qu'il faut faire un commit à part qui se nomme Prepare 1.9 en y mettant tout ce qui touche au bump de version, en s'insirant de 9736c8c

Le 🧐 d'or a encore frappé ! Et là encore j'ai honte, parce qu'évidemment quand j'avais commencé à bosser sur le sujet j'avais bien vu tous les commits Prepare <version> ! Encore faut-il brancher son cerveau quand c'est nécessaire ! Shame on me!
J'ai poussé une version corrigée et tassée.

Jamal-B · 2024-07-22T13:09:48Z

❤️ Merci pour vos reviews !
➡️ PR identique créée dans le repo du projet.
⚠️ Je ne merge pas celle-là chez nous pour le moment car :

@loliee veut y jeter un œil (merci !)
si ma PR était acceptée dans le repo du projet, je me demande s'il ne vaudrait pas mieux complètement supprimer celle-ci pour éviter les divergences ou galères de réconciliation. D'ailleurs dans ce cas, notre fork n'aurait de toute façon plus de raison d'être.

Jamal-B · 2024-07-23T10:56:19Z

J'ai complété les tests qui se trouvent dans la description de la PR (ici et celle du repo d'origine).

Jamal-B added the enhancement New feature or request label Jul 17, 2024

Jamal-B requested review from coox, ogrange, desbouis, yzoug, loliee and ceciliahaentjens July 17, 2024 11:43

desbouis reviewed Jul 17, 2024

View reviewed changes

pg_ivm--1.0.sql Outdated Show resolved Hide resolved

Jamal-B requested a review from desbouis July 17, 2024 13:07

desbouis approved these changes Jul 17, 2024

View reviewed changes

Jamal-B force-pushed the allow-to-create-unlogged-immv branch from 19f91db to 4a1124d Compare July 17, 2024 13:43

desbouis reviewed Jul 17, 2024

View reviewed changes

createas.c Outdated Show resolved Hide resolved

Jamal-B force-pushed the allow-to-create-unlogged-immv branch from 4a1124d to 726a766 Compare July 17, 2024 13:46

coox reviewed Jul 17, 2024

View reviewed changes

desbouis requested changes Jul 17, 2024

View reviewed changes

coox approved these changes Jul 19, 2024

View reviewed changes

COil approved these changes Jul 22, 2024

View reviewed changes

Jamal-B force-pushed the allow-to-create-unlogged-immv branch from e67f3aa to abc7d1f Compare July 22, 2024 07:49

desbouis approved these changes Jul 22, 2024

View reviewed changes

Jamal-B force-pushed the allow-to-create-unlogged-immv branch 2 times, most recently from d0d4a91 to 0cd7c28 Compare July 22, 2024 12:45

loliee approved these changes Jul 22, 2024

View reviewed changes

Jamal-B force-pushed the allow-to-create-unlogged-immv branch from 0cd7c28 to 434ff8f Compare August 26, 2024 15:21

Jamal-B added 3 commits August 26, 2024 17:52

Fix function names typos in README

90e28a8

Allow to create unlogged IMMVs

9ff132b

Prepare 1.10

e053a9e

Jamal-B force-pushed the allow-to-create-unlogged-immv branch from 434ff8f to e053a9e Compare August 26, 2024 15:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow to create unlogged IMMVs #1

Allow to create unlogged IMMVs #1

Jamal-B commented Jul 17, 2024 •

edited

Loading

desbouis commented Jul 17, 2024 •

edited

Loading

Jamal-B commented Jul 17, 2024

coox Jul 17, 2024

desbouis Jul 17, 2024

Jamal-B Jul 19, 2024

coox Jul 17, 2024

Jamal-B Jul 19, 2024 •

edited

Loading

coox commented Jul 17, 2024 •

edited

Loading

desbouis left a comment

Jamal-B commented Jul 19, 2024

Jamal-B commented Jul 19, 2024

coox left a comment

desbouis commented Jul 19, 2024

COil left a comment

COil Jul 22, 2024

COil Jul 22, 2024

Jamal-B Jul 22, 2024

COil Jul 22, 2024

Jamal-B Jul 22, 2024

Jamal-B Jul 22, 2024

COil Jul 22, 2024

Jamal-B Jul 22, 2024

Jamal-B commented Jul 22, 2024 •

edited

Loading

Jamal-B commented Jul 22, 2024

Jamal-B commented Jul 23, 2024

Allow to create unlogged IMMVs #1

Are you sure you want to change the base?

Allow to create unlogged IMMVs #1

Conversation

Jamal-B commented Jul 17, 2024 • edited Loading

Description

Tests

desbouis commented Jul 17, 2024 • edited Loading

Jamal-B commented Jul 17, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Jamal-B Jul 19, 2024 • edited Loading

Choose a reason for hiding this comment

coox commented Jul 17, 2024 • edited Loading

desbouis left a comment

Choose a reason for hiding this comment

Jamal-B commented Jul 19, 2024

Jamal-B commented Jul 19, 2024

coox left a comment

Choose a reason for hiding this comment

desbouis commented Jul 19, 2024

COil left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Jamal-B commented Jul 22, 2024 • edited Loading

Jamal-B commented Jul 22, 2024

Jamal-B commented Jul 23, 2024

Jamal-B commented Jul 17, 2024 •

edited

Loading

desbouis commented Jul 17, 2024 •

edited

Loading

Jamal-B Jul 19, 2024 •

edited

Loading

coox commented Jul 17, 2024 •

edited

Loading

Jamal-B commented Jul 22, 2024 •

edited

Loading