Base refresh materialized view #800

Gerych1984 · 2024-01-22T11:41:48Z

Q	A
Is bugfix?	❌
New feature?	✔️
Breaks BC?	❌

codecov · 2024-01-22T11:59:07Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.64%. Comparing base (ef81e8b) to head (4e86a29).
Report is 9 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff            @@
##             master     #800   +/-   ##
=========================================
  Coverage     99.64%   99.64%           
- Complexity     1271     1277    +6     
=========================================
  Files            63       63           
  Lines          3106     3119   +13     
=========================================
+ Hits           3095     3108   +13     
  Misses           11       11

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Tigrov

I think materialized views should be added in complex. With create, alter, drop and refresh methods. And take into account features of all supported DBMSs.

So, we should realize all these methods for all supported DBMSs which support materialized views.

Gerych1984 · 2024-01-31T06:26:20Z

I think materialized views should be added in complex. With create, alter, drop and refresh methods. And take into account features of all supported DBMSs.

So, we should realize all these methods for all supported DBMSs which support materialized views.

That's what this PR is made for. Without it, i can't go any further

Tigrov · 2024-01-31T07:13:57Z

src/Command/CommandInterface.php

+     * @param bool|null $withData
+     * @return bool
+     */
+    public function refreshMaterializedView(string $viewName, ?bool $concurrently = null, ?bool $withData = null): bool;


Suggested change

public function refreshMaterializedView(string $viewName, ?bool $concurrently = null, ?bool $withData = null): bool;

public function refreshMaterializedView(string $viewName, bool $concurrently = false, ?bool $withData = null): bool;

Why the interface is different from DDLQueryBuilderInterface::refreshMaterializedView()?
Looks like should be the same.

When execute $command->refreshMaterializedView($viewName); expect this is not concurently.

From what I see in MS SQL / Oracle / Clickhouse docs:

WITH [NO] DATA is PostgreSQL only option and pretty much useless one.

REFRESH exists only on PG and Oracle. MS SQL and Clickhouse do automatic updates, so their implementations should always return true.
There are a lot of variants in Oracle: 3 commands (REFRESH, REFREASH_ALL_MVIEWS, REFRESH_DEPENDENT) and at least 6 options. Option out_of_place seems to be equivalent of CONCURRENTLY in PostgreSQL.

So, the most common part is refresh + option for non-blocking update, but it will be very hard to take full advantage of database capabilities like Oracle's refresh method.

Tigrov · 2024-01-31T07:21:08Z

tests/Db/QueryBuilder/QueryBuilderTest.php

+        $sql = $db->getQueryBuilder()->refreshMaterializedView($viewName, $concurrently, $withData);
+        $actual = DbHelper::replaceQuotes($sql, $driver);


Suggested change

$sql = $db->getQueryBuilder()->refreshMaterializedView($viewName, $concurrently, $withData);

$actual = DbHelper::replaceQuotes($sql, $driver);

$actual = $db->getQueryBuilder()->refreshMaterializedView($viewName, $concurrently, $withData);

Result from $db->getQueryBuilder()->refreshMaterializedView() already quoted acording to the driver

Tigrov · 2024-01-31T07:26:33Z

tests/Db/QueryBuilder/QueryBuilderTest.php

@@ -178,7 +178,7 @@ public function testGetExpressionBuilderException(): void

        $this->expectException(Exception::class);

-        $expression = new class () implements ExpressionInterface {
+        $expression = new class() implements ExpressionInterface {


Style CI will change it back

https://github.styleci.io/analyses/kY9ABG?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link

Tigrov · 2024-01-31T07:46:10Z

Added this to the 2.0.0 milestone due to (almost) any change of interfaces breaks BC

Tigrov · 2024-01-31T08:08:36Z

src/QueryBuilder/DDLQueryBuilderInterface.php

+     *
+     * @return string The `REFRESH MATERIALIZED VIEW` SQL statement
+     */
+    public function refreshMaterializedView(string $viewName, bool $concurrently = false, ?bool $withData = null): string;


Suggested change

public function refreshMaterializedView(string $viewName, bool $concurrently = false, ?bool $withData = null): string;

public function refreshMaterializedView(string $viewName, bool $concurrently = false): string;

Not sure the argument $withData is needed. If it is false, it is equal to "truncate" the materialized view. Maybe better remove it and if the method is required add truncateMaterializedView()

Tigrov · 2024-02-02T14:58:06Z

tests/Db/Command/CommandTest.php

+    public static function refreshMaterializedViewDataProvider(): array
+    {
+        return [
+            [
+                'default_mt',
+                null,
+                null,
+            ],
+            [
+                'concurrently_mt',
+                true,
+                null,
+            ],
+            [
+                'concurrently_with_data_mt',
+                true,
+                true,
+            ],
+            [
+                'concurrently_without_data_mt',
+                true,
+                false,
+            ],
+        ];
+    }


NotSupportedException can be tested one time with one set of arguments.

Tigrov · 2024-02-02T14:58:56Z

tests/Db/QueryBuilder/QueryBuilderTest.php

+    public static function refreshMaterializedViewDataProvider(): array
+    {
+        return [
+            [
+                'concurrently_mt',
+                true,
+                null,
+                'REFRESH MATERIALIZED VIEW CONCURRENTLY [[concurrently_mt]]',
+            ],
+            [
+                'concurrently_with_data_mt',
+                true,
+                true,
+                'REFRESH MATERIALIZED VIEW CONCURRENTLY [[concurrently_with_data_mt]] WITH DATA',
+            ],
+            [
+                'concurrently_without_data_mt',
+                true,
+                false,
+                'REFRESH MATERIALIZED VIEW CONCURRENTLY [[concurrently_without_data_mt]] WITH NO DATA',
+            ],
+            [
+                'not_concurrently_mt',
+                false,
+                null,
+                'REFRESH MATERIALIZED VIEW [[not_concurrently_mt]]',
+            ],
+            [
+                'not_concurrently_with_data_mt',
+                false,
+                true,
+                'REFRESH MATERIALIZED VIEW [[not_concurrently_with_data_mt]] WITH DATA',
+            ],
+            [
+                'not_concurrently_without_data_mt',
+                false,
+                false,
+                'REFRESH MATERIALIZED VIEW [[not_concurrently_without_data_mt]] WITH NO DATA',
+            ],
+        ];
+    }


Better to move it to QueryBuilderProvider

Tigrov · 2024-02-02T15:14:48Z

tests/Db/QueryBuilder/QueryBuilderTest.php

+     * @param bool|null $withData
+     * @return void
+     */
+    public function testRefreshMaterializedView(string $viewName, bool $concurrently, ?bool $withData, string $expected): void


Looks need to move this test to AbstractQueryBuilderTest

smirnov-e · 2024-05-05T18:50:28Z

With create, alter, drop and refresh methods.

From what I see in MS SQL / Oracle / Clickhouse / PostgreSQL docs:

DROP: PostgreSQL requires DROP MATERIALIZED VIEW, other databases use simple DROP VIEW.

ALTER - quite different one from another

MS SQL only allows enable/disable MV
PostgreSQL allows modify name / column names / other options excluding source query
Clickhouse only allows to change source query
Oracle — refresh mode and type, enable/disable.

CREATE — MS SQL, PostgreSQL and Oracle has common syntax part, but Oracle has lots of useful extensions. All except MS SQL have option to skip data population (WITH NO DATA).

PostgreSQL — column names, storage options and other.
MS SQL — data distribution
Oracle — additional query for faster refresh, syntax for automatic refresh and partitioning.
Clickhouse — just query and SQL SECURITY.

Gerych1984 force-pushed the prepare-pgsql-mat-view branch from d38325b to 020d468 Compare January 22, 2024 11:52

Gerych1984 force-pushed the prepare-pgsql-mat-view branch from 020d468 to 348f586 Compare January 22, 2024 12:05

Base refresh materialized view

4e86a29

Gerych1984 force-pushed the prepare-pgsql-mat-view branch from 348f586 to 4e86a29 Compare January 22, 2024 12:19

Gerych1984 marked this pull request as ready for review January 22, 2024 12:40

Gerych1984 mentioned this pull request Jan 22, 2024

Add refreshMaterializedView command yiisoft/db-pgsql#332

Open

vjik requested review from Tigrov and a team January 22, 2024 13:02

vjik added the status:code review The pull request needs review. label Jan 22, 2024

Tigrov reviewed Jan 31, 2024

View reviewed changes

Tigrov requested changes Jan 31, 2024

View reviewed changes

Tigrov added this to the 2.0.0 milestone Jan 31, 2024

Tigrov requested changes Jan 31, 2024

View reviewed changes

Tigrov requested changes Feb 2, 2024

View reviewed changes

Tigrov added status:under development Someone is working on a pull request. and removed status:code review The pull request needs review. labels Apr 6, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Base refresh materialized view #800

Base refresh materialized view #800

Gerych1984 commented Jan 22, 2024

codecov bot commented Jan 22, 2024 •

edited

Tigrov left a comment

Gerych1984 commented Jan 31, 2024

Tigrov Jan 31, 2024

smirnov-e May 5, 2024 •

edited

Tigrov Jan 31, 2024

Tigrov Jan 31, 2024 •

edited

Tigrov commented Jan 31, 2024 •

edited

Tigrov Jan 31, 2024 •

edited

Tigrov Feb 2, 2024

Tigrov Feb 2, 2024

Tigrov Feb 2, 2024

smirnov-e commented May 5, 2024 •

edited

	public function refreshMaterializedView(string $viewName, ?bool $concurrently = null, ?bool $withData = null): bool;
	public function refreshMaterializedView(string $viewName, bool $concurrently = false, ?bool $withData = null): bool;

		$sql = $db->getQueryBuilder()->refreshMaterializedView($viewName, $concurrently, $withData);
		$actual = DbHelper::replaceQuotes($sql, $driver);

	$sql = $db->getQueryBuilder()->refreshMaterializedView($viewName, $concurrently, $withData);
	$actual = DbHelper::replaceQuotes($sql, $driver);
	$actual = $db->getQueryBuilder()->refreshMaterializedView($viewName, $concurrently, $withData);

Base refresh materialized view #800

Are you sure you want to change the base?

Base refresh materialized view #800

Conversation

Gerych1984 commented Jan 22, 2024

codecov bot commented Jan 22, 2024 • edited

Codecov Report

Tigrov left a comment

Choose a reason for hiding this comment

Gerych1984 commented Jan 31, 2024

Tigrov Jan 31, 2024

Choose a reason for hiding this comment

smirnov-e May 5, 2024 • edited

Choose a reason for hiding this comment

Tigrov Jan 31, 2024

Choose a reason for hiding this comment

Tigrov Jan 31, 2024 • edited

Choose a reason for hiding this comment

Tigrov commented Jan 31, 2024 • edited

Tigrov Jan 31, 2024 • edited

Choose a reason for hiding this comment

Tigrov Feb 2, 2024

Choose a reason for hiding this comment

Tigrov Feb 2, 2024

Choose a reason for hiding this comment

Tigrov Feb 2, 2024

Choose a reason for hiding this comment

smirnov-e commented May 5, 2024 • edited

codecov bot commented Jan 22, 2024 •

edited

smirnov-e May 5, 2024 •

edited

Tigrov Jan 31, 2024 •

edited

Tigrov commented Jan 31, 2024 •

edited

Tigrov Jan 31, 2024 •

edited

smirnov-e commented May 5, 2024 •

edited