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 2020/07/09 03:30:04 UTC
[GitHub] [incubator-doris] wuyunfeng opened a new pull request #4055: [Doris On ES] Add docvalue limitation for doc_values scan
wuyunfeng opened a new pull request #4055:
URL: https://github.com/apache/incubator-doris/pull/4055
----------------------------------------------------------------
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.
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] [incubator-doris] wutiangan commented on a change in pull request #4055: [Doris On ES] Add docvalue limitation for doc_values scan and enable doc_values scan default
Posted by GitBox <gi...@apache.org>.
wutiangan commented on a change in pull request #4055:
URL: https://github.com/apache/incubator-doris/pull/4055#discussion_r451983644
##########
File path: fe/src/main/java/org/apache/doris/catalog/EsTable.java
##########
@@ -294,6 +331,13 @@ public void readFields(DataInput in) throws IOException {
} else {
enableKeywordSniff = true;
}
+ if (tableContext.containsKey("maxDocValueFields")) {
+ try {
+ maxDocValueFields = Integer.parseInt(tableContext.get("maxDocValueFields"));
+ } catch (Exception e) {
+ maxDocValueFields = 30;
+ }
+ }
Review comment:
if the tableContext don't contain "maxDocValueFields", the maxDocValueFields should set Default_max_docvaues_fields or set MaxInt.
----------------------------------------------------------------
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.
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] [incubator-doris] wutiangan commented on a change in pull request #4055: [Doris On ES] Add docvalue limitation for doc_values scan and enable doc_values scan default
Posted by GitBox <gi...@apache.org>.
wutiangan commented on a change in pull request #4055:
URL: https://github.com/apache/incubator-doris/pull/4055#discussion_r451984322
##########
File path: be/src/exec/es/es_scan_reader.h
##########
@@ -40,6 +40,7 @@ class ESScanReader {
static constexpr const char* KEY_QUERY = "query";
static constexpr const char* KEY_BATCH_SIZE = "batch_size";
static constexpr const char* KEY_TERMINATE_AFTER = "limit";
+ static constexpr const char* KEY_DOC_VALUE_MODE = "doc_values_mode";
Review comment:
```suggestion
static constexpr const char* KEY_DOC_VALUES_MODE = "doc_values_mode";
```
----------------------------------------------------------------
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.
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] [incubator-doris] wutiangan commented on a change in pull request #4055: [Doris On ES] Add docvalue limitation for doc_values scan and enable doc_values scan default
Posted by GitBox <gi...@apache.org>.
wutiangan commented on a change in pull request #4055:
URL: https://github.com/apache/incubator-doris/pull/4055#discussion_r451983270
##########
File path: fe/src/main/java/org/apache/doris/catalog/EsTable.java
##########
@@ -294,6 +331,13 @@ public void readFields(DataInput in) throws IOException {
} else {
enableKeywordSniff = true;
}
+ if (tableContext.containsKey("maxDocValueFields")) {
+ try {
+ maxDocValueFields = Integer.parseInt(tableContext.get("maxDocValueFields"));
+ } catch (Exception e) {
+ maxDocValueFields = 30;
Review comment:
why set 30, not sed DEFAULT_MAX_DOCVALUE_FIELDS.
----------------------------------------------------------------
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.
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] [incubator-doris] blackfox1983 commented on a change in pull request #4055: [Doris On ES] Add docvalue limitation for doc_values scan and enable doc_values scan default
Posted by GitBox <gi...@apache.org>.
blackfox1983 commented on a change in pull request #4055:
URL: https://github.com/apache/incubator-doris/pull/4055#discussion_r451981961
##########
File path: be/src/exec/es/es_scroll_query.cpp
##########
@@ -76,14 +76,20 @@ std::string ESScrollQueryBuilder::build(const std::map<std::string, std::string>
// note: add `query` for this value....
es_query_dsl.AddMember("query", query_node, allocator);
bool pure_docvalue = true;
- // check docvalue sacan optimization
- if (docvalue_context.size() == 0 || docvalue_context.size() < fields.size()) {
- pure_docvalue = false;
+
+ // Doris FE already has checked docvalue-scan optimization
+ if (properties.find(ESScanReader::KEY_DOC_VALUE_MODE) != properties.end()) {
+ pure_docvalue = atoi(properties.at(ESScanReader::KEY_DOC_VALUE_MODE).c_str());
Review comment:
pure_docvalue = properties[ESScanReader::KEY_DOC_VALUE_MODE] == "1" ? true: false;
看到对bool变量赋值，这样子会不会更直观些？而不用考虑atoi返回值成功还是失败的情况下，docvalue是true还是false。
----------------------------------------------------------------
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.
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] [incubator-doris] wuyunfeng commented on a change in pull request #4055: [Doris On ES] Add docvalue limitation for doc_values scan and enable doc_values scan default
Posted by GitBox <gi...@apache.org>.
wuyunfeng commented on a change in pull request #4055:
URL: https://github.com/apache/incubator-doris/pull/4055#discussion_r451984684
##########
File path: fe/src/main/java/org/apache/doris/planner/EsScanNode.java
##########
@@ -109,6 +110,34 @@ public void finalize(Analyzer analyzer) throws UserException {
isFinalized = true;
}
+ /**
+ * return whether can use the doc_values scan
+ * 0 and 1 are returned to facilitate Doris BE processing
Review comment:
这和传递没有关系，因为有一个map结构当时留着就是为了在不改thrift结构的情况下容易扩展
----------------------------------------------------------------
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.
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] [incubator-doris] wuyunfeng commented on a change in pull request #4055: [Doris On ES] Add docvalue limitation for doc_values scan and enable doc_values scan default
Posted by GitBox <gi...@apache.org>.
wuyunfeng commented on a change in pull request #4055:
URL: https://github.com/apache/incubator-doris/pull/4055#discussion_r451983995
##########
File path: fe/src/main/java/org/apache/doris/catalog/EsTable.java
##########
@@ -294,6 +331,13 @@ public void readFields(DataInput in) throws IOException {
} else {
enableKeywordSniff = true;
}
+ if (tableContext.containsKey("maxDocValueFields")) {
+ try {
+ maxDocValueFields = Integer.parseInt(tableContext.get("maxDocValueFields"));
+ } catch (Exception e) {
+ maxDocValueFields = 30;
Review comment:
是的，我忘了
----------------------------------------------------------------
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.
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] [incubator-doris] wuyunfeng commented on a change in pull request #4055: [Doris On ES] Add docvalue limitation for doc_values scan and enable doc_values scan default
Posted by GitBox <gi...@apache.org>.
wuyunfeng commented on a change in pull request #4055:
URL: https://github.com/apache/incubator-doris/pull/4055#discussion_r451983832
##########
File path: fe/src/main/java/org/apache/doris/catalog/EsTable.java
##########
@@ -294,6 +331,13 @@ public void readFields(DataInput in) throws IOException {
} else {
enableKeywordSniff = true;
}
+ if (tableContext.containsKey("maxDocValueFields")) {
+ try {
+ maxDocValueFields = Integer.parseInt(tableContext.get("maxDocValueFields"));
+ } catch (Exception e) {
+ maxDocValueFields = 30;
Review comment:
Oh，I forget 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.
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] [incubator-doris] blackfox1983 commented on a change in pull request #4055: [Doris On ES] Add docvalue limitation for doc_values scan and enable doc_values scan default
Posted by GitBox <gi...@apache.org>.
blackfox1983 commented on a change in pull request #4055:
URL: https://github.com/apache/incubator-doris/pull/4055#discussion_r451983749
##########
File path: fe/src/main/java/org/apache/doris/catalog/EsTable.java
##########
@@ -294,6 +331,13 @@ public void readFields(DataInput in) throws IOException {
} else {
enableKeywordSniff = true;
}
+ if (tableContext.containsKey("maxDocValueFields")) {
+ try {
+ maxDocValueFields = Integer.parseInt(tableContext.get("maxDocValueFields"));
+ } catch (Exception e) {
+ maxDocValueFields = 30;
Review comment:
这个地方是不是可以使用定义的值？DEFAULT_MAX_DOC_VALUS_FILEDS
----------------------------------------------------------------
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.
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] [incubator-doris] wutiangan commented on a change in pull request #4055: [Doris On ES] Add docvalue limitation for doc_values scan and enable doc_values scan default
Posted by GitBox <gi...@apache.org>.
wutiangan commented on a change in pull request #4055:
URL: https://github.com/apache/incubator-doris/pull/4055#discussion_r451983270
##########
File path: fe/src/main/java/org/apache/doris/catalog/EsTable.java
##########
@@ -294,6 +331,13 @@ public void readFields(DataInput in) throws IOException {
} else {
enableKeywordSniff = true;
}
+ if (tableContext.containsKey("maxDocValueFields")) {
+ try {
+ maxDocValueFields = Integer.parseInt(tableContext.get("maxDocValueFields"));
+ } catch (Exception e) {
+ maxDocValueFields = 30;
Review comment:
why set 30, not set DEFAULT_MAX_DOCVALUE_FIELDS.
----------------------------------------------------------------
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.
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] [incubator-doris] morningman merged pull request #4055: [Doris On ES] Add docvalue limitation for doc_values scan and enable doc_values scan default
Posted by GitBox <gi...@apache.org>.
morningman merged pull request #4055:
URL: https://github.com/apache/incubator-doris/pull/4055
----------------------------------------------------------------
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.
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] [incubator-doris] blackfox1983 commented on a change in pull request #4055: [Doris On ES] Add docvalue limitation for doc_values scan and enable doc_values scan default
Posted by GitBox <gi...@apache.org>.
blackfox1983 commented on a change in pull request #4055:
URL: https://github.com/apache/incubator-doris/pull/4055#discussion_r451984363
##########
File path: fe/src/main/java/org/apache/doris/planner/EsScanNode.java
##########
@@ -109,6 +110,34 @@ public void finalize(Analyzer analyzer) throws UserException {
isFinalized = true;
}
+ /**
+ * return whether can use the doc_values scan
+ * 0 and 1 are returned to facilitate Doris BE processing
Review comment:
我不太确定 fe通过thrift传递给be的参数是否支持bool。这个地方读起来bool更直观。
否则我得带着c++的思维去理解。（1是true，0是false）
----------------------------------------------------------------
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.
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] [incubator-doris] wuyunfeng commented on a change in pull request #4055: [Doris On ES] Add docvalue limitation for doc_values scan and enable doc_values scan default
Posted by GitBox <gi...@apache.org>.
wuyunfeng commented on a change in pull request #4055:
URL: https://github.com/apache/incubator-doris/pull/4055#discussion_r451982340
##########
File path: be/src/exec/es/es_scroll_query.cpp
##########
@@ -76,14 +76,20 @@ std::string ESScrollQueryBuilder::build(const std::map<std::string, std::string>
// note: add `query` for this value....
es_query_dsl.AddMember("query", query_node, allocator);
bool pure_docvalue = true;
- // check docvalue sacan optimization
- if (docvalue_context.size() == 0 || docvalue_context.size() < fields.size()) {
- pure_docvalue = false;
+
+ // Doris FE already has checked docvalue-scan optimization
+ if (properties.find(ESScanReader::KEY_DOC_VALUE_MODE) != properties.end()) {
+ pure_docvalue = atoi(properties.at(ESScanReader::KEY_DOC_VALUE_MODE).c_str());
Review comment:
个人感觉这种值的设定是由FE完全控制的，倒还好
----------------------------------------------------------------
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.
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] [incubator-doris] wutiangan commented on a change in pull request #4055: [Doris On ES] Add docvalue limitation for doc_values scan and enable doc_values scan default
Posted by GitBox <gi...@apache.org>.
wutiangan commented on a change in pull request #4055:
URL: https://github.com/apache/incubator-doris/pull/4055#discussion_r451979387
##########
File path: fe/src/main/java/org/apache/doris/catalog/EsTable.java
##########
@@ -294,6 +331,13 @@ public void readFields(DataInput in) throws IOException {
} else {
enableKeywordSniff = true;
}
+ if (tableContext.containsKey("maxDocValueFields")) {
Review comment:
use a const string variables to replace stringLiterals
----------------------------------------------------------------
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.
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] [incubator-doris] blackfox1983 commented on a change in pull request #4055: [Doris On ES] Add docvalue limitation for doc_values scan and enable doc_values scan default
Posted by GitBox <gi...@apache.org>.
blackfox1983 commented on a change in pull request #4055:
URL: https://github.com/apache/incubator-doris/pull/4055#discussion_r451981036
##########
File path: be/src/exec/es/es_scan_reader.h
##########
@@ -40,6 +40,7 @@ class ESScanReader {
static constexpr const char* KEY_QUERY = "query";
static constexpr const char* KEY_BATCH_SIZE = "batch_size";
static constexpr const char* KEY_TERMINATE_AFTER = "limit";
+ static constexpr const char* KEY_DOC_VALUE_MODE = "doc_values_mode";
Review comment:
定义了这个，但是没看到在哪儿使用。
----------------------------------------------------------------
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.
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] [incubator-doris] blackfox1983 commented on a change in pull request #4055: [Doris On ES] Add docvalue limitation for doc_values scan and enable doc_values scan default
Posted by GitBox <gi...@apache.org>.
blackfox1983 commented on a change in pull request #4055:
URL: https://github.com/apache/incubator-doris/pull/4055#discussion_r451983508
##########
File path: fe/src/main/java/org/apache/doris/catalog/EsTable.java
##########
@@ -194,6 +219,17 @@ private void validate(Map<String, String> properties) throws DdlException {
+ " but value is " + transport);
}
}
+
+ if (properties.containsKey(MAX_DOCVALUE_FIELDS)) {
+ try {
+ maxDocValueFields = Integer.parseInt(properties.get(MAX_DOCVALUE_FIELDS).trim());
+ if (maxDocValueFields < 1) {
+ maxDocValueFields = 1;
Review comment:
是不是直接报错得了？我们不允许这类低级错误。比如设置为0或者-1. 可能用户想关闭docvalue查询？
----------------------------------------------------------------
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.
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] [incubator-doris] wuyunfeng commented on a change in pull request #4055: [Doris On ES] Add docvalue limitation for doc_values scan and enable doc_values scan default
Posted by GitBox <gi...@apache.org>.
wuyunfeng commented on a change in pull request #4055:
URL: https://github.com/apache/incubator-doris/pull/4055#discussion_r451991400
##########
File path: fe/src/main/java/org/apache/doris/catalog/EsTable.java
##########
@@ -194,6 +219,17 @@ private void validate(Map<String, String> properties) throws DdlException {
+ " but value is " + transport);
}
}
+
+ if (properties.containsKey(MAX_DOCVALUE_FIELDS)) {
+ try {
+ maxDocValueFields = Integer.parseInt(properties.get(MAX_DOCVALUE_FIELDS).trim());
+ if (maxDocValueFields < 1) {
+ maxDocValueFields = 1;
Review comment:
可以设置为0，不过控制开关的话还是用上面那个？
----------------------------------------------------------------
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.
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