zabetak commented on a change in pull request #2551:
URL: https://github.com/apache/hive/pull/2551#discussion_r685832922
##########
File path: ql/src/java/org/apache/hadoop/hive/ql/exec/HiveStructComparator.java
##########
@@ -45,16 +48,14 @@ public int compare(Object key1, Object key2) {
if (a1.size() == 0) {
return 0;
}
- if (comparator == null) {
- comparator = new WritableComparator[a1.size()];
- // For struct all elements may not be of same type, so create
comparator for each entry.
- for (int i = 0; i < a1.size(); i++) {
- comparator[i] = WritableComparatorFactory.get(a1.get(i),
nullSafe, nullOrdering);
- }
+ // For array, the length may not be fixed, so extend comparators on
demand
+ for (int i = comparators.size(); i < a1.size(); i++) {
+ // For struct, all elements may not be of same type, so create
comparator for each entry.
+ comparators.add(i, WritableComparatorFactory.get(a1.get(i),
nullSafe, nullOrdering));
}
result = 0;
for (int i = 0; i < a1.size(); i++) {
- result = comparator[i].compare(a1.get(i), a2.get(i));
+ result = comparators.get(i).compare(a1.get(i), a2.get(i));
Review comment:
For `ARRAY` comparisons we could even initialize and use only one
comparator instead of creating the same one multiple times. If the size of the
ARRAYs is small as in the example it will not make any real difference but for
bigger ARRAYs we may feel both in terms of memory and CPU. Since this might
need a bit of refactoring we can tackle it in a follow up JIRA, what do you
think @okumin ?
--
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]
