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.

Sign up for GitHub

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

Jump to bottom

[Feature] support analyze predicate columns #53013

Merged
merged 25 commits into from
Dec 13, 2024
Merged

[Feature] support analyze predicate columns #53013

merged 25 commits into from
Dec 13, 2024

Conversation

murphyatwork
Copy link
Contributor

@murphyatwork murphyatwork commented Nov 19, 2024 •
edited
Loading

Why I'm doing:

What I'm doing:

Covered:

  • Track column usage and expose it through information_schema.column_stats_usage
  • Persist column usage into internal table
  • Support information_schema.analyze_status
  • Support analyze predicate columns

Not Covered:

  • Support auto analyze predicate columns

Fixes #53204

What type of PR is this:

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

Does this PR entail a change in behavior?

  • Yes, this PR will result in a change in behavior.
  • No, this PR will not result in a change in behavior.

If yes, please specify the type of change:

  • Interface/UI changes: syntax, type conversion, expression evaluation, display information
  • Parameter changes: default values, similar parameters but with different default values
  • Policy changes: use new policy to replace old one, functionality automatically enabled
  • Feature removed
  • Miscellaneous: upgrade & downgrade compatibility, etc.

Checklist:

  • I have added test cases for my bug fix or my new feature
  • This pr needs user documentation (for new or modified features or behaviors)
    • I have added documentation for my new feature or new function
  • This is a backport pr

Bugfix cherry-pick branch check:

  • I have checked the version labels which the pr will be auto-backported to the target branch
    • 3.3
    • 3.2
    • 3.1
    • 3.0
    • 2.5

@wanpengfei-git wanpengfei-git requested review from a team November 19, 2024 12:32
@murphyatwork murphyatwork force-pushed the murphy_feat_predicate_columns branch 11 times, most recently from 2d2aacf to d89d6ba Compare November 26, 2024 07:09
@murphyatwork murphyatwork marked this pull request as ready for review November 26, 2024 07:09
@murphyatwork murphyatwork requested review from a team as code owners November 26, 2024 07:09
@murphyatwork murphyatwork force-pushed the murphy_feat_predicate_columns branch from d89d6ba to ef86122 Compare November 26, 2024 07:38
satanson
satanson previously approved these changes Nov 26, 2024
View reviewed changes
@murphyatwork murphyatwork force-pushed the murphy_feat_predicate_columns branch from ef86122 to 1b559fd Compare November 29, 2024 07:49
dirtysalt
dirtysalt reviewed Nov 29, 2024
View reviewed changes
fe/fe-core/src/main/java/com/starrocks/catalog/OlapTable.java
@@ -348,6 +351,17 @@ public OlapTable(long id, String tableName, List<Column> baseSchema, KeysType ke
this.tableProperty = null;
}

@Override
public synchronized Optional<String> mayGetDatabaseName() {
Copy link
Contributor

Choose a reason for hiding this comment

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

why synchronized? Can not think of a situation of data race.

Plus, may I ask why to use Optional<String>? Wont it be staightforward to return null directly?

Copy link
Contributor

Choose a reason for hiding this comment

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

@dirtysalt Optional is better than null, the former would force coder to check if the value is null, the latter would not.

@murphyatwork murphyatwork force-pushed the murphy_feat_predicate_columns branch 2 times, most recently from e0e7061 to 51e9707 Compare December 2, 2024 05:17
@murphyatwork
vacuum
7011d10 
Signed-off-by: Murphy <[email protected]>
@murphyatwork
persist db_id and table_id
ce737d4 
Signed-off-by: Murphy <[email protected]>
@murphyatwork
use columnUniqueId instead of column_id
388e8c0 
Signed-off-by: Murphy <[email protected]>
@murphyatwork
rename package
5cb91ff 
Signed-off-by: Murphy <[email protected]>
@murphyatwork
be impl
56071bd 
Signed-off-by: Murphy <[email protected]>
@murphyatwork
fe schema
9740e5b 
Signed-off-by: Murphy <[email protected]>
@murphyatwork
change to datetime type
56dc74d 
Signed-off-by: Murphy <[email protected]>
@murphyatwork
pack with json
f95a561 
Signed-off-by: Murphy <[email protected]>
@murphyatwork
fix
b7e79ec 
Signed-off-by: Murphy <[email protected]>
@murphyatwork
add schema table analyze_status
a92d5b9 
Signed-off-by: Murphy <[email protected]>
@murphyatwork
sql test
8fe7b53 
Signed-off-by: Murphy <[email protected]>
@murphyatwork
be analyze_status
cf16937 
Signed-off-by: Murphy <[email protected]>
@murphyatwork
fix sql test
99646d7 
Signed-off-by: Murphy <[email protected]>
@murphyatwork
fix recursive overflow
145c476 
Signed-off-by: Murphy <[email protected]>
@murphyatwork
fix test
8667202 
Signed-off-by: Murphy <[email protected]>
@murphyatwork
fix
7d0b117 
Signed-off-by: Murphy <[email protected]>
@murphyatwork
check table type
f29c972 
Signed-off-by: Murphy <[email protected]>
@murphyatwork
fix sql test
3f00624 
Signed-off-by: Murphy <[email protected]>
@murphyatwork murphyatwork force-pushed the murphy_feat_predicate_columns branch from 29fcc50 to 3f00624 Compare December 9, 2024 05:50
@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

@github-actions GitHub Actions
Copy link

[Java-Extensions Incremental Coverage Report]

✅ pass : 0 / 0 (0%)

@github-actions GitHub Actions
Copy link

[FE Incremental Coverage Report]

✅ pass : 508 / 556 (91.37%)

file detail

path covered_line new_line coverage not_covered_line_detail
🔵 com/starrocks/statistic/AnalyzeStatus.java 0 2 00.00% [44, 50]
🔵 com/starrocks/catalog/Table.java 1 2 50.00% [273]
🔵 com/starrocks/statistic/columns/ColumnFullId.java 21 30 70.00% [50, 51, 52, 53, 54, 56, 65, 74, 77]
🔵 com/starrocks/service/FrontendServiceImpl.java 12 16 75.00% [728, 729, 745, 746]
🔵 com/starrocks/statistic/columns/ColumnUsage.java 49 65 75.38% [86, 87, 135, 136, 137, 138, 139, 145, 148, 161, 165, 170, 171, 172, 174, 193]
🔵 com/starrocks/sql/optimizer/statistics/StatisticsCalculator.java 13 15 86.67% [1711, 1712]
🔵 com/starrocks/statistic/columns/TableNamePredicate.java 11 12 91.67% [36]
🔵 com/starrocks/statistic/columns/PredicateColumnsMgr.java 91 96 94.79% [167, 168, 169, 180, 181]
🔵 com/starrocks/sql/analyzer/AnalyzeStmtAnalyzer.java 16 17 94.12% [164]
🔵 com/starrocks/catalog/system/information/ColumnStatsUsageSystemTable.java 58 61 95.08% [90, 92, 97]
🔵 com/starrocks/statistic/columns/PredicateColumnsStorage.java 121 124 97.58% [172, 311, 312]
🔵 com/starrocks/catalog/system/information/AnalyzeStatusSystemTable.java 52 53 98.11% [104]
🔵 com/starrocks/catalog/Database.java 4 4 100.00% []
🔵 com/starrocks/common/util/TimeUtils.java 2 2 100.00% []
🔵 com/starrocks/catalog/system/information/InfoSchemaDb.java 2 2 100.00% []
🔵 com/starrocks/sql/ast/ShowAnalyzeStatusStmt.java 1 1 100.00% []
🔵 com/starrocks/catalog/OlapTable.java 5 5 100.00% []
🔵 com/starrocks/common/Config.java 2 2 100.00% []
🔵 com/starrocks/sql/optimizer/Utils.java 22 22 100.00% []
🔵 com/starrocks/sql/analyzer/QueryAnalyzer.java 2 2 100.00% []
🔵 com/starrocks/sql/ast/AnalyzeStmt.java 4 4 100.00% []
🔵 com/starrocks/server/GlobalStateMgr.java 1 1 100.00% []
🔵 com/starrocks/sql/optimizer/base/ColumnRefFactory.java 4 4 100.00% []
🔵 com/starrocks/scheduler/history/TableKeeper.java 6 6 100.00% []
🔵 com/starrocks/sql/optimizer/operator/Projection.java 3 3 100.00% []
🔵 com/starrocks/analysis/TableName.java 2 2 100.00% []
🔵 com/starrocks/catalog/system/SystemTable.java 3 3 100.00% []

@github-actions GitHub Actions
Copy link

[BE Incremental Coverage Report]

✅ pass : 109 / 112 (97.32%)

file detail

path covered_line new_line coverage not_covered_line_detail
🔵 be/src/exec/schema_scanner/schema_analyze_status.cpp 43 45 95.56% [54, 60]
🔵 be/src/exec/schema_scanner/schema_column_stats_usage_scanner.cpp 54 55 98.18% [49]
🔵 be/src/exec/schema_scanner.cpp 4 4 100.00% []
🔵 be/src/exec/schema_scanner/schema_helper.cpp 8 8 100.00% []

@murphyatwork murphyatwork merged commit 9f37188 into StarRocks:main Dec 13, 2024
52 of 59 checks passed
@wanpengfei-git wanpengfei-git mentioned this pull request Dec 13, 2024
Open
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dirtysalt dirtysalt dirtysalt left review comments

@satanson satanson satanson approved these changes

@LiShuMing LiShuMing LiShuMing approved these changes

@kangkaisen kangkaisen kangkaisen approved these changes

@gengjun-git gengjun-git gengjun-git approved these changes

Assignees

@murphyatwork murphyatwork

Labels
META-REVIEW PROTO-REVIEW version:3.5
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support analyze predicate columns
7 participants
@murphyatwork @dirtysalt @satanson @LiShuMing @kangkaisen @gengjun-git @wanpengfei-git