You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by GitBox <gi...@apache.org> on 2020/03/25 12:03:28 UTC
[GitHub] [drill] weijietong opened a new pull request #2039: DRILL-7663:
code refactor to DefaultFunctionResolver
weijietong opened a new pull request #2039: DRILL-7663: code refactor to DefaultFunctionResolver
URL: https://github.com/apache/drill/pull/2039
# [DRILL-7663](https://issues.apache.org/jira/browse/DRILL-7663): code refactor to DefaultFunctionResolver
## Description
extract common static logic from the for loop
## Documentation
NO
## Testing
NO
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [drill] arina-ielchiieva commented on a change in pull request
#2039: DRILL-7663: code refactor to DefaultFunctionResolver
Posted by GitBox <gi...@apache.org>.
arina-ielchiieva commented on a change in pull request #2039: DRILL-7663: code refactor to DefaultFunctionResolver
URL: https://github.com/apache/drill/pull/2039#discussion_r397849195
##########
File path: exec/java-exec/src/main/java/org/apache/drill/exec/resolver/DefaultFunctionResolver.java
##########
@@ -38,12 +38,11 @@ public DrillFuncHolder getBestMatch(List<DrillFuncHolder> methods, FunctionCall
int currcost = Integer.MAX_VALUE;
DrillFuncHolder bestmatch = null;
final List<DrillFuncHolder> bestMatchAlternatives = new LinkedList<>();
-
+ List<TypeProtos.MajorType> argumentTypes = Lists.newArrayList();
Review comment:
```
List<TypeProtos.MajorType> argumentTypes = call.args().stream()
.map(LogicalExpression::getMajorType)
.collect(Collectors.toList());
```
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [drill] arina-ielchiieva commented on a change in pull request
#2039: DRILL-7663: Code refactor to DefaultFunctionResolver
Posted by GitBox <gi...@apache.org>.
arina-ielchiieva commented on a change in pull request #2039: DRILL-7663: Code refactor to DefaultFunctionResolver
URL: https://github.com/apache/drill/pull/2039#discussion_r397879964
##########
File path: exec/java-exec/src/main/java/org/apache/drill/exec/resolver/DefaultFunctionResolver.java
##########
@@ -38,12 +38,11 @@ public DrillFuncHolder getBestMatch(List<DrillFuncHolder> methods, FunctionCall
int currcost = Integer.MAX_VALUE;
DrillFuncHolder bestmatch = null;
final List<DrillFuncHolder> bestMatchAlternatives = new LinkedList<>();
-
+ List<TypeProtos.MajorType> argumentTypes = Lists.newArrayList();
Review comment:
Well, it's just a suggestion... it's looks much better though.
If you want to leave as it you are welcome but than `Lists.newArrayList();` replace Guava usage then.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [drill] arina-ielchiieva commented on a change in pull request
#2039: DRILL-7663: Code refactor to DefaultFunctionResolver
Posted by GitBox <gi...@apache.org>.
arina-ielchiieva commented on a change in pull request #2039: DRILL-7663: Code refactor to DefaultFunctionResolver
URL: https://github.com/apache/drill/pull/2039#discussion_r397879964
##########
File path: exec/java-exec/src/main/java/org/apache/drill/exec/resolver/DefaultFunctionResolver.java
##########
@@ -38,12 +38,11 @@ public DrillFuncHolder getBestMatch(List<DrillFuncHolder> methods, FunctionCall
int currcost = Integer.MAX_VALUE;
DrillFuncHolder bestmatch = null;
final List<DrillFuncHolder> bestMatchAlternatives = new LinkedList<>();
-
+ List<TypeProtos.MajorType> argumentTypes = Lists.newArrayList();
Review comment:
Well, it's just a suggestion... it's looks much better.
If you want to leave as it you welcome but than `Lists.newArrayList();` replace Guava usage then.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [drill] arina-ielchiieva commented on a change in pull request
#2039: DRILL-7663: Code refactor to DefaultFunctionResolver
Posted by GitBox <gi...@apache.org>.
arina-ielchiieva commented on a change in pull request #2039: DRILL-7663: Code refactor to DefaultFunctionResolver
URL: https://github.com/apache/drill/pull/2039#discussion_r397879964
##########
File path: exec/java-exec/src/main/java/org/apache/drill/exec/resolver/DefaultFunctionResolver.java
##########
@@ -38,12 +38,11 @@ public DrillFuncHolder getBestMatch(List<DrillFuncHolder> methods, FunctionCall
int currcost = Integer.MAX_VALUE;
DrillFuncHolder bestmatch = null;
final List<DrillFuncHolder> bestMatchAlternatives = new LinkedList<>();
-
+ List<TypeProtos.MajorType> argumentTypes = Lists.newArrayList();
Review comment:
Well, it's just a suggestion... it's looks much better.
If you want to leave as it you welcome but than `Lists.newArrayList();` replace Guave usage then.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [drill] arina-ielchiieva merged pull request #2039: DRILL-7663:
Code refactor to DefaultFunctionResolver
Posted by GitBox <gi...@apache.org>.
arina-ielchiieva merged pull request #2039: DRILL-7663: Code refactor to DefaultFunctionResolver
URL: https://github.com/apache/drill/pull/2039
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [drill] weijietong commented on a change in pull request #2039:
DRILL-7663: Code refactor to DefaultFunctionResolver
Posted by GitBox <gi...@apache.org>.
weijietong commented on a change in pull request #2039: DRILL-7663: Code refactor to DefaultFunctionResolver
URL: https://github.com/apache/drill/pull/2039#discussion_r397885954
##########
File path: exec/java-exec/src/main/java/org/apache/drill/exec/resolver/DefaultFunctionResolver.java
##########
@@ -38,12 +38,11 @@ public DrillFuncHolder getBestMatch(List<DrillFuncHolder> methods, FunctionCall
int currcost = Integer.MAX_VALUE;
DrillFuncHolder bestmatch = null;
final List<DrillFuncHolder> bestMatchAlternatives = new LinkedList<>();
-
+ List<TypeProtos.MajorType> argumentTypes = Lists.newArrayList();
Review comment:
got it. I have applied your advice. thanks @arina-ielchiieva
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [drill] arina-ielchiieva commented on a change in pull request
#2039: DRILL-7663: Code refactor to DefaultFunctionResolver
Posted by GitBox <gi...@apache.org>.
arina-ielchiieva commented on a change in pull request #2039: DRILL-7663: Code refactor to DefaultFunctionResolver
URL: https://github.com/apache/drill/pull/2039#discussion_r397879964
##########
File path: exec/java-exec/src/main/java/org/apache/drill/exec/resolver/DefaultFunctionResolver.java
##########
@@ -38,12 +38,11 @@ public DrillFuncHolder getBestMatch(List<DrillFuncHolder> methods, FunctionCall
int currcost = Integer.MAX_VALUE;
DrillFuncHolder bestmatch = null;
final List<DrillFuncHolder> bestMatchAlternatives = new LinkedList<>();
-
+ List<TypeProtos.MajorType> argumentTypes = Lists.newArrayList();
Review comment:
Well, it's just a suggestion... it's looks much better though.
If you want to leave as it you are welcome but then replace `Lists.newArrayList();` Guava usage.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [drill] weijietong commented on a change in pull request #2039:
DRILL-7663: Code refactor to DefaultFunctionResolver
Posted by GitBox <gi...@apache.org>.
weijietong commented on a change in pull request #2039: DRILL-7663: Code refactor to DefaultFunctionResolver
URL: https://github.com/apache/drill/pull/2039#discussion_r397874647
##########
File path: exec/java-exec/src/main/java/org/apache/drill/exec/resolver/DefaultFunctionResolver.java
##########
@@ -38,12 +38,11 @@ public DrillFuncHolder getBestMatch(List<DrillFuncHolder> methods, FunctionCall
int currcost = Integer.MAX_VALUE;
DrillFuncHolder bestmatch = null;
final List<DrillFuncHolder> bestMatchAlternatives = new LinkedList<>();
-
+ List<TypeProtos.MajorType> argumentTypes = Lists.newArrayList();
Review comment:
ok, but I think we should open to contributors to choose the function programing or command one.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [drill] arina-ielchiieva commented on issue #2039: DRILL-7663: Code
refactor to DefaultFunctionResolver
Posted by GitBox <gi...@apache.org>.
arina-ielchiieva commented on issue #2039: DRILL-7663: Code refactor to DefaultFunctionResolver
URL: https://github.com/apache/drill/pull/2039#issuecomment-603838932
@weijietong please re-write for loop to use streams and rename PR commit message to start with upper letter: `DRILL-7663: code refactor to DefaultFunctionResolver` -> `DRILL-7663: Code refactor to DefaultFunctionResolver`.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [drill] arina-ielchiieva commented on a change in pull request
#2039: DRILL-7663: Code refactor to DefaultFunctionResolver
Posted by GitBox <gi...@apache.org>.
arina-ielchiieva commented on a change in pull request #2039: DRILL-7663: Code refactor to DefaultFunctionResolver
URL: https://github.com/apache/drill/pull/2039#discussion_r397879964
##########
File path: exec/java-exec/src/main/java/org/apache/drill/exec/resolver/DefaultFunctionResolver.java
##########
@@ -38,12 +38,11 @@ public DrillFuncHolder getBestMatch(List<DrillFuncHolder> methods, FunctionCall
int currcost = Integer.MAX_VALUE;
DrillFuncHolder bestmatch = null;
final List<DrillFuncHolder> bestMatchAlternatives = new LinkedList<>();
-
+ List<TypeProtos.MajorType> argumentTypes = Lists.newArrayList();
Review comment:
Well, it's just a suggestion... it's looks much better though.
If you want to leave as it you are welcome but then replace `Lists.newArrayList();` Guava usage then.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [drill] arina-ielchiieva commented on issue #2039: DRILL-7663: Code
refactor to DefaultFunctionResolver
Posted by GitBox <gi...@apache.org>.
arina-ielchiieva commented on issue #2039: DRILL-7663: Code refactor to DefaultFunctionResolver
URL: https://github.com/apache/drill/pull/2039#issuecomment-603863803
@weijietong thanks for making the changes. +1
----------------------------------------------------------------
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
With regards,
Apache Git Services