Fix string comparisons. #8253
Fix string comparisons. #8253
Conversation
Codecov Report
@@ Coverage Diff @@
## master #8253 +/- ##
=========================================
Coverage 70.69% 70.69%
- Complexity 4238 4242 +4
=========================================
Files 1629 1631 +2
Lines 85215 85290 +75
Branches 12830 12850 +20
=========================================
+ Hits 60243 60298 +55
- Misses 20814 20832 +18
- Partials 4158 4160 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
| private static Set<String> getStringOutputFunctionList() { | ||
| Set<String> set = new HashSet<>(); | ||
| Method[] methods = StringFunctions.class.getDeclaredMethods(); | ||
| for (Method method : methods) { | ||
| if (method.getReturnType() == String.class) { | ||
| set.add(method.getName().toUpperCase()); | ||
| } | ||
| } | ||
| return set; |
There was a problem hiding this comment.
@ScalarFunction now has a names array parameter - so you need to read the names from the annotation to do this correctly.
There was a problem hiding this comment.
Wouldn't using @ScalarFunction return names of all scalar functions and how would the output type of the function be resolved? We just want String scalar functions and that too only the once that return a string as output. All the string scalar functions are declared in StringFunctions and hence this code.
There was a problem hiding this comment.
Sorry, allow me to clarify:
Set<String> set = new HashSet<>();
Method[] methods = StringFunctions.class.getDeclaredMethods();
for (Method method : methods) {
if (method.getReturnType() == String.class) {
if (method.isAnnotationPresent(ScalarFunction.class)) {
ScalarFunction annotation = method.getAnnotation(ScalarFunction.class);
for (String name : annotation.names()) {
set.add(name.toUpperCase());
}
}
set.add(method.getName().toUpperCase());
}
}
return set;This way your logic works even if there is an alias in use. Does that make sense?
There was a problem hiding this comment.
Hmm, I see. Thanks for clarifying.
Done |
Description
A query such as
SELECT count(*) FROM mytable WHERE trim(OriginState) = DestStatecurrently throws exception, because internally the predicate gets rewritten toMINUS(trim(OriginState), DestState) = 0and MINUS function fails to evaluate STRING arguments. This PR modifiesStringPredicateFilterOptizer.javato properly rewrite predicate to use STRCMP instead of MINUS (STRCMP(trim(OriginState), DestState) = 0).Note that the same issue was fixed for string columns in #7394. This PR is extending the original fix to include string functions as well.
Upgrade Notes
Does this PR prevent a zero down-time upgrade? (Assume upgrade order: Controller, Broker, Server, Minion)
backward-incompat, and complete the section below on Release Notes)Does this PR fix a zero-downtime upgrade introduced earlier?
backward-incompat, and complete the section below on Release Notes)Does this PR otherwise need attention when creating release notes? Things to consider:
release-notesand complete the section on Release Notes)Release Notes
Documentation