Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Base refresh materialized view #800

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions src/Command/AbstractCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
use Closure;
use Throwable;
use Yiisoft\Db\Exception\Exception;
use Yiisoft\Db\Exception\NotSupportedException;
use Yiisoft\Db\Expression\Expression;
use Yiisoft\Db\Query\Data\DataReaderInterface;
use Yiisoft\Db\Query\QueryInterface;
Expand All @@ -21,6 +22,7 @@
use function is_scalar;
use function is_string;
use function preg_replace_callback;
use function sprintf;
use function str_starts_with;
use function stream_get_contents;

Expand Down Expand Up @@ -526,6 +528,13 @@
return $this->setSql($sql)->bindValues($params);
}

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

Check warning on line 531 in src/Command/AbstractCommand.php

View check run for this annotation

Codecov / codecov/patch

src/Command/AbstractCommand.php#L531

Added line #L531 was not covered by tests
{
$sql = $this->getQueryBuilder()->refreshMaterializedView($viewName, $concurrently ?? false, $withData);

Check warning on line 533 in src/Command/AbstractCommand.php

View check run for this annotation

Codecov / codecov/patch

src/Command/AbstractCommand.php#L533

Added line #L533 was not covered by tests

throw new NotSupportedException(sprintf('"%s" command not supported', $sql));

Check warning on line 535 in src/Command/AbstractCommand.php

View check run for this annotation

Codecov / codecov/patch

src/Command/AbstractCommand.php#L535

Added line #L535 was not covered by tests
}

/**
* @return QueryBuilderInterface The query builder instance.
*/
Expand Down
10 changes: 10 additions & 0 deletions src/Command/CommandInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -833,4 +833,14 @@ public function upsert(
bool|array $updateColumns = true,
array $params = []
): static;

/**
*
* Execute `REFRESH MATERIALIZED VIEW` command
* @param string $viewName
* @param bool|null $concurrently If `null` then auto choice from depends on DBMS
* @param bool|null $withData
* @return bool
*/
public function refreshMaterializedView(string $viewName, ?bool $concurrently = null, ?bool $withData = null): bool;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link

@smirnov-e smirnov-e May 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

  1. WITH [NO] DATA is PostgreSQL only option and pretty much useless one.
  2. 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.

}
18 changes: 18 additions & 0 deletions src/QueryBuilder/AbstractDDLQueryBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
use Yiisoft\Db\Schema\SchemaInterface;

use function implode;
use function is_bool;
use function is_string;
use function preg_split;

Expand Down Expand Up @@ -159,7 +160,7 @@
string $indexType = null,
string $indexMethod = null
): string {
return 'CREATE ' . ($indexType ? ($indexType . ' ') : '') . 'INDEX '

Check failure on line 163 in src/QueryBuilder/AbstractDDLQueryBuilder.php

View workflow job for this annotation

GitHub Actions / psalm / PHP 8.1-ubuntu-latest

RiskyTruthyFalsyComparison

src/QueryBuilder/AbstractDDLQueryBuilder.php:163:29: RiskyTruthyFalsyComparison: Operand of type null|string contains type string, which can be falsy and truthy. This can cause possibly unexpected behavior. Use strict comparison instead. (see https://psalm.dev/356)

Check failure on line 163 in src/QueryBuilder/AbstractDDLQueryBuilder.php

View workflow job for this annotation

GitHub Actions / psalm / PHP 8.2-ubuntu-latest

RiskyTruthyFalsyComparison

src/QueryBuilder/AbstractDDLQueryBuilder.php:163:29: RiskyTruthyFalsyComparison: Operand of type null|string contains type string, which can be falsy and truthy. This can cause possibly unexpected behavior. Use strict comparison instead. (see https://psalm.dev/356)
. $this->quoter->quoteTableName($name)
. ' ON ' . $this->quoter->quoteTableName($table)
. ' (' . $this->queryBuilder->buildColumns($columns) . ')';
Expand Down Expand Up @@ -302,4 +303,21 @@
{
return 'TRUNCATE TABLE ' . $this->quoter->quoteTableName($table);
}

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

Check warning on line 307 in src/QueryBuilder/AbstractDDLQueryBuilder.php

View check run for this annotation

Codecov / codecov/patch

src/QueryBuilder/AbstractDDLQueryBuilder.php#L307

Added line #L307 was not covered by tests
{
$sql = 'REFRESH MATERIALIZED VIEW ';

Check warning on line 309 in src/QueryBuilder/AbstractDDLQueryBuilder.php

View check run for this annotation

Codecov / codecov/patch

src/QueryBuilder/AbstractDDLQueryBuilder.php#L309

Added line #L309 was not covered by tests

if ($concurrently) {
$sql .= 'CONCURRENTLY ';

Check warning on line 312 in src/QueryBuilder/AbstractDDLQueryBuilder.php

View check run for this annotation

Codecov / codecov/patch

src/QueryBuilder/AbstractDDLQueryBuilder.php#L311-L312

Added lines #L311 - L312 were not covered by tests
}

$sql .= $this->quoter->quoteTableName($viewName);

Check warning on line 315 in src/QueryBuilder/AbstractDDLQueryBuilder.php

View check run for this annotation

Codecov / codecov/patch

src/QueryBuilder/AbstractDDLQueryBuilder.php#L315

Added line #L315 was not covered by tests

if (is_bool($withData)) {
$sql .= ' WITH ' . ($withData ? 'DATA' : 'NO DATA');

Check warning on line 318 in src/QueryBuilder/AbstractDDLQueryBuilder.php

View check run for this annotation

Codecov / codecov/patch

src/QueryBuilder/AbstractDDLQueryBuilder.php#L317-L318

Added lines #L317 - L318 were not covered by tests
}

return $sql;

Check warning on line 321 in src/QueryBuilder/AbstractDDLQueryBuilder.php

View check run for this annotation

Codecov / codecov/patch

src/QueryBuilder/AbstractDDLQueryBuilder.php#L321

Added line #L321 was not covered by tests
}
}
5 changes: 5 additions & 0 deletions src/QueryBuilder/AbstractQueryBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -405,4 +405,9 @@
): string {
return $this->dmlBuilder->upsert($table, $insertColumns, $updateColumns, $params);
}

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

Check warning on line 409 in src/QueryBuilder/AbstractQueryBuilder.php

View check run for this annotation

Codecov / codecov/patch

src/QueryBuilder/AbstractQueryBuilder.php#L409

Added line #L409 was not covered by tests
{
return $this->ddlBuilder->refreshMaterializedView($viewName, $concurrently, $withData);

Check warning on line 411 in src/QueryBuilder/AbstractQueryBuilder.php

View check run for this annotation

Codecov / codecov/patch

src/QueryBuilder/AbstractQueryBuilder.php#L411

Added line #L411 was not covered by tests
}
}
11 changes: 11 additions & 0 deletions src/QueryBuilder/DDLQueryBuilderInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -424,4 +424,15 @@ public function renameTable(string $oldName, string $newName): string;
* Note: The method will quote the `table` parameter before using it in the generated SQL.
*/
public function truncateTable(string $table): string;

/**
* Refresh materialized view
*
* @param string $viewName The name of the view to refresh.
* @param bool $concurrently Refresh the materialized view without locking out concurrent selects on the materialized view.
* @param bool|null $withData When `true` then the backing query is executed to provide the new data. Otherwise if `false` then no new data is generated and the materialized view is left in an unscannable state
*
* @return string The `REFRESH MATERIALIZED VIEW` SQL statement
*/
public function refreshMaterializedView(string $viewName, bool $concurrently = false, ?bool $withData = null): string;
Copy link
Member

@Tigrov Tigrov Jan 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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()

}
44 changes: 44 additions & 0 deletions tests/Db/Command/CommandTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -727,4 +727,48 @@ public function testProfilerData(string $sql = null): void
);
parent::testProfilerData();
}

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,
],
];
}
Comment on lines +731 to +755
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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



/**
* @dataProvider refreshMaterializedViewDataProvider
* @param string $viewName
* @param bool|null $concurrently
* @param bool|null $withData
* @return void
* @throws \Yiisoft\Db\Exception\Exception
* @throws \Yiisoft\Db\Exception\InvalidConfigException
*/
public function testRefreshMaterializedView(string $viewName, ?bool $concurrently, ?bool $withData): void
{
$db = $this->getConnection();
$this->expectException(NotSupportedException::class);

$db->createCommand()->refreshMaterializedView($viewName, $concurrently, $withData);
}
}
62 changes: 61 additions & 1 deletion tests/Db/QueryBuilder/QueryBuilderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ public function testGetExpressionBuilderException(): void

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

$expression = new class () implements ExpressionInterface {
$expression = new class() implements ExpressionInterface {
Copy link
Member

@Tigrov Tigrov Jan 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

};
$qb = $db->getQueryBuilder();
$qb->getExpressionBuilder($expression);
Expand Down Expand Up @@ -310,4 +310,64 @@ public function testUpsertExecute(
$actualParams = [];
$actualSQL = $db->getQueryBuilder()->upsert($table, $insertColumns, $updateColumns, $actualParams);
}


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',
],
];
}
Comment on lines +315 to +355
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better to move it to QueryBuilderProvider


/**
* @dataProvider refreshMaterializedViewDataProvider
* @param bool $concurrently
* @param bool|null $withData
* @return void
*/
public function testRefreshMaterializedView(string $viewName, bool $concurrently, ?bool $withData, string $expected): void
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks need to move this test to AbstractQueryBuilderTest

{
$db = $this->getConnection();
$driver = $db->getDriverName();
$sql = $db->getQueryBuilder()->refreshMaterializedView($viewName, $concurrently, $withData);
$actual = DbHelper::replaceQuotes($sql, $driver);
Comment on lines +367 to +368
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

$expected = DbHelper::replaceQuotes($expected, $driver);

$this->assertSame($expected, $actual);
}
}
Loading