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/09/20 03:37:48 UTC
[GitHub] [doris] weizhengte opened a new pull request, #12765: [feature-wip](statistics) collect statistics by sql task
weizhengte opened a new pull request, #12765:
URL: https://github.com/apache/doris/pull/12765
# Proposed changes
This pull request includes some implementations of the statistics(#6370), it Implement sql-task to collect statistics based on internal-query(#9983).
After the `ANALYZE` statement is parsed, statistical tasks will be generated. The statistical tasks includes mata-task(get statistics from metadata) and sql-task(get statistics by sql query). For sql-task, it will get statistics such as the row_count, the number of null values, and the maximum value by SQL query.
Issue Number: close #xxx
## Problem summary
Describe your 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 (If Yes, please explain WHY)
- [ ] 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] weizhengte commented on a diff in pull request #12765: [feature-wip](statistics) collect statistics by sql task
Posted by GitBox <gi...@apache.org>.
weizhengte commented on code in PR #12765:
URL: https://github.com/apache/doris/pull/12765#discussion_r977089097
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/SQLStatisticsTask.java:
##########
@@ -17,47 +17,119 @@
package org.apache.doris.statistics;
-import org.apache.doris.analysis.SelectStmt;
+import org.apache.doris.catalog.Database;
+import org.apache.doris.catalog.Env;
+import org.apache.doris.catalog.Table;
+import org.apache.doris.common.DdlException;
+import org.apache.doris.common.InvalidFormatException;
+import org.apache.doris.statistics.StatisticsTaskResult.TaskResult;
+import org.apache.doris.statistics.StatsGranularity.Granularity;
+import org.apache.doris.statistics.util.InternalQuery;
+import org.apache.doris.statistics.util.InternalQueryResult;
+import org.apache.doris.statistics.util.InternalQueryResult.ResultRow;
+import org.apache.doris.statistics.util.InternalSqlTemplate;
+
+import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
import java.util.List;
+import java.util.Map;
/**
* A statistics task that collects statistics by executing query.
* The results of the query will be returned as @StatisticsTaskResult.
*/
public class SQLStatisticsTask extends StatisticsTask {
- private SelectStmt query;
+ private String statement;
public SQLStatisticsTask(long jobId, List<StatisticsDesc> statsDescs) {
super(jobId, statsDescs);
}
@Override
public StatisticsTaskResult call() throws Exception {
- // TODO
- // step1: construct query by statsDescList
- constructQuery();
- // step2: execute query
- // the result should be sequence by @statsTypeList
- List<String> queryResultList = executeQuery(query);
- // step3: construct StatisticsTaskResult by query result
- constructTaskResult(queryResultList);
- return null;
+ checkStatisticsDesc();
+ List<TaskResult> taskResults = Lists.newArrayList();
+
+ for (StatisticsDesc statsDesc : statsDescs) {
+ statement = constructQuery(statsDesc);
+ TaskResult taskResult = executeQuery(statsDesc);
+ taskResults.add(taskResult);
+ }
+
+ return new StatisticsTaskResult(taskResults);
}
- protected void constructQuery() {
- // TODO
- // step1: construct FROM by @granularityDesc
- // step2: construct SELECT LIST by @statsTypeList
+ protected String constructQuery(StatisticsDesc statsDesc) throws DdlException,
+ InvalidFormatException {
+ Map<String, String> params = getQueryParams(statsDesc);
+ List<StatsType> statsTypes = statsDesc.getStatsTypes();
+ StatsType type = statsTypes.get(0);
+
+ StatsGranularity statsGranularity = statsDesc.getStatsGranularity();
+ Granularity granularity = statsGranularity.getGranularity();
+ boolean nonPartitioned = granularity != Granularity.PARTITION;
+
+ switch (type) {
Review Comment:
I think both ways can work. through testing, we found that when a single SQL collects multiple metrics, its query time increases proportionally with the number of metrics. For NDV, if the number of rows is 100 million, it takes 3s. When there is a large amount of data and a lot of columns, it takes longer. The current strategy is collecting statistics in parallel. Test data from @EmmyMiao87
docs：https://docs.google.com/document/d/1u1L6XhyzKShoyYRwFQ6kE1rnvY2iFwauwg289au5Qq0/edit
--
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 #12765: [feature-wip](statistics) collect statistics by sql task
Posted by GitBox <gi...@apache.org>.
morrySnow merged PR #12765:
URL: https://github.com/apache/doris/pull/12765
--
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 #12765: [feature-wip](statistics) collect statistics by sql task
Posted by GitBox <gi...@apache.org>.
weizhengte commented on code in PR #12765:
URL: https://github.com/apache/doris/pull/12765#discussion_r975378534
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/SQLStatisticsTask.java:
##########
@@ -17,47 +17,119 @@
package org.apache.doris.statistics;
-import org.apache.doris.analysis.SelectStmt;
+import org.apache.doris.catalog.Database;
+import org.apache.doris.catalog.Env;
+import org.apache.doris.catalog.Table;
+import org.apache.doris.common.DdlException;
+import org.apache.doris.common.InvalidFormatException;
+import org.apache.doris.statistics.StatisticsTaskResult.TaskResult;
+import org.apache.doris.statistics.StatsGranularity.Granularity;
+import org.apache.doris.statistics.util.InternalQuery;
+import org.apache.doris.statistics.util.InternalQueryResult;
+import org.apache.doris.statistics.util.InternalQueryResult.ResultRow;
+import org.apache.doris.statistics.util.InternalSqlTemplate;
+
+import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
import java.util.List;
+import java.util.Map;
/**
* A statistics task that collects statistics by executing query.
* The results of the query will be returned as @StatisticsTaskResult.
*/
public class SQLStatisticsTask extends StatisticsTask {
- private SelectStmt query;
+ private String statement;
public SQLStatisticsTask(long jobId, List<StatisticsDesc> statsDescs) {
super(jobId, statsDescs);
}
@Override
public StatisticsTaskResult call() throws Exception {
- // TODO
- // step1: construct query by statsDescList
- constructQuery();
- // step2: execute query
- // the result should be sequence by @statsTypeList
- List<String> queryResultList = executeQuery(query);
- // step3: construct StatisticsTaskResult by query result
- constructTaskResult(queryResultList);
- return null;
+ checkStatisticsDesc();
+ List<TaskResult> taskResults = Lists.newArrayList();
+
+ for (StatisticsDesc statsDesc : statsDescs) {
+ statement = constructQuery(statsDesc);
+ TaskResult taskResult = executeQuery(statsDesc);
+ taskResults.add(taskResult);
+ }
+
+ return new StatisticsTaskResult(taskResults);
}
- protected void constructQuery() {
- // TODO
- // step1: construct FROM by @granularityDesc
- // step2: construct SELECT LIST by @statsTypeList
+ protected String constructQuery(StatisticsDesc statsDesc) throws DdlException,
+ InvalidFormatException {
+ Map<String, String> params = getQueryParams(statsDesc);
+ List<StatsType> statsTypes = statsDesc.getStatsTypes();
+ StatsType type = statsTypes.get(0);
+
+ StatsGranularity statsGranularity = statsDesc.getStatsGranularity();
+ Granularity granularity = statsGranularity.getGranularity();
+ boolean nonPartitioned = granularity != Granularity.PARTITION;
+
+ switch (type) {
Review Comment:
It is true that you can collect multiple different statistics in the same statement. However, if you collect too many statistics in a task at the same time, the cluster pressure may be excessive and normal query may be affected.
Therefore, statistical tasks are divided into groups according to the type of statistics.
--
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 #12765: [feature-wip](statistics) collect statistics by sql task
Posted by GitBox <gi...@apache.org>.
Kikyou1997 commented on code in PR #12765:
URL: https://github.com/apache/doris/pull/12765#discussion_r976151896
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/SQLStatisticsTask.java:
##########
@@ -17,47 +17,119 @@
package org.apache.doris.statistics;
-import org.apache.doris.analysis.SelectStmt;
+import org.apache.doris.catalog.Database;
+import org.apache.doris.catalog.Env;
+import org.apache.doris.catalog.Table;
+import org.apache.doris.common.DdlException;
+import org.apache.doris.common.InvalidFormatException;
+import org.apache.doris.statistics.StatisticsTaskResult.TaskResult;
+import org.apache.doris.statistics.StatsGranularity.Granularity;
+import org.apache.doris.statistics.util.InternalQuery;
+import org.apache.doris.statistics.util.InternalQueryResult;
+import org.apache.doris.statistics.util.InternalQueryResult.ResultRow;
+import org.apache.doris.statistics.util.InternalSqlTemplate;
+
+import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
import java.util.List;
+import java.util.Map;
/**
* A statistics task that collects statistics by executing query.
* The results of the query will be returned as @StatisticsTaskResult.
*/
public class SQLStatisticsTask extends StatisticsTask {
- private SelectStmt query;
+ private String statement;
public SQLStatisticsTask(long jobId, List<StatisticsDesc> statsDescs) {
super(jobId, statsDescs);
}
@Override
public StatisticsTaskResult call() throws Exception {
- // TODO
- // step1: construct query by statsDescList
- constructQuery();
- // step2: execute query
- // the result should be sequence by @statsTypeList
- List<String> queryResultList = executeQuery(query);
- // step3: construct StatisticsTaskResult by query result
- constructTaskResult(queryResultList);
- return null;
+ checkStatisticsDesc();
+ List<TaskResult> taskResults = Lists.newArrayList();
+
+ for (StatisticsDesc statsDesc : statsDescs) {
+ statement = constructQuery(statsDesc);
+ TaskResult taskResult = executeQuery(statsDesc);
+ taskResults.add(taskResult);
+ }
+
+ return new StatisticsTaskResult(taskResults);
}
- protected void constructQuery() {
- // TODO
- // step1: construct FROM by @granularityDesc
- // step2: construct SELECT LIST by @statsTypeList
+ protected String constructQuery(StatisticsDesc statsDesc) throws DdlException,
+ InvalidFormatException {
+ Map<String, String> params = getQueryParams(statsDesc);
+ List<StatsType> statsTypes = statsDesc.getStatsTypes();
+ StatsType type = statsTypes.get(0);
+
+ StatsGranularity statsGranularity = statsDesc.getStatsGranularity();
+ Granularity granularity = statsGranularity.getGranularity();
+ boolean nonPartitioned = granularity != Granularity.PARTITION;
+
+ switch (type) {
Review Comment:
First, I don't think it would take much longer exectuion time than separete it to many small SQL, besides combine those SQL into single statement actually make the resource control of statistcs more easily, otherwise you should set resource limitations for each small statistic SQL and control the total resource occupation of these SQL and should make sure there aren't two many small SQL of statistics are executing, all about these is unnessary and make resource management and code maintenance much more complicated.
And, Is really neccessary to enable users setting which concrete metric of column is needed by analyze SQL statement? I think this is totally 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 #12765: [feature-wip](statistics) collect statistics by sql task
Posted by GitBox <gi...@apache.org>.
Kikyou1997 commented on code in PR #12765:
URL: https://github.com/apache/doris/pull/12765#discussion_r976151896
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/SQLStatisticsTask.java:
##########
@@ -17,47 +17,119 @@
package org.apache.doris.statistics;
-import org.apache.doris.analysis.SelectStmt;
+import org.apache.doris.catalog.Database;
+import org.apache.doris.catalog.Env;
+import org.apache.doris.catalog.Table;
+import org.apache.doris.common.DdlException;
+import org.apache.doris.common.InvalidFormatException;
+import org.apache.doris.statistics.StatisticsTaskResult.TaskResult;
+import org.apache.doris.statistics.StatsGranularity.Granularity;
+import org.apache.doris.statistics.util.InternalQuery;
+import org.apache.doris.statistics.util.InternalQueryResult;
+import org.apache.doris.statistics.util.InternalQueryResult.ResultRow;
+import org.apache.doris.statistics.util.InternalSqlTemplate;
+
+import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
import java.util.List;
+import java.util.Map;
/**
* A statistics task that collects statistics by executing query.
* The results of the query will be returned as @StatisticsTaskResult.
*/
public class SQLStatisticsTask extends StatisticsTask {
- private SelectStmt query;
+ private String statement;
public SQLStatisticsTask(long jobId, List<StatisticsDesc> statsDescs) {
super(jobId, statsDescs);
}
@Override
public StatisticsTaskResult call() throws Exception {
- // TODO
- // step1: construct query by statsDescList
- constructQuery();
- // step2: execute query
- // the result should be sequence by @statsTypeList
- List<String> queryResultList = executeQuery(query);
- // step3: construct StatisticsTaskResult by query result
- constructTaskResult(queryResultList);
- return null;
+ checkStatisticsDesc();
+ List<TaskResult> taskResults = Lists.newArrayList();
+
+ for (StatisticsDesc statsDesc : statsDescs) {
+ statement = constructQuery(statsDesc);
+ TaskResult taskResult = executeQuery(statsDesc);
+ taskResults.add(taskResult);
+ }
+
+ return new StatisticsTaskResult(taskResults);
}
- protected void constructQuery() {
- // TODO
- // step1: construct FROM by @granularityDesc
- // step2: construct SELECT LIST by @statsTypeList
+ protected String constructQuery(StatisticsDesc statsDesc) throws DdlException,
+ InvalidFormatException {
+ Map<String, String> params = getQueryParams(statsDesc);
+ List<StatsType> statsTypes = statsDesc.getStatsTypes();
+ StatsType type = statsTypes.get(0);
+
+ StatsGranularity statsGranularity = statsDesc.getStatsGranularity();
+ Granularity granularity = statsGranularity.getGranularity();
+ boolean nonPartitioned = granularity != Granularity.PARTITION;
+
+ switch (type) {
Review Comment:
First, I don't think it would take much longer time than separete it to many small SQL, besides put those SQL into single statement actually make the resource control of statistcs more easy, otherwise you should set resource limitations for each statistic SQL and control the total resource occupation of these SQL and should make sure there aren't two many small SQL of statistics executing, all about these is unnessary and make resource management and code maintenance much more complicated.
And, Is really neccessary to enable users setting which concrete metric of column is needed by analyze SQL statement? I think this is totally 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 #12765: [feature-wip](statistics) collect statistics by sql task
Posted by GitBox <gi...@apache.org>.
Kikyou1997 commented on code in PR #12765:
URL: https://github.com/apache/doris/pull/12765#discussion_r976007853
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/SQLStatisticsTask.java:
##########
@@ -17,47 +17,119 @@
package org.apache.doris.statistics;
-import org.apache.doris.analysis.SelectStmt;
+import org.apache.doris.catalog.Database;
+import org.apache.doris.catalog.Env;
+import org.apache.doris.catalog.Table;
+import org.apache.doris.common.DdlException;
+import org.apache.doris.common.InvalidFormatException;
+import org.apache.doris.statistics.StatisticsTaskResult.TaskResult;
+import org.apache.doris.statistics.StatsGranularity.Granularity;
+import org.apache.doris.statistics.util.InternalQuery;
+import org.apache.doris.statistics.util.InternalQueryResult;
+import org.apache.doris.statistics.util.InternalQueryResult.ResultRow;
+import org.apache.doris.statistics.util.InternalSqlTemplate;
+
+import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
import java.util.List;
+import java.util.Map;
/**
* A statistics task that collects statistics by executing query.
* The results of the query will be returned as @StatisticsTaskResult.
*/
public class SQLStatisticsTask extends StatisticsTask {
- private SelectStmt query;
+ private String statement;
public SQLStatisticsTask(long jobId, List<StatisticsDesc> statsDescs) {
super(jobId, statsDescs);
}
@Override
public StatisticsTaskResult call() throws Exception {
- // TODO
- // step1: construct query by statsDescList
- constructQuery();
- // step2: execute query
- // the result should be sequence by @statsTypeList
- List<String> queryResultList = executeQuery(query);
- // step3: construct StatisticsTaskResult by query result
- constructTaskResult(queryResultList);
- return null;
+ checkStatisticsDesc();
+ List<TaskResult> taskResults = Lists.newArrayList();
+
+ for (StatisticsDesc statsDesc : statsDescs) {
+ statement = constructQuery(statsDesc);
+ TaskResult taskResult = executeQuery(statsDesc);
+ taskResults.add(taskResult);
+ }
+
+ return new StatisticsTaskResult(taskResults);
}
- protected void constructQuery() {
- // TODO
- // step1: construct FROM by @granularityDesc
- // step2: construct SELECT LIST by @statsTypeList
+ protected String constructQuery(StatisticsDesc statsDesc) throws DdlException,
+ InvalidFormatException {
+ Map<String, String> params = getQueryParams(statsDesc);
+ List<StatsType> statsTypes = statsDesc.getStatsTypes();
+ StatsType type = statsTypes.get(0);
+
+ StatsGranularity statsGranularity = statsDesc.getStatsGranularity();
+ Granularity granularity = statsGranularity.getGranularity();
+ boolean nonPartitioned = granularity != Granularity.PARTITION;
+
+ switch (type) {
Review Comment:
you could just limit execution resource by set the memory upperbound and parallel execution instatnce and so on for the stats SQL though, so I think it's not a reasonable motivation for this
--
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 #12765: [feature-wip](statistics) collect statistics by sql task
Posted by GitBox <gi...@apache.org>.
Kikyou1997 commented on code in PR #12765:
URL: https://github.com/apache/doris/pull/12765#discussion_r976151896
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/SQLStatisticsTask.java:
##########
@@ -17,47 +17,119 @@
package org.apache.doris.statistics;
-import org.apache.doris.analysis.SelectStmt;
+import org.apache.doris.catalog.Database;
+import org.apache.doris.catalog.Env;
+import org.apache.doris.catalog.Table;
+import org.apache.doris.common.DdlException;
+import org.apache.doris.common.InvalidFormatException;
+import org.apache.doris.statistics.StatisticsTaskResult.TaskResult;
+import org.apache.doris.statistics.StatsGranularity.Granularity;
+import org.apache.doris.statistics.util.InternalQuery;
+import org.apache.doris.statistics.util.InternalQueryResult;
+import org.apache.doris.statistics.util.InternalQueryResult.ResultRow;
+import org.apache.doris.statistics.util.InternalSqlTemplate;
+
+import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
import java.util.List;
+import java.util.Map;
/**
* A statistics task that collects statistics by executing query.
* The results of the query will be returned as @StatisticsTaskResult.
*/
public class SQLStatisticsTask extends StatisticsTask {
- private SelectStmt query;
+ private String statement;
public SQLStatisticsTask(long jobId, List<StatisticsDesc> statsDescs) {
super(jobId, statsDescs);
}
@Override
public StatisticsTaskResult call() throws Exception {
- // TODO
- // step1: construct query by statsDescList
- constructQuery();
- // step2: execute query
- // the result should be sequence by @statsTypeList
- List<String> queryResultList = executeQuery(query);
- // step3: construct StatisticsTaskResult by query result
- constructTaskResult(queryResultList);
- return null;
+ checkStatisticsDesc();
+ List<TaskResult> taskResults = Lists.newArrayList();
+
+ for (StatisticsDesc statsDesc : statsDescs) {
+ statement = constructQuery(statsDesc);
+ TaskResult taskResult = executeQuery(statsDesc);
+ taskResults.add(taskResult);
+ }
+
+ return new StatisticsTaskResult(taskResults);
}
- protected void constructQuery() {
- // TODO
- // step1: construct FROM by @granularityDesc
- // step2: construct SELECT LIST by @statsTypeList
+ protected String constructQuery(StatisticsDesc statsDesc) throws DdlException,
+ InvalidFormatException {
+ Map<String, String> params = getQueryParams(statsDesc);
+ List<StatsType> statsTypes = statsDesc.getStatsTypes();
+ StatsType type = statsTypes.get(0);
+
+ StatsGranularity statsGranularity = statsDesc.getStatsGranularity();
+ Granularity granularity = statsGranularity.getGranularity();
+ boolean nonPartitioned = granularity != Granularity.PARTITION;
+
+ switch (type) {
Review Comment:
First, I don't think it would take much longer exectuion time than separete it to many small SQL, besides, combine those SQL into single statement actually make the resource control of statistcs more easily, otherwise you should set resource limitations for each small statistic SQL and control the total resource occupation of these SQL and should make sure there aren't two many small SQL of statistics are executing, all about these is unneccessary and make resource management and code maintenance much more complicated.
And, Is really neccessary to enable users setting which concrete metric of column is needed by analyze SQL statement? I think this is totally 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] weizhengte commented on a diff in pull request #12765: [feature-wip](statistics) collect statistics by sql task
Posted by GitBox <gi...@apache.org>.
weizhengte commented on code in PR #12765:
URL: https://github.com/apache/doris/pull/12765#discussion_r976075630
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/SQLStatisticsTask.java:
##########
@@ -17,47 +17,119 @@
package org.apache.doris.statistics;
-import org.apache.doris.analysis.SelectStmt;
+import org.apache.doris.catalog.Database;
+import org.apache.doris.catalog.Env;
+import org.apache.doris.catalog.Table;
+import org.apache.doris.common.DdlException;
+import org.apache.doris.common.InvalidFormatException;
+import org.apache.doris.statistics.StatisticsTaskResult.TaskResult;
+import org.apache.doris.statistics.StatsGranularity.Granularity;
+import org.apache.doris.statistics.util.InternalQuery;
+import org.apache.doris.statistics.util.InternalQueryResult;
+import org.apache.doris.statistics.util.InternalQueryResult.ResultRow;
+import org.apache.doris.statistics.util.InternalSqlTemplate;
+
+import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
import java.util.List;
+import java.util.Map;
/**
* A statistics task that collects statistics by executing query.
* The results of the query will be returned as @StatisticsTaskResult.
*/
public class SQLStatisticsTask extends StatisticsTask {
- private SelectStmt query;
+ private String statement;
public SQLStatisticsTask(long jobId, List<StatisticsDesc> statsDescs) {
super(jobId, statsDescs);
}
@Override
public StatisticsTaskResult call() throws Exception {
- // TODO
- // step1: construct query by statsDescList
- constructQuery();
- // step2: execute query
- // the result should be sequence by @statsTypeList
- List<String> queryResultList = executeQuery(query);
- // step3: construct StatisticsTaskResult by query result
- constructTaskResult(queryResultList);
- return null;
+ checkStatisticsDesc();
+ List<TaskResult> taskResults = Lists.newArrayList();
+
+ for (StatisticsDesc statsDesc : statsDescs) {
+ statement = constructQuery(statsDesc);
+ TaskResult taskResult = executeQuery(statsDesc);
+ taskResults.add(taskResult);
+ }
+
+ return new StatisticsTaskResult(taskResults);
}
- protected void constructQuery() {
- // TODO
- // step1: construct FROM by @granularityDesc
- // step2: construct SELECT LIST by @statsTypeList
+ protected String constructQuery(StatisticsDesc statsDesc) throws DdlException,
+ InvalidFormatException {
+ Map<String, String> params = getQueryParams(statsDesc);
+ List<StatsType> statsTypes = statsDesc.getStatsTypes();
+ StatsType type = statsTypes.get(0);
+
+ StatsGranularity statsGranularity = statsDesc.getStatsGranularity();
+ Granularity granularity = statsGranularity.getGranularity();
+ boolean nonPartitioned = granularity != Granularity.PARTITION;
+
+ switch (type) {
Review Comment:
In fact, as much as possible, we have put statistics that can be collected together (such as MIN, MAX, NDV) into one group and collected in one SQL. At the same time, it also makes corresponding resource restrictions (such as parallelism, etc.).
However, if all the aggregated metrics are put into a SQL query and the resources are limited, the collection time may be longer; In addition, when there is a large amount of data, some indicators (e.g., MAX/AVG_COL_LEN) can be collected by sampling, but some indicators (e.g., MIN, MAX, NDV) need to be fully collected, so it may not be appropriate to put them all in one statement.
Also, if the user only wants to quickly collect statistics for one statistical category, then in fact all statistics will be collected.
Including the problems mentioned above, as well as some other reasons, we have divided the tasks according to some rules~
--
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 #12765: [feature-wip](statistics) collect statistics by sql task
Posted by GitBox <gi...@apache.org>.
Kikyou1997 commented on code in PR #12765:
URL: https://github.com/apache/doris/pull/12765#discussion_r976007853
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/SQLStatisticsTask.java:
##########
@@ -17,47 +17,119 @@
package org.apache.doris.statistics;
-import org.apache.doris.analysis.SelectStmt;
+import org.apache.doris.catalog.Database;
+import org.apache.doris.catalog.Env;
+import org.apache.doris.catalog.Table;
+import org.apache.doris.common.DdlException;
+import org.apache.doris.common.InvalidFormatException;
+import org.apache.doris.statistics.StatisticsTaskResult.TaskResult;
+import org.apache.doris.statistics.StatsGranularity.Granularity;
+import org.apache.doris.statistics.util.InternalQuery;
+import org.apache.doris.statistics.util.InternalQueryResult;
+import org.apache.doris.statistics.util.InternalQueryResult.ResultRow;
+import org.apache.doris.statistics.util.InternalSqlTemplate;
+
+import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
import java.util.List;
+import java.util.Map;
/**
* A statistics task that collects statistics by executing query.
* The results of the query will be returned as @StatisticsTaskResult.
*/
public class SQLStatisticsTask extends StatisticsTask {
- private SelectStmt query;
+ private String statement;
public SQLStatisticsTask(long jobId, List<StatisticsDesc> statsDescs) {
super(jobId, statsDescs);
}
@Override
public StatisticsTaskResult call() throws Exception {
- // TODO
- // step1: construct query by statsDescList
- constructQuery();
- // step2: execute query
- // the result should be sequence by @statsTypeList
- List<String> queryResultList = executeQuery(query);
- // step3: construct StatisticsTaskResult by query result
- constructTaskResult(queryResultList);
- return null;
+ checkStatisticsDesc();
+ List<TaskResult> taskResults = Lists.newArrayList();
+
+ for (StatisticsDesc statsDesc : statsDescs) {
+ statement = constructQuery(statsDesc);
+ TaskResult taskResult = executeQuery(statsDesc);
+ taskResults.add(taskResult);
+ }
+
+ return new StatisticsTaskResult(taskResults);
}
- protected void constructQuery() {
- // TODO
- // step1: construct FROM by @granularityDesc
- // step2: construct SELECT LIST by @statsTypeList
+ protected String constructQuery(StatisticsDesc statsDesc) throws DdlException,
+ InvalidFormatException {
+ Map<String, String> params = getQueryParams(statsDesc);
+ List<StatsType> statsTypes = statsDesc.getStatsTypes();
+ StatsType type = statsTypes.get(0);
+
+ StatsGranularity statsGranularity = statsDesc.getStatsGranularity();
+ Granularity granularity = statsGranularity.getGranularity();
+ boolean nonPartitioned = granularity != Granularity.PARTITION;
+
+ switch (type) {
Review Comment:
you can just limit execution resource by set the memory upperbound and prallel execution instatnce for the stats SQL though, so I think it's not a good reason for that.....
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/SQLStatisticsTask.java:
##########
@@ -17,47 +17,119 @@
package org.apache.doris.statistics;
-import org.apache.doris.analysis.SelectStmt;
+import org.apache.doris.catalog.Database;
+import org.apache.doris.catalog.Env;
+import org.apache.doris.catalog.Table;
+import org.apache.doris.common.DdlException;
+import org.apache.doris.common.InvalidFormatException;
+import org.apache.doris.statistics.StatisticsTaskResult.TaskResult;
+import org.apache.doris.statistics.StatsGranularity.Granularity;
+import org.apache.doris.statistics.util.InternalQuery;
+import org.apache.doris.statistics.util.InternalQueryResult;
+import org.apache.doris.statistics.util.InternalQueryResult.ResultRow;
+import org.apache.doris.statistics.util.InternalSqlTemplate;
+
+import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
import java.util.List;
+import java.util.Map;
/**
* A statistics task that collects statistics by executing query.
* The results of the query will be returned as @StatisticsTaskResult.
*/
public class SQLStatisticsTask extends StatisticsTask {
- private SelectStmt query;
+ private String statement;
public SQLStatisticsTask(long jobId, List<StatisticsDesc> statsDescs) {
super(jobId, statsDescs);
}
@Override
public StatisticsTaskResult call() throws Exception {
- // TODO
- // step1: construct query by statsDescList
- constructQuery();
- // step2: execute query
- // the result should be sequence by @statsTypeList
- List<String> queryResultList = executeQuery(query);
- // step3: construct StatisticsTaskResult by query result
- constructTaskResult(queryResultList);
- return null;
+ checkStatisticsDesc();
+ List<TaskResult> taskResults = Lists.newArrayList();
+
+ for (StatisticsDesc statsDesc : statsDescs) {
+ statement = constructQuery(statsDesc);
+ TaskResult taskResult = executeQuery(statsDesc);
+ taskResults.add(taskResult);
+ }
+
+ return new StatisticsTaskResult(taskResults);
}
- protected void constructQuery() {
- // TODO
- // step1: construct FROM by @granularityDesc
- // step2: construct SELECT LIST by @statsTypeList
+ protected String constructQuery(StatisticsDesc statsDesc) throws DdlException,
+ InvalidFormatException {
+ Map<String, String> params = getQueryParams(statsDesc);
+ List<StatsType> statsTypes = statsDesc.getStatsTypes();
+ StatsType type = statsTypes.get(0);
+
+ StatsGranularity statsGranularity = statsDesc.getStatsGranularity();
+ Granularity granularity = statsGranularity.getGranularity();
+ boolean nonPartitioned = granularity != Granularity.PARTITION;
+
+ switch (type) {
Review Comment:
you could just limit execution resource by set the memory upperbound and prallel execution instatnce for the stats SQL though, so I think it's not a good reason for that.....
--
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 #12765: [feature-wip](statistics) collect statistics by sql task
Posted by GitBox <gi...@apache.org>.
Kikyou1997 commented on code in PR #12765:
URL: https://github.com/apache/doris/pull/12765#discussion_r976151896
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/SQLStatisticsTask.java:
##########
@@ -17,47 +17,119 @@
package org.apache.doris.statistics;
-import org.apache.doris.analysis.SelectStmt;
+import org.apache.doris.catalog.Database;
+import org.apache.doris.catalog.Env;
+import org.apache.doris.catalog.Table;
+import org.apache.doris.common.DdlException;
+import org.apache.doris.common.InvalidFormatException;
+import org.apache.doris.statistics.StatisticsTaskResult.TaskResult;
+import org.apache.doris.statistics.StatsGranularity.Granularity;
+import org.apache.doris.statistics.util.InternalQuery;
+import org.apache.doris.statistics.util.InternalQueryResult;
+import org.apache.doris.statistics.util.InternalQueryResult.ResultRow;
+import org.apache.doris.statistics.util.InternalSqlTemplate;
+
+import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
import java.util.List;
+import java.util.Map;
/**
* A statistics task that collects statistics by executing query.
* The results of the query will be returned as @StatisticsTaskResult.
*/
public class SQLStatisticsTask extends StatisticsTask {
- private SelectStmt query;
+ private String statement;
public SQLStatisticsTask(long jobId, List<StatisticsDesc> statsDescs) {
super(jobId, statsDescs);
}
@Override
public StatisticsTaskResult call() throws Exception {
- // TODO
- // step1: construct query by statsDescList
- constructQuery();
- // step2: execute query
- // the result should be sequence by @statsTypeList
- List<String> queryResultList = executeQuery(query);
- // step3: construct StatisticsTaskResult by query result
- constructTaskResult(queryResultList);
- return null;
+ checkStatisticsDesc();
+ List<TaskResult> taskResults = Lists.newArrayList();
+
+ for (StatisticsDesc statsDesc : statsDescs) {
+ statement = constructQuery(statsDesc);
+ TaskResult taskResult = executeQuery(statsDesc);
+ taskResults.add(taskResult);
+ }
+
+ return new StatisticsTaskResult(taskResults);
}
- protected void constructQuery() {
- // TODO
- // step1: construct FROM by @granularityDesc
- // step2: construct SELECT LIST by @statsTypeList
+ protected String constructQuery(StatisticsDesc statsDesc) throws DdlException,
+ InvalidFormatException {
+ Map<String, String> params = getQueryParams(statsDesc);
+ List<StatsType> statsTypes = statsDesc.getStatsTypes();
+ StatsType type = statsTypes.get(0);
+
+ StatsGranularity statsGranularity = statsDesc.getStatsGranularity();
+ Granularity granularity = statsGranularity.getGranularity();
+ boolean nonPartitioned = granularity != Granularity.PARTITION;
+
+ switch (type) {
Review Comment:
First, I don't think it would take much longer exectuion time than separete it to many small SQL, besides combine those SQL into single statement actually make the resource control of statistcs more easy, otherwise you should set resource limitations for each statistic SQL and control the total resource occupation of these SQL and should make sure there aren't two many small SQL of statistics executing, all about these is unnessary and make resource management and code maintenance much more complicated.
And, Is really neccessary to enable users setting which concrete metric of column is needed by analyze SQL statement? I think this is totally 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] weizhengte commented on a diff in pull request #12765: [feature-wip](statistics) collect statistics by sql task
Posted by GitBox <gi...@apache.org>.
weizhengte commented on code in PR #12765:
URL: https://github.com/apache/doris/pull/12765#discussion_r976076296
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/SQLStatisticsTask.java:
##########
@@ -17,47 +17,119 @@
package org.apache.doris.statistics;
-import org.apache.doris.analysis.SelectStmt;
+import org.apache.doris.catalog.Database;
+import org.apache.doris.catalog.Env;
+import org.apache.doris.catalog.Table;
+import org.apache.doris.common.DdlException;
+import org.apache.doris.common.InvalidFormatException;
+import org.apache.doris.statistics.StatisticsTaskResult.TaskResult;
+import org.apache.doris.statistics.StatsGranularity.Granularity;
+import org.apache.doris.statistics.util.InternalQuery;
+import org.apache.doris.statistics.util.InternalQueryResult;
+import org.apache.doris.statistics.util.InternalQueryResult.ResultRow;
+import org.apache.doris.statistics.util.InternalSqlTemplate;
+
+import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
import java.util.List;
+import java.util.Map;
/**
* A statistics task that collects statistics by executing query.
* The results of the query will be returned as @StatisticsTaskResult.
*/
public class SQLStatisticsTask extends StatisticsTask {
- private SelectStmt query;
+ private String statement;
public SQLStatisticsTask(long jobId, List<StatisticsDesc> statsDescs) {
super(jobId, statsDescs);
}
@Override
public StatisticsTaskResult call() throws Exception {
- // TODO
- // step1: construct query by statsDescList
- constructQuery();
- // step2: execute query
- // the result should be sequence by @statsTypeList
- List<String> queryResultList = executeQuery(query);
- // step3: construct StatisticsTaskResult by query result
- constructTaskResult(queryResultList);
- return null;
+ checkStatisticsDesc();
+ List<TaskResult> taskResults = Lists.newArrayList();
+
+ for (StatisticsDesc statsDesc : statsDescs) {
+ statement = constructQuery(statsDesc);
+ TaskResult taskResult = executeQuery(statsDesc);
+ taskResults.add(taskResult);
+ }
+
+ return new StatisticsTaskResult(taskResults);
}
- protected void constructQuery() {
- // TODO
- // step1: construct FROM by @granularityDesc
- // step2: construct SELECT LIST by @statsTypeList
+ protected String constructQuery(StatisticsDesc statsDesc) throws DdlException,
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 #12765: [feature-wip](statistics) collect statistics by sql task
Posted by GitBox <gi...@apache.org>.
Kikyou1997 commented on code in PR #12765:
URL: https://github.com/apache/doris/pull/12765#discussion_r976151896
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/SQLStatisticsTask.java:
##########
@@ -17,47 +17,119 @@
package org.apache.doris.statistics;
-import org.apache.doris.analysis.SelectStmt;
+import org.apache.doris.catalog.Database;
+import org.apache.doris.catalog.Env;
+import org.apache.doris.catalog.Table;
+import org.apache.doris.common.DdlException;
+import org.apache.doris.common.InvalidFormatException;
+import org.apache.doris.statistics.StatisticsTaskResult.TaskResult;
+import org.apache.doris.statistics.StatsGranularity.Granularity;
+import org.apache.doris.statistics.util.InternalQuery;
+import org.apache.doris.statistics.util.InternalQueryResult;
+import org.apache.doris.statistics.util.InternalQueryResult.ResultRow;
+import org.apache.doris.statistics.util.InternalSqlTemplate;
+
+import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
import java.util.List;
+import java.util.Map;
/**
* A statistics task that collects statistics by executing query.
* The results of the query will be returned as @StatisticsTaskResult.
*/
public class SQLStatisticsTask extends StatisticsTask {
- private SelectStmt query;
+ private String statement;
public SQLStatisticsTask(long jobId, List<StatisticsDesc> statsDescs) {
super(jobId, statsDescs);
}
@Override
public StatisticsTaskResult call() throws Exception {
- // TODO
- // step1: construct query by statsDescList
- constructQuery();
- // step2: execute query
- // the result should be sequence by @statsTypeList
- List<String> queryResultList = executeQuery(query);
- // step3: construct StatisticsTaskResult by query result
- constructTaskResult(queryResultList);
- return null;
+ checkStatisticsDesc();
+ List<TaskResult> taskResults = Lists.newArrayList();
+
+ for (StatisticsDesc statsDesc : statsDescs) {
+ statement = constructQuery(statsDesc);
+ TaskResult taskResult = executeQuery(statsDesc);
+ taskResults.add(taskResult);
+ }
+
+ return new StatisticsTaskResult(taskResults);
}
- protected void constructQuery() {
- // TODO
- // step1: construct FROM by @granularityDesc
- // step2: construct SELECT LIST by @statsTypeList
+ protected String constructQuery(StatisticsDesc statsDesc) throws DdlException,
+ InvalidFormatException {
+ Map<String, String> params = getQueryParams(statsDesc);
+ List<StatsType> statsTypes = statsDesc.getStatsTypes();
+ StatsType type = statsTypes.get(0);
+
+ StatsGranularity statsGranularity = statsDesc.getStatsGranularity();
+ Granularity granularity = statsGranularity.getGranularity();
+ boolean nonPartitioned = granularity != Granularity.PARTITION;
+
+ switch (type) {
Review Comment:
First, I don't think it would take much longer exectuion time than separete it to many small SQL, besides combine those SQL into single statement actually make the resource control of statistcs more easily, otherwise you should set resource limitations for each statistic SQL and control the total resource occupation of these SQL and should make sure there aren't two many small SQL of statistics executing, all about these is unnessary and make resource management and code maintenance much more complicated.
And, Is really neccessary to enable users setting which concrete metric of column is needed by analyze SQL statement? I think this is totally 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 #12765: [feature-wip](statistics) collect statistics by sql task
Posted by GitBox <gi...@apache.org>.
Kikyou1997 commented on code in PR #12765:
URL: https://github.com/apache/doris/pull/12765#discussion_r976151896
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/SQLStatisticsTask.java:
##########
@@ -17,47 +17,119 @@
package org.apache.doris.statistics;
-import org.apache.doris.analysis.SelectStmt;
+import org.apache.doris.catalog.Database;
+import org.apache.doris.catalog.Env;
+import org.apache.doris.catalog.Table;
+import org.apache.doris.common.DdlException;
+import org.apache.doris.common.InvalidFormatException;
+import org.apache.doris.statistics.StatisticsTaskResult.TaskResult;
+import org.apache.doris.statistics.StatsGranularity.Granularity;
+import org.apache.doris.statistics.util.InternalQuery;
+import org.apache.doris.statistics.util.InternalQueryResult;
+import org.apache.doris.statistics.util.InternalQueryResult.ResultRow;
+import org.apache.doris.statistics.util.InternalSqlTemplate;
+
+import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
import java.util.List;
+import java.util.Map;
/**
* A statistics task that collects statistics by executing query.
* The results of the query will be returned as @StatisticsTaskResult.
*/
public class SQLStatisticsTask extends StatisticsTask {
- private SelectStmt query;
+ private String statement;
public SQLStatisticsTask(long jobId, List<StatisticsDesc> statsDescs) {
super(jobId, statsDescs);
}
@Override
public StatisticsTaskResult call() throws Exception {
- // TODO
- // step1: construct query by statsDescList
- constructQuery();
- // step2: execute query
- // the result should be sequence by @statsTypeList
- List<String> queryResultList = executeQuery(query);
- // step3: construct StatisticsTaskResult by query result
- constructTaskResult(queryResultList);
- return null;
+ checkStatisticsDesc();
+ List<TaskResult> taskResults = Lists.newArrayList();
+
+ for (StatisticsDesc statsDesc : statsDescs) {
+ statement = constructQuery(statsDesc);
+ TaskResult taskResult = executeQuery(statsDesc);
+ taskResults.add(taskResult);
+ }
+
+ return new StatisticsTaskResult(taskResults);
}
- protected void constructQuery() {
- // TODO
- // step1: construct FROM by @granularityDesc
- // step2: construct SELECT LIST by @statsTypeList
+ protected String constructQuery(StatisticsDesc statsDesc) throws DdlException,
+ InvalidFormatException {
+ Map<String, String> params = getQueryParams(statsDesc);
+ List<StatsType> statsTypes = statsDesc.getStatsTypes();
+ StatsType type = statsTypes.get(0);
+
+ StatsGranularity statsGranularity = statsDesc.getStatsGranularity();
+ Granularity granularity = statsGranularity.getGranularity();
+ boolean nonPartitioned = granularity != Granularity.PARTITION;
+
+ switch (type) {
Review Comment:
First, I don't think it would take much longer exectuion time than separete it to many small SQL, besides combine those SQL into single statement actually make the resource control of statistcs more easily, otherwise you should set resource limitations for each small statistic SQL and control the total resource occupation of these SQL and should make sure there aren't two many small SQL of statistics are executing, all about these is unneccessary and make resource management and code maintenance much more complicated.
And, Is really neccessary to enable users setting which concrete metric of column is needed by analyze SQL statement? I think this is totally 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 #12765: [feature-wip](statistics) collect statistics by sql task
Posted by GitBox <gi...@apache.org>.
Kikyou1997 commented on code in PR #12765:
URL: https://github.com/apache/doris/pull/12765#discussion_r975326292
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/SQLStatisticsTask.java:
##########
@@ -17,47 +17,119 @@
package org.apache.doris.statistics;
-import org.apache.doris.analysis.SelectStmt;
+import org.apache.doris.catalog.Database;
+import org.apache.doris.catalog.Env;
+import org.apache.doris.catalog.Table;
+import org.apache.doris.common.DdlException;
+import org.apache.doris.common.InvalidFormatException;
+import org.apache.doris.statistics.StatisticsTaskResult.TaskResult;
+import org.apache.doris.statistics.StatsGranularity.Granularity;
+import org.apache.doris.statistics.util.InternalQuery;
+import org.apache.doris.statistics.util.InternalQueryResult;
+import org.apache.doris.statistics.util.InternalQueryResult.ResultRow;
+import org.apache.doris.statistics.util.InternalSqlTemplate;
+
+import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
import java.util.List;
+import java.util.Map;
/**
* A statistics task that collects statistics by executing query.
* The results of the query will be returned as @StatisticsTaskResult.
*/
public class SQLStatisticsTask extends StatisticsTask {
- private SelectStmt query;
+ private String statement;
public SQLStatisticsTask(long jobId, List<StatisticsDesc> statsDescs) {
super(jobId, statsDescs);
}
@Override
public StatisticsTaskResult call() throws Exception {
- // TODO
- // step1: construct query by statsDescList
- constructQuery();
- // step2: execute query
- // the result should be sequence by @statsTypeList
- List<String> queryResultList = executeQuery(query);
- // step3: construct StatisticsTaskResult by query result
- constructTaskResult(queryResultList);
- return null;
+ checkStatisticsDesc();
+ List<TaskResult> taskResults = Lists.newArrayList();
+
+ for (StatisticsDesc statsDesc : statsDescs) {
+ statement = constructQuery(statsDesc);
+ TaskResult taskResult = executeQuery(statsDesc);
+ taskResults.add(taskResult);
+ }
+
+ return new StatisticsTaskResult(taskResults);
}
- protected void constructQuery() {
- // TODO
- // step1: construct FROM by @granularityDesc
- // step2: construct SELECT LIST by @statsTypeList
+ protected String constructQuery(StatisticsDesc statsDesc) throws DdlException,
+ InvalidFormatException {
+ Map<String, String> params = getQueryParams(statsDesc);
+ List<StatsType> statsTypes = statsDesc.getStatsTypes();
+ StatsType type = statsTypes.get(0);
+
+ StatsGranularity statsGranularity = statsDesc.getStatsGranularity();
+ Granularity granularity = statsGranularity.getGranularity();
+ boolean nonPartitioned = granularity != Granularity.PARTITION;
+
+ switch (type) {
Review Comment:
I think we could finish the collection for these stats in single SQL, such as
```sql
select * from (select max(col1) as max from t1) as tt, (select count(col1) as num from t1) as ttt;
```
--
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 #12765: [feature-wip](statistics) collect statistics by sql task
Posted by GitBox <gi...@apache.org>.
weizhengte commented on code in PR #12765:
URL: https://github.com/apache/doris/pull/12765#discussion_r975378534
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/SQLStatisticsTask.java:
##########
@@ -17,47 +17,119 @@
package org.apache.doris.statistics;
-import org.apache.doris.analysis.SelectStmt;
+import org.apache.doris.catalog.Database;
+import org.apache.doris.catalog.Env;
+import org.apache.doris.catalog.Table;
+import org.apache.doris.common.DdlException;
+import org.apache.doris.common.InvalidFormatException;
+import org.apache.doris.statistics.StatisticsTaskResult.TaskResult;
+import org.apache.doris.statistics.StatsGranularity.Granularity;
+import org.apache.doris.statistics.util.InternalQuery;
+import org.apache.doris.statistics.util.InternalQueryResult;
+import org.apache.doris.statistics.util.InternalQueryResult.ResultRow;
+import org.apache.doris.statistics.util.InternalSqlTemplate;
+
+import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
import java.util.List;
+import java.util.Map;
/**
* A statistics task that collects statistics by executing query.
* The results of the query will be returned as @StatisticsTaskResult.
*/
public class SQLStatisticsTask extends StatisticsTask {
- private SelectStmt query;
+ private String statement;
public SQLStatisticsTask(long jobId, List<StatisticsDesc> statsDescs) {
super(jobId, statsDescs);
}
@Override
public StatisticsTaskResult call() throws Exception {
- // TODO
- // step1: construct query by statsDescList
- constructQuery();
- // step2: execute query
- // the result should be sequence by @statsTypeList
- List<String> queryResultList = executeQuery(query);
- // step3: construct StatisticsTaskResult by query result
- constructTaskResult(queryResultList);
- return null;
+ checkStatisticsDesc();
+ List<TaskResult> taskResults = Lists.newArrayList();
+
+ for (StatisticsDesc statsDesc : statsDescs) {
+ statement = constructQuery(statsDesc);
+ TaskResult taskResult = executeQuery(statsDesc);
+ taskResults.add(taskResult);
+ }
+
+ return new StatisticsTaskResult(taskResults);
}
- protected void constructQuery() {
- // TODO
- // step1: construct FROM by @granularityDesc
- // step2: construct SELECT LIST by @statsTypeList
+ protected String constructQuery(StatisticsDesc statsDesc) throws DdlException,
+ InvalidFormatException {
+ Map<String, String> params = getQueryParams(statsDesc);
+ List<StatsType> statsTypes = statsDesc.getStatsTypes();
+ StatsType type = statsTypes.get(0);
+
+ StatsGranularity statsGranularity = statsDesc.getStatsGranularity();
+ Granularity granularity = statsGranularity.getGranularity();
+ boolean nonPartitioned = granularity != Granularity.PARTITION;
+
+ switch (type) {
Review Comment:
It is true that you can collect multiple different statistics in the same statement. However, if you collect too many statistics in a task at the same time, the cluster pressure may be excessive and normal query services may be affected.
Therefore, statistical tasks are divided into groups according to the type of statistics.
--
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 #12765: [feature-wip](statistics) collect statistics by sql task
Posted by GitBox <gi...@apache.org>.
Kikyou1997 commented on code in PR #12765:
URL: https://github.com/apache/doris/pull/12765#discussion_r976007853
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/SQLStatisticsTask.java:
##########
@@ -17,47 +17,119 @@
package org.apache.doris.statistics;
-import org.apache.doris.analysis.SelectStmt;
+import org.apache.doris.catalog.Database;
+import org.apache.doris.catalog.Env;
+import org.apache.doris.catalog.Table;
+import org.apache.doris.common.DdlException;
+import org.apache.doris.common.InvalidFormatException;
+import org.apache.doris.statistics.StatisticsTaskResult.TaskResult;
+import org.apache.doris.statistics.StatsGranularity.Granularity;
+import org.apache.doris.statistics.util.InternalQuery;
+import org.apache.doris.statistics.util.InternalQueryResult;
+import org.apache.doris.statistics.util.InternalQueryResult.ResultRow;
+import org.apache.doris.statistics.util.InternalSqlTemplate;
+
+import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
import java.util.List;
+import java.util.Map;
/**
* A statistics task that collects statistics by executing query.
* The results of the query will be returned as @StatisticsTaskResult.
*/
public class SQLStatisticsTask extends StatisticsTask {
- private SelectStmt query;
+ private String statement;
public SQLStatisticsTask(long jobId, List<StatisticsDesc> statsDescs) {
super(jobId, statsDescs);
}
@Override
public StatisticsTaskResult call() throws Exception {
- // TODO
- // step1: construct query by statsDescList
- constructQuery();
- // step2: execute query
- // the result should be sequence by @statsTypeList
- List<String> queryResultList = executeQuery(query);
- // step3: construct StatisticsTaskResult by query result
- constructTaskResult(queryResultList);
- return null;
+ checkStatisticsDesc();
+ List<TaskResult> taskResults = Lists.newArrayList();
+
+ for (StatisticsDesc statsDesc : statsDescs) {
+ statement = constructQuery(statsDesc);
+ TaskResult taskResult = executeQuery(statsDesc);
+ taskResults.add(taskResult);
+ }
+
+ return new StatisticsTaskResult(taskResults);
}
- protected void constructQuery() {
- // TODO
- // step1: construct FROM by @granularityDesc
- // step2: construct SELECT LIST by @statsTypeList
+ protected String constructQuery(StatisticsDesc statsDesc) throws DdlException,
+ InvalidFormatException {
+ Map<String, String> params = getQueryParams(statsDesc);
+ List<StatsType> statsTypes = statsDesc.getStatsTypes();
+ StatsType type = statsTypes.get(0);
+
+ StatsGranularity statsGranularity = statsDesc.getStatsGranularity();
+ Granularity granularity = statsGranularity.getGranularity();
+ boolean nonPartitioned = granularity != Granularity.PARTITION;
+
+ switch (type) {
Review Comment:
you could just limit execution resource by set the memory upperbound and parallel execution instatnce for the stats SQL though, so I think it's not a good reason for that.....
--
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 #12765: [feature-wip](statistics) collect statistics by sql task
Posted by GitBox <gi...@apache.org>.
Kikyou1997 commented on code in PR #12765:
URL: https://github.com/apache/doris/pull/12765#discussion_r976007853
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/SQLStatisticsTask.java:
##########
@@ -17,47 +17,119 @@
package org.apache.doris.statistics;
-import org.apache.doris.analysis.SelectStmt;
+import org.apache.doris.catalog.Database;
+import org.apache.doris.catalog.Env;
+import org.apache.doris.catalog.Table;
+import org.apache.doris.common.DdlException;
+import org.apache.doris.common.InvalidFormatException;
+import org.apache.doris.statistics.StatisticsTaskResult.TaskResult;
+import org.apache.doris.statistics.StatsGranularity.Granularity;
+import org.apache.doris.statistics.util.InternalQuery;
+import org.apache.doris.statistics.util.InternalQueryResult;
+import org.apache.doris.statistics.util.InternalQueryResult.ResultRow;
+import org.apache.doris.statistics.util.InternalSqlTemplate;
+
+import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
import java.util.List;
+import java.util.Map;
/**
* A statistics task that collects statistics by executing query.
* The results of the query will be returned as @StatisticsTaskResult.
*/
public class SQLStatisticsTask extends StatisticsTask {
- private SelectStmt query;
+ private String statement;
public SQLStatisticsTask(long jobId, List<StatisticsDesc> statsDescs) {
super(jobId, statsDescs);
}
@Override
public StatisticsTaskResult call() throws Exception {
- // TODO
- // step1: construct query by statsDescList
- constructQuery();
- // step2: execute query
- // the result should be sequence by @statsTypeList
- List<String> queryResultList = executeQuery(query);
- // step3: construct StatisticsTaskResult by query result
- constructTaskResult(queryResultList);
- return null;
+ checkStatisticsDesc();
+ List<TaskResult> taskResults = Lists.newArrayList();
+
+ for (StatisticsDesc statsDesc : statsDescs) {
+ statement = constructQuery(statsDesc);
+ TaskResult taskResult = executeQuery(statsDesc);
+ taskResults.add(taskResult);
+ }
+
+ return new StatisticsTaskResult(taskResults);
}
- protected void constructQuery() {
- // TODO
- // step1: construct FROM by @granularityDesc
- // step2: construct SELECT LIST by @statsTypeList
+ protected String constructQuery(StatisticsDesc statsDesc) throws DdlException,
+ InvalidFormatException {
+ Map<String, String> params = getQueryParams(statsDesc);
+ List<StatsType> statsTypes = statsDesc.getStatsTypes();
+ StatsType type = statsTypes.get(0);
+
+ StatsGranularity statsGranularity = statsDesc.getStatsGranularity();
+ Granularity granularity = statsGranularity.getGranularity();
+ boolean nonPartitioned = granularity != Granularity.PARTITION;
+
+ switch (type) {
Review Comment:
you could just limit execution resource by set the memory upperbound and parallel execution instatnce for the stats SQL though, so I think it's not a good reason for this
--
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 #12765: [feature-wip](statistics) collect statistics by sql task
Posted by GitBox <gi...@apache.org>.
weizhengte commented on code in PR #12765:
URL: https://github.com/apache/doris/pull/12765#discussion_r976116625
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/SQLStatisticsTask.java:
##########
@@ -17,47 +17,119 @@
package org.apache.doris.statistics;
-import org.apache.doris.analysis.SelectStmt;
+import org.apache.doris.catalog.Database;
+import org.apache.doris.catalog.Env;
+import org.apache.doris.catalog.Table;
+import org.apache.doris.common.DdlException;
+import org.apache.doris.common.InvalidFormatException;
+import org.apache.doris.statistics.StatisticsTaskResult.TaskResult;
+import org.apache.doris.statistics.StatsGranularity.Granularity;
+import org.apache.doris.statistics.util.InternalQuery;
+import org.apache.doris.statistics.util.InternalQueryResult;
+import org.apache.doris.statistics.util.InternalQueryResult.ResultRow;
+import org.apache.doris.statistics.util.InternalSqlTemplate;
+
+import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
import java.util.List;
+import java.util.Map;
/**
* A statistics task that collects statistics by executing query.
* The results of the query will be returned as @StatisticsTaskResult.
*/
public class SQLStatisticsTask extends StatisticsTask {
- private SelectStmt query;
+ private String statement;
public SQLStatisticsTask(long jobId, List<StatisticsDesc> statsDescs) {
super(jobId, statsDescs);
}
@Override
public StatisticsTaskResult call() throws Exception {
- // TODO
- // step1: construct query by statsDescList
- constructQuery();
- // step2: execute query
- // the result should be sequence by @statsTypeList
- List<String> queryResultList = executeQuery(query);
- // step3: construct StatisticsTaskResult by query result
- constructTaskResult(queryResultList);
- return null;
+ checkStatisticsDesc();
+ List<TaskResult> taskResults = Lists.newArrayList();
+
+ for (StatisticsDesc statsDesc : statsDescs) {
+ statement = constructQuery(statsDesc);
+ TaskResult taskResult = executeQuery(statsDesc);
+ taskResults.add(taskResult);
+ }
+
+ return new StatisticsTaskResult(taskResults);
}
- protected void constructQuery() {
- // TODO
- // step1: construct FROM by @granularityDesc
- // step2: construct SELECT LIST by @statsTypeList
+ protected String constructQuery(StatisticsDesc statsDesc) throws DdlException,
+ InvalidFormatException {
+ Map<String, String> params = getQueryParams(statsDesc);
+ List<StatsType> statsTypes = statsDesc.getStatsTypes();
+ StatsType type = statsTypes.get(0);
+
+ StatsGranularity statsGranularity = statsDesc.getStatsGranularity();
+ Granularity granularity = statsGranularity.getGranularity();
+ boolean nonPartitioned = granularity != Granularity.PARTITION;
+
+ switch (type) {
+ case ROW_COUNT:
+ return nonPartitioned ? InternalSqlTemplate.buildStatsRowCountSql(params)
+ : InternalSqlTemplate.buildStatsPartitionRowCountSql(params);
+ case NUM_NULLS:
+ return nonPartitioned ? InternalSqlTemplate.buildStatsNumNullsSql(params)
+ : InternalSqlTemplate.buildStatsPartitionNumNullsSql(params);
+ case MAX_SIZE:
+ case AVG_SIZE:
+ return nonPartitioned ? InternalSqlTemplate.buildStatsMaxAvgSizeSql(params)
+ : InternalSqlTemplate.buildStatsPartitionMaxAvgSizeSql(params);
+ case NDV:
+ case MAX_VALUE:
+ case MIN_VALUE:
+ return nonPartitioned ? InternalSqlTemplate.buildStatsMinMaxNdvValueSql(params)
+ : InternalSqlTemplate.buildStatsPartitionMinMaxNdvValueSql(params);
+ case DATA_SIZE:
+ default:
+ throw new DdlException("Unsupported statistics type: " + type);
+ }
}
- protected List<String> executeQuery(SelectStmt query) {
- // TODO (ML)
- return null;
+ protected TaskResult executeQuery(StatisticsDesc statsDesc) throws Exception {
+ StatsGranularity granularity = statsDesc.getStatsGranularity();
+ List<StatsType> statsTypes = statsDesc.getStatsTypes();
+ StatsCategory category = statsDesc.getStatsCategory();
+
+ String dbName = Env.getCurrentInternalCatalog()
+ .getDbOrDdlException(category.getDbId()).getFullName();
+ InternalQuery query = new InternalQuery(dbName, statement);
+ InternalQueryResult queryResult = query.query();
+ List<ResultRow> resultRows = queryResult.getResultRows();
+
+ if (resultRows != null && resultRows.size() == 1) {
+ ResultRow resultRow = resultRows.get(0);
+ List<String> columns = resultRow.getColumns();
+ TaskResult result = createNewTaskResult(category, granularity);
+
+ if (columns.size() == statsTypes.size()) {
+ for (int i = 0; i < columns.size(); i++) {
+ StatsType statsType = StatsType.fromString(columns.get(i));
+ result.getStatsTypeToValue().put(statsType, resultRow.getString(i));
+ }
+ return result;
+ }
+ }
+
+ // Statistics statements are executed singly and return only one row data
+ throw new DdlException("Statistics query result is incorrect, " + queryResult);
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 #12765: [feature-wip](statistics) collect statistics by sql task
Posted by GitBox <gi...@apache.org>.
Kikyou1997 commented on code in PR #12765:
URL: https://github.com/apache/doris/pull/12765#discussion_r975326292
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/SQLStatisticsTask.java:
##########
@@ -17,47 +17,119 @@
package org.apache.doris.statistics;
-import org.apache.doris.analysis.SelectStmt;
+import org.apache.doris.catalog.Database;
+import org.apache.doris.catalog.Env;
+import org.apache.doris.catalog.Table;
+import org.apache.doris.common.DdlException;
+import org.apache.doris.common.InvalidFormatException;
+import org.apache.doris.statistics.StatisticsTaskResult.TaskResult;
+import org.apache.doris.statistics.StatsGranularity.Granularity;
+import org.apache.doris.statistics.util.InternalQuery;
+import org.apache.doris.statistics.util.InternalQueryResult;
+import org.apache.doris.statistics.util.InternalQueryResult.ResultRow;
+import org.apache.doris.statistics.util.InternalSqlTemplate;
+
+import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
import java.util.List;
+import java.util.Map;
/**
* A statistics task that collects statistics by executing query.
* The results of the query will be returned as @StatisticsTaskResult.
*/
public class SQLStatisticsTask extends StatisticsTask {
- private SelectStmt query;
+ private String statement;
public SQLStatisticsTask(long jobId, List<StatisticsDesc> statsDescs) {
super(jobId, statsDescs);
}
@Override
public StatisticsTaskResult call() throws Exception {
- // TODO
- // step1: construct query by statsDescList
- constructQuery();
- // step2: execute query
- // the result should be sequence by @statsTypeList
- List<String> queryResultList = executeQuery(query);
- // step3: construct StatisticsTaskResult by query result
- constructTaskResult(queryResultList);
- return null;
+ checkStatisticsDesc();
+ List<TaskResult> taskResults = Lists.newArrayList();
+
+ for (StatisticsDesc statsDesc : statsDescs) {
+ statement = constructQuery(statsDesc);
+ TaskResult taskResult = executeQuery(statsDesc);
+ taskResults.add(taskResult);
+ }
+
+ return new StatisticsTaskResult(taskResults);
}
- protected void constructQuery() {
- // TODO
- // step1: construct FROM by @granularityDesc
- // step2: construct SELECT LIST by @statsTypeList
+ protected String constructQuery(StatisticsDesc statsDesc) throws DdlException,
+ InvalidFormatException {
+ Map<String, String> params = getQueryParams(statsDesc);
+ List<StatsType> statsTypes = statsDesc.getStatsTypes();
+ StatsType type = statsTypes.get(0);
+
+ StatsGranularity statsGranularity = statsDesc.getStatsGranularity();
+ Granularity granularity = statsGranularity.getGranularity();
+ boolean nonPartitioned = granularity != Granularity.PARTITION;
+
+ switch (type) {
Review Comment:
I think we could finish the collection for these stats in single SQL, such as
```sql
select * from (select max(col1) as size from t1) as tt, (select count(col1) as num from t1) as ttt;
```
--
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 #12765: [feature-wip](statistics) collect statistics by sql task
Posted by GitBox <gi...@apache.org>.
Kikyou1997 commented on code in PR #12765:
URL: https://github.com/apache/doris/pull/12765#discussion_r976007853
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/SQLStatisticsTask.java:
##########
@@ -17,47 +17,119 @@
package org.apache.doris.statistics;
-import org.apache.doris.analysis.SelectStmt;
+import org.apache.doris.catalog.Database;
+import org.apache.doris.catalog.Env;
+import org.apache.doris.catalog.Table;
+import org.apache.doris.common.DdlException;
+import org.apache.doris.common.InvalidFormatException;
+import org.apache.doris.statistics.StatisticsTaskResult.TaskResult;
+import org.apache.doris.statistics.StatsGranularity.Granularity;
+import org.apache.doris.statistics.util.InternalQuery;
+import org.apache.doris.statistics.util.InternalQueryResult;
+import org.apache.doris.statistics.util.InternalQueryResult.ResultRow;
+import org.apache.doris.statistics.util.InternalSqlTemplate;
+
+import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
import java.util.List;
+import java.util.Map;
/**
* A statistics task that collects statistics by executing query.
* The results of the query will be returned as @StatisticsTaskResult.
*/
public class SQLStatisticsTask extends StatisticsTask {
- private SelectStmt query;
+ private String statement;
public SQLStatisticsTask(long jobId, List<StatisticsDesc> statsDescs) {
super(jobId, statsDescs);
}
@Override
public StatisticsTaskResult call() throws Exception {
- // TODO
- // step1: construct query by statsDescList
- constructQuery();
- // step2: execute query
- // the result should be sequence by @statsTypeList
- List<String> queryResultList = executeQuery(query);
- // step3: construct StatisticsTaskResult by query result
- constructTaskResult(queryResultList);
- return null;
+ checkStatisticsDesc();
+ List<TaskResult> taskResults = Lists.newArrayList();
+
+ for (StatisticsDesc statsDesc : statsDescs) {
+ statement = constructQuery(statsDesc);
+ TaskResult taskResult = executeQuery(statsDesc);
+ taskResults.add(taskResult);
+ }
+
+ return new StatisticsTaskResult(taskResults);
}
- protected void constructQuery() {
- // TODO
- // step1: construct FROM by @granularityDesc
- // step2: construct SELECT LIST by @statsTypeList
+ protected String constructQuery(StatisticsDesc statsDesc) throws DdlException,
+ InvalidFormatException {
+ Map<String, String> params = getQueryParams(statsDesc);
+ List<StatsType> statsTypes = statsDesc.getStatsTypes();
+ StatsType type = statsTypes.get(0);
+
+ StatsGranularity statsGranularity = statsDesc.getStatsGranularity();
+ Granularity granularity = statsGranularity.getGranularity();
+ boolean nonPartitioned = granularity != Granularity.PARTITION;
+
+ switch (type) {
Review Comment:
you could just limit execution resource by set the memory upperbound and parallel execution instatnce and so on for the stats SQL though, so I think it's not a good reason for this
--
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 commented on a diff in pull request #12765: [feature-wip](statistics) collect statistics by sql task
Posted by GitBox <gi...@apache.org>.
morrySnow commented on code in PR #12765:
URL: https://github.com/apache/doris/pull/12765#discussion_r976007432
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/SQLStatisticsTask.java:
##########
@@ -17,47 +17,119 @@
package org.apache.doris.statistics;
-import org.apache.doris.analysis.SelectStmt;
+import org.apache.doris.catalog.Database;
+import org.apache.doris.catalog.Env;
+import org.apache.doris.catalog.Table;
+import org.apache.doris.common.DdlException;
+import org.apache.doris.common.InvalidFormatException;
+import org.apache.doris.statistics.StatisticsTaskResult.TaskResult;
+import org.apache.doris.statistics.StatsGranularity.Granularity;
+import org.apache.doris.statistics.util.InternalQuery;
+import org.apache.doris.statistics.util.InternalQueryResult;
+import org.apache.doris.statistics.util.InternalQueryResult.ResultRow;
+import org.apache.doris.statistics.util.InternalSqlTemplate;
+
+import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
import java.util.List;
+import java.util.Map;
/**
* A statistics task that collects statistics by executing query.
* The results of the query will be returned as @StatisticsTaskResult.
*/
public class SQLStatisticsTask extends StatisticsTask {
- private SelectStmt query;
+ private String statement;
public SQLStatisticsTask(long jobId, List<StatisticsDesc> statsDescs) {
super(jobId, statsDescs);
}
@Override
public StatisticsTaskResult call() throws Exception {
- // TODO
- // step1: construct query by statsDescList
- constructQuery();
- // step2: execute query
- // the result should be sequence by @statsTypeList
- List<String> queryResultList = executeQuery(query);
- // step3: construct StatisticsTaskResult by query result
- constructTaskResult(queryResultList);
- return null;
+ checkStatisticsDesc();
+ List<TaskResult> taskResults = Lists.newArrayList();
+
+ for (StatisticsDesc statsDesc : statsDescs) {
+ statement = constructQuery(statsDesc);
+ TaskResult taskResult = executeQuery(statsDesc);
+ taskResults.add(taskResult);
+ }
+
+ return new StatisticsTaskResult(taskResults);
}
- protected void constructQuery() {
- // TODO
- // step1: construct FROM by @granularityDesc
- // step2: construct SELECT LIST by @statsTypeList
+ protected String constructQuery(StatisticsDesc statsDesc) throws DdlException,
Review Comment:
we need to add a UT for it
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/SQLStatisticsTask.java:
##########
@@ -17,47 +17,119 @@
package org.apache.doris.statistics;
-import org.apache.doris.analysis.SelectStmt;
+import org.apache.doris.catalog.Database;
+import org.apache.doris.catalog.Env;
+import org.apache.doris.catalog.Table;
+import org.apache.doris.common.DdlException;
+import org.apache.doris.common.InvalidFormatException;
+import org.apache.doris.statistics.StatisticsTaskResult.TaskResult;
+import org.apache.doris.statistics.StatsGranularity.Granularity;
+import org.apache.doris.statistics.util.InternalQuery;
+import org.apache.doris.statistics.util.InternalQueryResult;
+import org.apache.doris.statistics.util.InternalQueryResult.ResultRow;
+import org.apache.doris.statistics.util.InternalSqlTemplate;
+
+import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
import java.util.List;
+import java.util.Map;
/**
* A statistics task that collects statistics by executing query.
* The results of the query will be returned as @StatisticsTaskResult.
*/
public class SQLStatisticsTask extends StatisticsTask {
- private SelectStmt query;
+ private String statement;
public SQLStatisticsTask(long jobId, List<StatisticsDesc> statsDescs) {
super(jobId, statsDescs);
}
@Override
public StatisticsTaskResult call() throws Exception {
- // TODO
- // step1: construct query by statsDescList
- constructQuery();
- // step2: execute query
- // the result should be sequence by @statsTypeList
- List<String> queryResultList = executeQuery(query);
- // step3: construct StatisticsTaskResult by query result
- constructTaskResult(queryResultList);
- return null;
+ checkStatisticsDesc();
+ List<TaskResult> taskResults = Lists.newArrayList();
+
+ for (StatisticsDesc statsDesc : statsDescs) {
+ statement = constructQuery(statsDesc);
+ TaskResult taskResult = executeQuery(statsDesc);
+ taskResults.add(taskResult);
+ }
+
+ return new StatisticsTaskResult(taskResults);
}
- protected void constructQuery() {
- // TODO
- // step1: construct FROM by @granularityDesc
- // step2: construct SELECT LIST by @statsTypeList
+ protected String constructQuery(StatisticsDesc statsDesc) throws DdlException,
+ InvalidFormatException {
+ Map<String, String> params = getQueryParams(statsDesc);
+ List<StatsType> statsTypes = statsDesc.getStatsTypes();
+ StatsType type = statsTypes.get(0);
+
+ StatsGranularity statsGranularity = statsDesc.getStatsGranularity();
+ Granularity granularity = statsGranularity.getGranularity();
+ boolean nonPartitioned = granularity != Granularity.PARTITION;
+
+ switch (type) {
+ case ROW_COUNT:
+ return nonPartitioned ? InternalSqlTemplate.buildStatsRowCountSql(params)
+ : InternalSqlTemplate.buildStatsPartitionRowCountSql(params);
+ case NUM_NULLS:
+ return nonPartitioned ? InternalSqlTemplate.buildStatsNumNullsSql(params)
+ : InternalSqlTemplate.buildStatsPartitionNumNullsSql(params);
+ case MAX_SIZE:
+ case AVG_SIZE:
+ return nonPartitioned ? InternalSqlTemplate.buildStatsMaxAvgSizeSql(params)
+ : InternalSqlTemplate.buildStatsPartitionMaxAvgSizeSql(params);
+ case NDV:
+ case MAX_VALUE:
+ case MIN_VALUE:
+ return nonPartitioned ? InternalSqlTemplate.buildStatsMinMaxNdvValueSql(params)
+ : InternalSqlTemplate.buildStatsPartitionMinMaxNdvValueSql(params);
+ case DATA_SIZE:
+ default:
+ throw new DdlException("Unsupported statistics type: " + type);
+ }
}
- protected List<String> executeQuery(SelectStmt query) {
- // TODO (ML)
- return null;
+ protected TaskResult executeQuery(StatisticsDesc statsDesc) throws Exception {
+ StatsGranularity granularity = statsDesc.getStatsGranularity();
+ List<StatsType> statsTypes = statsDesc.getStatsTypes();
+ StatsCategory category = statsDesc.getStatsCategory();
+
+ String dbName = Env.getCurrentInternalCatalog()
+ .getDbOrDdlException(category.getDbId()).getFullName();
+ InternalQuery query = new InternalQuery(dbName, statement);
+ InternalQueryResult queryResult = query.query();
+ List<ResultRow> resultRows = queryResult.getResultRows();
+
+ if (resultRows != null && resultRows.size() == 1) {
+ ResultRow resultRow = resultRows.get(0);
+ List<String> columns = resultRow.getColumns();
+ TaskResult result = createNewTaskResult(category, granularity);
+
+ if (columns.size() == statsTypes.size()) {
+ for (int i = 0; i < columns.size(); i++) {
+ StatsType statsType = StatsType.fromString(columns.get(i));
+ result.getStatsTypeToValue().put(statsType, resultRow.getString(i));
+ }
+ return result;
+ }
+ }
+
+ // Statistics statements are executed singly and return only one row data
+ throw new DdlException("Statistics query result is incorrect, " + queryResult);
Review Comment:
do we need to add more info about with stat we collect now?
--
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 #12765: [feature-wip](statistics) collect statistics by sql task
Posted by GitBox <gi...@apache.org>.
Kikyou1997 commented on code in PR #12765:
URL: https://github.com/apache/doris/pull/12765#discussion_r976151896
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/SQLStatisticsTask.java:
##########
@@ -17,47 +17,119 @@
package org.apache.doris.statistics;
-import org.apache.doris.analysis.SelectStmt;
+import org.apache.doris.catalog.Database;
+import org.apache.doris.catalog.Env;
+import org.apache.doris.catalog.Table;
+import org.apache.doris.common.DdlException;
+import org.apache.doris.common.InvalidFormatException;
+import org.apache.doris.statistics.StatisticsTaskResult.TaskResult;
+import org.apache.doris.statistics.StatsGranularity.Granularity;
+import org.apache.doris.statistics.util.InternalQuery;
+import org.apache.doris.statistics.util.InternalQueryResult;
+import org.apache.doris.statistics.util.InternalQueryResult.ResultRow;
+import org.apache.doris.statistics.util.InternalSqlTemplate;
+
+import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
import java.util.List;
+import java.util.Map;
/**
* A statistics task that collects statistics by executing query.
* The results of the query will be returned as @StatisticsTaskResult.
*/
public class SQLStatisticsTask extends StatisticsTask {
- private SelectStmt query;
+ private String statement;
public SQLStatisticsTask(long jobId, List<StatisticsDesc> statsDescs) {
super(jobId, statsDescs);
}
@Override
public StatisticsTaskResult call() throws Exception {
- // TODO
- // step1: construct query by statsDescList
- constructQuery();
- // step2: execute query
- // the result should be sequence by @statsTypeList
- List<String> queryResultList = executeQuery(query);
- // step3: construct StatisticsTaskResult by query result
- constructTaskResult(queryResultList);
- return null;
+ checkStatisticsDesc();
+ List<TaskResult> taskResults = Lists.newArrayList();
+
+ for (StatisticsDesc statsDesc : statsDescs) {
+ statement = constructQuery(statsDesc);
+ TaskResult taskResult = executeQuery(statsDesc);
+ taskResults.add(taskResult);
+ }
+
+ return new StatisticsTaskResult(taskResults);
}
- protected void constructQuery() {
- // TODO
- // step1: construct FROM by @granularityDesc
- // step2: construct SELECT LIST by @statsTypeList
+ protected String constructQuery(StatisticsDesc statsDesc) throws DdlException,
+ InvalidFormatException {
+ Map<String, String> params = getQueryParams(statsDesc);
+ List<StatsType> statsTypes = statsDesc.getStatsTypes();
+ StatsType type = statsTypes.get(0);
+
+ StatsGranularity statsGranularity = statsDesc.getStatsGranularity();
+ Granularity granularity = statsGranularity.getGranularity();
+ boolean nonPartitioned = granularity != Granularity.PARTITION;
+
+ switch (type) {
Review Comment:
First, I don't think it would take much longer exectuion time than separete it to many small SQL, besides, combine those SQL into single statement actually make the resource control of statistcs more easily, otherwise you should set resource limitations for each small statistic SQL and control the total resource occupation of these SQL and should make sure there aren't two many small SQL of statistics are executing, all about these is unneccessary and make resource management and code maintenance much more complicated.
And, Is really neccessary to enable users setting which concrete metric of column is needed by analyze SQL statement? I think this consideration is totally 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 #12765: [feature-wip](statistics) collect statistics by sql task
Posted by GitBox <gi...@apache.org>.
Kikyou1997 commented on code in PR #12765:
URL: https://github.com/apache/doris/pull/12765#discussion_r976151896
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/SQLStatisticsTask.java:
##########
@@ -17,47 +17,119 @@
package org.apache.doris.statistics;
-import org.apache.doris.analysis.SelectStmt;
+import org.apache.doris.catalog.Database;
+import org.apache.doris.catalog.Env;
+import org.apache.doris.catalog.Table;
+import org.apache.doris.common.DdlException;
+import org.apache.doris.common.InvalidFormatException;
+import org.apache.doris.statistics.StatisticsTaskResult.TaskResult;
+import org.apache.doris.statistics.StatsGranularity.Granularity;
+import org.apache.doris.statistics.util.InternalQuery;
+import org.apache.doris.statistics.util.InternalQueryResult;
+import org.apache.doris.statistics.util.InternalQueryResult.ResultRow;
+import org.apache.doris.statistics.util.InternalSqlTemplate;
+
+import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
import java.util.List;
+import java.util.Map;
/**
* A statistics task that collects statistics by executing query.
* The results of the query will be returned as @StatisticsTaskResult.
*/
public class SQLStatisticsTask extends StatisticsTask {
- private SelectStmt query;
+ private String statement;
public SQLStatisticsTask(long jobId, List<StatisticsDesc> statsDescs) {
super(jobId, statsDescs);
}
@Override
public StatisticsTaskResult call() throws Exception {
- // TODO
- // step1: construct query by statsDescList
- constructQuery();
- // step2: execute query
- // the result should be sequence by @statsTypeList
- List<String> queryResultList = executeQuery(query);
- // step3: construct StatisticsTaskResult by query result
- constructTaskResult(queryResultList);
- return null;
+ checkStatisticsDesc();
+ List<TaskResult> taskResults = Lists.newArrayList();
+
+ for (StatisticsDesc statsDesc : statsDescs) {
+ statement = constructQuery(statsDesc);
+ TaskResult taskResult = executeQuery(statsDesc);
+ taskResults.add(taskResult);
+ }
+
+ return new StatisticsTaskResult(taskResults);
}
- protected void constructQuery() {
- // TODO
- // step1: construct FROM by @granularityDesc
- // step2: construct SELECT LIST by @statsTypeList
+ protected String constructQuery(StatisticsDesc statsDesc) throws DdlException,
+ InvalidFormatException {
+ Map<String, String> params = getQueryParams(statsDesc);
+ List<StatsType> statsTypes = statsDesc.getStatsTypes();
+ StatsType type = statsTypes.get(0);
+
+ StatsGranularity statsGranularity = statsDesc.getStatsGranularity();
+ Granularity granularity = statsGranularity.getGranularity();
+ boolean nonPartitioned = granularity != Granularity.PARTITION;
+
+ switch (type) {
Review Comment:
First, I don't think it would take much longer exectuion time than separete it to many small SQL, besides combine those SQL into single statement actually make the resource control of statistcs more easily, otherwise you should set resource limitations for each small statistic SQL and control the total resource occupation of these SQL and should make sure there aren't two many small SQL of statistics executing, all about these is unnessary and make resource management and code maintenance much more complicated.
And, Is really neccessary to enable users setting which concrete metric of column is needed by analyze SQL statement? I think this is totally 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 #12765: [feature-wip](statistics) collect statistics by sql task
Posted by GitBox <gi...@apache.org>.
Kikyou1997 commented on code in PR #12765:
URL: https://github.com/apache/doris/pull/12765#discussion_r976151896
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/SQLStatisticsTask.java:
##########
@@ -17,47 +17,119 @@
package org.apache.doris.statistics;
-import org.apache.doris.analysis.SelectStmt;
+import org.apache.doris.catalog.Database;
+import org.apache.doris.catalog.Env;
+import org.apache.doris.catalog.Table;
+import org.apache.doris.common.DdlException;
+import org.apache.doris.common.InvalidFormatException;
+import org.apache.doris.statistics.StatisticsTaskResult.TaskResult;
+import org.apache.doris.statistics.StatsGranularity.Granularity;
+import org.apache.doris.statistics.util.InternalQuery;
+import org.apache.doris.statistics.util.InternalQueryResult;
+import org.apache.doris.statistics.util.InternalQueryResult.ResultRow;
+import org.apache.doris.statistics.util.InternalSqlTemplate;
+
+import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
import java.util.List;
+import java.util.Map;
/**
* A statistics task that collects statistics by executing query.
* The results of the query will be returned as @StatisticsTaskResult.
*/
public class SQLStatisticsTask extends StatisticsTask {
- private SelectStmt query;
+ private String statement;
public SQLStatisticsTask(long jobId, List<StatisticsDesc> statsDescs) {
super(jobId, statsDescs);
}
@Override
public StatisticsTaskResult call() throws Exception {
- // TODO
- // step1: construct query by statsDescList
- constructQuery();
- // step2: execute query
- // the result should be sequence by @statsTypeList
- List<String> queryResultList = executeQuery(query);
- // step3: construct StatisticsTaskResult by query result
- constructTaskResult(queryResultList);
- return null;
+ checkStatisticsDesc();
+ List<TaskResult> taskResults = Lists.newArrayList();
+
+ for (StatisticsDesc statsDesc : statsDescs) {
+ statement = constructQuery(statsDesc);
+ TaskResult taskResult = executeQuery(statsDesc);
+ taskResults.add(taskResult);
+ }
+
+ return new StatisticsTaskResult(taskResults);
}
- protected void constructQuery() {
- // TODO
- // step1: construct FROM by @granularityDesc
- // step2: construct SELECT LIST by @statsTypeList
+ protected String constructQuery(StatisticsDesc statsDesc) throws DdlException,
+ InvalidFormatException {
+ Map<String, String> params = getQueryParams(statsDesc);
+ List<StatsType> statsTypes = statsDesc.getStatsTypes();
+ StatsType type = statsTypes.get(0);
+
+ StatsGranularity statsGranularity = statsDesc.getStatsGranularity();
+ Granularity granularity = statsGranularity.getGranularity();
+ boolean nonPartitioned = granularity != Granularity.PARTITION;
+
+ switch (type) {
Review Comment:
First, I don't think it would take much longer exectuion time than separete it to many small SQL, besides put those SQL into single statement actually make the resource control of statistcs more easy, otherwise you should set resource limitations for each statistic SQL and control the total resource occupation of these SQL and should make sure there aren't two many small SQL of statistics executing, all about these is unnessary and make resource management and code maintenance much more complicated.
And, Is really neccessary to enable users setting which concrete metric of column is needed by analyze SQL statement? I think this is totally 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