Hive
  1. Hive
  2. HIVE-472

HiveFileFormatUtils's checkInputFormat does not include RCFile

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.4.0
    • Component/s: None
    • Labels:
      None
    • Hadoop Flags:
      Reviewed
    1. hive.472.1.patch
      2 kB
      He Yongqiang
    2. hive-472-2009-06-08.patch
      9 kB
      He Yongqiang
    3. hive-472-2009-06-15.patch
      9 kB
      He Yongqiang
    4. hive-472-2009-06-20.patch
      9 kB
      He Yongqiang
    5. hive-472-2009-06-20-2.patch
      14 kB
      He Yongqiang
    6. hive-472-2009-06-24.patch
      15 kB
      He Yongqiang

      Activity

      Hide
      Namit Jain added a comment -

      Committed. Thanks Yongqiang.

      Show
      Namit Jain added a comment - Committed. Thanks Yongqiang.
      Hide
      Namit Jain added a comment -

      no problems - will commit if the tests pass

      Show
      Namit Jain added a comment - no problems - will commit if the tests pass
      Hide
      He Yongqiang added a comment -

      Sorry, my fault. the previous patch does not include changes for MoveTask. I use the same workspace working on hive-472 and hive-560, and forgot to add MoveTask in the patch.
      Please try the new one hive-472-2009-06-24.patch. Again, sorry for taking your time on the wrong patch.
      I think in order to reduce committer's time for reviewing and testing wrong patches( like the previous wrong one), we may need to let hudson to pick the patch and do the test.

      Show
      He Yongqiang added a comment - Sorry, my fault. the previous patch does not include changes for MoveTask. I use the same workspace working on hive-472 and hive-560, and forgot to add MoveTask in the patch. Please try the new one hive-472-2009-06-24.patch. Again, sorry for taking your time on the wrong patch. I think in order to reduce committer's time for reviewing and testing wrong patches( like the previous wrong one), we may need to let hudson to pick the patch and do the test.
      Hide
      Namit Jain added a comment -

      All the new tests are failing for me - you need to debug them

      Show
      Namit Jain added a comment - All the new tests are failing for me - you need to debug them
      Hide
      Namit Jain added a comment -

      +1

      The changes look good - I will merge if the tests pass

      Show
      Namit Jain added a comment - +1 The changes look good - I will merge if the tests pass
      Hide
      He Yongqiang added a comment -

      Namit, thanks for pointing the error query. I should throw an exception if a mismatch occured.
      And the new patch added in two more testcases.

      Show
      He Yongqiang added a comment - Namit, thanks for pointing the error query. I should throw an exception if a mismatch occured. And the new patch added in two more testcases.
      Hide
      Namit Jain added a comment -

      load_wrong_fileformat.q

      The above test is failing

      Show
      Namit Jain added a comment - load_wrong_fileformat.q The above test is failing
      Hide
      He Yongqiang added a comment -

      I use eclipse as the work env, and use two spaces instead of tabs.

      Show
      He Yongqiang added a comment - I use eclipse as the work env, and use two spaces instead of tabs.
      Hide
      Namit Jain added a comment -

      The changes look good but there are some tabs - can you remove them and I will merge it in

      Show
      Namit Jain added a comment - The changes look good but there are some tabs - can you remove them and I will merge it in
      Hide
      He Yongqiang added a comment -

      Update a new patch, which passed tests in my local. The previous one has a null pointer exception.

      Show
      He Yongqiang added a comment - Update a new patch, which passed tests in my local. The previous one has a null pointer exception.
      Hide
      He Yongqiang added a comment -

      1) added a new interface InputFormatChecker
      2) added a new SequenceFileInputFormatChecker implementing the newly added checker interface
      3) let RCFileInputFormat implement the newly added checker interface

      It seems 'ant test' can not successfully run with currently truck. So I did not run this patch on my local machine.

      Show
      He Yongqiang added a comment - 1) added a new interface InputFormatChecker 2) added a new SequenceFileInputFormatChecker implementing the newly added checker interface 3) let RCFileInputFormat implement the newly added checker interface It seems 'ant test' can not successfully run with currently truck. So I did not run this patch on my local machine.
      Hide
      He Yongqiang added a comment -

      Yes. It will be better if we can move this method out. But it has some problem since hive does not have its own InputFormat definition.
      The input formats hive uses right now are mainly from hadoop core, such as SequenceFileInputFormat, IgnoreKeyFileInputFormat etc. And we can not add a validInput() to hadoop's input format classes.
      One option is to create another class hierarchy (InputFormatChecker) in Hive parallized with FileFomat.
      Another option is to keep the FileFormat's checkInputFormat, and check whether the input format argument has a method "hiveValidInput". If find one, use it. And if not use the old logic in HiveFormatUtils.

      Show
      He Yongqiang added a comment - Yes. It will be better if we can move this method out. But it has some problem since hive does not have its own InputFormat definition. The input formats hive uses right now are mainly from hadoop core, such as SequenceFileInputFormat, IgnoreKeyFileInputFormat etc. And we can not add a validInput() to hadoop's input format classes. One option is to create another class hierarchy (InputFormatChecker) in Hive parallized with FileFomat. Another option is to keep the FileFormat's checkInputFormat, and check whether the input format argument has a method "hiveValidInput". If find one, use it. And if not use the old logic in HiveFormatUtils.
      Hide
      Zheng Shao added a comment -

      @Yongqiang: I think this is a good time to move the checkFileFormat code from the utility to the InputFileFormat class. What do you think?

      Show
      Zheng Shao added a comment - @Yongqiang: I think this is a good time to move the checkFileFormat code from the utility to the InputFileFormat class. What do you think?
      Hide
      He Yongqiang added a comment -

      Thanks, Ashish.
      This bug will cause insert overwrite into table XXX select YYY from table_rcfile not work.

      btw, once Hive-460 is resolved, i will start working on Hive-461. Before that, i would like to do some profiling, trying to find some optimization, taking a look at the hive's index and join(especially with small files).

      Show
      He Yongqiang added a comment - Thanks, Ashish. This bug will cause insert overwrite into table XXX select YYY from table_rcfile not work. btw, once Hive-460 is resolved, i will start working on Hive-461. Before that, i would like to do some profiling, trying to find some optimization, taking a look at the hive's index and join(especially with small files).
      Hide
      Ashish Thusoo added a comment -

      Assigned.

      Show
      Ashish Thusoo added a comment - Assigned.
      Hide
      He Yongqiang added a comment -

      hive.472.1.patch adds the check of RCFile.

      If we get more and more FileFormats, we may need a more elegant method to do the check.

      Show
      He Yongqiang added a comment - hive.472.1.patch adds the check of RCFile. If we get more and more FileFormats, we may need a more elegant method to do the check.

        People

        • Assignee:
          He Yongqiang
          Reporter:
          He Yongqiang
        • Votes:
          0 Vote for this issue
          Watchers:
          0 Start watching this issue

          Dates

          • Created:
            Updated:
            Resolved:

            Development