> On March 22, 2017, 12:49 a.m., Vihang Karajgaonkar wrote: > > itests/util/src/main/java/org/apache/hive/beeline/QFileOutputFormat.java > > Lines 62 (patched) > > <https://reviews.apache.org/r/57693/diff/1/?file=1666196#file1666196line62> > > > > Can you please add some documentation for this method so that it is > > easier to understand. It would be great if you could add a sample String[] > > vals and output String from this method.
With the rework this is removed > On March 22, 2017, 12:49 a.m., Vihang Karajgaonkar wrote: > > itests/util/src/main/java/org/apache/hive/beeline/QFileOutputFormat.java > > Lines 65 (patched) > > <https://reviews.apache.org/r/57693/diff/1/?file=1666196#file1666196line65> > > > > Do we need a null check here? With the rework this is removed > On March 22, 2017, 12:49 a.m., Vihang Karajgaonkar wrote: > > itests/util/src/main/java/org/apache/hive/beeline/QFilePostExecutePrinter.java > > Lines 27 (patched) > > <https://reviews.apache.org/r/57693/diff/1/?file=1666197#file1666197line27> > > > > Do we need two (QFilePost/QFilePre ExecutePrinter) separate classes? > > seems like both are doing the same thing. With the rework this change is removed > On March 22, 2017, 12:49 a.m., Vihang Karajgaonkar wrote: > > itests/util/src/main/java/org/apache/hive/beeline/qfile/QFile.java > > Lines 78 (patched) > > <https://reviews.apache.org/r/57693/diff/1/?file=1666199#file1666199line87> > > > > Not sure I understand this. Can you add some comments so that it is > > easier to understand? Is it because BOUNDING_CHAR is both at the starting > > and the ending of the message? With the rework this change is removed > On March 22, 2017, 12:49 a.m., Vihang Karajgaonkar wrote: > > ql/src/java/org/apache/hadoop/hive/ql/hooks/PrinterHook.java > > Lines 31 (patched) > > <https://reviews.apache.org/r/57693/diff/1/?file=1666204#file1666204line31> > > > > Does this need to be abstract? With the rework this change is removed - Peter ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57693/#review169650 ----------------------------------------------------------- On April 7, 2017, 9:25 a.m., Peter Vary wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/57693/ > ----------------------------------------------------------- > > (Updated April 7, 2017, 9:25 a.m.) > > > Review request for hive, Zoltan Haindrich, Marta Kuczora, Miklos Csanady, > Naveen Gangam, Vihang Karajgaonkar, and Barna Zsombor Klara. > > > Bugs: HIVE-16146 > https://issues.apache.org/jira/browse/HIVE-16146 > > > Repository: hive-git > > > Description > ------- > > The goal was to generate '\0' markers around the raw log items we want to > keep in the golden files. > > To archive this I had to do a small functional change and some small refactor: > - Removed the immutability of the format map in BeeLine, so the test could > add the QFile specific OutputFromat as a possible format > - The PostExecutePrinter and the PreExecutePrinter got a common ancestor, > which was due anyway because PostExecutePrinter reused static methods from > PreExecutePrinter. This way I was able to create QFile specific printers > which are generating the desired markers. > - Moved the QFile test to the org.apache.hive.beeline package, so the test > classes can use the package private classes and methods > > For one reason or other BeeLine added an extra space character at the end of > the lines for multiline commands - I have removed this space - Will see if > this effects any unit test or not. > > With the above mentioned *OutputFromat*, *QFilePreExecutePrinter*, > *QFilePostExecutePrinter* we can mark the lines which are needed in the q.out > file, and during the filtering we can remove the unneeded parts - I prefer > to keep the log level high in the raw files, so in case of a test failure we > can have better understanding of what has happened. > > In the test files: > - Updated the beforeExecute, and afterExecute methods to set the new > outputformat and the new hooks > - Removed the query specific filters, since they are non exitstent in the CLI > tests > - Simplified the static filterset - currently only contains the filters which > are really neccessary for the actual tests - might grow to the same size than > in the QTestUtils - but if we do not want to run all of the test we would > like to keep this list as small as possible > - Removed unnecessary configurations from QFileBuilder which will be not > needed in case we want to mimic the CLI results > > > Diffs > ----- > > beeline/src/java/org/apache/hive/beeline/BeeLine.java 11526a7 > beeline/src/java/org/apache/hive/beeline/Commands.java 2578728 > common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 7d4a6a0 > itests/src/test/resources/testconfiguration.properties 7a70c9c > > itests/util/src/main/java/org/apache/hadoop/hive/cli/control/CoreBeeLineDriver.java > 0d63f5d > itests/util/src/main/java/org/apache/hive/beeline/qfile/QFile.java ae5a349 > > itests/util/src/main/java/org/apache/hive/beeline/qfile/QFileBeeLineClient.java > 760fde6 > itests/util/src/main/java/org/apache/hive/beeline/qfile/package-info.java > fcd50ec > ql/src/java/org/apache/hadoop/hive/ql/exec/mr/ExecDriver.java a5c0fcd > ql/src/java/org/apache/hadoop/hive/ql/log/LogDivertAppenderForTest.java > PRE-CREATION > ql/src/java/org/apache/hadoop/hive/ql/session/OperationLog.java c37a633 > ql/src/test/results/clientpositive/beeline/drop_with_concurrency.q.out > 385f9b7 > ql/src/test/results/clientpositive/beeline/escape_comments.q.out abc0fee > ql/src/test/results/clientpositive/beeline/select_dummy_source.q.out > PRE-CREATION > > service/src/java/org/apache/hive/service/cli/operation/OperationManager.java > f62ee4e > service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java > 418f453 > > > Diff: https://reviews.apache.org/r/57693/diff/2/ > > > Testing > ------- > > Added a new simple query file from CLI driver, and checked that the generated > output is the same > > > Thanks, > > Peter Vary > >
