asterixdb-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "abdullah alamoudi (Code Review)" <do-not-re...@asterixdb.incubator.apache.org>
Subject Change in hyracks[master]: Add Support for Upsert Operation
Date Sun, 17 Jan 2016 12:22:27 GMT
abdullah alamoudi has posted comments on this change.

Change subject: Add Support for Upsert Operation
......................................................................


Patch Set 6:

(9 comments)

https://asterix-gerrit.ics.uci.edu/#/c/476/6/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/logical/ReplicateOperator.java
File algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/logical/ReplicateOperator.java:

Line 76:         return true;
> What's the meaning of Map in isMap()?
It means that the input variables == the output variables.


https://asterix-gerrit.ics.uci.edu/#/c/476/6/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/logical/SinkOperator.java
File algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/logical/SinkOperator.java:

Line 93:     }
> Why does sink operator have any variable, now?
I don't know how or why this change was made. I reverted this one.


https://asterix-gerrit.ics.uci.edu/#/c/476/6/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/logical/visitors/VariableUtilities.java
File algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/logical/visitors/VariableUtilities.java:

Line 47:      */
> Can we explain here the definition of the following terms?
Done


https://asterix-gerrit.ics.uci.edu/#/c/476/6/algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/physical/InsertDeleteUpsertPOperator.java
File algebricks/algebricks-core/src/main/java/org/apache/hyracks/algebricks/core/algebra/operators/physical/InsertDeleteUpsertPOperator.java:

Line 116:         }
> better throw exception by adding else { } here.
Done


https://asterix-gerrit.ics.uci.edu/#/c/476/6/hyracks/hyracks-storage-am-btree/src/main/java/org/apache/hyracks/storage/am/btree/impls/RangePredicate.java
File hyracks/hyracks-storage-am-btree/src/main/java/org/apache/hyracks/storage/am/btree/impls/RangePredicate.java:

Line 141:     }
> Can we just name as getLowKey()?  I know this key is only used for prior to
renamed


https://asterix-gerrit.ics.uci.edu/#/c/476/6/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/api/ISearchPredicate.java
File hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/api/ISearchPredicate.java:

Line 37:     public ITupleReference getSearchKey();
> again, can we rename getLowKey()?
Done


https://asterix-gerrit.ics.uci.edu/#/c/476/6/hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/impls/LSMBTreeOpContext.java
File hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/impls/LSMBTreeOpContext.java:

Line 219:     @Override
> Where are the following two methods used?
removed them. I was using them somewhere but with refactoring, I didn't need them anymore.


https://asterix-gerrit.ics.uci.edu/#/c/476/6/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/api/ILSMIndexOperationContext.java
File hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/api/ILSMIndexOperationContext.java:

Line 44: 
> Where are the following two methods used?
Removed. Good catch :)


https://asterix-gerrit.ics.uci.edu/#/c/476/6/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/search/InvertedIndexSearchPredicate.java
File hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/search/InvertedIndexSearchPredicate.java:

Line 86:         return queryTuple;
> Probably, better be throwing exception here since this method is only legal
If this throws an exception, then the search of the inverted index will always gets an exception
since this method gets called with the beforeOp. Instead, I made it return a null.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2705f43b6e6d187ee29b9ba5a7946d422990022a
Gerrit-PatchSet: 6
Gerrit-Project: hyracks
Gerrit-Branch: master
Gerrit-Owner: abdullah alamoudi <bamousaa@gmail.com>
Gerrit-Reviewer: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Taewoo Kim <wangsaeu@gmail.com>
Gerrit-Reviewer: Till Westmann <tillw@apache.org>
Gerrit-Reviewer: Young-Seok Kim <kisskys@gmail.com>
Gerrit-Reviewer: abdullah alamoudi <bamousaa@gmail.com>
Gerrit-HasComments: Yes

Mime
View raw message