Ben-Zvi commented on a change in pull request #1598: DRILL-6880: For Hash-Join
hash-table build - treat null keys as an equal match
URL: https://github.com/apache/drill/pull/1598#discussion_r246241524
##########
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/HashTableTemplate.java
##########
@@ -661,16 +669,15 @@ public PutStatus put(int incomingRowIdx, IndexPointer
htIdxHolder, int hashCode,
// if startIdx is non-empty, follow the hash chain links until we find a
matching
// key or reach the end of the chain (and remember the last link there)
- for ( currentIdxHolder.value = startIdx;
- currentIdxHolder.value != EMPTY_SLOT;
- /* isKeyMatch() below also advances the currentIdxHolder to the next
link */) {
-
+ for ( int currentIndex = startIdx;
+ currentIndex != EMPTY_SLOT;
+ currentIndex = lastEntryBatch.nextLinkInHashChain(currentIndex)) {
Review comment:
Given that this method is so simple and short, and being called a lot - it
is the ultimate candidate for JVM inlining.
I initially tried to build an iterator for this use, but it gets very
complicated as the next links jump from one BatchHolder to another; still the
for() loop tries to show a classic traversal of a linked chain; moving the
"next" method inside the body spoils this "iterator like" logic.
The comment should have also said "never call this when currentIndex ==
EMPTY_SLOT", which is trivial when used in the for().
I reworded that comment to reflect the "iterator like" use.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
