> On April 6, 2015, 3:51 a.m., Aman Sinha wrote: > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillValuesRel.java, > > line 60 > > <https://reviews.apache.org/r/32867/diff/2/?file=915514#file915514line60> > > > > The use of JSONOptions has some advantages in terms of representing > > heterogeneous values and in terms of serialization/deserialization; however > > it has some disadvantages - e.g there is no duplicate detection, so for > > large lists (e.g millions of values) containing duplicates, it will be less > > memory efficient compared to say a hash table. > > Jacques Nadeau wrote: > Since we're already holding the list on heap before this stage, I don't > think optimize this will help us change scaling properties.
Agree that there's already a copy of the list on heap as output of the parser. It would be good to keep only 1 copy and the other comment below addresses that. Discussed with Jacques and couple of additional notes: - The values list will be used not just for IN but also for the INSERT values. Since Drill does not have the context at the time the DrillValuesRel is created, it cannot determine whether or not to keep duplicates. Hence, at this time it will continue to keep duplicates. - If we had a notion of in-memory temporary table that is replicated, it would have worked better since for millions of items, the json overhead could become substantial. At this time this is not a major concern. - Aman ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32867/#review78898 ----------------------------------------------------------- On April 7, 2015, 7:25 p.m., Jacques Nadeau wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/32867/ > ----------------------------------------------------------- > > (Updated April 7, 2015, 7:25 p.m.) > > > Review request for drill, Aman Sinha and Hanifi Gunes. > > > Repository: drill-git > > > Description > ------- > > Add Support for large in conditions through the use of the Values operator. > Update JSON reader to support reading Extended JSON. > Update JSON writer to support writing extended JSON data. > Update JSON reader to automatically unwrap a file that includes a single > top-level array. > Update Options manager to use getOption(<Type>Validator) to directly retrieve > typed value. > Remove JSON rewinding > > > Diffs > ----- > > common/src/main/java/org/apache/drill/common/JSONOptions.java 945cd92 > common/src/main/java/org/apache/drill/common/logical/data/Constant.java > 460803d > common/src/main/java/org/apache/drill/common/logical/data/Values.java > PRE-CREATION > > common/src/main/java/org/apache/drill/common/logical/data/visitors/AbstractLogicalVisitor.java > 92e370f > > common/src/main/java/org/apache/drill/common/logical/data/visitors/LogicalVisitor.java > 3a426bf > > contrib/storage-mongo/src/main/java/org/apache/drill/exec/store/mongo/MongoRecordReader.java > 15ef197 > exec/java-exec/src/main/codegen/templates/JsonOutputRecordWriter.java > 1d0dc9d > exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java > bd93206 > > exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/DateUtility.java > a031bee > > exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/conv/JsonConvertFrom.java > c828cf4 > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/base/AbstractPhysicalVisitor.java > 27b0ecb > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/base/PhysicalVisitor.java > e6a89d0 > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/config/Values.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/values/ValuesBatchCreator.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillOptiq.java > 796f0f7 > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillRuleSets.java > b1a7189 > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillValuesRel.java > a3551e7 > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/ScanFieldDeterminer.java > 59c65f9 > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/ValuesPrel.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/ValuesPrule.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/server/options/BaseOptionManager.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/server/options/FallbackOptionManager.java > 45d393c > > exec/java-exec/src/main/java/org/apache/drill/exec/server/options/OptionManager.java > 4ffe9a3 > > exec/java-exec/src/main/java/org/apache/drill/exec/server/options/SystemOptionManager.java > 608fac7 > > exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/JSONFormatPlugin.java > 6cf1ce5 > > exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/JSONRecordReader.java > cc7cb83 > > exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/JsonProcessor.java > ce6017b > > exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/JsonRecordWriter.java > 76c4ace > > exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/RewindableUtf8Reader.java > b9075de > > exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/reader/BaseJsonProcessor.java > 509798a > > exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/reader/CountingJsonReader.java > 1ef71e7 > > exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/fn/BasicJsonOutput.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/fn/DateOutputFormat.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/fn/ExtendedJsonOutput.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/fn/ExtendedType.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/fn/ExtendedTypeName.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/fn/JsonOutput.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/fn/JsonReader.java > 9738ff8 > > exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/fn/JsonWriter.java > de52b73 > > exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/fn/VectorOutput.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/fn/WorkingBuffer.java > PRE-CREATION > > exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/filter/TestLargeInClause.java > PRE-CREATION > > exec/java-exec/src/test/java/org/apache/drill/exec/store/json/TestJsonRecordReader.java > c4bfcce > > exec/java-exec/src/test/java/org/apache/drill/exec/vector/complex/writer/TestExtendedTypes.java > PRE-CREATION > exec/java-exec/src/test/resources/store/json/listdoc.json PRE-CREATION > exec/java-exec/src/test/resources/vector/complex/extended.json PRE-CREATION > > Diff: https://reviews.apache.org/r/32867/diff/ > > > Testing > ------- > > Unit, Regression, SF100 > > > Thanks, > > Jacques Nadeau > >
