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

[Fix](multi catalog)(planner) Fix external table statistic collection bug. #16486

Merged
merged 1 commit into from
Feb 8, 2023

Conversation

Jibing-Li
Copy link
Contributor

Proposed changes

Issue Number: close #xxx

Problem summary

Add index id to column statistic id. Refresh statistic cache after analyze.

Checklist(Required)

  1. Does it affect the original behavior:
    • Yes
    • No
    • I don't know
  2. Has unit tests been added:
    • Yes
    • No
    • No Need
  3. Has document been added or modified:
    • Yes
    • No
    • No Need
  4. Does it need to update dependencies:
    • Yes
    • No
  5. Are there any changes that cannot be rolled back:
    • Yes (If Yes, please explain WHY)
    • No

Further comments

If this is a relatively large or complex change, kick off the discussion at [email protected] by explaining why you chose the solution you did and what alternatives you considered, etc...

@@ -85,7 +86,7 @@ protected void getColumnStatsByMeta() throws Exception {
Map<String, String> parameters = table.getRemoteTable().getParameters();
// Collect table level row count, null number and timestamp.
setParameterData(parameters, params);
params.put("id", String.valueOf(tbl.getId()) + "-" + String.valueOf(col.getName()));
params.put("id", String.valueOf(tbl.getId()) + "--1-" + col.getName());
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should extract a method to do this, currently, it is very error prone

@@ -294,6 +294,7 @@ public Partition getPartition(List<String> partitionValues) {

@Override
public List<Column> initSchema() {
makeSureInitialized();
Copy link
Contributor

Choose a reason for hiding this comment

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

Why need this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Without this initialization, FE will throw NPE when excuting analyze table hms_catalog.db.table immediately after FE started.

Copy link
Contributor

@morningman morningman left a comment

Choose a reason for hiding this comment

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

LGTM

@morningman morningman merged commit 666f709 into apache:master Feb 8, 2023
morningman pushed a commit that referenced this pull request Feb 8, 2023
… bug (#16486)

Add index id to column statistic id. Refresh statistic cache after analyze.
@Jibing-Li Jibing-Li deleted the planner branch February 9, 2023 05:54
YangShaw pushed a commit to YangShaw/doris that referenced this pull request Feb 17, 2023
… bug (apache#16486)

Add index id to column statistic id. Refresh statistic cache after analyze.
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.

2 participants