Preston Carman has posted comments on this change. Change subject: ASTERIXDB-1102: VarSize Encoding to store length of String and ByteArray ......................................................................
Patch Set 4: (10 comments) My big question is about why some of the method and classes were changed from static access? Also a few classes now are instantiated instead of using an INSTANCE created in the class. Could why you changed these items in your code? (I would have thought they should stay static and keep object creation down.) https://asterix-gerrit.ics.uci.edu/#/c/450/4/asterix-app/src/main/java/org/apache/asterix/file/ExternalIndexingOperations.java File asterix-app/src/main/java/org/apache/asterix/file/ExternalIndexingOperations.java: Line 238: FilesIndexDescription filesIndexDescription = new FilesIndexDescription(); Why change from a static reference? https://asterix-gerrit.ics.uci.edu/#/c/450/4/asterix-app/src/test/resources/runtimets/only.xml File asterix-app/src/test/resources/runtimets/only.xml: Line 1: <?xml version="1.0" encoding="UTF-8" standalone="yes"?> Keep the license header. https://asterix-gerrit.ics.uci.edu/#/c/450/4/asterix-app/src/test/resources/runtimets/queries/boolean/and_01/and_01.3.query.aql File asterix-app/src/test/resources/runtimets/queries/boolean/and_01/and_01.3.query.aql: Line 21: let $x := boolean("true") Are these queries testing the same thing? By specifying boolean(), it seems like a different query to me. https://asterix-gerrit.ics.uci.edu/#/c/450/4/asterix-app/src/test/resources/runtimets/queries/string/matches11/matches11.3.query.aql File asterix-app/src/test/resources/runtimets/queries/string/matches11/matches11.3.query.aql: Line 28: matches("hello",null), Does this test the null issue that was raised on the mailing list? Is this change a work around or is this correct? https://asterix-gerrit.ics.uci.edu/#/c/450/4/asterix-app/src/test/resources/runtimets/queries/string/startwith02/startwith02.3.query.aql File asterix-app/src/test/resources/runtimets/queries/string/startwith02/startwith02.3.query.aql: Line 27: null, Same question about null. https://asterix-gerrit.ics.uci.edu/#/c/450/4/asterix-external-data/src/main/java/org/apache/asterix/external/indexing/dataflow/FileIndexTupleTranslator.java File asterix-external-data/src/main/java/org/apache/asterix/external/indexing/dataflow/FileIndexTupleTranslator.java: Line 44: private final FilesIndexDescription filesIndexDescription = new FilesIndexDescription(); why change from static? https://asterix-gerrit.ics.uci.edu/#/c/450/4/asterix-external-data/src/main/java/org/apache/asterix/external/library/java/JObjectAccessors.java File asterix-external-data/src/main/java/org/apache/asterix/external/library/java/JObjectAccessors.java: Line 241: //v = new String(b, s+1, l, "UTF-8"); Can this comment be removed? https://asterix-gerrit.ics.uci.edu/#/c/450/4/asterix-external-data/src/main/java/org/apache/asterix/external/library/java/JObjectUtil.java File asterix-external-data/src/main/java/org/apache/asterix/external/library/java/JObjectUtil.java: Line 395: fieldNames[i] = new AStringSerializerDeserializer().deserialize(dis).getStringValue(); Why change? https://asterix-gerrit.ics.uci.edu/#/c/450/4/asterix-om/src/main/java/org/apache/asterix/dataflow/data/nontagged/serde/AStringSerializerDeserializer.java File asterix-om/src/main/java/org/apache/asterix/dataflow/data/nontagged/serde/AStringSerializerDeserializer.java: Line 34: private final UTF8StringSerializerDeserializer utf8SerDer = new UTF8StringSerializerDeserializer(); Can this be kept static? Seems like that would be a good idea. https://asterix-gerrit.ics.uci.edu/#/c/450/4/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/records/RecordFieldsUtil.java File asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/functions/records/RecordFieldsUtil.java: Line 80: @SuppressWarnings("unchecked") Why not? -- To view, visit https://asterix-gerrit.ics.uci.edu/450 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: comment Gerrit-Change-Id: I41fff780f5c071742ef10129d83c8f945d5886d7 Gerrit-PatchSet: 4 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: Jianfeng Jia <[email protected]> Gerrit-Reviewer: Ian Maxon <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Preston Carman <[email protected]> Gerrit-Reviewer: Taewoo Kim <[email protected]> Gerrit-Reviewer: Till Westmann <[email protected]> Gerrit-HasComments: Yes
