Jianfeng Jia 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()? the name is coming from the class name. I copied the comments of this class to the corresponding Utils functions. /* * This class provides the raw bytes-based comparison and hash function for UTF8 strings. * Note that the comparison may not deliver the correct ordering for certain languages that include 2 or 3 bytes characters. * But it works for single-byte character languages. */ 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? Done 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? Done 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? Done 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? Done 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? Done Line 42: appendUtf8Pointable(src, src.getCharStartOffset(), src.getUTFLength()); > I think getUTF8Length() is more clear. Done 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? Done 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? Done 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 r Done 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? Done -- 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
