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/10 17:28:51 UTC
[GitHub] [drill] paul-rogers opened a new pull request #2020: DRILL-7634:
Rollup of code cleanup changes
paul-rogers opened a new pull request #2020: DRILL-7634: Rollup of code cleanup changes
URL: https://github.com/apache/drill/pull/2020
# [DRILL-7634](https://issues.apache.org/jira/browseDRILL-7634): Rollup of code cleanup changes
## Description
Collection of code cleanup changes.
The most significant is to create constants for function names. We already had a pretty good list in `FunctionGenerationHelper`. I move this list to a new interface, `FunctionNames` and added to it. I then changed all references to hard-coded names to instead refer to these constants.
This change pointed out several places where code checks for variations of function names. Not sure why; perhaps an early version of the code did not have the map from alias to "canonical" name. Used constants for the canonical name, left the other names in place because I don't have enough knowledge to know if it is safe to remove them. Anyone know?
Also made the `args` member of `FunctionCall` private and added accessors.
## Documentation
None
## Testing
Reran all unit tests.
----------------------------------------------------------------
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] vvysotskyi commented on a change in pull request #2020:
DRILL-7634: Rollup of code cleanup changes
Posted by GitBox <gi...@apache.org>.
vvysotskyi commented on a change in pull request #2020: DRILL-7634: Rollup of code cleanup changes
URL: https://github.com/apache/drill/pull/2020#discussion_r390847156
##########
File path: exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/JoinPrel.java
##########
@@ -186,6 +188,7 @@ public boolean isSemiJoin() {
// fields, left fields, and right fields. Very similar to the
// output row type, except that fields have not yet been made due
// due to outer joins.
+ @SuppressWarnings("deprecation")
RexChecker checker =
new RexChecker(
getCluster().getTypeFactory().builder()
Review comment:
Please use Builder class, as it is recommended in the JavaDoc:
```suggestion
RexChecker checker =
new RexChecker(
new RelDataTypeFactory.Builder(getCluster().getTypeFactory())
```
----------------------------------------------------------------
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] asfgit closed pull request #2020: DRILL-7634: Rollup of
code cleanup changes
Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #2020: DRILL-7634: Rollup of code cleanup changes
URL: https://github.com/apache/drill/pull/2020
----------------------------------------------------------------
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] vvysotskyi commented on a change in pull request #2020:
DRILL-7634: Rollup of code cleanup changes
Posted by GitBox <gi...@apache.org>.
vvysotskyi commented on a change in pull request #2020: DRILL-7634: Rollup of code cleanup changes
URL: https://github.com/apache/drill/pull/2020#discussion_r390849669
##########
File path: exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/TopNPrel.java
##########
@@ -140,6 +140,7 @@ public Prel prepareForLateralUnnestPipeline(List<RelNode> children) {
fieldCollation.direction, fieldCollation.nullDirection));
}
+ @SuppressWarnings("deprecation")
RelCollation collationTrait = RelCollationImpl.of(relFieldCollations);
Review comment:
```suggestion
RelCollation collationTrait = RelCollations.of(relFieldCollations);
```
----------------------------------------------------------------
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] vvysotskyi commented on a change in pull request #2020:
DRILL-7634: Rollup of code cleanup changes
Posted by GitBox <gi...@apache.org>.
vvysotskyi commented on a change in pull request #2020: DRILL-7634: Rollup of code cleanup changes
URL: https://github.com/apache/drill/pull/2020#discussion_r390833446
##########
File path: exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillAggFuncHolder.java
##########
@@ -81,51 +79,47 @@ public boolean isAggregating() {
@Override
public JVar[] renderStart(ClassGenerator<?> g, HoldingContainer[] inputVariables, FieldReference fieldReference) {
if (!g.getMappingSet().isHashAggMapping()) { //Declare workspace vars for non-hash-aggregation.
- JVar[] workspaceJVars = declareWorkspaceVariables(g);
- generateBody(g, BlockType.SETUP, setup(), null, workspaceJVars, true);
- return workspaceJVars;
+ JVar[] workspaceJVars = declareWorkspaceVariables(g);
+ generateBody(g, BlockType.SETUP, setup(), null, workspaceJVars, true);
+ return workspaceJVars;
} else { //Declare workspace vars and workspace vectors for hash aggregation.
Review comment:
Please fix indentation for this line also.
----------------------------------------------------------------
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] vvysotskyi commented on a change in pull request #2020:
DRILL-7634: Rollup of code cleanup changes
Posted by GitBox <gi...@apache.org>.
vvysotskyi commented on a change in pull request #2020: DRILL-7634: Rollup of code cleanup changes
URL: https://github.com/apache/drill/pull/2020#discussion_r390840286
##########
File path: exec/java-exec/src/main/java/org/apache/drill/exec/planner/fragment/Wrapper.java
##########
@@ -225,12 +236,12 @@ public void computeCpuResources() {
return result;
};
- Function<DrillbitEndpoint, NodeResource> cpuPerEndpoint = (endpoint) -> new NodeResource(1, 0);
+ Function<DrillbitEndpoint, NodeResource> cpuPerEndpoint = (
+ endpoint) -> new NodeResource(1, 0);
Review comment:
Please remove brackets:
```suggestion
Function<DrillbitEndpoint, NodeResource> cpuPerEndpoint =
endpoint -> new NodeResource(1, 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
With regards,
Apache Git Services
[GitHub] [drill] vvysotskyi commented on a change in pull request #2020:
DRILL-7634: Rollup of code cleanup changes
Posted by GitBox <gi...@apache.org>.
vvysotskyi commented on a change in pull request #2020: DRILL-7634: Rollup of code cleanup changes
URL: https://github.com/apache/drill/pull/2020#discussion_r390847794
##########
File path: exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/MetadataControllerPrel.java
##########
@@ -85,6 +85,7 @@ public boolean needsFinalColumnReordering() {
return PrelUtil.iter(getLeft(), getRight());
}
+ @SuppressWarnings("deprecation")
@Override
protected RelDataType deriveRowType() {
return getCluster().getTypeFactory().builder()
Review comment:
The same for Builder there.
----------------------------------------------------------------
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] vvysotskyi commented on a change in pull request #2020:
DRILL-7634: Rollup of code cleanup changes
Posted by GitBox <gi...@apache.org>.
vvysotskyi commented on a change in pull request #2020: DRILL-7634: Rollup of code cleanup changes
URL: https://github.com/apache/drill/pull/2020#discussion_r390830839
##########
File path: contrib/storage-hive/core/src/main/java/org/apache/drill/exec/expr/fn/HiveFunctionRegistry.java
##########
@@ -65,9 +65,9 @@
OracleSqlOperatorTable.TRANSLATE3.getName().toLowerCase())
.build();
- private ArrayListMultimap<String, Class<? extends GenericUDF>> methodsGenericUDF = ArrayListMultimap.create();
- private ArrayListMultimap<String, Class<? extends UDF>> methodsUDF = ArrayListMultimap.create();
- private HashSet<Class<?>> nonDeterministicUDFs = new HashSet<>();
+ private final ArrayListMultimap<String, Class<? extends GenericUDF>> methodsGenericUDF = ArrayListMultimap.create();
Review comment:
Nit: Could you please replace field type with the `Multimap` interface here and in the line below.
```suggestion
private final Multimap<String, Class<? extends GenericUDF>> methodsGenericUDF = ArrayListMultimap.create();
```
----------------------------------------------------------------
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] vvysotskyi commented on a change in pull request #2020:
DRILL-7634: Rollup of code cleanup changes
Posted by GitBox <gi...@apache.org>.
vvysotskyi commented on a change in pull request #2020: DRILL-7634: Rollup of code cleanup changes
URL: https://github.com/apache/drill/pull/2020#discussion_r390831009
##########
File path: contrib/storage-hive/core/src/main/java/org/apache/drill/exec/expr/fn/HiveFunctionRegistry.java
##########
@@ -65,9 +65,9 @@
OracleSqlOperatorTable.TRANSLATE3.getName().toLowerCase())
.build();
- private ArrayListMultimap<String, Class<? extends GenericUDF>> methodsGenericUDF = ArrayListMultimap.create();
- private ArrayListMultimap<String, Class<? extends UDF>> methodsUDF = ArrayListMultimap.create();
- private HashSet<Class<?>> nonDeterministicUDFs = new HashSet<>();
+ private final ArrayListMultimap<String, Class<? extends GenericUDF>> methodsGenericUDF = ArrayListMultimap.create();
+ private final ArrayListMultimap<String, Class<? extends UDF>> methodsUDF = ArrayListMultimap.create();
+ private final HashSet<Class<?>> nonDeterministicUDFs = new HashSet<>();
Review comment:
```suggestion
private final Set<Class<?>> nonDeterministicUDFs = new HashSet<>();
```
----------------------------------------------------------------
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] vvysotskyi commented on a change in pull request #2020:
DRILL-7634: Rollup of code cleanup changes
Posted by GitBox <gi...@apache.org>.
vvysotskyi commented on a change in pull request #2020: DRILL-7634: Rollup of code cleanup changes
URL: https://github.com/apache/drill/pull/2020#discussion_r390844001
##########
File path: exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/DrillDistributionTrait.java
##########
@@ -27,6 +27,16 @@
public class DrillDistributionTrait implements RelTrait {
+ public enum DistributionType {
Review comment:
This class contains also inner classes in the end. Moving the only enum makes code more inconsistent. Could you please either revert this change, so all inner types would be at the end or move them all to the beginning of the file.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [drill] vvysotskyi commented on a change in pull request #2020:
DRILL-7634: Rollup of code cleanup changes
Posted by GitBox <gi...@apache.org>.
vvysotskyi commented on a change in pull request #2020: DRILL-7634: Rollup of code cleanup changes
URL: https://github.com/apache/drill/pull/2020#discussion_r390828975
##########
File path: contrib/format-maprdb/src/main/java/org/apache/drill/exec/store/mapr/db/json/OjaiFunctionsProcessor.java
##########
@@ -184,8 +184,8 @@ public Void visitFunctionCall(FunctionCall call, Void v) throws RuntimeException
case "ojai_condition": {
// ojai_condition(field, <serialized-condition>);
- final SchemaPath schemaPath = getSchemaPathArg(call.args.get(0));
- final String condString = getStringArg(call.args.get(1));
+ //final SchemaPath schemaPath = getSchemaPathArg(call.arg(0));
Review comment:
And please remove line here.
----------------------------------------------------------------
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] vvysotskyi commented on a change in pull request #2020:
DRILL-7634: Rollup of code cleanup changes
Posted by GitBox <gi...@apache.org>.
vvysotskyi commented on a change in pull request #2020: DRILL-7634: Rollup of code cleanup changes
URL: https://github.com/apache/drill/pull/2020#discussion_r390841188
##########
File path: exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillOptiq.java
##########
@@ -110,6 +110,7 @@ public static LogicalExpression toDrill(DrillParseContext context, RelDataType t
}
public static class RexToDrill extends RexVisitorImpl<LogicalExpression> {
+ @SuppressWarnings("unused")
Review comment:
Could you please remove the unused field?
----------------------------------------------------------------
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] vvysotskyi commented on a change in pull request #2020:
DRILL-7634: Rollup of code cleanup changes
Posted by GitBox <gi...@apache.org>.
vvysotskyi commented on a change in pull request #2020: DRILL-7634: Rollup of code cleanup changes
URL: https://github.com/apache/drill/pull/2020#discussion_r390828845
##########
File path: contrib/format-maprdb/src/main/java/org/apache/drill/exec/store/mapr/db/json/OjaiFunctionsProcessor.java
##########
@@ -169,8 +169,8 @@ public Void visitFunctionCall(FunctionCall call, Void v) throws RuntimeException
case "ojai_matches":
case "ojai_notmatches": {
// ojai_[not]matches(field, <regex>);
- final SchemaPath schemaPath = getSchemaPathArg(call.args.get(0));
- final String regex = getStringArg(call.args.get(1));
+ //final SchemaPath schemaPath = getSchemaPathArg(call.arg(0));
Review comment:
Could you please remove this line instead of commenting it out?
----------------------------------------------------------------
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