> On Feb. 28, 2015, 12:26 a.m., Chris Westin wrote: > > exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/DirectoryExplorers.java, > > line 71 > > <https://reviews.apache.org/r/30701/diff/3/?file=853387#file853387line71> > > > > Is there a reason to use java.io. here?
In the compiled classes we do not include the merged imports list (I do not know the reason for this). You can see references to static helper methods, for example several of the UDFs defined in StringFunctions refernce regex classes/utilties or a Drill specific toStringFromUTF8 method from Drill using the fully qualified name. > On Feb. 28, 2015, 12:26 a.m., Chris Westin wrote: > > exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/DirectoryExplorers.java, > > line 79 > > <https://reviews.apache.org/r/30701/diff/3/?file=853387#file853387line79> > > > > Why use this fully qualified name instead of just MAXDIR_NAME? Same > > below. see above. > On Feb. 28, 2015, 12:26 a.m., Chris Westin wrote: > > exec/java-exec/src/main/java/org/apache/drill/exec/store/PartitionExplorer.java, > > line 50 > > <https://reviews.apache.org/r/30701/diff/3/?file=853393#file853393line50> > > > > If this could be the set of all the files in a directory, it could be > > really large. Would it be better to define this to return an > > Iterator<String>? That doesn't necessarily mean the implementations have to > > change (the interator might just iterate over an array that's generated > > internally), but it would give implementations that would have a large > > result set more flexibility in how they work, and not necessarily require > > materializing a large result set all at once. That sounds good to me. To allow for use of the extended for loop by consumers of the interface I will have it return Iterable instead. > On Feb. 28, 2015, 12:26 a.m., Chris Westin wrote: > > exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/DirectoryExplorers.java, > > line 106 > > <https://reviews.apache.org/r/30701/diff/3/?file=853387#file853387line106> > > > > No blank line here. fixed > On Feb. 28, 2015, 12:26 a.m., Chris Westin wrote: > > exec/java-exec/src/main/java/org/apache/drill/exec/store/StoragePluginPartitionExplorer.java, > > line 50 > > <https://reviews.apache.org/r/30701/diff/3/?file=853396#file853396line50> > > > > Same comment as before re the array of Strings. Fixed - Jason ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30701/#review74629 ----------------------------------------------------------- On Feb. 28, 2015, 12:18 a.m., Jason Altekruse wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/30701/ > ----------------------------------------------------------- > > (Updated Feb. 28, 2015, 12:18 a.m.) > > > Review request for drill, Jacques Nadeau, Mehant Baid, and Parth Chandra. > > > Bugs: DRILL-2173 > https://issues.apache.org/jira/browse/DRILL-2173 > > > Repository: drill-git > > > Description > ------- > > Adds a new interface for UDFs to access partition information. Together with > 2060 which allows constant expression folding this will allow UDFs that > > > Diffs > ----- > > > exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/DrillFuncHolder.java > 279c428 > > exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionConverter.java > 0127e6e > > exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/DirectoryExplorers.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/interpreter/InterpreterEvaluator.java > 0fe36cb > exec/java-exec/src/main/java/org/apache/drill/exec/ops/FragmentContext.java > e413921 > exec/java-exec/src/main/java/org/apache/drill/exec/ops/QueryContext.java > c881432 > exec/java-exec/src/main/java/org/apache/drill/exec/ops/UdfUtilities.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/store/AbstractStoragePlugin.java > b032fce > > exec/java-exec/src/main/java/org/apache/drill/exec/store/PartitionExplorer.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/store/PartitionNotFoundException.java > PRE-CREATION > exec/java-exec/src/main/java/org/apache/drill/exec/store/StoragePlugin.java > ef5978c > > exec/java-exec/src/main/java/org/apache/drill/exec/store/StoragePluginPartitionExplorer.java > PRE-CREATION > > exec/java-exec/src/main/java/org/apache/drill/exec/store/StoragePluginRegistry.java > 5d0eed6 > > exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/FileSystemPlugin.java > 7a1f61e > > exec/java-exec/src/test/java/org/apache/drill/exec/fn/interp/ExpressionInterpreterTest.java > PRE-CREATION > > exec/java-exec/src/test/java/org/apache/drill/exec/fn/interp/TestConstantFolding.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/30701/diff/ > > > Testing > ------- > > Some unit tests on the functionality have been run, still a work in progress > so no full mvn build run yet > > > Thanks, > > Jason Altekruse > >
