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

[Enhancement] Support alter materialized view to active #23304

Closed
wants to merge 6 commits into from
Closed

[Enhancement] Support alter materialized view to active #23304

wants to merge 6 commits into from

Conversation

Astralidea
Copy link
Contributor

@Astralidea Astralidea commented May 11, 2023

What type of PR is this:

  • BugFix
  • Feature
  • Enhancement
  • Refactor
  • UT
  • Doc
  • Tool

Which issues of this PR fixes:

Fixes #

Problem Summary(Required):

CREATE TABLE t1 (
                            k1 date,
                            v1 INT,
                            v2 INT)
                        DUPLICATE KEY(k1)
                        PARTITION BY RANGE(`k1`)
                        (
                        PARTITION `p1` VALUES LESS THAN ('2023-01-01'),
                        PARTITION `p2` VALUES LESS THAN ('2023-02-01'),
                        PARTITION `p3` VALUES LESS THAN ('2023-03-01'),
                        PARTITION `p4` VALUES LESS THAN ('2023-04-01'),
                        PARTITION `p5` VALUES LESS THAN ('2023-05-01'),
                        PARTITION `p6` VALUES LESS THAN ('2023-06-01')
                        )
                        DISTRIBUTED BY HASH(k1)
PROPERTIES (
"replication_num" = "1",
"storage_format" = "v2"
);

insert into t1 values ("2019-01-01",1,1),("2019-01-01",1,2),("2019-01-01",2,1),("2019-01-01",2,2),
                                              ("2023-01-11",1,1),("2023-01-11",1,2),("2023-02-11",2,1),("2023-01-11",2,2),
                                              ("2023-03-11",1,1),("2023-05-11",1,2),("2023-04-11",2,1),("2023-05-01",2,2);

CREATE MATERIALIZED VIEW mv1
               PARTITION BY k1
               DISTRIBUTED BY HASH(k1) BUCKETS 10
               REFRESH ASYNC
               PROPERTIES(
               "partition_ttl_number"="3",
               "auto_refresh_partitions_limit"="3"
               )
               AS SELECT k1, sum(v1) as sum_v1 FROM t1 group by k1;

drop table t1;
refresh MATERIALIZED VIEW mv1; -- failed

CREATE TABLE t1 (
                            k1 date,
                            v1 INT,
                            v2 INT)
                        DUPLICATE KEY(k1)
                        PARTITION BY RANGE(`k1`)
                        (
                        PARTITION `p1` VALUES LESS THAN ('2023-01-01'),
                        PARTITION `p2` VALUES LESS THAN ('2023-02-01'),
                        PARTITION `p3` VALUES LESS THAN ('2023-03-01'),
                        PARTITION `p4` VALUES LESS THAN ('2023-04-01'),
                        PARTITION `p5` VALUES LESS THAN ('2023-05-01'),
                        PARTITION `p6` VALUES LESS THAN ('2023-06-01')
                        )
                        DISTRIBUTED BY HASH(k1)
PROPERTIES (
"replication_num" = "1",
"storage_format" = "v2"
);
insert into t1 values ("2019-01-01",1,1),("2019-01-01",1,2),("2019-01-01",2,1),("2019-01-01",2,2),
                                              ("2023-01-11",1,1),("2023-01-11",1,2),("2023-02-11",2,1),("2023-01-11",2,2),
                                              ("2023-03-22",1,1),("2023-05-22",1,2),("2023-04-22",2,1),("2023-05-01",2,2);
ALTER MATERIALIZED VIEW mv1 ACTIVE
refresh MATERIALIZED VIEW mv1; -- success

There are still some issues with this solution, which will be resolved in the next PR.

Checklist:

  • I have added test cases for my bug fix or my new feature
  • This pr will affect users' behaviors
  • This pr needs user documentation (for new or modified features or behaviors)
    • I have added documentation for my new feature or new function

Bugfix cherry-pick branch check:

  • I have checked the version labels which the pr will be auto backported to target branch
    • 3.0
    • 2.5
    • 2.4
    • 2.3

@Astralidea Astralidea requested a review from a team as a code owner May 11, 2023 11:46
Signed-off-by: Astralidea <[email protected]>
Map<TableName, Table> tableNameTableMap = AnalyzerUtils.collectAllConnectorTableAndView(queryStatement);
List<BaseTableInfo> baseTableInfos = MaterializedViewAnalyzer.getBaseTableInfos(tableNameTableMap);
materializedView.setBaseTableInfos(baseTableInfos);
materializedView.getRefreshScheme().getAsyncRefreshContext().clearVisibleVersionMap();
Copy link
Contributor

Choose a reason for hiding this comment

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

What about the MV's history data? Can it be queried again?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The historical data of the MV can be queried, and theoretically it can be forced to trigger another refresh. This issue can be discussed together with the next PR issue.

Copy link
Contributor

Choose a reason for hiding this comment

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

After set active, we need to refresh this mv to enable the query rewrite?

continue;
}
MvId mvId = new MvId(dbId, mv.getId());
table.addRelatedMaterializedView(mvId);
Copy link
Contributor

Choose a reason for hiding this comment

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

table may already contain the mvId?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

addRelatedMaterializedView use Set structure。
So even if you have it before, you won't add more. Normally, the MvID will be removed normally during Drop.

Copy link
Contributor

Choose a reason for hiding this comment

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

Other DDLs (like altering table column names) also can cause Mv's to be inactive, so need to be more careful.

fe/fe-core/src/main/java/com/starrocks/alter/Alter.java Outdated Show resolved Hide resolved
if (table == null) {
LOG.warn("Setting the materialized view {}({}) to invalid because " +
"the table {} was not exist.", mv.getName(), mv.getId(), baseTableInfo.getTableName());
mv.setActive(false);
Copy link
Contributor

Choose a reason for hiding this comment

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

please update the inactive reason at the same time

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know how to set maybe after your PR #21591 merge?

@sonarcloud
Copy link

sonarcloud bot commented May 19, 2023

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 6 Code Smells

0.0% 0.0% Coverage
4.8% 4.8% Duplication

@wanpengfei-git
Copy link
Collaborator

[FE PR Coverage Check]

😞 fail : 45 / 161 (27.95%)

file detail

path covered_line new_line coverage not_covered_line_detail
🔵 com/starrocks/catalog/MaterializedView.java 0 3 00.00% [173, 174, 175]
🔵 com/starrocks/sql/analyzer/SemanticException.java 0 1 00.00% [72]
🔵 com/starrocks/server/GlobalStateMgr.java 0 27 00.00% [1563, 1564, 1574, 1575, 1576, 1577, 1578, 1579, 1580, 1582, 1583, 1584, 1585, 1586, 1587, 1588, 1590, 1591, 1592, 1593, 1594, 1595, 1596, 1598, 1599, 3298, 3299]
🔵 com/starrocks/persist/AlterMaterializedViewStatusLog.java 0 17 00.00% [38, 39, 40, 41, 42, 45, 49, 50, 53, 57, 58, 61, 65, 66, 70, 71, 74]
🔵 com/starrocks/persist/EditLog.java 0 6 00.00% [329, 330, 331, 332, 1637, 1638]
🔵 com/starrocks/journal/JournalEntity.java 0 3 00.00% [326, 327, 328]
🔵 com/starrocks/alter/Alter.java 1 49 02.04% [315, 316, 317, 318, 320, 321, 322, 323, 324, 325, 326, 328, 330, 331, 333, 335, 336, 337, 524, 525, 526, 527, 528, 529, 531, 532, 534, 535, 536, 538, 539, 541, 542, 543, 544, 545, 547, 548, 549, 551, 633, 634, 635, 636, 638, 639, 641, 643]
🔵 com/starrocks/sql/parser/AstBuilder.java 3 4 75.00% [1642]
🔵 com/starrocks/sql/analyzer/MaterializedViewAnalyzer.java 36 46 78.26% [138, 140, 168, 169, 170, 171, 173, 756, 757, 758]
🔵 com/starrocks/sql/ast/AlterMaterializedViewStmt.java 5 5 100.00% []

@Astralidea Astralidea closed this May 23, 2023
auto-merge was automatically disabled May 23, 2023 13:02

Pull request was closed

@wanpengfei-git
Copy link
Collaborator

@Mergifyio backport branch-3.0

@github-actions github-actions bot removed the 3.0 label May 23, 2023
@wanpengfei-git
Copy link
Collaborator

@Mergifyio backport branch-2.5

@mergify
Copy link
Contributor

mergify bot commented May 23, 2023

backport branch-3.0

🟠 Waiting for conditions to match

  • merged [:pushpin: backport requirement]

@github-actions github-actions bot removed the 2.5 label May 23, 2023
@mergify
Copy link
Contributor

mergify bot commented May 23, 2023

backport branch-2.5

🟠 Waiting for conditions to match

  • merged [:pushpin: backport requirement]

murphyatwork pushed a commit that referenced this pull request May 24, 2023
mergify bot pushed a commit that referenced this pull request May 24, 2023
Fixes #23304

---------

Signed-off-by: Astralidea <[email protected]>
(cherry picked from commit f74e15e)

# Conflicts:
#	fe/fe-core/src/main/java/com/starrocks/alter/Alter.java
#	fe/fe-core/src/main/java/com/starrocks/server/GlobalStateMgr.java
#	fe/fe-core/src/main/java/com/starrocks/sql/analyzer/MaterializedViewAnalyzer.java
#	fe/fe-core/src/main/java/com/starrocks/sql/analyzer/SemanticException.java
#	fe/fe-core/src/main/java/com/starrocks/sql/ast/AlterMaterializedViewStmt.java
#	fe/fe-core/src/main/java/com/starrocks/sql/parser/AstBuilder.java
#	fe/fe-core/src/main/java/com/starrocks/sql/parser/StarRocks.g4
#	fe/fe-core/src/main/java/com/starrocks/sql/parser/StarRocksLex.g4
#	test/sql/test_materialized_column/R/test_materialized_column
#	test/sql/test_mv/R/basic
mergify bot pushed a commit that referenced this pull request May 24, 2023
Fixes #23304

---------

Signed-off-by: Astralidea <[email protected]>
(cherry picked from commit f74e15e)

# Conflicts:
#	fe/fe-core/src/main/java/com/starrocks/alter/Alter.java
#	fe/fe-core/src/main/java/com/starrocks/server/GlobalStateMgr.java
#	fe/fe-core/src/main/java/com/starrocks/sql/analyzer/MaterializedViewAnalyzer.java
#	fe/fe-core/src/main/java/com/starrocks/sql/analyzer/SemanticException.java
#	fe/fe-core/src/main/java/com/starrocks/sql/ast/AlterMaterializedViewStmt.java
#	fe/fe-core/src/main/java/com/starrocks/sql/parser/AstBuilder.java
#	fe/fe-core/src/main/java/com/starrocks/sql/parser/StarRocks.g4
#	fe/fe-core/src/main/java/com/starrocks/sql/parser/StarRocksLex.g4
#	test/sql/test_materialized_column/R/test_materialized_column
#	test/sql/test_mv/R/basic
wxl24life pushed a commit to wxl24life/starrocks that referenced this pull request May 25, 2023
abc982627271 pushed a commit to abc982627271/starrocks that referenced this pull request Jun 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants