asterixdb-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Michael Blow (Code Review)" <>
Subject Change in asterixdb[master]: ASTERIXDB-1479: Change storage valid int and add explicit ve...
Date Tue, 14 Jun 2016 15:07:04 GMT
Michael Blow has posted comments on this change.

Change subject: ASTERIXDB-1479: Change storage valid int and add explicit version

Patch Set 8:

File hyracks-fullstack/hyracks/hyracks-storage-am-btree/src/main/java/org/apache/hyracks/storage/am/btree/frames/

Line 464:         strBuilder.append("TUPLE_COUNT_OFF:             " + tupleCountOff + "\n");
Should we revert this?  Not sure why we want to change the name of the displayed keys only.
 If we're keeping, we probably should fix the spacing so that the values align with the others.
File hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/frames/

Line 284:         strBuilder.append("TUPLE_COUNT_OFF:     " + tupleCountOff + "\n");
same comment as before
File hyracks-fullstack/hyracks/hyracks-storage-am-common/src/main/java/org/apache/hyracks/storage/am/common/freepage/

Line 475:             return (long)(metadataPageNum * bufferCache.getPageSize()) + LIFOMetaDataFrame.LSN_OFF;
Should one of the operands to the multiply be casted to a long rather than the product?  I
think otherwise we risk overflow.
File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-common/src/main/java/org/apache/hyracks/storage/am/lsm/common/impls/

Line 53:     public enum TREE_INDEX_STATE {
enums should be named like classes (e.g. TreeIndexState)- not sure why this isn't caught by

Line 55:         WRONG_VERS,
Not loving this abbreviation- can we have WRONG_VERSION or perhaps VERSION_MISMATCH?

Line 105:                     return TREE_INDEX_STATE.WRONG_VERS;
We may have to revisit this in the future- we're going to need to know which specific version
the index file has in order to provide the detailed error message Till was proposing, I think.

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: I888ff0eacf5b3cb6ad7ec002c74f113c6ffcd496
Gerrit-PatchSet: 8
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Ian Maxon <>
Gerrit-Reviewer: Jenkins <>
Gerrit-Reviewer: Michael Blow <>
Gerrit-Reviewer: Till Westmann <>
Gerrit-Reviewer: abdullah alamoudi <>
Gerrit-HasComments: Yes

View raw message