Ali Alsuliman has posted comments on this change.

Change subject: [ASTERIXDB-2523][RT][COMP] add support for hashing array fields
......................................................................


Patch Set 2:

(4 comments)

https://asterix-gerrit.ics.uci.edu/#/c/3241/2/asterixdb/asterix-om/src/main/java/org/apache/asterix/dataflow/data/nontagged/hash/AMurmurHash3BinaryHashFunctionFamily.java
File 
asterixdb/asterix-om/src/main/java/org/apache/asterix/dataflow/data/nontagged/hash/AMurmurHash3BinaryHashFunctionFamily.java:

Line 54:         return new GenericHashFunction(type, seed);
> future thought. we could still have a singleton and return it here if type 
True.
I will need to get rid of the internal buffer used by the function for type 
promotion. My plan is to do it also for comparators.


Line 105:                         throw 
HyracksDataException.create(ErrorCode.NUMERIC_PROMOTION_ERROR, e.getMessage());
> minor. it'd be nice to have source location available here, so we could poi
Yes.
I will do it in a follow-up change since I want to do singleton for primitive 
types, and I need to figure out how to do it nicely if we want to pass source 
location and still keep it singleton for primitives.


https://asterix-gerrit.ics.uci.edu/#/c/3241/2/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/physical/InMemoryHashJoinPOperator.java
File 
hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/physical/InMemoryHashJoinPOperator.java:

Line 94:                 
JobGenHelper.variablesToBinaryHashFunctionFactories(keysLeftBranch, env, 
context);
> should be keysRightBranch
Good catch!


https://asterix-gerrit.ics.uci.edu/#/c/3241/2/hyracks-fullstack/hyracks/hyracks-api/src/main/resources/errormsg/en.properties
File 
hyracks-fullstack/hyracks/hyracks-api/src/main/resources/errormsg/en.properties:

Line 140: 121 = A numeric type promotion error has occurred before hashing a 
field: %1$s
> 1. I'd change this error message to be more generic, so avoid talking about
1. Done.
2. %1$s - it's the exact error message coming from the type promoter. I believe 
it's IO error.


-- 
To view, visit https://asterix-gerrit.ics.uci.edu/3241
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd0dc7f270730140226f54445705822049f5c863
Gerrit-PatchSet: 2
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Ali Alsuliman <ali.al.solai...@gmail.com>
Gerrit-Reviewer: Ali Alsuliman <ali.al.solai...@gmail.com>
Gerrit-Reviewer: Anon. E. Moose #1000171
Gerrit-Reviewer: Dmitry Lychagin <dmitry.lycha...@couchbase.com>
Gerrit-Reviewer: Jenkins <jenk...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Till Westmann <ti...@apache.org>
Gerrit-HasComments: Yes

Reply via email to