> On July 2, 2012, 5:15 a.m., Dmitriy Ryaboy wrote: > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/builtin/RollupDimensions.java, > > line 49 > > <https://reviews.apache.org/r/5521/diff/1/?file=115854#file115854line49> > > > > how are you planning to inherit null handling behavior from > > CubeDimensions? > > Prasanth_J wrote: > By using static function inside CubeDimensions function for converting > nulls to unknown. This seems to be an easy solution which solves the purpose > of reusing the same code for CubeDimensions and RollupDimensions. Ideally > this should be done outside the UDFs in some Util class or somewhere before > passing the tuples to UDFs.
Since this null handling behavior is required for holistic measures as well, I will move this null handling method outside of CubeDimensions. I will move this to Utils class in the next patch. > On July 2, 2012, 5:15 a.m., Dmitriy Ryaboy wrote: > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/builtin/RollupDimensions.java, > > line 71 > > <https://reviews.apache.org/r/5521/diff/1/?file=115854#file115854line71> > > > > I believe that's also in the null-handling patch. Since you need it > > both here and in CubeDimensions, it would be better to reuse that bit of > > code than to make copies of it. > > > > The comment about not allocating new tuples and copying into them > > unnecessarily applies here as well. > > Prasanth_J wrote: > Fixed. As explained above using static function. Will move it to Utils class in next patch. - Prasanth_J ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5521/#review8778 ----------------------------------------------------------- On July 11, 2012, 6:42 p.m., Prasanth_J wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/5521/ > ----------------------------------------------------------- > > (Updated July 11, 2012, 6:42 p.m.) > > > Review request for pig and Dmitriy Ryaboy. > > > Description > ------- > > This is a review board request for > https://issues.apache.org/jira/browse/PIG-2765 > > > This addresses bug PIG-2765. > https://issues.apache.org/jira/browse/PIG-2765 > > > Diffs > ----- > > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/builtin/CubeDimensions.java > 1360329 > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/builtin/RollupDimensions.java > PRE-CREATION > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/newplan/logical/relational/LOCube.java > 1360329 > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/AliasMasker.g > 1360329 > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/AstPrinter.g > 1360329 > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/AstValidator.g > 1360329 > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/LogicalPlanBuilder.java > 1360329 > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/LogicalPlanGenerator.g > 1360329 > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/QueryLexer.g > 1360329 > > http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/QueryParser.g > 1360329 > > http://svn.apache.org/repos/asf/pig/trunk/test/org/apache/pig/parser/TestLexer.pig > 1360329 > > http://svn.apache.org/repos/asf/pig/trunk/test/org/apache/pig/parser/TestLogicalPlanGenerator.java > 1360329 > > http://svn.apache.org/repos/asf/pig/trunk/test/org/apache/pig/parser/TestParser.pig > 1360329 > > http://svn.apache.org/repos/asf/pig/trunk/test/org/apache/pig/parser/TestQueryLexer.java > 1360329 > > http://svn.apache.org/repos/asf/pig/trunk/test/org/apache/pig/parser/TestQueryParser.java > 1360329 > > http://svn.apache.org/repos/asf/pig/trunk/test/org/apache/pig/test/TestCubeOperator.java > 1360329 > > http://svn.apache.org/repos/asf/pig/trunk/test/org/apache/pig/test/TestRollupDimensions.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/5521/diff/ > > > Testing > ------- > > Unit tests: All passed > > Pre-commit tests: All passed > ant clean test-commit > > > Thanks, > > Prasanth_J > >
