asdf2014 commented on code in PR #15622:
URL: https://github.com/apache/druid/pull/15622#discussion_r1461323549
##########
processing/src/main/java/org/apache/druid/query/expression/ContainsExprMacro.java:
##########
Review Comment:
Same as above, for example:
```java
* @see CaseInsensitiveContainsExprMacro for the case-insensitive version.
```
##########
processing/src/main/java/org/apache/druid/query/expression/CaseInsensitiveContainsExprMacro.java:
##########
Review Comment:
It would be better to use `@see` instead of `@link` here, like this way:
```java
* @see ContainsExprMacro for the case-sensitive version.
```
##########
processing/src/main/java/org/apache/druid/query/expression/NestedDataExpressions.java:
##########
@@ -818,14 +702,6 @@ public ExprEval eval(ObjectBinding bindings)
);
}
-
- @Override
- public Expr visit(Shuttle shuttle)
- {
- List<Expr> newArgs = args.stream().map(x ->
x.visit(shuttle)).collect(Collectors.toList());
- return shuttle.visit(new JsonKeysExpr(newArgs));
- }
-
@Nullable
Review Comment:
```suggestion
@NotNull
```
##########
processing/src/main/java/org/apache/druid/query/expression/LikeExprMacro.java:
##########
@@ -88,35 +87,14 @@ public ExprEval eval(final ObjectBinding bindings)
return ExprEval.ofLongBoolean(match.matches(false));
}
- @Override
- public Expr visit(Shuttle shuttle)
- {
- return shuttle.visit(apply(shuttle.visitAll(args)));
- }
-
@Nullable
@Override
public ExpressionType getOutputType(InputBindingInspector inspector)
{
return ExpressionType.LONG;
}
-
- @Override
- public String stringify()
- {
- if (escapeExpr != null) {
- return StringUtils.format(
- "%s(%s, %s, %s)",
- FN_NAME,
- arg.stringify(),
- patternExpr.stringify(),
- escapeExpr.stringify()
- );
- }
- return StringUtils.format("%s(%s, %s)", FN_NAME, arg.stringify(),
patternExpr.stringify());
- }
}
- return new LikeExtractExpr(arg);
+ return new LikeExtractExpr(args);
}
}
Review Comment:
```suggestion
```
##########
processing/src/main/java/org/apache/druid/query/expression/NestedDataExpressions.java:
##########
@@ -818,14 +702,6 @@ public ExprEval eval(ObjectBinding bindings)
);
}
-
- @Override
- public Expr visit(Shuttle shuttle)
- {
- List<Expr> newArgs = args.stream().map(x ->
x.visit(shuttle)).collect(Collectors.toList());
- return shuttle.visit(new JsonKeysExpr(newArgs));
- }
-
@Nullable
Review Comment:
It seems that the `getOutputType` method always returns
`ExpressionType.STRING`, so we should use `@NotNull` instead of `@Nullable` here
##########
processing/src/main/java/org/apache/druid/query/expression/LikeExprMacro.java:
##########
@@ -88,35 +87,14 @@ public ExprEval eval(final ObjectBinding bindings)
return ExprEval.ofLongBoolean(match.matches(false));
}
- @Override
- public Expr visit(Shuttle shuttle)
- {
- return shuttle.visit(apply(shuttle.visitAll(args)));
- }
-
@Nullable
@Override
public ExpressionType getOutputType(InputBindingInspector inspector)
{
return ExpressionType.LONG;
}
-
- @Override
- public String stringify()
- {
- if (escapeExpr != null) {
- return StringUtils.format(
- "%s(%s, %s, %s)",
- FN_NAME,
- arg.stringify(),
- patternExpr.stringify(),
- escapeExpr.stringify()
- );
- }
- return StringUtils.format("%s(%s, %s)", FN_NAME, arg.stringify(),
patternExpr.stringify());
- }
}
- return new LikeExtractExpr(arg);
+ return new LikeExtractExpr(args);
}
}
Review Comment:
nit: there are two blank lines
##########
integration-tests-ex/tools/src/main/java/org/apache/druid/testing/tools/SleepExprMacro.java:
##########
@@ -101,6 +95,6 @@ public ExpressionType getOutputType(InputBindingInspector
inspector)
return null;
Review Comment:
We can mark this method with `@Nullable` annotation
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
