----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34749/#review85646 -----------------------------------------------------------
Quick, initial comments. exec/java-exec/src/test/java/org/apache/drill/exec/vector/accessor/GenericAccessorTest.java <https://reviews.apache.org/r/34749/#comment137319> We should mark this static final. exec/java-exec/src/test/java/org/apache/drill/exec/vector/accessor/GenericAccessorTest.java <https://reviews.apache.org/r/34749/#comment137324> The accessor should throw IOOB rather than IAE. This complies with the contract as well (assuming bounds checking is enabled). exec/java-exec/src/test/java/org/apache/drill/exec/vector/accessor/GenericAccessorTest.java <https://reviews.apache.org/r/34749/#comment137320> Please be consistent with your styling. We should either use whitespaces everywhere inside paranthesis or never use them. - Hanifi Gunes On May 28, 2015, 11:12 p.m., Matt Burgess wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/34749/ > ----------------------------------------------------------- > > (Updated May 28, 2015, 11:12 p.m.) > > > Review request for drill. > > > Repository: drill-git > > > Description > ------- > > When calling isNull() from a result set, if the method gets > delegated/propagated to a GenericAccessor, it throws an > UnsupportedOperationException. Since GenericAccessor uses its underlying > ValueVector for most other methods, I think it should also delegate the > isNull() method and return whatever comes back from the ValueVector.isNull() > call. > > > Diffs > ----- > > > exec/java-exec/src/main/java/org/apache/drill/exec/vector/accessor/GenericAccessor.java > 347cf26 > > exec/java-exec/src/test/java/org/apache/drill/exec/vector/accessor/GenericAccessorTest.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/34749/diff/ > > > Testing > ------- > > Patch provides implementation of the isNull() method, along with unit tests > covering 100% of the GenericAccessor class > > > Thanks, > > Matt Burgess > >
