Hive
  1. Hive
  2. HIVE-2334

DESCRIBE TABLE causes NPE when hive.cli.print.header=true

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.7.1
    • Fix Version/s: 0.8.0
    • Component/s: CLI
    • Labels:
      None
    • Hadoop Flags:
      Reviewed
    1. h2334.patch
      13 kB
      Jakob Homan
    2. HIVE-2334-2.patch
      16 kB
      Jakob Homan
    3. HIVE-2334-3.patch
      16 kB
      Jakob Homan

      Activity

      Carl Steinbach made changes -
      Status Resolved [ 5 ] Closed [ 6 ]
      Hide
      Hudson added a comment -

      Integrated in Hive-trunk-h0.21 #907 (See https://builds.apache.org/job/Hive-trunk-h0.21/907/)
      HIVE-2334. DESCRIBE TABLE causes NPE when hive.cli.print.header=true (Jakob Homan via cws)

      cws : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1159742
      Files :

      • /hive/trunk/cli/src/test/org/apache
      • /hive/trunk/cli/src/test/org/apache/hadoop/hive/cli/TestCliDriverMethods.java
      • /hive/trunk/cli/src/test/org/apache/hadoop
      • /hive/trunk/cli/src/test
      • /hive/trunk/cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java
      • /hive/trunk/ql/src/test/queries/clientpositive/print_header.q
      • /hive/trunk/cli/ivy.xml
      • /hive/trunk/ql/src/test/results/clientpositive/print_header.q.out
      • /hive/trunk/cli/src/test/org
      • /hive/trunk/cli/src/test/org/apache/hadoop/hive
      • /hive/trunk/cli/build.xml
      • /hive/trunk/cli/src/test/org/apache/hadoop/hive/cli
      Show
      Hudson added a comment - Integrated in Hive-trunk-h0.21 #907 (See https://builds.apache.org/job/Hive-trunk-h0.21/907/ ) HIVE-2334 . DESCRIBE TABLE causes NPE when hive.cli.print.header=true (Jakob Homan via cws) cws : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1159742 Files : /hive/trunk/cli/src/test/org/apache /hive/trunk/cli/src/test/org/apache/hadoop/hive/cli/TestCliDriverMethods.java /hive/trunk/cli/src/test/org/apache/hadoop /hive/trunk/cli/src/test /hive/trunk/cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java /hive/trunk/ql/src/test/queries/clientpositive/print_header.q /hive/trunk/cli/ivy.xml /hive/trunk/ql/src/test/results/clientpositive/print_header.q.out /hive/trunk/cli/src/test/org /hive/trunk/cli/src/test/org/apache/hadoop/hive /hive/trunk/cli/build.xml /hive/trunk/cli/src/test/org/apache/hadoop/hive/cli
      Carl Steinbach made changes -
      Status Patch Available [ 10002 ] Resolved [ 5 ]
      Hadoop Flags [Reviewed]
      Fix Version/s 0.8.0 [ 12316178 ]
      Resolution Fixed [ 1 ]
      Hide
      Carl Steinbach added a comment -

      Committed to trunk. Thanks Jakob!

      Show
      Carl Steinbach added a comment - Committed to trunk. Thanks Jakob!
      Hide
      Carl Steinbach added a comment -

      +1. Will commit if tests pass.

      Show
      Carl Steinbach added a comment - +1. Will commit if tests pass.
      Hide
      jiraposter@reviews.apache.org added a comment -

      -----------------------------------------------------------
      This is an automatically generated e-mail. To reply, visit:
      https://reviews.apache.org/r/1300/
      -----------------------------------------------------------

      (Updated 2011-08-15 20:12:23.939812)

      Review request for hive and Carl Steinbach.

      Changes
      -------

      Updated diff now that HIVE-2171 has been committed. Removes inclusion of mockito, which was handled in the other JIRA. Otherwise, no changes.

      Summary
      -------

      Commands that don't return a schema cause NPE when print headers is on.

      This addresses bug HIVE-2334.
      https://issues.apache.org/jira/browse/HIVE-2334

      Diffs (updated)


      cli/build.xml c174b22
      cli/ivy.xml f272b91
      cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java a2976b5
      cli/src/test/org/apache/hadoop/hive/cli/TestCliDriverMethods.java PRE-CREATION
      ql/src/test/queries/clientpositive/print_header.q 96380ab
      ql/src/test/results/clientpositive/print_header.q.out e3946d7

      Diff: https://reviews.apache.org/r/1300/diff

      Testing
      -------

      New unit tests (both positive and negative) and verification on manual cluster.

      Thanks,

      Jakob

      Show
      jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1300/ ----------------------------------------------------------- (Updated 2011-08-15 20:12:23.939812) Review request for hive and Carl Steinbach. Changes ------- Updated diff now that HIVE-2171 has been committed. Removes inclusion of mockito, which was handled in the other JIRA. Otherwise, no changes. Summary ------- Commands that don't return a schema cause NPE when print headers is on. This addresses bug HIVE-2334 . https://issues.apache.org/jira/browse/HIVE-2334 Diffs (updated) cli/build.xml c174b22 cli/ivy.xml f272b91 cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java a2976b5 cli/src/test/org/apache/hadoop/hive/cli/TestCliDriverMethods.java PRE-CREATION ql/src/test/queries/clientpositive/print_header.q 96380ab ql/src/test/results/clientpositive/print_header.q.out e3946d7 Diff: https://reviews.apache.org/r/1300/diff Testing ------- New unit tests (both positive and negative) and verification on manual cluster. Thanks, Jakob
      Jakob Homan made changes -
      Status Open [ 1 ] Patch Available [ 10002 ]
      Hide
      Jakob Homan added a comment -

      re-submitting for review.

      Show
      Jakob Homan added a comment - re-submitting for review.
      Jakob Homan made changes -
      Attachment HIVE-2334-3.patch [ 12490467 ]
      Hide
      Jakob Homan added a comment -

      New patch that merges cleanly after HIVE-2171. Only change is to remove the inclusion of mockito into ivy, which was also done in the now-committed 2171. Otherwise patch is the same.

      Show
      Jakob Homan added a comment - New patch that merges cleanly after HIVE-2171 . Only change is to remove the inclusion of mockito into ivy, which was also done in the now-committed 2171. Otherwise patch is the same.
      Jakob Homan made changes -
      Status Patch Available [ 10002 ] Open [ 1 ]
      Hide
      Jakob Homan added a comment -

      canceling patch due to trunk drift.

      Show
      Jakob Homan added a comment - canceling patch due to trunk drift.
      Hide
      jiraposter@reviews.apache.org added a comment -

      -----------------------------------------------------------
      This is an automatically generated e-mail. To reply, visit:
      https://reviews.apache.org/r/1300/
      -----------------------------------------------------------

      (Updated 2011-08-08 22:42:22.232604)

      Review request for hive.

      Changes
      -------

      • Removed star imports in unit test.
      • Added test case to integration test. Call to use default NPEs before patch, succeeds afterwards
      • Removed no-op from cli/build.xml to ensure test runs
      • Added mockito into cli's ivy.xml. The top-level ivy.xml doesn't need changed since the ivy settings aren't inherited.
      • Didn't change the split call to tokenize, since it was as is in the original patch. No need to introduce unnecessary changes.

      Summary
      -------

      Commands that don't return a schema cause NPE when print headers is on.

      This addresses bug HIVE-2334.
      https://issues.apache.org/jira/browse/HIVE-2334

      Diffs (updated)


      cli/build.xml c174b22
      cli/ivy.xml f272b91
      cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java 9fa7bc6
      cli/src/test/org/apache/hadoop/hive/cli/TestCliDriverMethods.java PRE-CREATION
      ivy/libraries.properties af856bd
      ql/src/test/queries/clientpositive/print_header.q 96380ab
      ql/src/test/results/clientpositive/print_header.q.out e3946d7

      Diff: https://reviews.apache.org/r/1300/diff

      Testing
      -------

      New unit tests (both positive and negative) and verification on manual cluster.

      Thanks,

      Jakob

      Show
      jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1300/ ----------------------------------------------------------- (Updated 2011-08-08 22:42:22.232604) Review request for hive. Changes ------- Removed star imports in unit test. Added test case to integration test. Call to use default NPEs before patch, succeeds afterwards Removed no-op from cli/build.xml to ensure test runs Added mockito into cli's ivy.xml. The top-level ivy.xml doesn't need changed since the ivy settings aren't inherited. Didn't change the split call to tokenize, since it was as is in the original patch. No need to introduce unnecessary changes. Summary ------- Commands that don't return a schema cause NPE when print headers is on. This addresses bug HIVE-2334 . https://issues.apache.org/jira/browse/HIVE-2334 Diffs (updated) cli/build.xml c174b22 cli/ivy.xml f272b91 cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java 9fa7bc6 cli/src/test/org/apache/hadoop/hive/cli/TestCliDriverMethods.java PRE-CREATION ivy/libraries.properties af856bd ql/src/test/queries/clientpositive/print_header.q 96380ab ql/src/test/results/clientpositive/print_header.q.out e3946d7 Diff: https://reviews.apache.org/r/1300/diff Testing ------- New unit tests (both positive and negative) and verification on manual cluster. Thanks, Jakob
      Jakob Homan made changes -
      Status Open [ 1 ] Patch Available [ 10002 ]
      Jakob Homan made changes -
      Attachment HIVE-2334-2.patch [ 12489765 ]
      Hide
      Jakob Homan added a comment -

      Updated patch to address Carl's comments.

      Show
      Jakob Homan added a comment - Updated patch to address Carl's comments.
      Hide
      jiraposter@reviews.apache.org added a comment -

      On 2011-08-05 20:38:11, Carl Steinbach wrote:

      > cli/src/test/org/apache/hadoop/hive/cli/TestCliDriverMethods.java, line 1

      > <https://reviews.apache.org/r/1300/diff/1/?file=30860#file30860line1>

      >

      > cli/build.xml overrides the ant test target with a no-op, so this test is actually not getting run.

      Jakob Homan wrote:

      I'll update cli/build.xml to not be a no-op, unless there's some reason to?

      Yup, it needs to be update, or your test will never get run. Inheriting the test target definition from build-common.xml is probably the best option if it works.

      On 2011-08-05 20:38:11, Carl Steinbach wrote:

      > ivy/libraries.properties, line 47

      > <https://reviews.apache.org/r/1300/diff/1/?file=30861#file30861line47>

      >

      > We need to manage this dependency with Ivy. The Hive build currently runs against hadoop-0.20.1, which does not include mockito-all-1.8.2.jar

      Jakob Homan wrote:

      I'm sorry; I don't understand. This is being brought in by Ivy? As part of HIVE-2171, I had mentioned we need to make sure testing related jars don't get included during binary/package, but that should be done in a different JIRA.

      Right now the dependency on mockito is unsatisfied. I tried updating cli/build.xml to fix the test target problem and ended up getting a bunch of compilation errors since the mockito jar is not on the classpath.

      As part of HIVE-2171, I had mentioned we need to make sure testing related jars don't get included during binary/package, but that should be done in a different JIRA.

      Ivy supports this behavior through the use of configurations. We need to add a 'test' configuration to the root ivy.xml file:

      <conf name="test" extends="compile" visibility="private"/>

      And then add mockito to the list of dependencies:

      <dependency org="org.mockito" name="mockito-all" rev="$

      {mockito-all.version}

      " conf="test->default"/>

      On 2011-08-05 20:38:11, Carl Steinbach wrote:

      > cli/src/test/org/apache/hadoop/hive/cli/TestCliDriverMethods.java, line 37

      > <https://reviews.apache.org/r/1300/diff/1/?file=30860#file30860line37>

      >

      > There's already some very limited test coverage for the hive.cli.print.header feature in print_header.q. Why not extend this testcase instead of adding a new unit test?

      Jakob Homan wrote:

      Because this is an actual unit test against a specific regression, which has value separate from the print_header.q integration test. I can add additional content to print_header.q, since this test is easier to identify what's gone wrong and runs in about 0.2 seconds, this one seems more useful.

      Ok. Please update print_header.q with some additional tests.

      • Carl

      -----------------------------------------------------------
      This is an automatically generated e-mail. To reply, visit:
      https://reviews.apache.org/r/1300/#review1308
      -----------------------------------------------------------

      On 2011-08-05 01:22:01, Jakob Homan wrote:

      -----------------------------------------------------------

      This is an automatically generated e-mail. To reply, visit:

      https://reviews.apache.org/r/1300/

      -----------------------------------------------------------

      (Updated 2011-08-05 01:22:01)

      Review request for hive.

      Summary

      -------

      Commands that don't return a schema cause NPE when print headers is on.

      This addresses bug HIVE-2334.

      https://issues.apache.org/jira/browse/HIVE-2334

      Diffs

      -----

      cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java 9fa7bc6

      cli/src/test/org/apache/hadoop/hive/cli/TestCliDriverMethods.java PRE-CREATION

      ivy/libraries.properties af856bd

      Diff: https://reviews.apache.org/r/1300/diff

      Testing

      -------

      New unit tests (both positive and negative) and verification on manual cluster.

      Thanks,

      Jakob

      Show
      jiraposter@reviews.apache.org added a comment - On 2011-08-05 20:38:11, Carl Steinbach wrote: > cli/src/test/org/apache/hadoop/hive/cli/TestCliDriverMethods.java, line 1 > < https://reviews.apache.org/r/1300/diff/1/?file=30860#file30860line1 > > > cli/build.xml overrides the ant test target with a no-op, so this test is actually not getting run. Jakob Homan wrote: I'll update cli/build.xml to not be a no-op, unless there's some reason to? Yup, it needs to be update, or your test will never get run. Inheriting the test target definition from build-common.xml is probably the best option if it works. On 2011-08-05 20:38:11, Carl Steinbach wrote: > ivy/libraries.properties, line 47 > < https://reviews.apache.org/r/1300/diff/1/?file=30861#file30861line47 > > > We need to manage this dependency with Ivy. The Hive build currently runs against hadoop-0.20.1, which does not include mockito-all-1.8.2.jar Jakob Homan wrote: I'm sorry; I don't understand. This is being brought in by Ivy? As part of HIVE-2171 , I had mentioned we need to make sure testing related jars don't get included during binary/package, but that should be done in a different JIRA. Right now the dependency on mockito is unsatisfied. I tried updating cli/build.xml to fix the test target problem and ended up getting a bunch of compilation errors since the mockito jar is not on the classpath. As part of HIVE-2171 , I had mentioned we need to make sure testing related jars don't get included during binary/package, but that should be done in a different JIRA. Ivy supports this behavior through the use of configurations. We need to add a 'test' configuration to the root ivy.xml file: <conf name="test" extends="compile" visibility="private"/> And then add mockito to the list of dependencies: <dependency org="org.mockito" name="mockito-all" rev="$ {mockito-all.version} " conf="test->default"/> On 2011-08-05 20:38:11, Carl Steinbach wrote: > cli/src/test/org/apache/hadoop/hive/cli/TestCliDriverMethods.java, line 37 > < https://reviews.apache.org/r/1300/diff/1/?file=30860#file30860line37 > > > There's already some very limited test coverage for the hive.cli.print.header feature in print_header.q. Why not extend this testcase instead of adding a new unit test? Jakob Homan wrote: Because this is an actual unit test against a specific regression, which has value separate from the print_header.q integration test. I can add additional content to print_header.q, since this test is easier to identify what's gone wrong and runs in about 0.2 seconds, this one seems more useful. Ok. Please update print_header.q with some additional tests. Carl ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1300/#review1308 ----------------------------------------------------------- On 2011-08-05 01:22:01, Jakob Homan wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1300/ ----------------------------------------------------------- (Updated 2011-08-05 01:22:01) Review request for hive. Summary ------- Commands that don't return a schema cause NPE when print headers is on. This addresses bug HIVE-2334 . https://issues.apache.org/jira/browse/HIVE-2334 Diffs ----- cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java 9fa7bc6 cli/src/test/org/apache/hadoop/hive/cli/TestCliDriverMethods.java PRE-CREATION ivy/libraries.properties af856bd Diff: https://reviews.apache.org/r/1300/diff Testing ------- New unit tests (both positive and negative) and verification on manual cluster. Thanks, Jakob
      Hide
      jiraposter@reviews.apache.org added a comment -

      On 2011-08-05 20:38:11, Carl Steinbach wrote:

      > cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java, line 235

      > <https://reviews.apache.org/r/1300/diff/1/?file=30859#file30859line235>

      >

      > Might want to consider using StringTokenizer or StreamTokenizer here.

      This is how it was in the original code. All of this can actually be done quite a bit better. I'm happy to switch to tokenizer; the patch is a bit schizophrenic about refactoring/improving. I didn't change this since it's not directly related to what I was trying to test.

      On 2011-08-05 20:38:11, Carl Steinbach wrote:

      > cli/src/test/org/apache/hadoop/hive/cli/TestCliDriverMethods.java, line 37

      > <https://reviews.apache.org/r/1300/diff/1/?file=30860#file30860line37>

      >

      > There's already some very limited test coverage for the hive.cli.print.header feature in print_header.q. Why not extend this testcase instead of adding a new unit test?

      Because this is an actual unit test against a specific regression, which has value separate from the print_header.q integration test. I can add additional content to print_header.q, since this test is easier to identify what's gone wrong and runs in about 0.2 seconds, this one seems more useful.

      On 2011-08-05 20:38:11, Carl Steinbach wrote:

      > ivy/libraries.properties, line 47

      > <https://reviews.apache.org/r/1300/diff/1/?file=30861#file30861line47>

      >

      > We need to manage this dependency with Ivy. The Hive build currently runs against hadoop-0.20.1, which does not include mockito-all-1.8.2.jar

      I'm sorry; I don't understand. This is being brought in by Ivy? As part of HIVE-2171, I had mentioned we need to make sure testing related jars don't get included during binary/package, but that should be done in a different JIRA.

      On 2011-08-05 20:38:11, Carl Steinbach wrote:

      > cli/src/test/org/apache/hadoop/hive/cli/TestCliDriverMethods.java, line 1

      > <https://reviews.apache.org/r/1300/diff/1/?file=30860#file30860line1>

      >

      > cli/build.xml overrides the ant test target with a no-op, so this test is actually not getting run.

      I'll update cli/build.xml to not be a no-op, unless there's some reason to?

      • Jakob

      -----------------------------------------------------------
      This is an automatically generated e-mail. To reply, visit:
      https://reviews.apache.org/r/1300/#review1308
      -----------------------------------------------------------

      On 2011-08-05 01:22:01, Jakob Homan wrote:

      -----------------------------------------------------------

      This is an automatically generated e-mail. To reply, visit:

      https://reviews.apache.org/r/1300/

      -----------------------------------------------------------

      (Updated 2011-08-05 01:22:01)

      Review request for hive.

      Summary

      -------

      Commands that don't return a schema cause NPE when print headers is on.

      This addresses bug HIVE-2334.

      https://issues.apache.org/jira/browse/HIVE-2334

      Diffs

      -----

      cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java 9fa7bc6

      cli/src/test/org/apache/hadoop/hive/cli/TestCliDriverMethods.java PRE-CREATION

      ivy/libraries.properties af856bd

      Diff: https://reviews.apache.org/r/1300/diff

      Testing

      -------

      New unit tests (both positive and negative) and verification on manual cluster.

      Thanks,

      Jakob

      Show
      jiraposter@reviews.apache.org added a comment - On 2011-08-05 20:38:11, Carl Steinbach wrote: > cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java, line 235 > < https://reviews.apache.org/r/1300/diff/1/?file=30859#file30859line235 > > > Might want to consider using StringTokenizer or StreamTokenizer here. This is how it was in the original code. All of this can actually be done quite a bit better. I'm happy to switch to tokenizer; the patch is a bit schizophrenic about refactoring/improving. I didn't change this since it's not directly related to what I was trying to test. On 2011-08-05 20:38:11, Carl Steinbach wrote: > cli/src/test/org/apache/hadoop/hive/cli/TestCliDriverMethods.java, line 37 > < https://reviews.apache.org/r/1300/diff/1/?file=30860#file30860line37 > > > There's already some very limited test coverage for the hive.cli.print.header feature in print_header.q. Why not extend this testcase instead of adding a new unit test? Because this is an actual unit test against a specific regression, which has value separate from the print_header.q integration test. I can add additional content to print_header.q, since this test is easier to identify what's gone wrong and runs in about 0.2 seconds, this one seems more useful. On 2011-08-05 20:38:11, Carl Steinbach wrote: > ivy/libraries.properties, line 47 > < https://reviews.apache.org/r/1300/diff/1/?file=30861#file30861line47 > > > We need to manage this dependency with Ivy. The Hive build currently runs against hadoop-0.20.1, which does not include mockito-all-1.8.2.jar I'm sorry; I don't understand. This is being brought in by Ivy? As part of HIVE-2171 , I had mentioned we need to make sure testing related jars don't get included during binary/package, but that should be done in a different JIRA. On 2011-08-05 20:38:11, Carl Steinbach wrote: > cli/src/test/org/apache/hadoop/hive/cli/TestCliDriverMethods.java, line 1 > < https://reviews.apache.org/r/1300/diff/1/?file=30860#file30860line1 > > > cli/build.xml overrides the ant test target with a no-op, so this test is actually not getting run. I'll update cli/build.xml to not be a no-op, unless there's some reason to? Jakob ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1300/#review1308 ----------------------------------------------------------- On 2011-08-05 01:22:01, Jakob Homan wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1300/ ----------------------------------------------------------- (Updated 2011-08-05 01:22:01) Review request for hive. Summary ------- Commands that don't return a schema cause NPE when print headers is on. This addresses bug HIVE-2334 . https://issues.apache.org/jira/browse/HIVE-2334 Diffs ----- cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java 9fa7bc6 cli/src/test/org/apache/hadoop/hive/cli/TestCliDriverMethods.java PRE-CREATION ivy/libraries.properties af856bd Diff: https://reviews.apache.org/r/1300/diff Testing ------- New unit tests (both positive and negative) and verification on manual cluster. Thanks, Jakob
      Hide
      jiraposter@reviews.apache.org added a comment -

      -----------------------------------------------------------
      This is an automatically generated e-mail. To reply, visit:
      https://reviews.apache.org/r/1300/#review1308
      -----------------------------------------------------------

      cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java
      <https://reviews.apache.org/r/1300/#comment2967>

      Might want to consider using StringTokenizer or StreamTokenizer here.

      cli/src/test/org/apache/hadoop/hive/cli/TestCliDriverMethods.java
      <https://reviews.apache.org/r/1300/#comment2963>

      cli/build.xml overrides the ant test target with a no-op, so this test is actually not getting run.

      cli/src/test/org/apache/hadoop/hive/cli/TestCliDriverMethods.java
      <https://reviews.apache.org/r/1300/#comment2964>

      Wildcard imports violate the checkstyle guidelines.

      cli/src/test/org/apache/hadoop/hive/cli/TestCliDriverMethods.java
      <https://reviews.apache.org/r/1300/#comment2965>

      There's already some very limited test coverage for the hive.cli.print.header feature in print_header.q. Why not extend this testcase instead of adding a new unit test?

      ivy/libraries.properties
      <https://reviews.apache.org/r/1300/#comment2966>

      We need to manage this dependency with Ivy. The Hive build currently runs against hadoop-0.20.1, which does not include mockito-all-1.8.2.jar

      • Carl

      On 2011-08-05 01:22:01, Jakob Homan wrote:

      -----------------------------------------------------------

      This is an automatically generated e-mail. To reply, visit:

      https://reviews.apache.org/r/1300/

      -----------------------------------------------------------

      (Updated 2011-08-05 01:22:01)

      Review request for hive.

      Summary

      -------

      Commands that don't return a schema cause NPE when print headers is on.

      This addresses bug HIVE-2334.

      https://issues.apache.org/jira/browse/HIVE-2334

      Diffs

      -----

      cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java 9fa7bc6

      cli/src/test/org/apache/hadoop/hive/cli/TestCliDriverMethods.java PRE-CREATION

      ivy/libraries.properties af856bd

      Diff: https://reviews.apache.org/r/1300/diff

      Testing

      -------

      New unit tests (both positive and negative) and verification on manual cluster.

      Thanks,

      Jakob

      Show
      jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1300/#review1308 ----------------------------------------------------------- cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java < https://reviews.apache.org/r/1300/#comment2967 > Might want to consider using StringTokenizer or StreamTokenizer here. cli/src/test/org/apache/hadoop/hive/cli/TestCliDriverMethods.java < https://reviews.apache.org/r/1300/#comment2963 > cli/build.xml overrides the ant test target with a no-op, so this test is actually not getting run. cli/src/test/org/apache/hadoop/hive/cli/TestCliDriverMethods.java < https://reviews.apache.org/r/1300/#comment2964 > Wildcard imports violate the checkstyle guidelines. cli/src/test/org/apache/hadoop/hive/cli/TestCliDriverMethods.java < https://reviews.apache.org/r/1300/#comment2965 > There's already some very limited test coverage for the hive.cli.print.header feature in print_header.q. Why not extend this testcase instead of adding a new unit test? ivy/libraries.properties < https://reviews.apache.org/r/1300/#comment2966 > We need to manage this dependency with Ivy. The Hive build currently runs against hadoop-0.20.1, which does not include mockito-all-1.8.2.jar Carl On 2011-08-05 01:22:01, Jakob Homan wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1300/ ----------------------------------------------------------- (Updated 2011-08-05 01:22:01) Review request for hive. Summary ------- Commands that don't return a schema cause NPE when print headers is on. This addresses bug HIVE-2334 . https://issues.apache.org/jira/browse/HIVE-2334 Diffs ----- cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java 9fa7bc6 cli/src/test/org/apache/hadoop/hive/cli/TestCliDriverMethods.java PRE-CREATION ivy/libraries.properties af856bd Diff: https://reviews.apache.org/r/1300/diff Testing ------- New unit tests (both positive and negative) and verification on manual cluster. Thanks, Jakob
      Carl Steinbach made changes -
      Status Patch Available [ 10002 ] Open [ 1 ]
      Hide
      Jakob Homan added a comment -

      Ran full test suite. All pass (two failures in full suite, but can't reproduce after repeated runs - one in TestHBaseCliDriver and one for skewjoin.q). Patch is ready for review.

      Show
      Jakob Homan added a comment - Ran full test suite. All pass (two failures in full suite, but can't reproduce after repeated runs - one in TestHBaseCliDriver and one for skewjoin.q). Patch is ready for review.
      Hide
      jiraposter@reviews.apache.org added a comment -

      -----------------------------------------------------------
      This is an automatically generated e-mail. To reply, visit:
      https://reviews.apache.org/r/1300/
      -----------------------------------------------------------

      Review request for hive.

      Summary
      -------

      Commands that don't return a schema cause NPE when print headers is on.

      This addresses bug HIVE-2334.
      https://issues.apache.org/jira/browse/HIVE-2334

      Diffs


      cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java 9fa7bc6
      cli/src/test/org/apache/hadoop/hive/cli/TestCliDriverMethods.java PRE-CREATION
      ivy/libraries.properties af856bd

      Diff: https://reviews.apache.org/r/1300/diff

      Testing
      -------

      New unit tests (both positive and negative) and verification on manual cluster.

      Thanks,

      Jakob

      Show
      jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1300/ ----------------------------------------------------------- Review request for hive. Summary ------- Commands that don't return a schema cause NPE when print headers is on. This addresses bug HIVE-2334 . https://issues.apache.org/jira/browse/HIVE-2334 Diffs cli/src/java/org/apache/hadoop/hive/cli/CliDriver.java 9fa7bc6 cli/src/test/org/apache/hadoop/hive/cli/TestCliDriverMethods.java PRE-CREATION ivy/libraries.properties af856bd Diff: https://reviews.apache.org/r/1300/diff Testing ------- New unit tests (both positive and negative) and verification on manual cluster. Thanks, Jakob
      Jakob Homan made changes -
      Status Open [ 1 ] Patch Available [ 10002 ]
      Jakob Homan made changes -
      Attachment h2334.patch [ 12489430 ]
      Hide
      Jakob Homan added a comment -

      Patch to fix issue. The problem was that some commands, such as use default do not have an associated schema with them from the query processor, but this is expected in the header printing code. It's questionable if the QP should return a schema object with two null fields, but that should be addressed in another issue.

      After patch:

      hive> use default;
      OK
      Time taken: 4.013 seconds
      hive> set hive.cli.print.header=true;
      hive> use default;
      OK
      Time taken: 0.0090 seconds
      hive>

      The actual fix is simple, but a bit of refactoring was necessary to test it. I normally wouldn't want to include such refactoring in a bug fix, but this class is ripe for it and the refactorings are a good first step. The check this via an actual unit test, mockito is introduced, as was also the case in HIVE-2171; whichever goes in last will need to be updated to remove the new library.

      The patch is on trunk. I can generate one for 7.1 if requested.

      Show
      Jakob Homan added a comment - Patch to fix issue. The problem was that some commands, such as use default do not have an associated schema with them from the query processor, but this is expected in the header printing code. It's questionable if the QP should return a schema object with two null fields, but that should be addressed in another issue. After patch: hive> use default; OK Time taken: 4.013 seconds hive> set hive.cli.print.header=true; hive> use default; OK Time taken: 0.0090 seconds hive> The actual fix is simple, but a bit of refactoring was necessary to test it. I normally wouldn't want to include such refactoring in a bug fix, but this class is ripe for it and the refactorings are a good first step. The check this via an actual unit test, mockito is introduced, as was also the case in HIVE-2171 ; whichever goes in last will need to be updated to remove the new library. The patch is on trunk. I can generate one for 7.1 if requested.
      Jakob Homan made changes -
      Field Original Value New Value
      Assignee Jakob Homan [ jghoman ]
      Hide
      Carl Steinbach added a comment -
      hive> use default;
      OK
      hive> set hive.cli.print.header=true;
      OK
      hive> use default;
      11/08/02 14:52:29 INFO ql.Driver: OK
      Exception in thread "main" java.lang.NullPointerException
      	at org.apache.hadoop.hive.cli.CliDriver.processCmd(CliDriver.java:176)
      	at org.apache.hadoop.hive.cli.CliDriver.processLine(CliDriver.java:241)
      	at org.apache.hadoop.hive.cli.CliDriver.main(CliDriver.java:456)
      	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
      	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
      	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
      	at java.lang.reflect.Method.invoke(Method.java:597)
      	at org.apache.hadoop.util.RunJar.main(RunJar.java:156)
      %
      
      Show
      Carl Steinbach added a comment - hive> use default; OK hive> set hive.cli.print.header=true; OK hive> use default; 11/08/02 14:52:29 INFO ql.Driver: OK Exception in thread "main" java.lang.NullPointerException at org.apache.hadoop.hive.cli.CliDriver.processCmd(CliDriver.java:176) at org.apache.hadoop.hive.cli.CliDriver.processLine(CliDriver.java:241) at org.apache.hadoop.hive.cli.CliDriver.main(CliDriver.java:456) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25) at java.lang.reflect.Method.invoke(Method.java:597) at org.apache.hadoop.util.RunJar.main(RunJar.java:156) %
      Carl Steinbach created issue -

        People

        • Assignee:
          Jakob Homan
          Reporter:
          Carl Steinbach
        • Votes:
          0 Vote for this issue
          Watchers:
          1 Start watching this issue

          Dates

          • Created:
            Updated:
            Resolved:

            Development