Dmitry Lychagin has posted comments on this change.

Change subject: [ASTERIXDB-1972][COMP][RT][TX] index-only plan
......................................................................


Patch Set 43:

(4 comments)

https://asterix-gerrit.ics.uci.edu/#/c/1866/43/asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/declared/MetadataProvider.java
File 
asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/declared/MetadataProvider.java:

Line 1606:     private static byte[] computeByteArrayForIndexOnlyPlan(int 
resultValue) throws AlgebricksException {
This is a generic method that serializes integer value into byte array. Can we 
move it to SerializerDeserializerUtil? It's also unlikely to fail, so there's 
no need for a special error code for it 
(CANNOT_GENERATE_RESULT_VALUE_FOR_INSTANT_TRYLOCK_FOR_INDEX_ONLY_PLAN), we 
could throw some internal error instead.


https://asterix-gerrit.ics.uci.edu/#/c/1866/43/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/types/hierachy/DoubleToFloatTypeConvertComputer.java
File 
asterixdb/asterix-om/src/main/java/org/apache/asterix/om/types/hierachy/DoubleToFloatTypeConvertComputer.java:

Line 59:     public IAObject convertType(IAObject sourceObject, 
TypeCastingMathFunctionType mathFunction)
mathFunction is not used in the method. Seems like we should either only expect 
NONE and therefore throw an exception if it's something else, or accept any 
math function and apply it here.


https://asterix-gerrit.ics.uci.edu/#/c/1866/43/asterixdb/asterix-om/src/main/java/org/apache/asterix/om/types/hierachy/FloatToDoubleTypeConvertComputer.java
File 
asterixdb/asterix-om/src/main/java/org/apache/asterix/om/types/hierachy/FloatToDoubleTypeConvertComputer.java:

Line 52:     public IAObject convertType(IAObject sourceObject, 
TypeCastingMathFunctionType mathFunction)
mathFunction is not used in the method. Seems like we should either only expect 
NONE and therefore throw an exception if it's something else, or accept any 
math function and apply it here.


https://asterix-gerrit.ics.uci.edu/#/c/1866/43/hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/logical/AbstractUnnestMapOperator.java
File 
hyracks-fullstack/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/logical/AbstractUnnestMapOperator.java:

Line 142:     public void setGenerateCallBackProceedResultVar(boolean 
generateCallBackProceedResultVar) {
Can we add a comment that this variable must be defined as the 3rd variable 
inside 'variables' field?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifd5c9ab1cf2e4bedb7d8db582441919875e74d51
Gerrit-PatchSet: 43
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Taewoo Kim <[email protected]>
Gerrit-Reviewer: Anon. E. Moose #1000171
Gerrit-Reviewer: Dmitry Lychagin <[email protected]>
Gerrit-Reviewer: Ildar Absalyamov <[email protected]>
Gerrit-Reviewer: Jenkins <[email protected]>
Gerrit-Reviewer: Taewoo Kim <[email protected]>
Gerrit-Reviewer: Till Westmann <[email protected]>
Gerrit-Reviewer: Yingyi Bu <[email protected]>
Gerrit-HasComments: Yes

Reply via email to