Taewoo Kim has posted comments on this change. Change subject: ASTERIXDB-1102: VarSize Encoding to store length of String and ByteArray ......................................................................
Patch Set 10: (11 comments) https://asterix-gerrit.ics.uci.edu/#/c/449/10/hyracks/hyracks-data/hyracks-data-std/src/main/java/org/apache/hyracks/data/std/primitive/RawUTF8StringPointable.java File hyracks/hyracks-data/hyracks-data-std/src/main/java/org/apache/hyracks/data/std/primitive/RawUTF8StringPointable.java: Line 75: return UTF8StringUtil.rawBytehash(this.bytes, this.start); name: rawByteHash()? https://asterix-gerrit.ics.uci.edu/#/c/449/10/hyracks/hyracks-data/hyracks-data-std/src/main/java/org/apache/hyracks/data/std/util/AbstractVarLenObjectBuilder.java File hyracks/hyracks-data/hyracks-data-std/src/main/java/org/apache/hyracks/data/std/util/AbstractVarLenObjectBuilder.java: Line 45: public void finish() throws IOException { Can you put more comments on this method()? When is this function called? https://asterix-gerrit.ics.uci.edu/#/c/449/10/hyracks/hyracks-data/hyracks-data-std/src/main/java/org/apache/hyracks/data/std/util/ByteArrayAccessibleOutputStream.java File hyracks/hyracks-data/hyracks-data-std/src/main/java/org/apache/hyracks/data/std/util/ByteArrayAccessibleOutputStream.java: Line 50: public void rewindPositionBy(int delta) { Still, it is not clear to me why this is required. Can you elaborate? https://asterix-gerrit.ics.uci.edu/#/c/449/10/hyracks/hyracks-data/hyracks-data-std/src/main/java/org/apache/hyracks/data/std/util/GrowableArray.java File hyracks/hyracks-data/hyracks-data-std/src/main/java/org/apache/hyracks/data/std/util/GrowableArray.java: Line 49: public void rewindPositionBy(int delta) { Same comments here: why is it required? https://asterix-gerrit.ics.uci.edu/#/c/449/10/hyracks/hyracks-data/hyracks-data-std/src/main/java/org/apache/hyracks/data/std/util/RewindableDataOutputStream.java File hyracks/hyracks-data/hyracks-data-std/src/main/java/org/apache/hyracks/data/std/util/RewindableDataOutputStream.java: Line 26: public class RewindableDataOutputStream extends DataOutputStream { Same comments here: a usage of rewindable? https://asterix-gerrit.ics.uci.edu/#/c/449/10/hyracks/hyracks-data/hyracks-data-std/src/main/java/org/apache/hyracks/data/std/util/UTF8StringBuilder.java File hyracks/hyracks-data/hyracks-data-std/src/main/java/org/apache/hyracks/data/std/util/UTF8StringBuilder.java: Line 37: public void appendUtf8Pointable(UTF8StringPointable src, int byteStartOffset, int byteLength) throws IOException { name: appendUTF8Pointable? Line 42: appendUtf8Pointable(src, src.getCharStartOffset(), src.getUTFLength()); I think getUTF8Length() is more clear. https://asterix-gerrit.ics.uci.edu/#/c/449/10/hyracks/hyracks-storage-am-lsm-invertedindex/pom.xml File hyracks/hyracks-storage-am-lsm-invertedindex/pom.xml: Line 60: <groupId>org.apache.hyracks</groupId> Can we remove tabs here? https://asterix-gerrit.ics.uci.edu/#/c/449/10/hyracks/hyracks-tests/hyracks-storage-am-lsm-invertedindex-test/pom.xml File hyracks/hyracks-tests/hyracks-storage-am-lsm-invertedindex-test/pom.xml: Line 66: <dependency> Can we remove tabs here? https://asterix-gerrit.ics.uci.edu/#/c/449/10/hyracks/hyracks-util/src/main/java/org/apache/hyracks/util/encoding/VarLenIntEncoderDecoder.java File hyracks/hyracks-util/src/main/java/org/apache/hyracks/util/encoding/VarLenIntEncoderDecoder.java: Line 28: public class VarLenIntEncoderDecoder { Can you describe the logic here? Put comments and examples for the future references? https://asterix-gerrit.ics.uci.edu/#/c/449/10/pom.xml File pom.xml: Line 176: <groupId>org.apache.maven.plugins</groupId> Can we remove tabs here? -- To view, visit https://asterix-gerrit.ics.uci.edu/449 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7e95df0f06984b784ebac2c84b97e56a50207d27 Gerrit-PatchSet: 10 Gerrit-Project: hyracks Gerrit-Branch: master Gerrit-Owner: Jianfeng Jia <[email protected]> Gerrit-Reviewer: Ian Maxon <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Jianfeng Jia <[email protected]> Gerrit-Reviewer: Preston Carman <[email protected]> Gerrit-Reviewer: Taewoo Kim <[email protected]> Gerrit-Reviewer: Till Westmann <[email protected]> Gerrit-HasComments: Yes
