Skip to content

Add refreshMaterializedView command - #332

Open
Gerych1984 wants to merge 1 commit into
yiisoft:masterfrom
Gerych1984:refresh-mat-view
Open

Add refreshMaterializedView command#332
Gerych1984 wants to merge 1 commit into
yiisoft:masterfrom
Gerych1984:refresh-mat-view

Conversation

@Gerych1984

Copy link
Copy Markdown
Contributor
QA
Is bugfix?
New feature?✔️
Breaks BC?

Add new command to refresh PgSql materialized views. I think it can be added in CommandInterface

@Gerych1984
Gerych1984force-pushed the refresh-mat-view branch 2 times, most recently from aaff285 to 08b86ebCompareJanuary 19, 2024 14:14
@codecov

codecovBot commented Jan 19, 2024

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 95.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 99.85%. Comparing base (02a2a87) to head (f07182c).
⚠️ Report is 166 commits behind head on master.

Files with missing linesPatch %Lines
src/Command.php95.00%1 Missing ⚠️
Additional details and impacted files
@@ Coverage Diff @@## master #332 +/- ##
=============================================
- Coverage 100.00% 99.85% -0.15% - Complexity 241 252 +11 
=============================================
Files 16 16 Lines 694 714 +20 =============================================
+ Hits 694 713 +19 - Misses 0 1 +1 

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment threadsrc/Command.php
}

/**
* @see {https://www.postgresql.org/docs/current/sql-refreshmaterializedview.html}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
* @see {https://www.postgresql.org/docs/current/sql-refreshmaterializedview.html}
* @see https://www.postgresql.org/docs/current/sql-refreshmaterializedview.html

Comment threadsrc/Command.php
$tableSchema = $this->db->getTableSchema($viewName);

if ($tableSchema) {
$hasUnique = count($this->db->getSchema()->findUniqueIndexes($tableSchema)) > 0;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
$hasUnique = count($this->db->getSchema()->findUniqueIndexes($tableSchema)) > 0;
$hasUnique = !empty($this->db->getSchema()->findUniqueIndexes($tableSchema));

Comment threadsrc/Command.php
*/
public function refreshMaterializedView(string $viewName, ?bool $concurrently = null, ?bool $withData = null): bool
{
if ($concurrently || ($concurrently === null || $withData === null)) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
if ($concurrently|| ($concurrently === null || $withData === null)) {
if ($concurrently!== false) {

Comment threadsrc/Command.php
);
}

if ($concurrently && !$hasUnique) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
if ($concurrently && !$hasUnique) {
if ($concurrently=== true&& !$hasUnique) {

@vjik
vjik requested review from Tigrov and darkdefJanuary 20, 2024 11:40
@vjikvjik added the status:code review The pull request needs review. label Jan 20, 2024

@TigrovTigrov left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Good idea.

  • This method should be added in base yii-db package and should be consistent with other DBMS.

  • The part of generating SQL query shold be in DDLQueryBuilder

@Gerych1984

Copy link
Copy Markdown
ContributorAuthor

God idea.

  • This method should be added in base yii-db package and should be consistent with other DBMS.
  • The part of generating SQL query shold be in DDLQueryBuilder

I create this PR and i think they must be merged first

@TigrovTigrov added status:under development Someone is working on a pull request. and removed status:code review The pull request needs review. labels Feb 4, 2024
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status:under developmentSomeone is working on a pull request.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@Gerych1984@vjik@Tigrov