----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60445/#review179819 -----------------------------------------------------------
Fix it, then Ship it! common/src/java/org/apache/hive/common/util/HiveStringUtils.java Lines 1149-1150 (patched) <https://reviews.apache.org/r/60445/#comment254682> I didn't know that BeeLine supported "#" as a comment prefix (and I don't see any documentation for that on wiki as well). While BeeLine may be supporting it and we should probably not change that now, I am not sure if we should keep it as well here since now it will apply to all the other JDBC clients. Should we rather stick to the SQL92 comment prefix only here and let BeeLine keep using "#" in addition to "--"? What do you think? common/src/test/org/apache/hive/common/util/TestHiveStringUtils.java Lines 112 (patched) <https://reviews.apache.org/r/60445/#comment254672> nit, spellcheck comments - Vihang Karajgaonkar On July 5, 2017, 6:09 p.m., Andrew Sherman wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/60445/ > ----------------------------------------------------------- > > (Updated July 5, 2017, 6:09 p.m.) > > > Review request for hive and Sahil Takiar. > > > Bugs: HIVE-16935 > https://issues.apache.org/jira/browse/HIVE-16935 > > > Repository: hive-git > > > Description > ------- > > We strip sql comments from a command string. The stripped command is use to > determine which > CommandProcessor will execute the command. If the CommandProcessorFactory > does not select a special > CommandProcessor then we execute the original unstripped command so that the > sql parser can remove comments. > Move BeeLine's comment stripping code to HiveStringUtils and change BeeLine > to call it from there > Add a better test with separate tokens for "set role" in > TestCommandProcessorFactory. > Add a test case for comment removal in set_processor_namespaces.q using an > indented comment as > unindented comments are removed by the test driver. > > Change-Id: I166dc1e7588ec9802ba373d88e69e716aecd33c2 > > > Diffs > ----- > > beeline/src/java/org/apache/hive/beeline/Commands.java > 3b2d72ed79771e6198e62c47060a7f80665dbcb2 > beeline/src/test/org/apache/hive/beeline/TestCommands.java > 04c939a04c7a56768286743c2bb9c9797507e3aa > cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java > 27fd66d35ea89b0de0d17763625fbf564584fcca > common/src/java/org/apache/hive/common/util/HiveStringUtils.java > 4a6413a7c376ffb4de6d20d24707ac5bf89ebc0c > common/src/test/org/apache/hive/common/util/TestHiveStringUtils.java > 6bd7037152c6f809daec8af42708693c05fe00cf > > ql/src/test/org/apache/hadoop/hive/ql/processors/TestCommandProcessorFactory.java > 21bdcf44436a02b11f878fa439e916d4b55ac63d > ql/src/test/queries/clientpositive/set_processor_namespaces.q > 612807f0c871b1881446d088e1c2c399d1afe970 > ql/src/test/results/clientpositive/set_processor_namespaces.q.out > c05ce4d61d00a9ee6671d97f2fd178f18d44cc8c > > service/src/java/org/apache/hive/service/cli/operation/ExecuteStatementOperation.java > 2dd90b69b3bf789b1a3928129cf801b17884033f > > > Diff: https://reviews.apache.org/r/60445/diff/4/ > > > Testing > ------- > > Added new test case. > Hand tested with Hue and Jdbc. > > > Thanks, > > Andrew Sherman > >
