You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by GitBox <gi...@apache.org> on 2022/04/21 03:34:03 UTC
[GitHub] [incubator-doris] BiteTheDDDDt opened a new pull request, #9147: [Feature] [Lateral-View] support outer combinator of table function
BiteTheDDDDt opened a new pull request, #9147:
URL: https://github.com/apache/incubator-doris/pull/9147
# Proposed changes
Issue Number: close #9146
## Problem Summary:
Describe the overview of changes.
## Checklist(Required)
1. Does it affect the original behavior: (Yes/No/I Don't know)
2. Has unit tests been added: (Yes/No/No Need)
3. Has document been added or modified: (Yes/No/No Need)
4. Does it need to update dependencies: (Yes/No)
5. Are there any changes that cannot be rolled back: (Yes/No)
## Further comments
If this is a relatively large or complex change, kick off the discussion at [dev@doris.apache.org](mailto:dev@doris.apache.org) by explaining why you chose the solution you did and what alternatives you considered, etc...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] HappenLee merged pull request #9147: [Feature] [Lateral-View] support outer combinator of table function
Posted by GitBox <gi...@apache.org>.
HappenLee merged PR #9147:
URL: https://github.com/apache/incubator-doris/pull/9147
--
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] [incubator-doris] github-actions[bot] commented on pull request #9147: [Feature] [Lateral-View] support outer combinator of table function
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #9147:
URL: https://github.com/apache/incubator-doris/pull/9147#issuecomment-1106053721
PR approved by anyone and no changes requested.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] cambyzju commented on a diff in pull request #9147: [Feature] [Lateral-View] support outer combinator of table function
Posted by GitBox <gi...@apache.org>.
cambyzju commented on code in PR #9147:
URL: https://github.com/apache/incubator-doris/pull/9147#discussion_r855090552
##########
be/src/exprs/table_function/table_function_factory.cpp:
##########
@@ -92,14 +85,32 @@ const std::unordered_map<std::pair<std::string, bool>, std::function<TableFuncti
{{"explode_json_array_string", true}, VExplodeJsonArrayStringCreator},
{{"explode_bitmap", true},
TableFunctionCreator<vectorized::VExplodeBitmapTableFunction>()},
- {{"explode", true}, VExplodeCreator},
- {{"explode_outer", true}, VExplodeOuterCreator}}; // namespace doris
+ {{"explode", true}, TableFunctionCreator<vectorized::VExplodeTableFunction> {}}};
+
+Status TableFunctionFactory::get_fn(std::string fn_name, bool is_vectorized, ObjectPool* pool,
+ TableFunction** fn) {
+ bool is_outer = false;
+
+ auto match_suffix = [](const std::string& name, const std::string& suffix) -> bool {
+ return name.substr(name.length() - suffix.length()) == suffix;
Review Comment:
check length at first, maybe name.length() < suffix.length()
--
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] [incubator-doris] cambyzju commented on a diff in pull request #9147: [Feature] [Lateral-View] support outer combinator of table function
Posted by GitBox <gi...@apache.org>.
cambyzju commented on code in PR #9147:
URL: https://github.com/apache/incubator-doris/pull/9147#discussion_r855082368
##########
be/src/exprs/table_function/table_function.h:
##########
@@ -99,6 +99,14 @@ class TableFunction {
}
bool is_outer() const { return _is_outer; }
+ void set_outer() {
+ if (is_outer()) {
+ return;
+ }
+ _is_outer = true;
+ _fn_name += "_outer";
Review Comment:
use suffix_outer
--
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] [incubator-doris] HappenLee commented on a diff in pull request #9147: [Feature] [Lateral-View] support outer combinator of table function
Posted by GitBox <gi...@apache.org>.
HappenLee commented on code in PR #9147:
URL: https://github.com/apache/incubator-doris/pull/9147#discussion_r855384900
##########
be/src/vec/functions/function_fake.cpp:
##########
@@ -17,18 +17,43 @@
#include "vec/functions/function_fake.h"
+#include <boost/metaparse/string.hpp>
+#include <string_view>
+#include <type_traits>
+
namespace doris::vectorized {
+// We can use std::basic_fixed_string with c++20 in future
Review Comment:
in the future
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] cambyzju commented on a diff in pull request #9147: [Feature] [Lateral-View] support outer combinator of table function
Posted by GitBox <gi...@apache.org>.
cambyzju commented on code in PR #9147:
URL: https://github.com/apache/incubator-doris/pull/9147#discussion_r855087622
##########
be/src/vec/functions/simple_function_factory.h:
##########
@@ -109,6 +109,13 @@ class SimpleFunctionFactory {
register_function(Function::name, &Function::create);
}
+ template <class Function>
+ void register_table_function() {
+ function_creators[Function::name] = &createDefaultFunction<Function>;
+ function_creators[std::string(Function::name) + "_outer"] =
Review Comment:
use suffix_outer
--
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] [incubator-doris] github-actions[bot] commented on pull request #9147: [Feature] [Lateral-View] support outer combinator of table function
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #9147:
URL: https://github.com/apache/incubator-doris/pull/9147#issuecomment-1106053694
PR approved by at least one committer and no changes requested.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [incubator-doris] cambyzju commented on a diff in pull request #9147: [Feature] [Lateral-View] support outer combinator of table function
Posted by GitBox <gi...@apache.org>.
cambyzju commented on code in PR #9147:
URL: https://github.com/apache/incubator-doris/pull/9147#discussion_r855094379
##########
be/src/exprs/table_function/table_function_factory.cpp:
##########
@@ -92,14 +85,32 @@ const std::unordered_map<std::pair<std::string, bool>, std::function<TableFuncti
{{"explode_json_array_string", true}, VExplodeJsonArrayStringCreator},
{{"explode_bitmap", true},
TableFunctionCreator<vectorized::VExplodeBitmapTableFunction>()},
- {{"explode", true}, VExplodeCreator},
- {{"explode_outer", true}, VExplodeOuterCreator}}; // namespace doris
+ {{"explode", true}, TableFunctionCreator<vectorized::VExplodeTableFunction> {}}};
+
+Status TableFunctionFactory::get_fn(std::string fn_name, bool is_vectorized, ObjectPool* pool,
+ TableFunction** fn) {
+ bool is_outer = false;
+
+ auto match_suffix = [](const std::string& name, const std::string& suffix) -> bool {
+ return name.substr(name.length() - suffix.length()) == suffix;
+ };
+
+ auto remove_suffix = [](const std::string& name, const std::string& suffix) -> std::string {
+ return name.substr(0, name.length() - suffix.length());
+ };
+
+ if (match_suffix(fn_name, suffix_outer)) {
+ is_outer = true;
+ fn_name = remove_suffix(fn_name, suffix_outer);
+ }
-Status TableFunctionFactory::get_fn(const std::string& fn_name, bool is_vectorized,
Review Comment:
use a local variable, such as std::string search_fn_name for search the function.
Does it looks better than change argument of get_fn function?
--
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] [incubator-doris] HappenLee commented on a diff in pull request #9147: [Feature] [Lateral-View] support outer combinator of table function
Posted by GitBox <gi...@apache.org>.
HappenLee commented on code in PR #9147:
URL: https://github.com/apache/incubator-doris/pull/9147#discussion_r855383532
##########
be/src/exprs/table_function/table_function.h:
##########
@@ -30,9 +30,7 @@ namespace doris {
// Currently, the memory allocated from table function is from malloc directly.
class TableFunctionState {};
-namespace table_function_combinator_suffix {
-static const std::string outer = "_outer";
-}
+const std::string COMBINATOR_SUFFIX_OUTER = "_outer";
Review Comment:
constexpr auto
--
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] [incubator-doris] BiteTheDDDDt commented on pull request #9147: [Feature] [Lateral-View] support outer combinator of table function
Posted by GitBox <gi...@apache.org>.
BiteTheDDDDt commented on PR #9147:
URL: https://github.com/apache/incubator-doris/pull/9147#issuecomment-1105161800
@cambyzju fixed
--
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] [incubator-doris] BiteTheDDDDt commented on a diff in pull request #9147: [Feature] [Lateral-View] support outer combinator of table function
Posted by GitBox <gi...@apache.org>.
BiteTheDDDDt commented on code in PR #9147:
URL: https://github.com/apache/incubator-doris/pull/9147#discussion_r855103814
##########
be/src/exprs/table_function/table_function_factory.cpp:
##########
@@ -92,14 +85,32 @@ const std::unordered_map<std::pair<std::string, bool>, std::function<TableFuncti
{{"explode_json_array_string", true}, VExplodeJsonArrayStringCreator},
{{"explode_bitmap", true},
TableFunctionCreator<vectorized::VExplodeBitmapTableFunction>()},
- {{"explode", true}, VExplodeCreator},
- {{"explode_outer", true}, VExplodeOuterCreator}}; // namespace doris
+ {{"explode", true}, TableFunctionCreator<vectorized::VExplodeTableFunction> {}}};
+
+Status TableFunctionFactory::get_fn(std::string fn_name, bool is_vectorized, ObjectPool* pool,
+ TableFunction** fn) {
+ bool is_outer = false;
+
+ auto match_suffix = [](const std::string& name, const std::string& suffix) -> bool {
+ return name.substr(name.length() - suffix.length()) == suffix;
Review Comment:
I think maybe it's not necessary to do the check here?
http://www.cplusplus.com/reference/string/string/substr/
`Number of characters to include in the substring (if the string is shorter, as many characters as possible are used).`
--
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