You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by GitBox <gi...@apache.org> on 2022/04/06 08:58:16 UTC
[GitHub] [incubator-doris] weizhengte opened a new pull request, #8864: [feature-wip](statistics) step6: statistics is available
weizhengte opened a new pull request, #8864:
URL: https://github.com/apache/incubator-doris/pull/8864
# Proposed changes
This pull request includes some implementations of the statistics(https://github.com/apache/incubator-doris/issues/6370).
users will be able to execute these sql such as "ANALYZE, SHOW ANALYZE, SHOW TABLE/COLUMN STATS..." to collect statistics information and query them.
## Problem Summary:
Describe the overview of changes.
## 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/No)
## Further comments
If this is a relatively large or complex change, kick off the discussion at [dev@doris.apache.org](mailto:dev@doris.apache.org) by explaining why you chose the solution you did and what alternatives you considered, etc...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] Kikyou1997 commented on a diff in pull request #8864: [feature-wip](statistics) step6: statistics is available
Posted by GitBox <gi...@apache.org>.
Kikyou1997 commented on code in PR #8864:
URL: https://github.com/apache/doris/pull/8864#discussion_r979969582
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/ColumnStats.java:
##########
@@ -72,16 +75,94 @@ public class ColumnStats {
private float avgSize = -1; // in bytes
private long maxSize = -1; // in bytes
private long numNulls = -1;
- private LiteralExpr minValue;
- private LiteralExpr maxValue;
-
- public static ColumnStats createDefaultColumnStats() {
- ColumnStats columnStats = new ColumnStats();
- columnStats.setAvgSize(1);
- columnStats.setMaxSize(1);
- columnStats.setNdv(1);
- columnStats.setNumNulls(0);
- return columnStats;
+
+ private LiteralExpr minValue = new NullLiteral();
+ private LiteralExpr maxValue = new NullLiteral();
+
+ /**
+ * Return default column statistic.
+ */
+ public static ColumnStats getDefaultColumnStats() {
+ return new ColumnStats();
+ }
+
+ /**
+ * Merge column statistics(the original statistics should not be modified)
+ *
+ * @param left statistics to be merged
+ * @param right statistics to be merged
+ */
+ public static ColumnStats aggColumnStats(ColumnStats left, ColumnStats right) {
+ // merge ndv
+ long leftNdv = left.getNdv();
+ long rightNdv = right.getNdv();
+
+ if (leftNdv == -1) {
+ leftNdv = rightNdv;
+ } else {
+ leftNdv = rightNdv != -1 ? (leftNdv + rightNdv) : leftNdv;
+ }
+
+ // merge avg_size
+ float leftAvgSize = left.getAvgSize();
+ float rightAvgSize = right.getAvgSize();
+ if (leftAvgSize == -1) {
+ leftAvgSize = rightAvgSize;
+ } else {
+ leftAvgSize = rightAvgSize != -1 ? ((leftAvgSize + rightAvgSize) / 2) : leftAvgSize;
+ }
+
+ // merge max_size
+ long leftMaxSize = left.getMaxSize();
+ long rightMaxSize = right.getMaxSize();
+ if (leftMaxSize == -1) {
+ leftMaxSize = rightMaxSize;
+ } else {
+ leftMaxSize = rightMaxSize != -1 ? (Math.max(leftMaxSize, rightMaxSize)) : leftMaxSize;
Review Comment:
`rightMaxSize != -1` here is redundant
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] Kikyou1997 commented on a diff in pull request #8864: [feature-wip](statistics) step6: statistics is available
Posted by GitBox <gi...@apache.org>.
Kikyou1997 commented on code in PR #8864:
URL: https://github.com/apache/doris/pull/8864#discussion_r979972528
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/ColumnStats.java:
##########
@@ -72,16 +75,94 @@ public class ColumnStats {
private float avgSize = -1; // in bytes
private long maxSize = -1; // in bytes
private long numNulls = -1;
- private LiteralExpr minValue;
- private LiteralExpr maxValue;
-
- public static ColumnStats createDefaultColumnStats() {
- ColumnStats columnStats = new ColumnStats();
- columnStats.setAvgSize(1);
- columnStats.setMaxSize(1);
- columnStats.setNdv(1);
- columnStats.setNumNulls(0);
- return columnStats;
+
+ private LiteralExpr minValue = new NullLiteral();
+ private LiteralExpr maxValue = new NullLiteral();
+
+ /**
+ * Return default column statistic.
+ */
+ public static ColumnStats getDefaultColumnStats() {
+ return new ColumnStats();
+ }
+
+ /**
+ * Merge column statistics(the original statistics should not be modified)
+ *
+ * @param left statistics to be merged
+ * @param right statistics to be merged
+ */
+ public static ColumnStats aggColumnStats(ColumnStats left, ColumnStats right) {
+ // merge ndv
+ long leftNdv = left.getNdv();
+ long rightNdv = right.getNdv();
+
+ if (leftNdv == -1) {
+ leftNdv = rightNdv;
+ } else {
+ leftNdv = rightNdv != -1 ? (leftNdv + rightNdv) : leftNdv;
+ }
+
+ // merge avg_size
+ float leftAvgSize = left.getAvgSize();
+ float rightAvgSize = right.getAvgSize();
+ if (leftAvgSize == -1) {
+ leftAvgSize = rightAvgSize;
+ } else {
+ leftAvgSize = rightAvgSize != -1 ? ((leftAvgSize + rightAvgSize) / 2) : leftAvgSize;
+ }
+
+ // merge max_size
+ long leftMaxSize = left.getMaxSize();
+ long rightMaxSize = right.getMaxSize();
+ if (leftMaxSize == -1) {
+ leftMaxSize = rightMaxSize;
+ } else {
+ leftMaxSize = rightMaxSize != -1 ? (Math.max(leftMaxSize, rightMaxSize)) : leftMaxSize;
+ }
+
+ // merge num_nulls
+ long leftNumNulls = left.getNumNulls();
+ long rightNumNulls = right.getNumNulls();
+ if (leftNumNulls == -1) {
+ leftNumNulls = rightNumNulls;
+ } else {
+ leftNumNulls = rightNumNulls != -1 ? (leftNumNulls + rightNumNulls) : leftNumNulls;
+ }
+
+ // merge min_value
+ LiteralExpr leftMinValue = left.getMinValue();
+ LiteralExpr rightMinValue = right.getMinValue();
+ if (leftMinValue == null) {
+ leftMinValue = rightMinValue;
+ } else {
+ leftMinValue = leftMinValue.compareTo(rightMinValue) > 0 ? rightMinValue : leftMinValue;
Review Comment:
`Math.min` might be more clear
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/ColumnStats.java:
##########
@@ -72,16 +75,94 @@ public class ColumnStats {
private float avgSize = -1; // in bytes
private long maxSize = -1; // in bytes
private long numNulls = -1;
- private LiteralExpr minValue;
- private LiteralExpr maxValue;
-
- public static ColumnStats createDefaultColumnStats() {
- ColumnStats columnStats = new ColumnStats();
- columnStats.setAvgSize(1);
- columnStats.setMaxSize(1);
- columnStats.setNdv(1);
- columnStats.setNumNulls(0);
- return columnStats;
+
+ private LiteralExpr minValue = new NullLiteral();
+ private LiteralExpr maxValue = new NullLiteral();
+
+ /**
+ * Return default column statistic.
+ */
+ public static ColumnStats getDefaultColumnStats() {
+ return new ColumnStats();
+ }
+
+ /**
+ * Merge column statistics(the original statistics should not be modified)
+ *
+ * @param left statistics to be merged
+ * @param right statistics to be merged
+ */
+ public static ColumnStats aggColumnStats(ColumnStats left, ColumnStats right) {
+ // merge ndv
+ long leftNdv = left.getNdv();
+ long rightNdv = right.getNdv();
+
+ if (leftNdv == -1) {
+ leftNdv = rightNdv;
+ } else {
+ leftNdv = rightNdv != -1 ? (leftNdv + rightNdv) : leftNdv;
+ }
+
+ // merge avg_size
+ float leftAvgSize = left.getAvgSize();
+ float rightAvgSize = right.getAvgSize();
+ if (leftAvgSize == -1) {
+ leftAvgSize = rightAvgSize;
+ } else {
+ leftAvgSize = rightAvgSize != -1 ? ((leftAvgSize + rightAvgSize) / 2) : leftAvgSize;
+ }
+
+ // merge max_size
+ long leftMaxSize = left.getMaxSize();
+ long rightMaxSize = right.getMaxSize();
+ if (leftMaxSize == -1) {
+ leftMaxSize = rightMaxSize;
+ } else {
+ leftMaxSize = rightMaxSize != -1 ? (Math.max(leftMaxSize, rightMaxSize)) : leftMaxSize;
+ }
+
+ // merge num_nulls
+ long leftNumNulls = left.getNumNulls();
+ long rightNumNulls = right.getNumNulls();
+ if (leftNumNulls == -1) {
+ leftNumNulls = rightNumNulls;
+ } else {
+ leftNumNulls = rightNumNulls != -1 ? (leftNumNulls + rightNumNulls) : leftNumNulls;
+ }
+
+ // merge min_value
+ LiteralExpr leftMinValue = left.getMinValue();
+ LiteralExpr rightMinValue = right.getMinValue();
+ if (leftMinValue == null) {
+ leftMinValue = rightMinValue;
+ } else {
+ leftMinValue = leftMinValue.compareTo(rightMinValue) > 0 ? rightMinValue : leftMinValue;
Review Comment:
`Math.min` might looks more clear
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on pull request #8864: [feature-wip](statistics) step6: statistics is available
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #8864:
URL: https://github.com/apache/doris/pull/8864#issuecomment-1258883117
PR approved by anyone and no changes requested.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] weizhengte commented on pull request #8864: [feature-wip](statistics) step6: statistics is available
Posted by GitBox <gi...@apache.org>.
weizhengte commented on PR #8864:
URL: https://github.com/apache/doris/pull/8864#issuecomment-1258881956
@morrySnow review plz
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] Kikyou1997 commented on a diff in pull request #8864: [feature-wip](statistics) step6: statistics is available
Posted by GitBox <gi...@apache.org>.
Kikyou1997 commented on code in PR #8864:
URL: https://github.com/apache/doris/pull/8864#discussion_r979976545
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/ColumnStats.java:
##########
@@ -180,23 +262,10 @@ public void updateStats(Type columnType, Map<StatsType, String> statsTypeToValue
}
}
- public List<String> getShowInfo() {
- List<String> result = Lists.newArrayList();
- result.add(Long.toString(ndv));
- result.add(Float.toString(avgSize));
- result.add(Long.toString(maxSize));
- result.add(Long.toString(numNulls));
- if (minValue != null) {
- result.add(minValue.getStringValue());
- } else {
- result.add("N/A");
- }
- if (maxValue != null) {
- result.add(maxValue.getStringValue());
- } else {
- result.add("N/A");
- }
- return result;
+ // TODO: The generated statistics should not be modified
+ public void updateBySelectivity(double selectivity) {
+ ndv = (long) Math.ceil(ndv * selectivity);
Review Comment:
Why not use `Math.round`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] Kikyou1997 commented on a diff in pull request #8864: [feature-wip](statistics) step6: statistics is available
Posted by GitBox <gi...@apache.org>.
Kikyou1997 commented on code in PR #8864:
URL: https://github.com/apache/doris/pull/8864#discussion_r979983061
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/Statistics.java:
##########
@@ -225,12 +199,40 @@ public void mockTableStatsWithRowCount(long tableId, long rowCount) {
}
if (tableStats.getRowCount() != rowCount) {
- tableStats.setRowCount(rowCount);
+ // tableStats.setRowCount(rowCount);
}
}
- // Used for unit test
- public void putTableStats(long id, TableStats tableStats) {
- this.idToTableStats.put(id, tableStats);
+ /**
+ * if the table stats is not exist, create a new one.
+ *
+ * @param tableId table id
+ * @return @TableStats
+ */
+ private TableStats getNotNullTableStats(long tableId) {
Review Comment:
Do we have machanism to remove the stats of deleted table currently?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] Kikyou1997 commented on a diff in pull request #8864: [feature-wip](statistics) step6: statistics is available
Posted by GitBox <gi...@apache.org>.
Kikyou1997 commented on code in PR #8864:
URL: https://github.com/apache/doris/pull/8864#discussion_r980662059
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/ColumnStats.java:
##########
@@ -180,23 +262,10 @@ public void updateStats(Type columnType, Map<StatsType, String> statsTypeToValue
}
}
- public List<String> getShowInfo() {
- List<String> result = Lists.newArrayList();
- result.add(Long.toString(ndv));
- result.add(Float.toString(avgSize));
- result.add(Long.toString(maxSize));
- result.add(Long.toString(numNulls));
- if (minValue != null) {
- result.add(minValue.getStringValue());
- } else {
- result.add("N/A");
- }
- if (maxValue != null) {
- result.add(maxValue.getStringValue());
- } else {
- result.add("N/A");
- }
- return result;
+ // TODO: The generated statistics should not be modified
+ public void updateBySelectivity(double selectivity) {
+ ndv = (long) Math.ceil(ndv * selectivity);
Review Comment:
ok thx
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on pull request #8864: [feature-wip](statistics) step6: statistics is available
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #8864:
URL: https://github.com/apache/doris/pull/8864#issuecomment-1259023509
PR approved by at least one committer and no changes requested.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] weizhengte commented on a diff in pull request #8864: [feature-wip](statistics) step6: statistics is available
Posted by GitBox <gi...@apache.org>.
weizhengte commented on code in PR #8864:
URL: https://github.com/apache/doris/pull/8864#discussion_r980631453
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/ColumnStats.java:
##########
@@ -72,16 +75,94 @@ public class ColumnStats {
private float avgSize = -1; // in bytes
private long maxSize = -1; // in bytes
private long numNulls = -1;
- private LiteralExpr minValue;
- private LiteralExpr maxValue;
-
- public static ColumnStats createDefaultColumnStats() {
- ColumnStats columnStats = new ColumnStats();
- columnStats.setAvgSize(1);
- columnStats.setMaxSize(1);
- columnStats.setNdv(1);
- columnStats.setNumNulls(0);
- return columnStats;
+
+ private LiteralExpr minValue = new NullLiteral();
+ private LiteralExpr maxValue = new NullLiteral();
+
+ /**
+ * Return default column statistic.
+ */
+ public static ColumnStats getDefaultColumnStats() {
+ return new ColumnStats();
+ }
+
+ /**
+ * Merge column statistics(the original statistics should not be modified)
+ *
+ * @param left statistics to be merged
+ * @param right statistics to be merged
+ */
+ public static ColumnStats aggColumnStats(ColumnStats left, ColumnStats right) {
Review Comment:
ok
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/StatisticsJobManager.java:
##########
@@ -80,16 +80,24 @@ public Map<Long, StatisticsJob> getIdToStatisticsJob() {
}
public void createStatisticsJob(AnalyzeStmt analyzeStmt) throws UserException {
- // step1: init statistics job by analyzeStmt
- StatisticsJob statisticsJob = StatisticsJob.fromAnalyzeStmt(analyzeStmt);
- writeLock();
- try {
- // step2: check restrict
- checkRestrict(analyzeStmt.getDbId(), statisticsJob.getTblIds());
- // step3: create it
- createStatisticsJob(statisticsJob);
- } finally {
- writeUnlock();
+ // The current statistics are only used for CBO test,
+ // and are not available to users. (work in progress)
+ // TODO(wzt): Further tests are needed
+ if (Config.enable_cbo_statistics) {
Review Comment:
sure
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] Kikyou1997 commented on a diff in pull request #8864: [feature-wip](statistics) step6: statistics is available
Posted by GitBox <gi...@apache.org>.
Kikyou1997 commented on code in PR #8864:
URL: https://github.com/apache/doris/pull/8864#discussion_r979992528
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/StatisticsJobManager.java:
##########
@@ -80,16 +80,24 @@ public Map<Long, StatisticsJob> getIdToStatisticsJob() {
}
public void createStatisticsJob(AnalyzeStmt analyzeStmt) throws UserException {
- // step1: init statistics job by analyzeStmt
- StatisticsJob statisticsJob = StatisticsJob.fromAnalyzeStmt(analyzeStmt);
- writeLock();
- try {
- // step2: check restrict
- checkRestrict(analyzeStmt.getDbId(), statisticsJob.getTblIds());
- // step3: create it
- createStatisticsJob(statisticsJob);
- } finally {
- writeUnlock();
+ // The current statistics are only used for CBO test,
+ // and are not available to users. (work in progress)
+ // TODO(wzt): Further tests are needed
+ if (Config.enable_cbo_statistics) {
Review Comment:
Set it as a session variables rather than an options in configuration file
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] weizhengte commented on a diff in pull request #8864: [feature-wip](statistics) step6: statistics is available
Posted by GitBox <gi...@apache.org>.
weizhengte commented on code in PR #8864:
URL: https://github.com/apache/doris/pull/8864#discussion_r980632737
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/Statistics.java:
##########
@@ -225,12 +199,40 @@ public void mockTableStatsWithRowCount(long tableId, long rowCount) {
}
if (tableStats.getRowCount() != rowCount) {
- tableStats.setRowCount(rowCount);
+ // tableStats.setRowCount(rowCount);
}
}
- // Used for unit test
- public void putTableStats(long id, TableStats tableStats) {
- this.idToTableStats.put(id, tableStats);
+ /**
+ * if the table stats is not exist, create a new one.
+ *
+ * @param tableId table id
+ * @return @TableStats
+ */
+ private TableStats getNotNullTableStats(long tableId) {
Review Comment:
Not yet, it is planned to be implemented with automatic update of statistics in the future.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] morrySnow merged pull request #8864: [feature-wip](statistics) step6: statistics is available
Posted by GitBox <gi...@apache.org>.
morrySnow merged PR #8864:
URL: https://github.com/apache/doris/pull/8864
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] weizhengte commented on a diff in pull request #8864: [feature-wip](statistics) step6: statistics is available
Posted by GitBox <gi...@apache.org>.
weizhengte commented on code in PR #8864:
URL: https://github.com/apache/doris/pull/8864#discussion_r980638089
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/ColumnStats.java:
##########
@@ -180,23 +262,10 @@ public void updateStats(Type columnType, Map<StatsType, String> statsTypeToValue
}
}
- public List<String> getShowInfo() {
- List<String> result = Lists.newArrayList();
- result.add(Long.toString(ndv));
- result.add(Float.toString(avgSize));
- result.add(Long.toString(maxSize));
- result.add(Long.toString(numNulls));
- if (minValue != null) {
- result.add(minValue.getStringValue());
- } else {
- result.add("N/A");
- }
- if (maxValue != null) {
- result.add(maxValue.getStringValue());
- } else {
- result.add("N/A");
- }
- return result;
+ // TODO: The generated statistics should not be modified
+ public void updateBySelectivity(double selectivity) {
+ ndv = (long) Math.ceil(ndv * selectivity);
Review Comment:
It was previously implemented to update ndv by selectivity. the way I understand it is that `Math.round` might lead result to 0.
I intend to remove this method, the collected statistics should not be changed in this way.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] Kikyou1997 commented on a diff in pull request #8864: [feature-wip](statistics) step6: statistics is available
Posted by GitBox <gi...@apache.org>.
Kikyou1997 commented on code in PR #8864:
URL: https://github.com/apache/doris/pull/8864#discussion_r979964422
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/ColumnStats.java:
##########
@@ -72,16 +75,94 @@ public class ColumnStats {
private float avgSize = -1; // in bytes
private long maxSize = -1; // in bytes
private long numNulls = -1;
- private LiteralExpr minValue;
- private LiteralExpr maxValue;
-
- public static ColumnStats createDefaultColumnStats() {
- ColumnStats columnStats = new ColumnStats();
- columnStats.setAvgSize(1);
- columnStats.setMaxSize(1);
- columnStats.setNdv(1);
- columnStats.setNumNulls(0);
- return columnStats;
+
+ private LiteralExpr minValue = new NullLiteral();
+ private LiteralExpr maxValue = new NullLiteral();
+
+ /**
+ * Return default column statistic.
+ */
+ public static ColumnStats getDefaultColumnStats() {
+ return new ColumnStats();
+ }
+
+ /**
+ * Merge column statistics(the original statistics should not be modified)
+ *
+ * @param left statistics to be merged
+ * @param right statistics to be merged
+ */
+ public static ColumnStats aggColumnStats(ColumnStats left, ColumnStats right) {
Review Comment:
rename this method to `mergeColumnStats`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] Kikyou1997 commented on a diff in pull request #8864: [feature-wip](statistics) step6: statistics is available
Posted by GitBox <gi...@apache.org>.
Kikyou1997 commented on code in PR #8864:
URL: https://github.com/apache/doris/pull/8864#discussion_r980662194
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/Statistics.java:
##########
@@ -225,12 +199,40 @@ public void mockTableStatsWithRowCount(long tableId, long rowCount) {
}
if (tableStats.getRowCount() != rowCount) {
- tableStats.setRowCount(rowCount);
+ // tableStats.setRowCount(rowCount);
}
}
- // Used for unit test
- public void putTableStats(long id, TableStats tableStats) {
- this.idToTableStats.put(id, tableStats);
+ /**
+ * if the table stats is not exist, create a new one.
+ *
+ * @param tableId table id
+ * @return @TableStats
+ */
+ private TableStats getNotNullTableStats(long tableId) {
Review Comment:
fine
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] Kikyou1997 commented on a diff in pull request #8864: [feature-wip](statistics) step6: statistics is available
Posted by GitBox <gi...@apache.org>.
Kikyou1997 commented on code in PR #8864:
URL: https://github.com/apache/doris/pull/8864#discussion_r980019243
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/StatisticsJobManager.java:
##########
@@ -80,16 +80,24 @@ public Map<Long, StatisticsJob> getIdToStatisticsJob() {
}
public void createStatisticsJob(AnalyzeStmt analyzeStmt) throws UserException {
- // step1: init statistics job by analyzeStmt
- StatisticsJob statisticsJob = StatisticsJob.fromAnalyzeStmt(analyzeStmt);
- writeLock();
Review Comment:
Seems like the read lock is never used, so why not use object lock directly
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] weizhengte commented on a diff in pull request #8864: [feature-wip](statistics) step6: statistics is available
Posted by GitBox <gi...@apache.org>.
weizhengte commented on code in PR #8864:
URL: https://github.com/apache/doris/pull/8864#discussion_r980632045
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/StatisticsJobManager.java:
##########
@@ -80,16 +80,24 @@ public Map<Long, StatisticsJob> getIdToStatisticsJob() {
}
public void createStatisticsJob(AnalyzeStmt analyzeStmt) throws UserException {
- // step1: init statistics job by analyzeStmt
- StatisticsJob statisticsJob = StatisticsJob.fromAnalyzeStmt(analyzeStmt);
- writeLock();
Review Comment:
sure
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org