Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.4
    • Fix Version/s: 0.4
    • Component/s: storage handlers
    • Labels:
      None

      Description

      This JIRA covers changes to HCatInputFormat and InputJobInfo. See HCATALOG-237 for details and design notes.

      1. 239_1.patch
        29 kB
        Vikram Dixit K
      2. 239_2.patch
        124 kB
        Vikram Dixit K
      3. 239_3.patch
        34 kB
        Vikram Dixit K
      4. 239_4.patch
        47 kB
        Vikram Dixit K
      5. 239_5.patch
        94 kB
        Vikram Dixit K
      6. 239_6.patch
        92 kB
        Vikram Dixit K
      7. 239_7.patch
        95 kB
        Vikram Dixit K
      8. 239_8.patch
        96 kB
        Vikram Dixit K
      9. 239.patch
        29 kB
        Vikram Dixit K

        Issue Links

          Activity

          Hide
          Vikram Dixit K added a comment -

          First Iteration. Requires integration with https://issues.apache.org/jira/browse/HCATALOG-241

          Show
          Vikram Dixit K added a comment - First Iteration. Requires integration with https://issues.apache.org/jira/browse/HCATALOG-241
          Hide
          Vikram Dixit K added a comment -

          Removed the mapred implementation and retained only the mapreduce implementation for the HCatInputFormat class.

          Show
          Vikram Dixit K added a comment - Removed the mapred implementation and retained only the mapreduce implementation for the HCatInputFormat class.
          Hide
          jiraposter@reviews.apache.org added a comment -

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

          Review request for hcatalog, Sushanth Sowmyan and Francis Liu.

          Summary
          -------

          See HCATALOG-237 for details and design notes.

          This addresses bug HCATALOG-239.
          https://issues.apache.org/jira/browse/HCATALOG-239

          Diffs


          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java 1241719
          trunk/src/java/org/apache/hcatalog/mapreduce/HCatEximInputFormat.java 1241719
          trunk/src/java/org/apache/hcatalog/mapreduce/HCatInputFormat.java 1241719
          trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java 1241719
          trunk/src/java/org/apache/hcatalog/mapreduce/HCatSplit.java 1241719
          trunk/src/java/org/apache/hcatalog/mapreduce/InitializeInput.java 1241719
          trunk/src/java/org/apache/hcatalog/mapreduce/PartInfo.java 1241719

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

          Testing
          -------

          Thanks,

          Alan

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3784/ ----------------------------------------------------------- Review request for hcatalog, Sushanth Sowmyan and Francis Liu. Summary ------- See HCATALOG-237 for details and design notes. This addresses bug HCATALOG-239 . https://issues.apache.org/jira/browse/HCATALOG-239 Diffs trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java 1241719 trunk/src/java/org/apache/hcatalog/mapreduce/HCatEximInputFormat.java 1241719 trunk/src/java/org/apache/hcatalog/mapreduce/HCatInputFormat.java 1241719 trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java 1241719 trunk/src/java/org/apache/hcatalog/mapreduce/HCatSplit.java 1241719 trunk/src/java/org/apache/hcatalog/mapreduce/InitializeInput.java 1241719 trunk/src/java/org/apache/hcatalog/mapreduce/PartInfo.java 1241719 Diff: https://reviews.apache.org/r/3784/diff Testing ------- Thanks, Alan
          Hide
          jiraposter@reviews.apache.org added a comment -

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

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java
          <https://reviews.apache.org/r/3784/#comment10857>

          not related do this patch. Didn't know the outputSchema is not in inputJobInfo. We should file a jira to put it in for consistency.

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java
          <https://reviews.apache.org/r/3784/#comment10859>

          since jobConf is a deep copy of jobContext.getConfiguration() you should be consistent here and only use jobConf. In case one of these methods actually modifies the configuration then you know all of it is in one place. And update the jobContext with all the changes before this method ends.

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java
          <https://reviews.apache.org/r/3784/#comment10866>

          We should sync up on how we do this. Take a look at my patch and HCatUtil.getStorageHandler().

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java
          <https://reviews.apache.org/r/3784/#comment10874>

          Available in my patch use HCatUtil.getStorageHandler() instead

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java
          <https://reviews.apache.org/r/3784/#comment10863>

          Will jobProperties really have nothing in it? maybe you should append to the the contents instead?

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java
          <https://reviews.apache.org/r/3784/#comment10864>

          IOException would be better. Please add a message as well.

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java
          <https://reviews.apache.org/r/3784/#comment10869>

          should be moved into default/foster storageHandler logic.

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java
          <https://reviews.apache.org/r/3784/#comment10870>

          what happens if storageHandler == null? You should encapsulate if,of,serde into a "FosterStorageHandler" so you only have to take care of one code path.

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java
          <https://reviews.apache.org/r/3784/#comment10871>

          you don't need an explicit cast.

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java
          <https://reviews.apache.org/r/3784/#comment10872>

          For now use HCatStorageHandler since it contains some methods that are missing and will be added to HiveStorageHandler

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java
          <https://reviews.apache.org/r/3784/#comment10873>

          similar code as the previous...same comments. In HCatOutputFormat, I moved code such as this into HCatUtil.configureOutputJobProperties() maybe you should do something similar?

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java
          <https://reviews.apache.org/r/3784/#comment10875>

          I would just do:
          JobConf jobConf = new JobConf(jobContext.getConfiguration)

          One less variable to worry about.

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java
          <https://reviews.apache.org/r/3784/#comment10865>

          Since this is file based code. this logic should go into the Foster/Default StoragaHandler.configureInputJobProperties()

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java
          <https://reviews.apache.org/r/3784/#comment10876>

          you don't need the 2nd condition

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java
          <https://reviews.apache.org/r/3784/#comment10877>

          you can probably remove this already?

          • Francis

          On 2012-02-08 01:01:56, Alan Gates wrote:

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

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

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

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

          (Updated 2012-02-08 01:01:56)

          Review request for hcatalog, Sushanth Sowmyan and Francis Liu.

          Summary

          -------

          See HCATALOG-237 for details and design notes.

          This addresses bug HCATALOG-239.

          https://issues.apache.org/jira/browse/HCATALOG-239

          Diffs

          -----

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java 1241719

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatEximInputFormat.java 1241719

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatInputFormat.java 1241719

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java 1241719

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatSplit.java 1241719

          trunk/src/java/org/apache/hcatalog/mapreduce/InitializeInput.java 1241719

          trunk/src/java/org/apache/hcatalog/mapreduce/PartInfo.java 1241719

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

          Testing

          -------

          Thanks,

          Alan

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3784/#review4941 ----------------------------------------------------------- trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java < https://reviews.apache.org/r/3784/#comment10857 > not related do this patch. Didn't know the outputSchema is not in inputJobInfo. We should file a jira to put it in for consistency. trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java < https://reviews.apache.org/r/3784/#comment10859 > since jobConf is a deep copy of jobContext.getConfiguration() you should be consistent here and only use jobConf. In case one of these methods actually modifies the configuration then you know all of it is in one place. And update the jobContext with all the changes before this method ends. trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java < https://reviews.apache.org/r/3784/#comment10866 > We should sync up on how we do this. Take a look at my patch and HCatUtil.getStorageHandler(). trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java < https://reviews.apache.org/r/3784/#comment10874 > Available in my patch use HCatUtil.getStorageHandler() instead trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java < https://reviews.apache.org/r/3784/#comment10863 > Will jobProperties really have nothing in it? maybe you should append to the the contents instead? trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java < https://reviews.apache.org/r/3784/#comment10864 > IOException would be better. Please add a message as well. trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java < https://reviews.apache.org/r/3784/#comment10869 > should be moved into default/foster storageHandler logic. trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java < https://reviews.apache.org/r/3784/#comment10870 > what happens if storageHandler == null? You should encapsulate if,of,serde into a "FosterStorageHandler" so you only have to take care of one code path. trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java < https://reviews.apache.org/r/3784/#comment10871 > you don't need an explicit cast. trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java < https://reviews.apache.org/r/3784/#comment10872 > For now use HCatStorageHandler since it contains some methods that are missing and will be added to HiveStorageHandler trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java < https://reviews.apache.org/r/3784/#comment10873 > similar code as the previous...same comments. In HCatOutputFormat, I moved code such as this into HCatUtil.configureOutputJobProperties() maybe you should do something similar? trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java < https://reviews.apache.org/r/3784/#comment10875 > I would just do: JobConf jobConf = new JobConf(jobContext.getConfiguration) One less variable to worry about. trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java < https://reviews.apache.org/r/3784/#comment10865 > Since this is file based code. this logic should go into the Foster/Default StoragaHandler.configureInputJobProperties() trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java < https://reviews.apache.org/r/3784/#comment10876 > you don't need the 2nd condition trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java < https://reviews.apache.org/r/3784/#comment10877 > you can probably remove this already? Francis On 2012-02-08 01:01:56, Alan Gates wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3784/ ----------------------------------------------------------- (Updated 2012-02-08 01:01:56) Review request for hcatalog, Sushanth Sowmyan and Francis Liu. Summary ------- See HCATALOG-237 for details and design notes. This addresses bug HCATALOG-239 . https://issues.apache.org/jira/browse/HCATALOG-239 Diffs ----- trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java 1241719 trunk/src/java/org/apache/hcatalog/mapreduce/HCatEximInputFormat.java 1241719 trunk/src/java/org/apache/hcatalog/mapreduce/HCatInputFormat.java 1241719 trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java 1241719 trunk/src/java/org/apache/hcatalog/mapreduce/HCatSplit.java 1241719 trunk/src/java/org/apache/hcatalog/mapreduce/InitializeInput.java 1241719 trunk/src/java/org/apache/hcatalog/mapreduce/PartInfo.java 1241719 Diff: https://reviews.apache.org/r/3784/diff Testing ------- Thanks, Alan
          Hide
          Vikram Dixit K added a comment -

          Includes the entire changeset from JIRAs 240 and 241. To prevent accidental destruction of all changes through improper command line.

          Show
          Vikram Dixit K added a comment - Includes the entire changeset from JIRAs 240 and 241. To prevent accidental destruction of all changes through improper command line.
          Hide
          Vikram Dixit K added a comment -

          Latest integrated patch

          Show
          Vikram Dixit K added a comment - Latest integrated patch
          Hide
          jiraposter@reviews.apache.org added a comment -

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

          Review request for Alan Gates, Sushanth Sowmyan and Francis Liu.

          Summary
          -------

          This is a patch with input format specific changes integrated with record reader and output format changes.

          This addresses bug HCATALOG-239.
          https://issues.apache.org/jira/browse/HCATALOG-239

          Diffs


          trunk/src/java/org/apache/hcatalog/common/HCatUtil.java 1241662
          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java 1241662
          trunk/src/java/org/apache/hcatalog/mapreduce/HCatEximInputFormat.java 1241662
          trunk/src/java/org/apache/hcatalog/mapreduce/HCatInputFormat.java 1241662
          trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java 1241662
          trunk/src/java/org/apache/hcatalog/mapreduce/HCatSplit.java 1241662
          trunk/src/java/org/apache/hcatalog/mapreduce/HCatSplit.java 1241662

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

          Testing
          -------

          Thanks,

          Vikram

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3901/ ----------------------------------------------------------- Review request for Alan Gates, Sushanth Sowmyan and Francis Liu. Summary ------- This is a patch with input format specific changes integrated with record reader and output format changes. This addresses bug HCATALOG-239 . https://issues.apache.org/jira/browse/HCATALOG-239 Diffs trunk/src/java/org/apache/hcatalog/common/HCatUtil.java 1241662 trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java 1241662 trunk/src/java/org/apache/hcatalog/mapreduce/HCatEximInputFormat.java 1241662 trunk/src/java/org/apache/hcatalog/mapreduce/HCatInputFormat.java 1241662 trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java 1241662 trunk/src/java/org/apache/hcatalog/mapreduce/HCatSplit.java 1241662 trunk/src/java/org/apache/hcatalog/mapreduce/HCatSplit.java 1241662 Diff: https://reviews.apache.org/r/3901/diff Testing ------- Thanks, Vikram
          Hide
          jiraposter@reviews.apache.org added a comment -

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

          trunk/src/java/org/apache/hcatalog/common/HCatUtil.java
          <https://reviews.apache.org/r/3901/#comment11286>

          This looks like it came from my patch. I've updated my patch and moved this into InternalUtil. Please update.

          trunk/src/java/org/apache/hcatalog/common/HCatUtil.java
          <https://reviews.apache.org/r/3901/#comment11287>

          comments don't match the method.

          trunk/src/java/org/apache/hcatalog/common/HCatUtil.java
          <https://reviews.apache.org/r/3901/#comment11288>

          This should probably be moved to internal util?

          trunk/src/java/org/apache/hcatalog/common/HCatUtil.java
          <https://reviews.apache.org/r/3901/#comment11294>

          You don't really need the user to specify a class do you? I think you can arbitrarily pick one for HiveConf preferably one from the hive client library.

          trunk/src/java/org/apache/hcatalog/common/HCatUtil.java
          <https://reviews.apache.org/r/3901/#comment11289>

          include the original exception as part of the new throw exception. provide a better message.

          trunk/src/java/org/apache/hcatalog/common/HCatUtil.java
          <https://reviews.apache.org/r/3901/#comment11290>

          Sushanth has this in his patch as well...a more updated version I think.

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java
          <https://reviews.apache.org/r/3901/#comment11291>

          We might as well fix this...better to throw the specific exception, IOException. This should break backwards compatibility.

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java
          <https://reviews.apache.org/r/3901/#comment11292>

          Might as well fix this too.

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java
          <https://reviews.apache.org/r/3901/#comment11293>

          maybe call it getMapRedInputFormat(), to make things clearer.

          Is this used beyond this class? Consider making it private or package private.

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java
          <https://reviews.apache.org/r/3901/#comment11295>

          Are you sure tableDesc.getJobProperties() hasn't been set you prior to this?

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java
          <https://reviews.apache.org/r/3901/#comment11296>

          Add a message.

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java
          <https://reviews.apache.org/r/3901/#comment11299>

          change to private?

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatEximInputFormat.java
          <https://reviews.apache.org/r/3901/#comment11300>

          this class was removed in my patch. please update.

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java
          <https://reviews.apache.org/r/3901/#comment11301>

          shouldn't the contents of this be moved into mapreduce.initializ()?

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java
          <https://reviews.apache.org/r/3901/#comment11302>

          you should probably throw an exception if it's not an hcatsplit.

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java
          <https://reviews.apache.org/r/3901/#comment11304>

          add a message

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java
          <https://reviews.apache.org/r/3901/#comment11303>

          i'm wondering wether you should actually throw an exception as well. since the underlying inputformat did so itself?

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatSplit.java
          <https://reviews.apache.org/r/3901/#comment11306>

          shouldn't this be a mapreduce.InputSplit?

          • Francis

          On 2012-02-14 22:21:47, Vikram Dixit Kumaraswamy wrote:

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

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

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

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

          (Updated 2012-02-14 22:21:47)

          Review request for Alan Gates, Sushanth Sowmyan and Francis Liu.

          Summary

          -------

          This is a patch with input format specific changes integrated with record reader and output format changes.

          This addresses bug HCATALOG-239.

          https://issues.apache.org/jira/browse/HCATALOG-239

          Diffs

          -----

          trunk/src/java/org/apache/hcatalog/common/HCatUtil.java 1241662

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java 1241662

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatEximInputFormat.java 1241662

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatInputFormat.java 1241662

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java 1241662

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatSplit.java 1241662

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatSplit.java 1241662

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

          Testing

          -------

          Thanks,

          Vikram

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3901/#review5161 ----------------------------------------------------------- trunk/src/java/org/apache/hcatalog/common/HCatUtil.java < https://reviews.apache.org/r/3901/#comment11286 > This looks like it came from my patch. I've updated my patch and moved this into InternalUtil. Please update. trunk/src/java/org/apache/hcatalog/common/HCatUtil.java < https://reviews.apache.org/r/3901/#comment11287 > comments don't match the method. trunk/src/java/org/apache/hcatalog/common/HCatUtil.java < https://reviews.apache.org/r/3901/#comment11288 > This should probably be moved to internal util? trunk/src/java/org/apache/hcatalog/common/HCatUtil.java < https://reviews.apache.org/r/3901/#comment11294 > You don't really need the user to specify a class do you? I think you can arbitrarily pick one for HiveConf preferably one from the hive client library. trunk/src/java/org/apache/hcatalog/common/HCatUtil.java < https://reviews.apache.org/r/3901/#comment11289 > include the original exception as part of the new throw exception. provide a better message. trunk/src/java/org/apache/hcatalog/common/HCatUtil.java < https://reviews.apache.org/r/3901/#comment11290 > Sushanth has this in his patch as well...a more updated version I think. trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java < https://reviews.apache.org/r/3901/#comment11291 > We might as well fix this...better to throw the specific exception, IOException. This should break backwards compatibility. trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java < https://reviews.apache.org/r/3901/#comment11292 > Might as well fix this too. trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java < https://reviews.apache.org/r/3901/#comment11293 > maybe call it getMapRedInputFormat(), to make things clearer. Is this used beyond this class? Consider making it private or package private. trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java < https://reviews.apache.org/r/3901/#comment11295 > Are you sure tableDesc.getJobProperties() hasn't been set you prior to this? trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java < https://reviews.apache.org/r/3901/#comment11296 > Add a message. trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java < https://reviews.apache.org/r/3901/#comment11299 > change to private? trunk/src/java/org/apache/hcatalog/mapreduce/HCatEximInputFormat.java < https://reviews.apache.org/r/3901/#comment11300 > this class was removed in my patch. please update. trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java < https://reviews.apache.org/r/3901/#comment11301 > shouldn't the contents of this be moved into mapreduce.initializ()? trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java < https://reviews.apache.org/r/3901/#comment11302 > you should probably throw an exception if it's not an hcatsplit. trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java < https://reviews.apache.org/r/3901/#comment11304 > add a message trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java < https://reviews.apache.org/r/3901/#comment11303 > i'm wondering wether you should actually throw an exception as well. since the underlying inputformat did so itself? trunk/src/java/org/apache/hcatalog/mapreduce/HCatSplit.java < https://reviews.apache.org/r/3901/#comment11306 > shouldn't this be a mapreduce.InputSplit? Francis On 2012-02-14 22:21:47, Vikram Dixit Kumaraswamy wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3901/ ----------------------------------------------------------- (Updated 2012-02-14 22:21:47) Review request for Alan Gates, Sushanth Sowmyan and Francis Liu. Summary ------- This is a patch with input format specific changes integrated with record reader and output format changes. This addresses bug HCATALOG-239 . https://issues.apache.org/jira/browse/HCATALOG-239 Diffs ----- trunk/src/java/org/apache/hcatalog/common/HCatUtil.java 1241662 trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java 1241662 trunk/src/java/org/apache/hcatalog/mapreduce/HCatEximInputFormat.java 1241662 trunk/src/java/org/apache/hcatalog/mapreduce/HCatInputFormat.java 1241662 trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java 1241662 trunk/src/java/org/apache/hcatalog/mapreduce/HCatSplit.java 1241662 trunk/src/java/org/apache/hcatalog/mapreduce/HCatSplit.java 1241662 Diff: https://reviews.apache.org/r/3901/diff Testing ------- Thanks, Vikram
          Hide
          jiraposter@reviews.apache.org added a comment -

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

          I realized this is missing FosterStorageHandler changes for the input side. You might've forgotten to include it?

          • Francis

          On 2012-02-14 22:21:47, Vikram Dixit Kumaraswamy wrote:

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

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

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

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

          (Updated 2012-02-14 22:21:47)

          Review request for Alan Gates, Sushanth Sowmyan and Francis Liu.

          Summary

          -------

          This is a patch with input format specific changes integrated with record reader and output format changes.

          This addresses bug HCATALOG-239.

          https://issues.apache.org/jira/browse/HCATALOG-239

          Diffs

          -----

          trunk/src/java/org/apache/hcatalog/common/HCatUtil.java 1241662

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java 1241662

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatEximInputFormat.java 1241662

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatInputFormat.java 1241662

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java 1241662

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatSplit.java 1241662

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatSplit.java 1241662

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

          Testing

          -------

          Thanks,

          Vikram

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3901/#review5182 ----------------------------------------------------------- I realized this is missing FosterStorageHandler changes for the input side. You might've forgotten to include it? Francis On 2012-02-14 22:21:47, Vikram Dixit Kumaraswamy wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3901/ ----------------------------------------------------------- (Updated 2012-02-14 22:21:47) Review request for Alan Gates, Sushanth Sowmyan and Francis Liu. Summary ------- This is a patch with input format specific changes integrated with record reader and output format changes. This addresses bug HCATALOG-239 . https://issues.apache.org/jira/browse/HCATALOG-239 Diffs ----- trunk/src/java/org/apache/hcatalog/common/HCatUtil.java 1241662 trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java 1241662 trunk/src/java/org/apache/hcatalog/mapreduce/HCatEximInputFormat.java 1241662 trunk/src/java/org/apache/hcatalog/mapreduce/HCatInputFormat.java 1241662 trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java 1241662 trunk/src/java/org/apache/hcatalog/mapreduce/HCatSplit.java 1241662 trunk/src/java/org/apache/hcatalog/mapreduce/HCatSplit.java 1241662 Diff: https://reviews.apache.org/r/3901/diff Testing ------- Thanks, Vikram
          Hide
          Vikram Dixit K added a comment -

          Incorporated Francis' comments.

          Show
          Vikram Dixit K added a comment - Incorporated Francis' comments.
          Hide
          jiraposter@reviews.apache.org added a comment -

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

          trunk/src/java/org/apache/hcatalog/common/HCatUtil.java
          <https://reviews.apache.org/r/3901/#comment11344>

          Still seems to be in HCatUtil. Do I have to use another patch to see this change?

          trunk/src/java/org/apache/hcatalog/common/HCatUtil.java
          <https://reviews.apache.org/r/3901/#comment11348>

          Removed.

          trunk/src/java/org/apache/hcatalog/common/HCatUtil.java
          <https://reviews.apache.org/r/3901/#comment11350>

          Discussed with Alan about this. If we leave it as an empty constructor, this loads the HiveClient class. Alan was of the opinion that the class does not really matter. So have changed this to be the empty constructor. We can use another if the need arises.

          trunk/src/java/org/apache/hcatalog/common/HCatUtil.java
          <https://reviews.apache.org/r/3901/#comment11351>

          Fixed as part of Sushanth's changes?

          trunk/src/java/org/apache/hcatalog/common/HCatUtil.java
          <https://reviews.apache.org/r/3901/#comment11352>

          true.

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java
          <https://reviews.apache.org/r/3901/#comment11353>

          Done.

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java
          <https://reviews.apache.org/r/3901/#comment11354>

          Done.

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java
          <https://reviews.apache.org/r/3901/#comment11355>

          Done. Both.

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java
          <https://reviews.apache.org/r/3901/#comment11357>

          Fixed as part of the exception being thrown from getJobInfo.

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java
          <https://reviews.apache.org/r/3901/#comment11361>

          Done.

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java
          <https://reviews.apache.org/r/3901/#comment11362>

          The input split that is passed to the mapreduce initialize has to be the mapreduce inputsplit by definition. Added this new initialize function as an imitation. Can rename the function but it is doing the functionality of initialize.

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java
          <https://reviews.apache.org/r/3901/#comment11363>

          Done.

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java
          <https://reviews.apache.org/r/3901/#comment11364>

          Done.

          • Vikram

          On 2012-02-14 22:21:47, Vikram Dixit Kumaraswamy wrote:

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

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

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

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

          (Updated 2012-02-14 22:21:47)

          Review request for Alan Gates, Sushanth Sowmyan and Francis Liu.

          Summary

          -------

          This is a patch with input format specific changes integrated with record reader and output format changes.

          This addresses bug HCATALOG-239.

          https://issues.apache.org/jira/browse/HCATALOG-239

          Diffs

          -----

          trunk/src/java/org/apache/hcatalog/common/HCatUtil.java 1241662

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java 1241662

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatEximInputFormat.java 1241662

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatInputFormat.java 1241662

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java 1241662

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatSplit.java 1241662

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatSplit.java 1241662

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

          Testing

          -------

          Thanks,

          Vikram

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3901/#review5181 ----------------------------------------------------------- trunk/src/java/org/apache/hcatalog/common/HCatUtil.java < https://reviews.apache.org/r/3901/#comment11344 > Still seems to be in HCatUtil. Do I have to use another patch to see this change? trunk/src/java/org/apache/hcatalog/common/HCatUtil.java < https://reviews.apache.org/r/3901/#comment11348 > Removed. trunk/src/java/org/apache/hcatalog/common/HCatUtil.java < https://reviews.apache.org/r/3901/#comment11350 > Discussed with Alan about this. If we leave it as an empty constructor, this loads the HiveClient class. Alan was of the opinion that the class does not really matter. So have changed this to be the empty constructor. We can use another if the need arises. trunk/src/java/org/apache/hcatalog/common/HCatUtil.java < https://reviews.apache.org/r/3901/#comment11351 > Fixed as part of Sushanth's changes? trunk/src/java/org/apache/hcatalog/common/HCatUtil.java < https://reviews.apache.org/r/3901/#comment11352 > true. trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java < https://reviews.apache.org/r/3901/#comment11353 > Done. trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java < https://reviews.apache.org/r/3901/#comment11354 > Done. trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java < https://reviews.apache.org/r/3901/#comment11355 > Done. Both. trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java < https://reviews.apache.org/r/3901/#comment11357 > Fixed as part of the exception being thrown from getJobInfo. trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java < https://reviews.apache.org/r/3901/#comment11361 > Done. trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java < https://reviews.apache.org/r/3901/#comment11362 > The input split that is passed to the mapreduce initialize has to be the mapreduce inputsplit by definition. Added this new initialize function as an imitation. Can rename the function but it is doing the functionality of initialize. trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java < https://reviews.apache.org/r/3901/#comment11363 > Done. trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java < https://reviews.apache.org/r/3901/#comment11364 > Done. Vikram On 2012-02-14 22:21:47, Vikram Dixit Kumaraswamy wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3901/ ----------------------------------------------------------- (Updated 2012-02-14 22:21:47) Review request for Alan Gates, Sushanth Sowmyan and Francis Liu. Summary ------- This is a patch with input format specific changes integrated with record reader and output format changes. This addresses bug HCATALOG-239 . https://issues.apache.org/jira/browse/HCATALOG-239 Diffs ----- trunk/src/java/org/apache/hcatalog/common/HCatUtil.java 1241662 trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java 1241662 trunk/src/java/org/apache/hcatalog/mapreduce/HCatEximInputFormat.java 1241662 trunk/src/java/org/apache/hcatalog/mapreduce/HCatInputFormat.java 1241662 trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java 1241662 trunk/src/java/org/apache/hcatalog/mapreduce/HCatSplit.java 1241662 trunk/src/java/org/apache/hcatalog/mapreduce/HCatSplit.java 1241662 Diff: https://reviews.apache.org/r/3901/diff Testing ------- Thanks, Vikram
          Hide
          jiraposter@reviews.apache.org added a comment -

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

          Review request for Sushanth Sowmyan and Francis Liu.

          Summary
          -------

          Incorporated Francis' comments.

          This addresses bug HCATALOG-239.
          https://issues.apache.org/jira/browse/HCATALOG-239

          Diffs


          trunk/src/java/org/apache/hcatalog/common/HCatUtil.java 1245259
          trunk/src/java/org/apache/hcatalog/mapreduce/DefaultOutputCommitterContainer.java 1245259
          trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputCommitterContainer.java 1245259
          trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputFormatContainer.java 1245259
          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java 1245259
          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseOutputFormat.java 1245259
          trunk/src/java/org/apache/hcatalog/mapreduce/HCatOutputFormat.java 1245259
          trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java 1245259
          trunk/src/java/org/apache/hcatalog/mapreduce/HCatSplit.java 1245259
          trunk/src/java/org/apache/hcatalog/mapreduce/PartInfo.java 1245259

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

          Testing
          -------

          Thanks,

          Vikram

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3945/ ----------------------------------------------------------- Review request for Sushanth Sowmyan and Francis Liu. Summary ------- Incorporated Francis' comments. This addresses bug HCATALOG-239 . https://issues.apache.org/jira/browse/HCATALOG-239 Diffs trunk/src/java/org/apache/hcatalog/common/HCatUtil.java 1245259 trunk/src/java/org/apache/hcatalog/mapreduce/DefaultOutputCommitterContainer.java 1245259 trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputCommitterContainer.java 1245259 trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputFormatContainer.java 1245259 trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java 1245259 trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseOutputFormat.java 1245259 trunk/src/java/org/apache/hcatalog/mapreduce/HCatOutputFormat.java 1245259 trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java 1245259 trunk/src/java/org/apache/hcatalog/mapreduce/HCatSplit.java 1245259 trunk/src/java/org/apache/hcatalog/mapreduce/PartInfo.java 1245259 Diff: https://reviews.apache.org/r/3945/diff Testing ------- Thanks, Vikram
          Hide
          jiraposter@reviews.apache.org added a comment -

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

          Vikram, should there be changes in FosterStorageHandler as well? It's not included in the path?

          • Francis

          On 2012-02-17 18:31:55, Vikram Dixit Kumaraswamy wrote:

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

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

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

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

          (Updated 2012-02-17 18:31:55)

          Review request for Sushanth Sowmyan and Francis Liu.

          Summary

          -------

          Incorporated Francis' comments.

          This addresses bug HCATALOG-239.

          https://issues.apache.org/jira/browse/HCATALOG-239

          Diffs

          -----

          trunk/src/java/org/apache/hcatalog/common/HCatUtil.java 1245259

          trunk/src/java/org/apache/hcatalog/mapreduce/DefaultOutputCommitterContainer.java 1245259

          trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputCommitterContainer.java 1245259

          trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputFormatContainer.java 1245259

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java 1245259

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseOutputFormat.java 1245259

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatOutputFormat.java 1245259

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java 1245259

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatSplit.java 1245259

          trunk/src/java/org/apache/hcatalog/mapreduce/PartInfo.java 1245259

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

          Testing

          -------

          Thanks,

          Vikram

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3945/#review5194 ----------------------------------------------------------- Vikram, should there be changes in FosterStorageHandler as well? It's not included in the path? Francis On 2012-02-17 18:31:55, Vikram Dixit Kumaraswamy wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3945/ ----------------------------------------------------------- (Updated 2012-02-17 18:31:55) Review request for Sushanth Sowmyan and Francis Liu. Summary ------- Incorporated Francis' comments. This addresses bug HCATALOG-239 . https://issues.apache.org/jira/browse/HCATALOG-239 Diffs ----- trunk/src/java/org/apache/hcatalog/common/HCatUtil.java 1245259 trunk/src/java/org/apache/hcatalog/mapreduce/DefaultOutputCommitterContainer.java 1245259 trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputCommitterContainer.java 1245259 trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputFormatContainer.java 1245259 trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java 1245259 trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseOutputFormat.java 1245259 trunk/src/java/org/apache/hcatalog/mapreduce/HCatOutputFormat.java 1245259 trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java 1245259 trunk/src/java/org/apache/hcatalog/mapreduce/HCatSplit.java 1245259 trunk/src/java/org/apache/hcatalog/mapreduce/PartInfo.java 1245259 Diff: https://reviews.apache.org/r/3945/diff Testing ------- Thanks, Vikram
          Hide
          Alan Gates added a comment -

          When I apply this patch I see failures in several unit tests, including TestHCatNonPartitioned and TestHCatDynamicPartitioned with the following:

          org.apache.hcatalog.common.HCatException : 2001 : Error setting output information. Cause : java.lang.ClassCastException: org.apache.hcatalog.rcfile.RCFileInputDriver cannot be cast to org.apache.hcatalog.mapreduce.HCatStorageHandler
          org.apache.hcatalog.common.HCatException : 2001 : Error setting output information. Cause : java.lang.ClassCastException: org.apache.hcatalog.rcfile.RCFileInputDriver cannot be cast to org.apache.hcatalog.mapreduce.HCatStorageHandler
              at org.apache.hcatalog.mapreduce.HCatBaseInputFormat.getSplits(HCatBaseInputFormat.java:189)
              at org.apache.hadoop.mapred.JobClient.writeNewSplits(JobClient.java:962)
              at org.apache.hadoop.mapred.JobClient.writeSplits(JobClient.java:979)
              at org.apache.hadoop.mapred.JobClient.access$600(JobClient.java:174)
              at org.apache.hadoop.mapred.JobClient$2.run(JobClient.java:897)
              at org.apache.hadoop.mapred.JobClient$2.run(JobClient.java:850)
              at java.security.AccessController.doPrivileged(Native Method)
              at javax.security.auth.Subject.doAs(Subject.java:396)
              at org.apache.hadoop.security.UserGroupInformation.doAs(UserGroupInformation.java:1083)
              at org.apache.hadoop.mapred.JobClient.submitJobInternal(JobClient.java:850)
              at org.apache.hadoop.mapreduce.Job.submit(Job.java:465)
              at org.apache.hadoop.mapreduce.Job.waitForCompletion(Job.java:495)
              at org.apache.hcatalog.mapreduce.HCatMapReduceTest.runMRRead(HCatMapReduceTest.java:313)
              at org.apache.hcatalog.mapreduce.HCatMapReduceTest.runMRRead(HCatMapReduceTest.java:281)
              at org.apache.hcatalog.mapreduce.TestHCatNonPartitioned.testHCatNonPartitionedTable(TestHCatNonPartitioned.java:111)
          Caused by: java.lang.ClassCastException: org.apache.hcatalog.rcfile.RCFileInputDriver cannot be cast to org.apache.hcatalog.mapreduce.HCatStorageHandler
              at org.apache.hcatalog.common.HCatUtil.getStorageHandler(HCatUtil.java:514)
              at org.apache.hcatalog.mapreduce.HCatBaseInputFormat.getSplits(HCatBaseInputFormat.java:170)
          
          Show
          Alan Gates added a comment - When I apply this patch I see failures in several unit tests, including TestHCatNonPartitioned and TestHCatDynamicPartitioned with the following: org.apache.hcatalog.common.HCatException : 2001 : Error setting output information. Cause : java.lang.ClassCastException: org.apache.hcatalog.rcfile.RCFileInputDriver cannot be cast to org.apache.hcatalog.mapreduce.HCatStorageHandler org.apache.hcatalog.common.HCatException : 2001 : Error setting output information. Cause : java.lang.ClassCastException: org.apache.hcatalog.rcfile.RCFileInputDriver cannot be cast to org.apache.hcatalog.mapreduce.HCatStorageHandler at org.apache.hcatalog.mapreduce.HCatBaseInputFormat.getSplits(HCatBaseInputFormat.java:189) at org.apache.hadoop.mapred.JobClient.writeNewSplits(JobClient.java:962) at org.apache.hadoop.mapred.JobClient.writeSplits(JobClient.java:979) at org.apache.hadoop.mapred.JobClient.access$600(JobClient.java:174) at org.apache.hadoop.mapred.JobClient$2.run(JobClient.java:897) at org.apache.hadoop.mapred.JobClient$2.run(JobClient.java:850) at java.security.AccessController.doPrivileged(Native Method) at javax.security.auth.Subject.doAs(Subject.java:396) at org.apache.hadoop.security.UserGroupInformation.doAs(UserGroupInformation.java:1083) at org.apache.hadoop.mapred.JobClient.submitJobInternal(JobClient.java:850) at org.apache.hadoop.mapreduce.Job.submit(Job.java:465) at org.apache.hadoop.mapreduce.Job.waitForCompletion(Job.java:495) at org.apache.hcatalog.mapreduce.HCatMapReduceTest.runMRRead(HCatMapReduceTest.java:313) at org.apache.hcatalog.mapreduce.HCatMapReduceTest.runMRRead(HCatMapReduceTest.java:281) at org.apache.hcatalog.mapreduce.TestHCatNonPartitioned.testHCatNonPartitionedTable(TestHCatNonPartitioned.java:111) Caused by: java.lang.ClassCastException: org.apache.hcatalog.rcfile.RCFileInputDriver cannot be cast to org.apache.hcatalog.mapreduce.HCatStorageHandler at org.apache.hcatalog.common.HCatUtil.getStorageHandler(HCatUtil.java:514) at org.apache.hcatalog.mapreduce.HCatBaseInputFormat.getSplits(HCatBaseInputFormat.java:170)
          Hide
          Vikram Dixit K added a comment -

          Final patch?

          Show
          Vikram Dixit K added a comment - Final patch?
          Hide
          Vikram Dixit K added a comment -

          Reverted HCatRecordSerDe change because of JIRA-270 (which needs to be committed to earlier version (0.3?)) and a change to a test case. This fails 1 unit test - TestHCatLoaderComplexSchema with the revert. Apply the patch from 270 for all unit tests to succeed.

          Show
          Vikram Dixit K added a comment - Reverted HCatRecordSerDe change because of JIRA-270 (which needs to be committed to earlier version (0.3?)) and a change to a test case. This fails 1 unit test - TestHCatLoaderComplexSchema with the revert. Apply the patch from 270 for all unit tests to succeed.
          Hide
          jiraposter@reviews.apache.org added a comment -

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

          (Updated 2012-02-23 19:27:43.741693)

          Review request for Sushanth Sowmyan and Francis Liu.

          Changes
          -------

          Almost all unit tests pass except for org.apache.hcatalog.pig.TestHCatLoaderComplexSchema which requires JIRA-270 patches to pass.

          Summary
          -------

          Incorporated Francis' comments.

          This addresses bug HCATALOG-239.
          https://issues.apache.org/jira/browse/HCATALOG-239

          Diffs (updated)


          trunk/src/java/org/apache/hcatalog/common/ErrorType.java 1292521
          trunk/src/java/org/apache/hcatalog/common/HCatUtil.java 1292521
          trunk/src/java/org/apache/hcatalog/data/DataType.java 1292521
          trunk/src/java/org/apache/hcatalog/data/HCatRecordObjectInspectorFactory.java 1292521
          trunk/src/java/org/apache/hcatalog/data/LazyHCatRecord.java 1292521
          trunk/src/java/org/apache/hcatalog/mapreduce/DefaultOutputCommitterContainer.java 1292521
          trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputCommitterContainer.java 1292521
          trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputFormatContainer.java 1292521
          trunk/src/java/org/apache/hcatalog/mapreduce/FosterStorageHandler.java 1292521
          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java 1292521
          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseOutputFormat.java 1292521
          trunk/src/java/org/apache/hcatalog/mapreduce/HCatOutputFormat.java 1292521
          trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java 1292521
          trunk/src/java/org/apache/hcatalog/mapreduce/HCatSplit.java 1292521
          trunk/src/java/org/apache/hcatalog/mapreduce/HCatTableInfo.java 1292521
          trunk/src/java/org/apache/hcatalog/mapreduce/InitializeInput.java 1292521
          trunk/src/java/org/apache/hcatalog/mapreduce/InputJobInfo.java 1292521
          trunk/src/java/org/apache/hcatalog/mapreduce/InternalUtil.java 1292521
          trunk/src/java/org/apache/hcatalog/mapreduce/PartInfo.java 1292521
          trunk/src/java/org/apache/hcatalog/pig/HCatLoader.java 1292521
          trunk/src/test/org/apache/hcatalog/data/TestHCatRecordSerDe.java 1292521
          trunk/src/test/org/apache/hcatalog/data/TestLazyHCatRecord.java 1292521
          trunk/src/test/org/apache/hcatalog/mapreduce/TestHCatHiveCompatibility.java 1292521
          trunk/src/test/org/apache/hcatalog/pig/TestHCatLoaderComplexSchema.java 1292521

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

          Testing
          -------

          Thanks,

          Vikram

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3945/ ----------------------------------------------------------- (Updated 2012-02-23 19:27:43.741693) Review request for Sushanth Sowmyan and Francis Liu. Changes ------- Almost all unit tests pass except for org.apache.hcatalog.pig.TestHCatLoaderComplexSchema which requires JIRA-270 patches to pass. Summary ------- Incorporated Francis' comments. This addresses bug HCATALOG-239 . https://issues.apache.org/jira/browse/HCATALOG-239 Diffs (updated) trunk/src/java/org/apache/hcatalog/common/ErrorType.java 1292521 trunk/src/java/org/apache/hcatalog/common/HCatUtil.java 1292521 trunk/src/java/org/apache/hcatalog/data/DataType.java 1292521 trunk/src/java/org/apache/hcatalog/data/HCatRecordObjectInspectorFactory.java 1292521 trunk/src/java/org/apache/hcatalog/data/LazyHCatRecord.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/DefaultOutputCommitterContainer.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputCommitterContainer.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputFormatContainer.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/FosterStorageHandler.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseOutputFormat.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatOutputFormat.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatSplit.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatTableInfo.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/InitializeInput.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/InputJobInfo.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/InternalUtil.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/PartInfo.java 1292521 trunk/src/java/org/apache/hcatalog/pig/HCatLoader.java 1292521 trunk/src/test/org/apache/hcatalog/data/TestHCatRecordSerDe.java 1292521 trunk/src/test/org/apache/hcatalog/data/TestLazyHCatRecord.java 1292521 trunk/src/test/org/apache/hcatalog/mapreduce/TestHCatHiveCompatibility.java 1292521 trunk/src/test/org/apache/hcatalog/pig/TestHCatLoaderComplexSchema.java 1292521 Diff: https://reviews.apache.org/r/3945/diff Testing ------- Thanks, Vikram
          Hide
          jiraposter@reviews.apache.org added a comment -

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

          trunk/src/java/org/apache/hcatalog/common/HCatUtil.java
          <https://reviews.apache.org/r/3945/#comment11561>

          Change to logger.debug

          trunk/src/java/org/apache/hcatalog/common/HCatUtil.java
          <https://reviews.apache.org/r/3945/#comment11562>

          Please remove this method - it was a utility method that didn't prove to be useful. Also, due to the initializeSerDe methods that we've introduced, this one is incomplete as well.

          trunk/src/java/org/apache/hcatalog/common/HCatUtil.java
          <https://reviews.apache.org/r/3945/#comment11565>

          This will not work unless getHiveConf has already been called - it needs to check before it returns.

          trunk/src/java/org/apache/hcatalog/common/HCatUtil.java
          <https://reviews.apache.org/r/3945/#comment11564>

          getharRequested should be called getHarRequested, and this will not work unless getHiveConf has already been called - it needs to check before it returns.

          trunk/src/java/org/apache/hcatalog/data/HCatRecordObjectInspectorFactory.java
          <https://reviews.apache.org/r/3945/#comment11563>

          Please remove this try block - it was probably meant as a debug print - it should go now.

          trunk/src/java/org/apache/hcatalog/mapreduce/InitializeInput.java
          <https://reviews.apache.org/r/3945/#comment11566>

          This TODO/FIXME is important! We should probably create a new jira to fix this.

          • Sushanth

          On 2012-02-23 19:27:43, Vikram Dixit Kumaraswamy wrote:

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

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

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

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

          (Updated 2012-02-23 19:27:43)

          Review request for Sushanth Sowmyan and Francis Liu.

          Summary

          -------

          Incorporated Francis' comments.

          This addresses bug HCATALOG-239.

          https://issues.apache.org/jira/browse/HCATALOG-239

          Diffs

          -----

          trunk/src/java/org/apache/hcatalog/common/ErrorType.java 1292521

          trunk/src/java/org/apache/hcatalog/common/HCatUtil.java 1292521

          trunk/src/java/org/apache/hcatalog/data/DataType.java 1292521

          trunk/src/java/org/apache/hcatalog/data/HCatRecordObjectInspectorFactory.java 1292521

          trunk/src/java/org/apache/hcatalog/data/LazyHCatRecord.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/DefaultOutputCommitterContainer.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputCommitterContainer.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputFormatContainer.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/FosterStorageHandler.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseOutputFormat.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatOutputFormat.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatSplit.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatTableInfo.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/InitializeInput.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/InputJobInfo.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/InternalUtil.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/PartInfo.java 1292521

          trunk/src/java/org/apache/hcatalog/pig/HCatLoader.java 1292521

          trunk/src/test/org/apache/hcatalog/data/TestHCatRecordSerDe.java 1292521

          trunk/src/test/org/apache/hcatalog/data/TestLazyHCatRecord.java 1292521

          trunk/src/test/org/apache/hcatalog/mapreduce/TestHCatHiveCompatibility.java 1292521

          trunk/src/test/org/apache/hcatalog/pig/TestHCatLoaderComplexSchema.java 1292521

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

          Testing

          -------

          Thanks,

          Vikram

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3945/#review5315 ----------------------------------------------------------- trunk/src/java/org/apache/hcatalog/common/HCatUtil.java < https://reviews.apache.org/r/3945/#comment11561 > Change to logger.debug trunk/src/java/org/apache/hcatalog/common/HCatUtil.java < https://reviews.apache.org/r/3945/#comment11562 > Please remove this method - it was a utility method that didn't prove to be useful. Also, due to the initializeSerDe methods that we've introduced, this one is incomplete as well. trunk/src/java/org/apache/hcatalog/common/HCatUtil.java < https://reviews.apache.org/r/3945/#comment11565 > This will not work unless getHiveConf has already been called - it needs to check before it returns. trunk/src/java/org/apache/hcatalog/common/HCatUtil.java < https://reviews.apache.org/r/3945/#comment11564 > getharRequested should be called getHarRequested, and this will not work unless getHiveConf has already been called - it needs to check before it returns. trunk/src/java/org/apache/hcatalog/data/HCatRecordObjectInspectorFactory.java < https://reviews.apache.org/r/3945/#comment11563 > Please remove this try block - it was probably meant as a debug print - it should go now. trunk/src/java/org/apache/hcatalog/mapreduce/InitializeInput.java < https://reviews.apache.org/r/3945/#comment11566 > This TODO/FIXME is important! We should probably create a new jira to fix this. Sushanth On 2012-02-23 19:27:43, Vikram Dixit Kumaraswamy wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3945/ ----------------------------------------------------------- (Updated 2012-02-23 19:27:43) Review request for Sushanth Sowmyan and Francis Liu. Summary ------- Incorporated Francis' comments. This addresses bug HCATALOG-239 . https://issues.apache.org/jira/browse/HCATALOG-239 Diffs ----- trunk/src/java/org/apache/hcatalog/common/ErrorType.java 1292521 trunk/src/java/org/apache/hcatalog/common/HCatUtil.java 1292521 trunk/src/java/org/apache/hcatalog/data/DataType.java 1292521 trunk/src/java/org/apache/hcatalog/data/HCatRecordObjectInspectorFactory.java 1292521 trunk/src/java/org/apache/hcatalog/data/LazyHCatRecord.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/DefaultOutputCommitterContainer.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputCommitterContainer.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputFormatContainer.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/FosterStorageHandler.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseOutputFormat.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatOutputFormat.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatSplit.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatTableInfo.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/InitializeInput.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/InputJobInfo.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/InternalUtil.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/PartInfo.java 1292521 trunk/src/java/org/apache/hcatalog/pig/HCatLoader.java 1292521 trunk/src/test/org/apache/hcatalog/data/TestHCatRecordSerDe.java 1292521 trunk/src/test/org/apache/hcatalog/data/TestLazyHCatRecord.java 1292521 trunk/src/test/org/apache/hcatalog/mapreduce/TestHCatHiveCompatibility.java 1292521 trunk/src/test/org/apache/hcatalog/pig/TestHCatLoaderComplexSchema.java 1292521 Diff: https://reviews.apache.org/r/3945/diff Testing ------- Thanks, Vikram
          Hide
          jiraposter@reviews.apache.org added a comment -

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

          trunk/src/test/org/apache/hcatalog/data/TestLazyHCatRecord.java
          <https://reviews.apache.org/r/3945/#comment11568>

          comma needed here.

          trunk/src/test/org/apache/hcatalog/data/TestLazyHCatRecord.java
          <https://reviews.apache.org/r/3945/#comment11567>

          Replace this line with :

          HCatSchema schema = HCatSchemaUtils.getHCatSchema(ti).get(0).getStructSubSchema();

          And this test will work. Basically, it's the difference between the following schemas:

          earlier: struct<an_int:int,a_long:bigint,a_double:double,a_string:string>
          now : an_int:int,a_long:bigint,a_double:double,a_string:string

          trunk/src/test/org/apache/hcatalog/data/TestLazyHCatRecord.java
          <https://reviews.apache.org/r/3945/#comment11570>

          comma needed here

          trunk/src/test/org/apache/hcatalog/data/TestLazyHCatRecord.java
          <https://reviews.apache.org/r/3945/#comment11569>

          Replace this line with :

          HCatSchema schema = HCatSchemaUtils.getHCatSchema(ti).get(0).getStructSubSchema();

          And this test will work. Basically, it's the difference between the following schemas:

          earlier: struct<an_int:int,a_long:bigint,a_double:double,a_string:string>
          now : an_int:int,a_long:bigint,a_double:double,a_string:string

          • Sushanth

          On 2012-02-23 19:27:43, Vikram Dixit Kumaraswamy wrote:

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

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

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

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

          (Updated 2012-02-23 19:27:43)

          Review request for Sushanth Sowmyan and Francis Liu.

          Summary

          -------

          Incorporated Francis' comments.

          This addresses bug HCATALOG-239.

          https://issues.apache.org/jira/browse/HCATALOG-239

          Diffs

          -----

          trunk/src/java/org/apache/hcatalog/common/ErrorType.java 1292521

          trunk/src/java/org/apache/hcatalog/common/HCatUtil.java 1292521

          trunk/src/java/org/apache/hcatalog/data/DataType.java 1292521

          trunk/src/java/org/apache/hcatalog/data/HCatRecordObjectInspectorFactory.java 1292521

          trunk/src/java/org/apache/hcatalog/data/LazyHCatRecord.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/DefaultOutputCommitterContainer.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputCommitterContainer.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputFormatContainer.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/FosterStorageHandler.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseOutputFormat.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatOutputFormat.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatSplit.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatTableInfo.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/InitializeInput.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/InputJobInfo.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/InternalUtil.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/PartInfo.java 1292521

          trunk/src/java/org/apache/hcatalog/pig/HCatLoader.java 1292521

          trunk/src/test/org/apache/hcatalog/data/TestHCatRecordSerDe.java 1292521

          trunk/src/test/org/apache/hcatalog/data/TestLazyHCatRecord.java 1292521

          trunk/src/test/org/apache/hcatalog/mapreduce/TestHCatHiveCompatibility.java 1292521

          trunk/src/test/org/apache/hcatalog/pig/TestHCatLoaderComplexSchema.java 1292521

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

          Testing

          -------

          Thanks,

          Vikram

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3945/#review5316 ----------------------------------------------------------- trunk/src/test/org/apache/hcatalog/data/TestLazyHCatRecord.java < https://reviews.apache.org/r/3945/#comment11568 > comma needed here. trunk/src/test/org/apache/hcatalog/data/TestLazyHCatRecord.java < https://reviews.apache.org/r/3945/#comment11567 > Replace this line with : HCatSchema schema = HCatSchemaUtils.getHCatSchema(ti).get(0).getStructSubSchema(); And this test will work. Basically, it's the difference between the following schemas: earlier: struct<an_int:int,a_long:bigint,a_double:double,a_string:string> now : an_int:int,a_long:bigint,a_double:double,a_string:string trunk/src/test/org/apache/hcatalog/data/TestLazyHCatRecord.java < https://reviews.apache.org/r/3945/#comment11570 > comma needed here trunk/src/test/org/apache/hcatalog/data/TestLazyHCatRecord.java < https://reviews.apache.org/r/3945/#comment11569 > Replace this line with : HCatSchema schema = HCatSchemaUtils.getHCatSchema(ti).get(0).getStructSubSchema(); And this test will work. Basically, it's the difference between the following schemas: earlier: struct<an_int:int,a_long:bigint,a_double:double,a_string:string> now : an_int:int,a_long:bigint,a_double:double,a_string:string Sushanth On 2012-02-23 19:27:43, Vikram Dixit Kumaraswamy wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3945/ ----------------------------------------------------------- (Updated 2012-02-23 19:27:43) Review request for Sushanth Sowmyan and Francis Liu. Summary ------- Incorporated Francis' comments. This addresses bug HCATALOG-239 . https://issues.apache.org/jira/browse/HCATALOG-239 Diffs ----- trunk/src/java/org/apache/hcatalog/common/ErrorType.java 1292521 trunk/src/java/org/apache/hcatalog/common/HCatUtil.java 1292521 trunk/src/java/org/apache/hcatalog/data/DataType.java 1292521 trunk/src/java/org/apache/hcatalog/data/HCatRecordObjectInspectorFactory.java 1292521 trunk/src/java/org/apache/hcatalog/data/LazyHCatRecord.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/DefaultOutputCommitterContainer.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputCommitterContainer.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputFormatContainer.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/FosterStorageHandler.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseOutputFormat.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatOutputFormat.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatSplit.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatTableInfo.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/InitializeInput.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/InputJobInfo.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/InternalUtil.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/PartInfo.java 1292521 trunk/src/java/org/apache/hcatalog/pig/HCatLoader.java 1292521 trunk/src/test/org/apache/hcatalog/data/TestHCatRecordSerDe.java 1292521 trunk/src/test/org/apache/hcatalog/data/TestLazyHCatRecord.java 1292521 trunk/src/test/org/apache/hcatalog/mapreduce/TestHCatHiveCompatibility.java 1292521 trunk/src/test/org/apache/hcatalog/pig/TestHCatLoaderComplexSchema.java 1292521 Diff: https://reviews.apache.org/r/3945/diff Testing ------- Thanks, Vikram
          Hide
          jiraposter@reviews.apache.org added a comment -

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

          trunk/src/java/org/apache/hcatalog/common/HCatUtil.java
          <https://reviews.apache.org/r/3945/#comment11571>

          DONE.

          trunk/src/java/org/apache/hcatalog/common/HCatUtil.java
          <https://reviews.apache.org/r/3945/#comment11572>

          DONE.

          trunk/src/java/org/apache/hcatalog/common/HCatUtil.java
          <https://reviews.apache.org/r/3945/#comment11584>

          Had to do quite some refactoring to get this right with the getHiveConf api being a helper to the createHiveClient.

          trunk/src/java/org/apache/hcatalog/common/HCatUtil.java
          <https://reviews.apache.org/r/3945/#comment11585>

          DONE. Please see previous comments.

          trunk/src/java/org/apache/hcatalog/data/HCatRecordObjectInspectorFactory.java
          <https://reviews.apache.org/r/3945/#comment11573>

          DONE.

          trunk/src/java/org/apache/hcatalog/mapreduce/InitializeInput.java
          <https://reviews.apache.org/r/3945/#comment11575>

          DONE. JIRA-274

          trunk/src/test/org/apache/hcatalog/data/TestLazyHCatRecord.java
          <https://reviews.apache.org/r/3945/#comment11576>

          DONE.

          trunk/src/test/org/apache/hcatalog/data/TestLazyHCatRecord.java
          <https://reviews.apache.org/r/3945/#comment11577>

          DONE.

          trunk/src/test/org/apache/hcatalog/data/TestLazyHCatRecord.java
          <https://reviews.apache.org/r/3945/#comment11578>

          DONE.

          trunk/src/test/org/apache/hcatalog/data/TestLazyHCatRecord.java
          <https://reviews.apache.org/r/3945/#comment11579>

          DONE.

          • Vikram

          On 2012-02-23 19:27:43, Vikram Dixit Kumaraswamy wrote:

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

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

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

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

          (Updated 2012-02-23 19:27:43)

          Review request for Sushanth Sowmyan and Francis Liu.

          Summary

          -------

          Incorporated Francis' comments.

          This addresses bug HCATALOG-239.

          https://issues.apache.org/jira/browse/HCATALOG-239

          Diffs

          -----

          trunk/src/java/org/apache/hcatalog/common/ErrorType.java 1292521

          trunk/src/java/org/apache/hcatalog/common/HCatUtil.java 1292521

          trunk/src/java/org/apache/hcatalog/data/DataType.java 1292521

          trunk/src/java/org/apache/hcatalog/data/HCatRecordObjectInspectorFactory.java 1292521

          trunk/src/java/org/apache/hcatalog/data/LazyHCatRecord.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/DefaultOutputCommitterContainer.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputCommitterContainer.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputFormatContainer.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/FosterStorageHandler.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseOutputFormat.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatOutputFormat.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatSplit.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatTableInfo.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/InitializeInput.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/InputJobInfo.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/InternalUtil.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/PartInfo.java 1292521

          trunk/src/java/org/apache/hcatalog/pig/HCatLoader.java 1292521

          trunk/src/test/org/apache/hcatalog/data/TestHCatRecordSerDe.java 1292521

          trunk/src/test/org/apache/hcatalog/data/TestLazyHCatRecord.java 1292521

          trunk/src/test/org/apache/hcatalog/mapreduce/TestHCatHiveCompatibility.java 1292521

          trunk/src/test/org/apache/hcatalog/pig/TestHCatLoaderComplexSchema.java 1292521

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

          Testing

          -------

          Thanks,

          Vikram

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3945/#review5318 ----------------------------------------------------------- trunk/src/java/org/apache/hcatalog/common/HCatUtil.java < https://reviews.apache.org/r/3945/#comment11571 > DONE. trunk/src/java/org/apache/hcatalog/common/HCatUtil.java < https://reviews.apache.org/r/3945/#comment11572 > DONE. trunk/src/java/org/apache/hcatalog/common/HCatUtil.java < https://reviews.apache.org/r/3945/#comment11584 > Had to do quite some refactoring to get this right with the getHiveConf api being a helper to the createHiveClient. trunk/src/java/org/apache/hcatalog/common/HCatUtil.java < https://reviews.apache.org/r/3945/#comment11585 > DONE. Please see previous comments. trunk/src/java/org/apache/hcatalog/data/HCatRecordObjectInspectorFactory.java < https://reviews.apache.org/r/3945/#comment11573 > DONE. trunk/src/java/org/apache/hcatalog/mapreduce/InitializeInput.java < https://reviews.apache.org/r/3945/#comment11575 > DONE. JIRA-274 trunk/src/test/org/apache/hcatalog/data/TestLazyHCatRecord.java < https://reviews.apache.org/r/3945/#comment11576 > DONE. trunk/src/test/org/apache/hcatalog/data/TestLazyHCatRecord.java < https://reviews.apache.org/r/3945/#comment11577 > DONE. trunk/src/test/org/apache/hcatalog/data/TestLazyHCatRecord.java < https://reviews.apache.org/r/3945/#comment11578 > DONE. trunk/src/test/org/apache/hcatalog/data/TestLazyHCatRecord.java < https://reviews.apache.org/r/3945/#comment11579 > DONE. Vikram On 2012-02-23 19:27:43, Vikram Dixit Kumaraswamy wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3945/ ----------------------------------------------------------- (Updated 2012-02-23 19:27:43) Review request for Sushanth Sowmyan and Francis Liu. Summary ------- Incorporated Francis' comments. This addresses bug HCATALOG-239 . https://issues.apache.org/jira/browse/HCATALOG-239 Diffs ----- trunk/src/java/org/apache/hcatalog/common/ErrorType.java 1292521 trunk/src/java/org/apache/hcatalog/common/HCatUtil.java 1292521 trunk/src/java/org/apache/hcatalog/data/DataType.java 1292521 trunk/src/java/org/apache/hcatalog/data/HCatRecordObjectInspectorFactory.java 1292521 trunk/src/java/org/apache/hcatalog/data/LazyHCatRecord.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/DefaultOutputCommitterContainer.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputCommitterContainer.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputFormatContainer.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/FosterStorageHandler.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseOutputFormat.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatOutputFormat.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatSplit.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatTableInfo.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/InitializeInput.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/InputJobInfo.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/InternalUtil.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/PartInfo.java 1292521 trunk/src/java/org/apache/hcatalog/pig/HCatLoader.java 1292521 trunk/src/test/org/apache/hcatalog/data/TestHCatRecordSerDe.java 1292521 trunk/src/test/org/apache/hcatalog/data/TestLazyHCatRecord.java 1292521 trunk/src/test/org/apache/hcatalog/mapreduce/TestHCatHiveCompatibility.java 1292521 trunk/src/test/org/apache/hcatalog/pig/TestHCatLoaderComplexSchema.java 1292521 Diff: https://reviews.apache.org/r/3945/diff Testing ------- Thanks, Vikram
          Hide
          jiraposter@reviews.apache.org added a comment -

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

          (Updated 2012-02-24 03:32:30.165133)

          Review request for Sushanth Sowmyan and Francis Liu.

          Changes
          -------

          Incorporated Sushanth's comments. Quite some refactoring done and re-enabled couple of the unit-tests related to LazyHCatRecord. All test cases pass with JIRA-270 changes incorporated.

          Summary
          -------

          Incorporated Francis' comments.

          This addresses bug HCATALOG-239.
          https://issues.apache.org/jira/browse/HCATALOG-239

          Diffs (updated)


          trunk/src/java/org/apache/hcatalog/common/ErrorType.java 1292521
          trunk/src/java/org/apache/hcatalog/common/HCatUtil.java 1292521
          trunk/src/java/org/apache/hcatalog/data/DataType.java 1292521
          trunk/src/java/org/apache/hcatalog/data/HCatRecordObjectInspectorFactory.java 1292521
          trunk/src/java/org/apache/hcatalog/data/LazyHCatRecord.java 1292521
          trunk/src/java/org/apache/hcatalog/mapreduce/DefaultOutputCommitterContainer.java 1292521
          trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputCommitterContainer.java 1292521
          trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputFormatContainer.java 1292521
          trunk/src/java/org/apache/hcatalog/mapreduce/FosterStorageHandler.java 1292521
          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java 1292521
          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseOutputFormat.java 1292521
          trunk/src/java/org/apache/hcatalog/mapreduce/HCatOutputFormat.java 1292521
          trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java 1292521
          trunk/src/java/org/apache/hcatalog/mapreduce/HCatSplit.java 1292521
          trunk/src/java/org/apache/hcatalog/mapreduce/HCatTableInfo.java 1292521
          trunk/src/java/org/apache/hcatalog/mapreduce/InitializeInput.java 1292521
          trunk/src/java/org/apache/hcatalog/mapreduce/InputJobInfo.java 1292521
          trunk/src/java/org/apache/hcatalog/mapreduce/InternalUtil.java 1292521
          trunk/src/java/org/apache/hcatalog/mapreduce/PartInfo.java 1292521
          trunk/src/java/org/apache/hcatalog/pig/HCatLoader.java 1292521
          trunk/src/test/org/apache/hcatalog/data/TestHCatRecordSerDe.java 1292521
          trunk/src/test/org/apache/hcatalog/data/TestLazyHCatRecord.java 1292521
          trunk/src/test/org/apache/hcatalog/mapreduce/TestHCatHiveCompatibility.java 1292521
          trunk/src/test/org/apache/hcatalog/pig/TestHCatLoaderComplexSchema.java 1292521

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

          Testing
          -------

          Thanks,

          Vikram

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3945/ ----------------------------------------------------------- (Updated 2012-02-24 03:32:30.165133) Review request for Sushanth Sowmyan and Francis Liu. Changes ------- Incorporated Sushanth's comments. Quite some refactoring done and re-enabled couple of the unit-tests related to LazyHCatRecord. All test cases pass with JIRA-270 changes incorporated. Summary ------- Incorporated Francis' comments. This addresses bug HCATALOG-239 . https://issues.apache.org/jira/browse/HCATALOG-239 Diffs (updated) trunk/src/java/org/apache/hcatalog/common/ErrorType.java 1292521 trunk/src/java/org/apache/hcatalog/common/HCatUtil.java 1292521 trunk/src/java/org/apache/hcatalog/data/DataType.java 1292521 trunk/src/java/org/apache/hcatalog/data/HCatRecordObjectInspectorFactory.java 1292521 trunk/src/java/org/apache/hcatalog/data/LazyHCatRecord.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/DefaultOutputCommitterContainer.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputCommitterContainer.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputFormatContainer.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/FosterStorageHandler.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseOutputFormat.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatOutputFormat.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatSplit.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatTableInfo.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/InitializeInput.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/InputJobInfo.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/InternalUtil.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/PartInfo.java 1292521 trunk/src/java/org/apache/hcatalog/pig/HCatLoader.java 1292521 trunk/src/test/org/apache/hcatalog/data/TestHCatRecordSerDe.java 1292521 trunk/src/test/org/apache/hcatalog/data/TestLazyHCatRecord.java 1292521 trunk/src/test/org/apache/hcatalog/mapreduce/TestHCatHiveCompatibility.java 1292521 trunk/src/test/org/apache/hcatalog/pig/TestHCatLoaderComplexSchema.java 1292521 Diff: https://reviews.apache.org/r/3945/diff Testing ------- Thanks, Vikram
          Hide
          Vikram Dixit K added a comment -

          Incorporated Sushanth's review comments.

          Show
          Vikram Dixit K added a comment - Incorporated Sushanth's review comments.
          Hide
          jiraposter@reviews.apache.org added a comment -

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

          trunk/src/java/org/apache/hcatalog/common/HCatUtil.java
          <https://reviews.apache.org/r/3945/#comment11582>

          Shouldn't this be stored in OutputJobInfo. This could be troublesome if you have multiple stores put in the same same pig MR Job?

          trunk/src/java/org/apache/hcatalog/common/HCatUtil.java
          <https://reviews.apache.org/r/3945/#comment11558>

          since you're just rethrowing might as well not catch the exception.

          trunk/src/java/org/apache/hcatalog/common/HCatUtil.java
          <https://reviews.apache.org/r/3945/#comment11559>

          clean up unused code.

          trunk/src/java/org/apache/hcatalog/common/HCatUtil.java
          <https://reviews.apache.org/r/3945/#comment11581>

          curly brackets should be at the end of the method.

          trunk/src/java/org/apache/hcatalog/data/LazyHCatRecord.java
          <https://reviews.apache.org/r/3945/#comment11574>

          Does it really ahve have to be throws exception? Can you be more specific?

          trunk/src/java/org/apache/hcatalog/mapreduce/FosterStorageHandler.java
          <https://reviews.apache.org/r/3945/#comment11580>

          not sure why it has to be serializable?

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java
          <https://reviews.apache.org/r/3945/#comment11583>

          We should prolly move this into InputJobInfo. If we can't do the refactor here please add a TODO.

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java
          <https://reviews.apache.org/r/3945/#comment11592>

          is this used?

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java
          <https://reviews.apache.org/r/3945/#comment11602>

          This doesn't look right, with this the current conf, partinfo will get the location of the previous partinfo? You really should put this in configureInput, that way partInto.jobProperties would've been set with the correct input locaiton prior to this.

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java
          <https://reviews.apache.org/r/3945/#comment11593>

          jobConf must be copied back to JobContext.

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java
          <https://reviews.apache.org/r/3945/#comment11589>

          you don't need the cast?

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java
          <https://reviews.apache.org/r/3945/#comment11590>

          remove unused code.

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java
          <https://reviews.apache.org/r/3945/#comment11594>

          Why not use ReflectionUtils?

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java
          <https://reviews.apache.org/r/3945/#comment11586>

          As I've mentioned before, this should go into FosterStorageHandler.configureInputJobProperties()

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatOutputFormat.java
          <https://reviews.apache.org/r/3945/#comment11591>

          I don't see why these have to be coming from hcatUtil instead of take from the conf?

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java
          <https://reviews.apache.org/r/3945/#comment11595>

          update comment

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java
          <https://reviews.apache.org/r/3945/#comment11597>

          be more specific?

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java
          <https://reviews.apache.org/r/3945/#comment11596>

          message?

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatSplit.java
          <https://reviews.apache.org/r/3945/#comment11598>

          curly bracket should be after method.

          trunk/src/java/org/apache/hcatalog/mapreduce/InitializeInput.java
          <https://reviews.apache.org/r/3945/#comment11600>

          does this have to be public?

          trunk/src/java/org/apache/hcatalog/mapreduce/InitializeInput.java
          <https://reviews.apache.org/r/3945/#comment11599>

          Message, IllegalStateException would be better

          trunk/src/java/org/apache/hcatalog/mapreduce/InitializeInput.java
          <https://reviews.apache.org/r/3945/#comment11601>

          This seems simple enough not to need a method?

          • Francis

          On 2012-02-24 03:32:30, Vikram Dixit Kumaraswamy wrote:

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

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

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

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

          (Updated 2012-02-24 03:32:30)

          Review request for Sushanth Sowmyan and Francis Liu.

          Summary

          -------

          Incorporated Francis' comments.

          This addresses bug HCATALOG-239.

          https://issues.apache.org/jira/browse/HCATALOG-239

          Diffs

          -----

          trunk/src/java/org/apache/hcatalog/common/ErrorType.java 1292521

          trunk/src/java/org/apache/hcatalog/common/HCatUtil.java 1292521

          trunk/src/java/org/apache/hcatalog/data/DataType.java 1292521

          trunk/src/java/org/apache/hcatalog/data/HCatRecordObjectInspectorFactory.java 1292521

          trunk/src/java/org/apache/hcatalog/data/LazyHCatRecord.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/DefaultOutputCommitterContainer.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputCommitterContainer.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputFormatContainer.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/FosterStorageHandler.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseOutputFormat.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatOutputFormat.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatSplit.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatTableInfo.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/InitializeInput.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/InputJobInfo.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/InternalUtil.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/PartInfo.java 1292521

          trunk/src/java/org/apache/hcatalog/pig/HCatLoader.java 1292521

          trunk/src/test/org/apache/hcatalog/data/TestHCatRecordSerDe.java 1292521

          trunk/src/test/org/apache/hcatalog/data/TestLazyHCatRecord.java 1292521

          trunk/src/test/org/apache/hcatalog/mapreduce/TestHCatHiveCompatibility.java 1292521

          trunk/src/test/org/apache/hcatalog/pig/TestHCatLoaderComplexSchema.java 1292521

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

          Testing

          -------

          Thanks,

          Vikram

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3945/#review5313 ----------------------------------------------------------- trunk/src/java/org/apache/hcatalog/common/HCatUtil.java < https://reviews.apache.org/r/3945/#comment11582 > Shouldn't this be stored in OutputJobInfo. This could be troublesome if you have multiple stores put in the same same pig MR Job? trunk/src/java/org/apache/hcatalog/common/HCatUtil.java < https://reviews.apache.org/r/3945/#comment11558 > since you're just rethrowing might as well not catch the exception. trunk/src/java/org/apache/hcatalog/common/HCatUtil.java < https://reviews.apache.org/r/3945/#comment11559 > clean up unused code. trunk/src/java/org/apache/hcatalog/common/HCatUtil.java < https://reviews.apache.org/r/3945/#comment11581 > curly brackets should be at the end of the method. trunk/src/java/org/apache/hcatalog/data/LazyHCatRecord.java < https://reviews.apache.org/r/3945/#comment11574 > Does it really ahve have to be throws exception? Can you be more specific? trunk/src/java/org/apache/hcatalog/mapreduce/FosterStorageHandler.java < https://reviews.apache.org/r/3945/#comment11580 > not sure why it has to be serializable? trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java < https://reviews.apache.org/r/3945/#comment11583 > We should prolly move this into InputJobInfo. If we can't do the refactor here please add a TODO. trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java < https://reviews.apache.org/r/3945/#comment11592 > is this used? trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java < https://reviews.apache.org/r/3945/#comment11602 > This doesn't look right, with this the current conf, partinfo will get the location of the previous partinfo? You really should put this in configureInput, that way partInto.jobProperties would've been set with the correct input locaiton prior to this. trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java < https://reviews.apache.org/r/3945/#comment11593 > jobConf must be copied back to JobContext. trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java < https://reviews.apache.org/r/3945/#comment11589 > you don't need the cast? trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java < https://reviews.apache.org/r/3945/#comment11590 > remove unused code. trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java < https://reviews.apache.org/r/3945/#comment11594 > Why not use ReflectionUtils? trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java < https://reviews.apache.org/r/3945/#comment11586 > As I've mentioned before, this should go into FosterStorageHandler.configureInputJobProperties() trunk/src/java/org/apache/hcatalog/mapreduce/HCatOutputFormat.java < https://reviews.apache.org/r/3945/#comment11591 > I don't see why these have to be coming from hcatUtil instead of take from the conf? trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java < https://reviews.apache.org/r/3945/#comment11595 > update comment trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java < https://reviews.apache.org/r/3945/#comment11597 > be more specific? trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java < https://reviews.apache.org/r/3945/#comment11596 > message? trunk/src/java/org/apache/hcatalog/mapreduce/HCatSplit.java < https://reviews.apache.org/r/3945/#comment11598 > curly bracket should be after method. trunk/src/java/org/apache/hcatalog/mapreduce/InitializeInput.java < https://reviews.apache.org/r/3945/#comment11600 > does this have to be public? trunk/src/java/org/apache/hcatalog/mapreduce/InitializeInput.java < https://reviews.apache.org/r/3945/#comment11599 > Message, IllegalStateException would be better trunk/src/java/org/apache/hcatalog/mapreduce/InitializeInput.java < https://reviews.apache.org/r/3945/#comment11601 > This seems simple enough not to need a method? Francis On 2012-02-24 03:32:30, Vikram Dixit Kumaraswamy wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3945/ ----------------------------------------------------------- (Updated 2012-02-24 03:32:30) Review request for Sushanth Sowmyan and Francis Liu. Summary ------- Incorporated Francis' comments. This addresses bug HCATALOG-239 . https://issues.apache.org/jira/browse/HCATALOG-239 Diffs ----- trunk/src/java/org/apache/hcatalog/common/ErrorType.java 1292521 trunk/src/java/org/apache/hcatalog/common/HCatUtil.java 1292521 trunk/src/java/org/apache/hcatalog/data/DataType.java 1292521 trunk/src/java/org/apache/hcatalog/data/HCatRecordObjectInspectorFactory.java 1292521 trunk/src/java/org/apache/hcatalog/data/LazyHCatRecord.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/DefaultOutputCommitterContainer.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputCommitterContainer.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputFormatContainer.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/FosterStorageHandler.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseOutputFormat.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatOutputFormat.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatSplit.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatTableInfo.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/InitializeInput.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/InputJobInfo.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/InternalUtil.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/PartInfo.java 1292521 trunk/src/java/org/apache/hcatalog/pig/HCatLoader.java 1292521 trunk/src/test/org/apache/hcatalog/data/TestHCatRecordSerDe.java 1292521 trunk/src/test/org/apache/hcatalog/data/TestLazyHCatRecord.java 1292521 trunk/src/test/org/apache/hcatalog/mapreduce/TestHCatHiveCompatibility.java 1292521 trunk/src/test/org/apache/hcatalog/pig/TestHCatLoaderComplexSchema.java 1292521 Diff: https://reviews.apache.org/r/3945/diff Testing ------- Thanks, Vikram
          Hide
          jiraposter@reviews.apache.org added a comment -

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

          trunk/src/java/org/apache/hcatalog/data/LazyHCatRecord.java
          <https://reviews.apache.org/r/3945/#comment11622>

          Will get Sushanth's comments and modify appropriately.

          trunk/src/java/org/apache/hcatalog/mapreduce/FosterStorageHandler.java
          <https://reviews.apache.org/r/3945/#comment11610>

          Artifact, removed.

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java
          <https://reviews.apache.org/r/3945/#comment11608>

          Removed. Artifact of earlier change.

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java
          <https://reviews.apache.org/r/3945/#comment11612>

          PartInfo.jobProperties is set as part of InitializeInput for each partition. I am not sure of partInfo will get the location of previous partInfo.. We should chat sometime today about this.

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java
          <https://reviews.apache.org/r/3945/#comment11607>

          Isn't the JobContext a local copy and not affect future computations even if modified at this stage?

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java
          <https://reviews.apache.org/r/3945/#comment11613>

          Removed.

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java
          <https://reviews.apache.org/r/3945/#comment11606>

          Done.

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java
          <https://reviews.apache.org/r/3945/#comment11605>

          Changed to use ReflectionUtils.

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatOutputFormat.java
          <https://reviews.apache.org/r/3945/#comment11603>

          After refactoring, OutputFormat no longer had the hiveConf available within the class. Changed that and moved these APIs to HCatOutputFormat.

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java
          <https://reviews.apache.org/r/3945/#comment11614>

          Done.

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java
          <https://reviews.apache.org/r/3945/#comment11615>

          LazyHCatRecord constructor throws Exception. Added a message to reflect the actual error.

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java
          <https://reviews.apache.org/r/3945/#comment11616>

          Done.

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatSplit.java
          <https://reviews.apache.org/r/3945/#comment11617>

          Done.

          trunk/src/java/org/apache/hcatalog/mapreduce/InitializeInput.java
          <https://reviews.apache.org/r/3945/#comment11618>

          Removed this artifact.

          trunk/src/java/org/apache/hcatalog/mapreduce/InitializeInput.java
          <https://reviews.apache.org/r/3945/#comment11621>

          Removed api.

          trunk/src/java/org/apache/hcatalog/mapreduce/InitializeInput.java
          <https://reviews.apache.org/r/3945/#comment11619>

          Removed artifact.

          • Vikram

          On 2012-02-24 03:32:30, Vikram Dixit Kumaraswamy wrote:

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

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

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

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

          (Updated 2012-02-24 03:32:30)

          Review request for Sushanth Sowmyan and Francis Liu.

          Summary

          -------

          Incorporated Francis' comments.

          This addresses bug HCATALOG-239.

          https://issues.apache.org/jira/browse/HCATALOG-239

          Diffs

          -----

          trunk/src/java/org/apache/hcatalog/common/ErrorType.java 1292521

          trunk/src/java/org/apache/hcatalog/common/HCatUtil.java 1292521

          trunk/src/java/org/apache/hcatalog/data/DataType.java 1292521

          trunk/src/java/org/apache/hcatalog/data/HCatRecordObjectInspectorFactory.java 1292521

          trunk/src/java/org/apache/hcatalog/data/LazyHCatRecord.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/DefaultOutputCommitterContainer.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputCommitterContainer.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputFormatContainer.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/FosterStorageHandler.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseOutputFormat.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatOutputFormat.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatSplit.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatTableInfo.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/InitializeInput.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/InputJobInfo.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/InternalUtil.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/PartInfo.java 1292521

          trunk/src/java/org/apache/hcatalog/pig/HCatLoader.java 1292521

          trunk/src/test/org/apache/hcatalog/data/TestHCatRecordSerDe.java 1292521

          trunk/src/test/org/apache/hcatalog/data/TestLazyHCatRecord.java 1292521

          trunk/src/test/org/apache/hcatalog/mapreduce/TestHCatHiveCompatibility.java 1292521

          trunk/src/test/org/apache/hcatalog/pig/TestHCatLoaderComplexSchema.java 1292521

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

          Testing

          -------

          Thanks,

          Vikram

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3945/#review5321 ----------------------------------------------------------- trunk/src/java/org/apache/hcatalog/data/LazyHCatRecord.java < https://reviews.apache.org/r/3945/#comment11622 > Will get Sushanth's comments and modify appropriately. trunk/src/java/org/apache/hcatalog/mapreduce/FosterStorageHandler.java < https://reviews.apache.org/r/3945/#comment11610 > Artifact, removed. trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java < https://reviews.apache.org/r/3945/#comment11608 > Removed. Artifact of earlier change. trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java < https://reviews.apache.org/r/3945/#comment11612 > PartInfo.jobProperties is set as part of InitializeInput for each partition. I am not sure of partInfo will get the location of previous partInfo.. We should chat sometime today about this. trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java < https://reviews.apache.org/r/3945/#comment11607 > Isn't the JobContext a local copy and not affect future computations even if modified at this stage? trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java < https://reviews.apache.org/r/3945/#comment11613 > Removed. trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java < https://reviews.apache.org/r/3945/#comment11606 > Done. trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java < https://reviews.apache.org/r/3945/#comment11605 > Changed to use ReflectionUtils. trunk/src/java/org/apache/hcatalog/mapreduce/HCatOutputFormat.java < https://reviews.apache.org/r/3945/#comment11603 > After refactoring, OutputFormat no longer had the hiveConf available within the class. Changed that and moved these APIs to HCatOutputFormat. trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java < https://reviews.apache.org/r/3945/#comment11614 > Done. trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java < https://reviews.apache.org/r/3945/#comment11615 > LazyHCatRecord constructor throws Exception. Added a message to reflect the actual error. trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java < https://reviews.apache.org/r/3945/#comment11616 > Done. trunk/src/java/org/apache/hcatalog/mapreduce/HCatSplit.java < https://reviews.apache.org/r/3945/#comment11617 > Done. trunk/src/java/org/apache/hcatalog/mapreduce/InitializeInput.java < https://reviews.apache.org/r/3945/#comment11618 > Removed this artifact. trunk/src/java/org/apache/hcatalog/mapreduce/InitializeInput.java < https://reviews.apache.org/r/3945/#comment11621 > Removed api. trunk/src/java/org/apache/hcatalog/mapreduce/InitializeInput.java < https://reviews.apache.org/r/3945/#comment11619 > Removed artifact. Vikram On 2012-02-24 03:32:30, Vikram Dixit Kumaraswamy wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3945/ ----------------------------------------------------------- (Updated 2012-02-24 03:32:30) Review request for Sushanth Sowmyan and Francis Liu. Summary ------- Incorporated Francis' comments. This addresses bug HCATALOG-239 . https://issues.apache.org/jira/browse/HCATALOG-239 Diffs ----- trunk/src/java/org/apache/hcatalog/common/ErrorType.java 1292521 trunk/src/java/org/apache/hcatalog/common/HCatUtil.java 1292521 trunk/src/java/org/apache/hcatalog/data/DataType.java 1292521 trunk/src/java/org/apache/hcatalog/data/HCatRecordObjectInspectorFactory.java 1292521 trunk/src/java/org/apache/hcatalog/data/LazyHCatRecord.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/DefaultOutputCommitterContainer.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputCommitterContainer.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputFormatContainer.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/FosterStorageHandler.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseOutputFormat.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatOutputFormat.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatSplit.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatTableInfo.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/InitializeInput.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/InputJobInfo.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/InternalUtil.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/PartInfo.java 1292521 trunk/src/java/org/apache/hcatalog/pig/HCatLoader.java 1292521 trunk/src/test/org/apache/hcatalog/data/TestHCatRecordSerDe.java 1292521 trunk/src/test/org/apache/hcatalog/data/TestLazyHCatRecord.java 1292521 trunk/src/test/org/apache/hcatalog/mapreduce/TestHCatHiveCompatibility.java 1292521 trunk/src/test/org/apache/hcatalog/pig/TestHCatLoaderComplexSchema.java 1292521 Diff: https://reviews.apache.org/r/3945/diff Testing ------- Thanks, Vikram
          Hide
          jiraposter@reviews.apache.org added a comment -

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

          trunk/src/java/org/apache/hcatalog/common/HCatUtil.java
          <https://reviews.apache.org/r/3945/#comment11611>

          remove TODO?

          trunk/src/java/org/apache/hcatalog/mapreduce/InitializeInput.java
          <https://reviews.apache.org/r/3945/#comment11624>

          There are two extract storerInfo methods, one here and one in internalUtil. Let's pick one.

          trunk/src/java/org/apache/hcatalog/mapreduce/InitializeInput.java
          <https://reviews.apache.org/r/3945/#comment11623>

          can we remove this restriction to hcat only properties?

          • Francis

          On 2012-02-24 03:32:30, Vikram Dixit Kumaraswamy wrote:

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

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

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

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

          (Updated 2012-02-24 03:32:30)

          Review request for Sushanth Sowmyan and Francis Liu.

          Summary

          -------

          Incorporated Francis' comments.

          This addresses bug HCATALOG-239.

          https://issues.apache.org/jira/browse/HCATALOG-239

          Diffs

          -----

          trunk/src/java/org/apache/hcatalog/common/ErrorType.java 1292521

          trunk/src/java/org/apache/hcatalog/common/HCatUtil.java 1292521

          trunk/src/java/org/apache/hcatalog/data/DataType.java 1292521

          trunk/src/java/org/apache/hcatalog/data/HCatRecordObjectInspectorFactory.java 1292521

          trunk/src/java/org/apache/hcatalog/data/LazyHCatRecord.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/DefaultOutputCommitterContainer.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputCommitterContainer.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputFormatContainer.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/FosterStorageHandler.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseOutputFormat.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatOutputFormat.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatSplit.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatTableInfo.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/InitializeInput.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/InputJobInfo.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/InternalUtil.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/PartInfo.java 1292521

          trunk/src/java/org/apache/hcatalog/pig/HCatLoader.java 1292521

          trunk/src/test/org/apache/hcatalog/data/TestHCatRecordSerDe.java 1292521

          trunk/src/test/org/apache/hcatalog/data/TestLazyHCatRecord.java 1292521

          trunk/src/test/org/apache/hcatalog/mapreduce/TestHCatHiveCompatibility.java 1292521

          trunk/src/test/org/apache/hcatalog/pig/TestHCatLoaderComplexSchema.java 1292521

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

          Testing

          -------

          Thanks,

          Vikram

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3945/#review5323 ----------------------------------------------------------- trunk/src/java/org/apache/hcatalog/common/HCatUtil.java < https://reviews.apache.org/r/3945/#comment11611 > remove TODO? trunk/src/java/org/apache/hcatalog/mapreduce/InitializeInput.java < https://reviews.apache.org/r/3945/#comment11624 > There are two extract storerInfo methods, one here and one in internalUtil. Let's pick one. trunk/src/java/org/apache/hcatalog/mapreduce/InitializeInput.java < https://reviews.apache.org/r/3945/#comment11623 > can we remove this restriction to hcat only properties? Francis On 2012-02-24 03:32:30, Vikram Dixit Kumaraswamy wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3945/ ----------------------------------------------------------- (Updated 2012-02-24 03:32:30) Review request for Sushanth Sowmyan and Francis Liu. Summary ------- Incorporated Francis' comments. This addresses bug HCATALOG-239 . https://issues.apache.org/jira/browse/HCATALOG-239 Diffs ----- trunk/src/java/org/apache/hcatalog/common/ErrorType.java 1292521 trunk/src/java/org/apache/hcatalog/common/HCatUtil.java 1292521 trunk/src/java/org/apache/hcatalog/data/DataType.java 1292521 trunk/src/java/org/apache/hcatalog/data/HCatRecordObjectInspectorFactory.java 1292521 trunk/src/java/org/apache/hcatalog/data/LazyHCatRecord.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/DefaultOutputCommitterContainer.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputCommitterContainer.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputFormatContainer.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/FosterStorageHandler.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseOutputFormat.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatOutputFormat.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatSplit.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatTableInfo.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/InitializeInput.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/InputJobInfo.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/InternalUtil.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/PartInfo.java 1292521 trunk/src/java/org/apache/hcatalog/pig/HCatLoader.java 1292521 trunk/src/test/org/apache/hcatalog/data/TestHCatRecordSerDe.java 1292521 trunk/src/test/org/apache/hcatalog/data/TestLazyHCatRecord.java 1292521 trunk/src/test/org/apache/hcatalog/mapreduce/TestHCatHiveCompatibility.java 1292521 trunk/src/test/org/apache/hcatalog/pig/TestHCatLoaderComplexSchema.java 1292521 Diff: https://reviews.apache.org/r/3945/diff Testing ------- Thanks, Vikram
          Hide
          jiraposter@reviews.apache.org added a comment -

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

          trunk/src/java/org/apache/hcatalog/mapreduce/InitializeInput.java
          <https://reviews.apache.org/r/3945/#comment11626>

          remove restriction

          • Francis

          On 2012-02-24 03:32:30, Vikram Dixit Kumaraswamy wrote:

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

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

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

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

          (Updated 2012-02-24 03:32:30)

          Review request for Sushanth Sowmyan and Francis Liu.

          Summary

          -------

          Incorporated Francis' comments.

          This addresses bug HCATALOG-239.

          https://issues.apache.org/jira/browse/HCATALOG-239

          Diffs

          -----

          trunk/src/java/org/apache/hcatalog/common/ErrorType.java 1292521

          trunk/src/java/org/apache/hcatalog/common/HCatUtil.java 1292521

          trunk/src/java/org/apache/hcatalog/data/DataType.java 1292521

          trunk/src/java/org/apache/hcatalog/data/HCatRecordObjectInspectorFactory.java 1292521

          trunk/src/java/org/apache/hcatalog/data/LazyHCatRecord.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/DefaultOutputCommitterContainer.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputCommitterContainer.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputFormatContainer.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/FosterStorageHandler.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseOutputFormat.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatOutputFormat.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatSplit.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatTableInfo.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/InitializeInput.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/InputJobInfo.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/InternalUtil.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/PartInfo.java 1292521

          trunk/src/java/org/apache/hcatalog/pig/HCatLoader.java 1292521

          trunk/src/test/org/apache/hcatalog/data/TestHCatRecordSerDe.java 1292521

          trunk/src/test/org/apache/hcatalog/data/TestLazyHCatRecord.java 1292521

          trunk/src/test/org/apache/hcatalog/mapreduce/TestHCatHiveCompatibility.java 1292521

          trunk/src/test/org/apache/hcatalog/pig/TestHCatLoaderComplexSchema.java 1292521

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

          Testing

          -------

          Thanks,

          Vikram

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3945/#review5326 ----------------------------------------------------------- trunk/src/java/org/apache/hcatalog/mapreduce/InitializeInput.java < https://reviews.apache.org/r/3945/#comment11626 > remove restriction Francis On 2012-02-24 03:32:30, Vikram Dixit Kumaraswamy wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3945/ ----------------------------------------------------------- (Updated 2012-02-24 03:32:30) Review request for Sushanth Sowmyan and Francis Liu. Summary ------- Incorporated Francis' comments. This addresses bug HCATALOG-239 . https://issues.apache.org/jira/browse/HCATALOG-239 Diffs ----- trunk/src/java/org/apache/hcatalog/common/ErrorType.java 1292521 trunk/src/java/org/apache/hcatalog/common/HCatUtil.java 1292521 trunk/src/java/org/apache/hcatalog/data/DataType.java 1292521 trunk/src/java/org/apache/hcatalog/data/HCatRecordObjectInspectorFactory.java 1292521 trunk/src/java/org/apache/hcatalog/data/LazyHCatRecord.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/DefaultOutputCommitterContainer.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputCommitterContainer.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputFormatContainer.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/FosterStorageHandler.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseOutputFormat.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatOutputFormat.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatSplit.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatTableInfo.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/InitializeInput.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/InputJobInfo.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/InternalUtil.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/PartInfo.java 1292521 trunk/src/java/org/apache/hcatalog/pig/HCatLoader.java 1292521 trunk/src/test/org/apache/hcatalog/data/TestHCatRecordSerDe.java 1292521 trunk/src/test/org/apache/hcatalog/data/TestLazyHCatRecord.java 1292521 trunk/src/test/org/apache/hcatalog/mapreduce/TestHCatHiveCompatibility.java 1292521 trunk/src/test/org/apache/hcatalog/pig/TestHCatLoaderComplexSchema.java 1292521 Diff: https://reviews.apache.org/r/3945/diff Testing ------- Thanks, Vikram
          Hide
          jiraposter@reviews.apache.org added a comment -

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

          trunk/src/java/org/apache/hcatalog/mapreduce/InternalUtil.java
          <https://reviews.apache.org/r/3945/#comment11627>

          you can already clean this up. the output part is gone.

          • Francis

          On 2012-02-24 03:32:30, Vikram Dixit Kumaraswamy wrote:

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

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

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

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

          (Updated 2012-02-24 03:32:30)

          Review request for Sushanth Sowmyan and Francis Liu.

          Summary

          -------

          Incorporated Francis' comments.

          This addresses bug HCATALOG-239.

          https://issues.apache.org/jira/browse/HCATALOG-239

          Diffs

          -----

          trunk/src/java/org/apache/hcatalog/common/ErrorType.java 1292521

          trunk/src/java/org/apache/hcatalog/common/HCatUtil.java 1292521

          trunk/src/java/org/apache/hcatalog/data/DataType.java 1292521

          trunk/src/java/org/apache/hcatalog/data/HCatRecordObjectInspectorFactory.java 1292521

          trunk/src/java/org/apache/hcatalog/data/LazyHCatRecord.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/DefaultOutputCommitterContainer.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputCommitterContainer.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputFormatContainer.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/FosterStorageHandler.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseOutputFormat.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatOutputFormat.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatSplit.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatTableInfo.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/InitializeInput.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/InputJobInfo.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/InternalUtil.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/PartInfo.java 1292521

          trunk/src/java/org/apache/hcatalog/pig/HCatLoader.java 1292521

          trunk/src/test/org/apache/hcatalog/data/TestHCatRecordSerDe.java 1292521

          trunk/src/test/org/apache/hcatalog/data/TestLazyHCatRecord.java 1292521

          trunk/src/test/org/apache/hcatalog/mapreduce/TestHCatHiveCompatibility.java 1292521

          trunk/src/test/org/apache/hcatalog/pig/TestHCatLoaderComplexSchema.java 1292521

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

          Testing

          -------

          Thanks,

          Vikram

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3945/#review5327 ----------------------------------------------------------- trunk/src/java/org/apache/hcatalog/mapreduce/InternalUtil.java < https://reviews.apache.org/r/3945/#comment11627 > you can already clean this up. the output part is gone. Francis On 2012-02-24 03:32:30, Vikram Dixit Kumaraswamy wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3945/ ----------------------------------------------------------- (Updated 2012-02-24 03:32:30) Review request for Sushanth Sowmyan and Francis Liu. Summary ------- Incorporated Francis' comments. This addresses bug HCATALOG-239 . https://issues.apache.org/jira/browse/HCATALOG-239 Diffs ----- trunk/src/java/org/apache/hcatalog/common/ErrorType.java 1292521 trunk/src/java/org/apache/hcatalog/common/HCatUtil.java 1292521 trunk/src/java/org/apache/hcatalog/data/DataType.java 1292521 trunk/src/java/org/apache/hcatalog/data/HCatRecordObjectInspectorFactory.java 1292521 trunk/src/java/org/apache/hcatalog/data/LazyHCatRecord.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/DefaultOutputCommitterContainer.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputCommitterContainer.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputFormatContainer.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/FosterStorageHandler.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseOutputFormat.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatOutputFormat.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatSplit.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatTableInfo.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/InitializeInput.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/InputJobInfo.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/InternalUtil.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/PartInfo.java 1292521 trunk/src/java/org/apache/hcatalog/pig/HCatLoader.java 1292521 trunk/src/test/org/apache/hcatalog/data/TestHCatRecordSerDe.java 1292521 trunk/src/test/org/apache/hcatalog/data/TestLazyHCatRecord.java 1292521 trunk/src/test/org/apache/hcatalog/mapreduce/TestHCatHiveCompatibility.java 1292521 trunk/src/test/org/apache/hcatalog/pig/TestHCatLoaderComplexSchema.java 1292521 Diff: https://reviews.apache.org/r/3945/diff Testing ------- Thanks, Vikram
          Hide
          jiraposter@reviews.apache.org added a comment -

          On 2012-02-24 17:36:48, Vikram Dixit Kumaraswamy wrote:

          > trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java, line 171

          > <https://reviews.apache.org/r/3945/diff/2/?file=85622#file85622line171>

          >

          > Isn't the JobContext a local copy and not affect future computations even if modified at this stage?

          stuff that's set in jobcontext here gets passed downstream if not then all the setting jobconf here would be all for nothing .

          On 2012-02-24 17:36:48, Vikram Dixit Kumaraswamy wrote:

          > trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java, line 144

          > <https://reviews.apache.org/r/3945/diff/2/?file=85622#file85622line144>

          >

          > PartInfo.jobProperties is set as part of InitializeInput for each partition. I am not sure of partInfo will get the location of previous partInfo.. We should chat sometime today about this.

          Hmmm it's a bit hard to explain, can you see the difference between the two? In any case this is file specific stuff it would be cleaner to put setInputPath in FosterStorageHandler.

          Job localJob = new Job(conf);
          setInputPath(jobContext, partitionInfo.getLocation());

          and

          setInputPath(jobContext, partitionInfo.getLocation());
          Job localJob = new Job(conf);

          • Francis

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

          On 2012-02-24 03:32:30, Vikram Dixit Kumaraswamy wrote:

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

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

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

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

          (Updated 2012-02-24 03:32:30)

          Review request for Sushanth Sowmyan and Francis Liu.

          Summary

          -------

          Incorporated Francis' comments.

          This addresses bug HCATALOG-239.

          https://issues.apache.org/jira/browse/HCATALOG-239

          Diffs

          -----

          trunk/src/java/org/apache/hcatalog/common/ErrorType.java 1292521

          trunk/src/java/org/apache/hcatalog/common/HCatUtil.java 1292521

          trunk/src/java/org/apache/hcatalog/data/DataType.java 1292521

          trunk/src/java/org/apache/hcatalog/data/HCatRecordObjectInspectorFactory.java 1292521

          trunk/src/java/org/apache/hcatalog/data/LazyHCatRecord.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/DefaultOutputCommitterContainer.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputCommitterContainer.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputFormatContainer.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/FosterStorageHandler.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseOutputFormat.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatOutputFormat.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatSplit.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatTableInfo.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/InitializeInput.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/InputJobInfo.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/InternalUtil.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/PartInfo.java 1292521

          trunk/src/java/org/apache/hcatalog/pig/HCatLoader.java 1292521

          trunk/src/test/org/apache/hcatalog/data/TestHCatRecordSerDe.java 1292521

          trunk/src/test/org/apache/hcatalog/data/TestLazyHCatRecord.java 1292521

          trunk/src/test/org/apache/hcatalog/mapreduce/TestHCatHiveCompatibility.java 1292521

          trunk/src/test/org/apache/hcatalog/pig/TestHCatLoaderComplexSchema.java 1292521

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

          Testing

          -------

          Thanks,

          Vikram

          Show
          jiraposter@reviews.apache.org added a comment - On 2012-02-24 17:36:48, Vikram Dixit Kumaraswamy wrote: > trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java, line 171 > < https://reviews.apache.org/r/3945/diff/2/?file=85622#file85622line171 > > > Isn't the JobContext a local copy and not affect future computations even if modified at this stage? stuff that's set in jobcontext here gets passed downstream if not then all the setting jobconf here would be all for nothing . On 2012-02-24 17:36:48, Vikram Dixit Kumaraswamy wrote: > trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java, line 144 > < https://reviews.apache.org/r/3945/diff/2/?file=85622#file85622line144 > > > PartInfo.jobProperties is set as part of InitializeInput for each partition. I am not sure of partInfo will get the location of previous partInfo.. We should chat sometime today about this. Hmmm it's a bit hard to explain, can you see the difference between the two? In any case this is file specific stuff it would be cleaner to put setInputPath in FosterStorageHandler. Job localJob = new Job(conf); setInputPath(jobContext, partitionInfo.getLocation()); and setInputPath(jobContext, partitionInfo.getLocation()); Job localJob = new Job(conf); Francis ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3945/#review5321 ----------------------------------------------------------- On 2012-02-24 03:32:30, Vikram Dixit Kumaraswamy wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3945/ ----------------------------------------------------------- (Updated 2012-02-24 03:32:30) Review request for Sushanth Sowmyan and Francis Liu. Summary ------- Incorporated Francis' comments. This addresses bug HCATALOG-239 . https://issues.apache.org/jira/browse/HCATALOG-239 Diffs ----- trunk/src/java/org/apache/hcatalog/common/ErrorType.java 1292521 trunk/src/java/org/apache/hcatalog/common/HCatUtil.java 1292521 trunk/src/java/org/apache/hcatalog/data/DataType.java 1292521 trunk/src/java/org/apache/hcatalog/data/HCatRecordObjectInspectorFactory.java 1292521 trunk/src/java/org/apache/hcatalog/data/LazyHCatRecord.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/DefaultOutputCommitterContainer.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputCommitterContainer.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputFormatContainer.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/FosterStorageHandler.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseOutputFormat.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatOutputFormat.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatSplit.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatTableInfo.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/InitializeInput.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/InputJobInfo.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/InternalUtil.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/PartInfo.java 1292521 trunk/src/java/org/apache/hcatalog/pig/HCatLoader.java 1292521 trunk/src/test/org/apache/hcatalog/data/TestHCatRecordSerDe.java 1292521 trunk/src/test/org/apache/hcatalog/data/TestLazyHCatRecord.java 1292521 trunk/src/test/org/apache/hcatalog/mapreduce/TestHCatHiveCompatibility.java 1292521 trunk/src/test/org/apache/hcatalog/pig/TestHCatLoaderComplexSchema.java 1292521 Diff: https://reviews.apache.org/r/3945/diff Testing ------- Thanks, Vikram
          Hide
          jiraposter@reviews.apache.org added a comment -

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

          trunk/src/java/org/apache/hcatalog/mapreduce/FosterStorageHandler.java
          <https://reviews.apache.org/r/3945/#comment11632>

          Actually needed it for unit-test to pass.

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java
          <https://reviews.apache.org/r/3945/#comment11630>

          As far as my understanding goes, the jobConf is dropped as well which is why I reconstruct it again in createRecordReader time. The jobConf in getSplits is just to provide the conf for the underlying getSplits. I would like to discuss and update accordingly.

          • Vikram

          On 2012-02-24 03:32:30, Vikram Dixit Kumaraswamy wrote:

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

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

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

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

          (Updated 2012-02-24 03:32:30)

          Review request for Sushanth Sowmyan and Francis Liu.

          Summary

          -------

          Incorporated Francis' comments.

          This addresses bug HCATALOG-239.

          https://issues.apache.org/jira/browse/HCATALOG-239

          Diffs

          -----

          trunk/src/java/org/apache/hcatalog/common/ErrorType.java 1292521

          trunk/src/java/org/apache/hcatalog/common/HCatUtil.java 1292521

          trunk/src/java/org/apache/hcatalog/data/DataType.java 1292521

          trunk/src/java/org/apache/hcatalog/data/HCatRecordObjectInspectorFactory.java 1292521

          trunk/src/java/org/apache/hcatalog/data/LazyHCatRecord.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/DefaultOutputCommitterContainer.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputCommitterContainer.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputFormatContainer.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/FosterStorageHandler.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseOutputFormat.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatOutputFormat.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatSplit.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatTableInfo.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/InitializeInput.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/InputJobInfo.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/InternalUtil.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/PartInfo.java 1292521

          trunk/src/java/org/apache/hcatalog/pig/HCatLoader.java 1292521

          trunk/src/test/org/apache/hcatalog/data/TestHCatRecordSerDe.java 1292521

          trunk/src/test/org/apache/hcatalog/data/TestLazyHCatRecord.java 1292521

          trunk/src/test/org/apache/hcatalog/mapreduce/TestHCatHiveCompatibility.java 1292521

          trunk/src/test/org/apache/hcatalog/pig/TestHCatLoaderComplexSchema.java 1292521

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

          Testing

          -------

          Thanks,

          Vikram

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3945/#review5329 ----------------------------------------------------------- trunk/src/java/org/apache/hcatalog/mapreduce/FosterStorageHandler.java < https://reviews.apache.org/r/3945/#comment11632 > Actually needed it for unit-test to pass. trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java < https://reviews.apache.org/r/3945/#comment11630 > As far as my understanding goes, the jobConf is dropped as well which is why I reconstruct it again in createRecordReader time. The jobConf in getSplits is just to provide the conf for the underlying getSplits. I would like to discuss and update accordingly. Vikram On 2012-02-24 03:32:30, Vikram Dixit Kumaraswamy wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3945/ ----------------------------------------------------------- (Updated 2012-02-24 03:32:30) Review request for Sushanth Sowmyan and Francis Liu. Summary ------- Incorporated Francis' comments. This addresses bug HCATALOG-239 . https://issues.apache.org/jira/browse/HCATALOG-239 Diffs ----- trunk/src/java/org/apache/hcatalog/common/ErrorType.java 1292521 trunk/src/java/org/apache/hcatalog/common/HCatUtil.java 1292521 trunk/src/java/org/apache/hcatalog/data/DataType.java 1292521 trunk/src/java/org/apache/hcatalog/data/HCatRecordObjectInspectorFactory.java 1292521 trunk/src/java/org/apache/hcatalog/data/LazyHCatRecord.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/DefaultOutputCommitterContainer.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputCommitterContainer.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputFormatContainer.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/FosterStorageHandler.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseOutputFormat.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatOutputFormat.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatSplit.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatTableInfo.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/InitializeInput.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/InputJobInfo.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/InternalUtil.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/PartInfo.java 1292521 trunk/src/java/org/apache/hcatalog/pig/HCatLoader.java 1292521 trunk/src/test/org/apache/hcatalog/data/TestHCatRecordSerDe.java 1292521 trunk/src/test/org/apache/hcatalog/data/TestLazyHCatRecord.java 1292521 trunk/src/test/org/apache/hcatalog/mapreduce/TestHCatHiveCompatibility.java 1292521 trunk/src/test/org/apache/hcatalog/pig/TestHCatLoaderComplexSchema.java 1292521 Diff: https://reviews.apache.org/r/3945/diff Testing ------- Thanks, Vikram
          Hide
          jiraposter@reviews.apache.org added a comment -

          On 2012-02-24 17:52:26, Francis Liu wrote:

          > trunk/src/java/org/apache/hcatalog/mapreduce/InternalUtil.java, line 39

          > <https://reviews.apache.org/r/3945/diff/2-3/?file=85630#file85630line39>

          >

          > you can already clean this up. the output part is gone.

          This will be cleaned up as part of Daniel's patch removing ISD/OSD info. It's best to leave this here for now, since further tests that check for its existence still need to be changed.

          • Sushanth

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

          On 2012-02-24 03:32:30, Vikram Dixit Kumaraswamy wrote:

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

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

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

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

          (Updated 2012-02-24 03:32:30)

          Review request for Sushanth Sowmyan and Francis Liu.

          Summary

          -------

          Incorporated Francis' comments.

          This addresses bug HCATALOG-239.

          https://issues.apache.org/jira/browse/HCATALOG-239

          Diffs

          -----

          trunk/src/java/org/apache/hcatalog/common/ErrorType.java 1292521

          trunk/src/java/org/apache/hcatalog/common/HCatUtil.java 1292521

          trunk/src/java/org/apache/hcatalog/data/DataType.java 1292521

          trunk/src/java/org/apache/hcatalog/data/HCatRecordObjectInspectorFactory.java 1292521

          trunk/src/java/org/apache/hcatalog/data/LazyHCatRecord.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/DefaultOutputCommitterContainer.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputCommitterContainer.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputFormatContainer.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/FosterStorageHandler.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseOutputFormat.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatOutputFormat.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatSplit.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatTableInfo.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/InitializeInput.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/InputJobInfo.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/InternalUtil.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/PartInfo.java 1292521

          trunk/src/java/org/apache/hcatalog/pig/HCatLoader.java 1292521

          trunk/src/test/org/apache/hcatalog/data/TestHCatRecordSerDe.java 1292521

          trunk/src/test/org/apache/hcatalog/data/TestLazyHCatRecord.java 1292521

          trunk/src/test/org/apache/hcatalog/mapreduce/TestHCatHiveCompatibility.java 1292521

          trunk/src/test/org/apache/hcatalog/pig/TestHCatLoaderComplexSchema.java 1292521

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

          Testing

          -------

          Thanks,

          Vikram

          Show
          jiraposter@reviews.apache.org added a comment - On 2012-02-24 17:52:26, Francis Liu wrote: > trunk/src/java/org/apache/hcatalog/mapreduce/InternalUtil.java, line 39 > < https://reviews.apache.org/r/3945/diff/2-3/?file=85630#file85630line39 > > > you can already clean this up. the output part is gone. This will be cleaned up as part of Daniel's patch removing ISD/OSD info. It's best to leave this here for now, since further tests that check for its existence still need to be changed. Sushanth ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3945/#review5327 ----------------------------------------------------------- On 2012-02-24 03:32:30, Vikram Dixit Kumaraswamy wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3945/ ----------------------------------------------------------- (Updated 2012-02-24 03:32:30) Review request for Sushanth Sowmyan and Francis Liu. Summary ------- Incorporated Francis' comments. This addresses bug HCATALOG-239 . https://issues.apache.org/jira/browse/HCATALOG-239 Diffs ----- trunk/src/java/org/apache/hcatalog/common/ErrorType.java 1292521 trunk/src/java/org/apache/hcatalog/common/HCatUtil.java 1292521 trunk/src/java/org/apache/hcatalog/data/DataType.java 1292521 trunk/src/java/org/apache/hcatalog/data/HCatRecordObjectInspectorFactory.java 1292521 trunk/src/java/org/apache/hcatalog/data/LazyHCatRecord.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/DefaultOutputCommitterContainer.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputCommitterContainer.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputFormatContainer.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/FosterStorageHandler.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseOutputFormat.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatOutputFormat.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatSplit.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatTableInfo.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/InitializeInput.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/InputJobInfo.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/InternalUtil.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/PartInfo.java 1292521 trunk/src/java/org/apache/hcatalog/pig/HCatLoader.java 1292521 trunk/src/test/org/apache/hcatalog/data/TestHCatRecordSerDe.java 1292521 trunk/src/test/org/apache/hcatalog/data/TestLazyHCatRecord.java 1292521 trunk/src/test/org/apache/hcatalog/mapreduce/TestHCatHiveCompatibility.java 1292521 trunk/src/test/org/apache/hcatalog/pig/TestHCatLoaderComplexSchema.java 1292521 Diff: https://reviews.apache.org/r/3945/diff Testing ------- Thanks, Vikram
          Hide
          jiraposter@reviews.apache.org added a comment -

          On 2012-02-24 07:07:57, Francis Liu wrote:

          > trunk/src/java/org/apache/hcatalog/data/LazyHCatRecord.java, line 106

          > <https://reviews.apache.org/r/3945/diff/2/?file=85617#file85617line106>

          >

          > Does it really ahve have to be throws exception? Can you be more specific?

          It winds up being SerDeException, but we do not want to leak knowledge of that exception upstream. It could have been cast to IOException for clarity - that'd probably be a better ctor.

          On 2012-02-24 07:07:57, Francis Liu wrote:

          > trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java, line 144

          > <https://reviews.apache.org/r/3945/diff/2/?file=85622#file85622line144>

          >

          > This doesn't look right, with this the current conf, partinfo will get the location of the previous partinfo? You really should put this in configureInput, that way partInto.jobProperties would've been set with the correct input locaiton prior to this.

          I'm not sure about moving it to another location, but Francis is right - you need isolation of the confs being passed in to setInputPath. An easy way to do this is to change the signature of setInputPath to JobConf, since you're initializing a new one for each partition, and pass in the newly created JobConf to it.

          On 2012-02-24 07:07:57, Francis Liu wrote:

          > trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java, line 171

          > <https://reviews.apache.org/r/3945/diff/2/?file=85622#file85622line171>

          >

          > jobConf must be copied back to JobContext.

          Disagree. By the time getSplits is called, we have no more control over what gets stored in JobContext. There's no value in storing it back into JobContext, that info will be lost anyway.

          • Sushanth

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

          On 2012-02-24 03:32:30, Vikram Dixit Kumaraswamy wrote:

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

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

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

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

          (Updated 2012-02-24 03:32:30)

          Review request for Sushanth Sowmyan and Francis Liu.

          Summary

          -------

          Incorporated Francis' comments.

          This addresses bug HCATALOG-239.

          https://issues.apache.org/jira/browse/HCATALOG-239

          Diffs

          -----

          trunk/src/java/org/apache/hcatalog/common/ErrorType.java 1292521

          trunk/src/java/org/apache/hcatalog/common/HCatUtil.java 1292521

          trunk/src/java/org/apache/hcatalog/data/DataType.java 1292521

          trunk/src/java/org/apache/hcatalog/data/HCatRecordObjectInspectorFactory.java 1292521

          trunk/src/java/org/apache/hcatalog/data/LazyHCatRecord.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/DefaultOutputCommitterContainer.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputCommitterContainer.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputFormatContainer.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/FosterStorageHandler.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseOutputFormat.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatOutputFormat.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatSplit.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatTableInfo.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/InitializeInput.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/InputJobInfo.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/InternalUtil.java 1292521

          trunk/src/java/org/apache/hcatalog/mapreduce/PartInfo.java 1292521

          trunk/src/java/org/apache/hcatalog/pig/HCatLoader.java 1292521

          trunk/src/test/org/apache/hcatalog/data/TestHCatRecordSerDe.java 1292521

          trunk/src/test/org/apache/hcatalog/data/TestLazyHCatRecord.java 1292521

          trunk/src/test/org/apache/hcatalog/mapreduce/TestHCatHiveCompatibility.java 1292521

          trunk/src/test/org/apache/hcatalog/pig/TestHCatLoaderComplexSchema.java 1292521

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

          Testing

          -------

          Thanks,

          Vikram

          Show
          jiraposter@reviews.apache.org added a comment - On 2012-02-24 07:07:57, Francis Liu wrote: > trunk/src/java/org/apache/hcatalog/data/LazyHCatRecord.java, line 106 > < https://reviews.apache.org/r/3945/diff/2/?file=85617#file85617line106 > > > Does it really ahve have to be throws exception? Can you be more specific? It winds up being SerDeException, but we do not want to leak knowledge of that exception upstream. It could have been cast to IOException for clarity - that'd probably be a better ctor. On 2012-02-24 07:07:57, Francis Liu wrote: > trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java, line 144 > < https://reviews.apache.org/r/3945/diff/2/?file=85622#file85622line144 > > > This doesn't look right, with this the current conf, partinfo will get the location of the previous partinfo? You really should put this in configureInput, that way partInto.jobProperties would've been set with the correct input locaiton prior to this. I'm not sure about moving it to another location, but Francis is right - you need isolation of the confs being passed in to setInputPath. An easy way to do this is to change the signature of setInputPath to JobConf, since you're initializing a new one for each partition, and pass in the newly created JobConf to it. On 2012-02-24 07:07:57, Francis Liu wrote: > trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java, line 171 > < https://reviews.apache.org/r/3945/diff/2/?file=85622#file85622line171 > > > jobConf must be copied back to JobContext. Disagree. By the time getSplits is called, we have no more control over what gets stored in JobContext. There's no value in storing it back into JobContext, that info will be lost anyway. Sushanth ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3945/#review5313 ----------------------------------------------------------- On 2012-02-24 03:32:30, Vikram Dixit Kumaraswamy wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3945/ ----------------------------------------------------------- (Updated 2012-02-24 03:32:30) Review request for Sushanth Sowmyan and Francis Liu. Summary ------- Incorporated Francis' comments. This addresses bug HCATALOG-239 . https://issues.apache.org/jira/browse/HCATALOG-239 Diffs ----- trunk/src/java/org/apache/hcatalog/common/ErrorType.java 1292521 trunk/src/java/org/apache/hcatalog/common/HCatUtil.java 1292521 trunk/src/java/org/apache/hcatalog/data/DataType.java 1292521 trunk/src/java/org/apache/hcatalog/data/HCatRecordObjectInspectorFactory.java 1292521 trunk/src/java/org/apache/hcatalog/data/LazyHCatRecord.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/DefaultOutputCommitterContainer.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputCommitterContainer.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputFormatContainer.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/FosterStorageHandler.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseOutputFormat.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatOutputFormat.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatSplit.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/HCatTableInfo.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/InitializeInput.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/InputJobInfo.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/InternalUtil.java 1292521 trunk/src/java/org/apache/hcatalog/mapreduce/PartInfo.java 1292521 trunk/src/java/org/apache/hcatalog/pig/HCatLoader.java 1292521 trunk/src/test/org/apache/hcatalog/data/TestHCatRecordSerDe.java 1292521 trunk/src/test/org/apache/hcatalog/data/TestLazyHCatRecord.java 1292521 trunk/src/test/org/apache/hcatalog/mapreduce/TestHCatHiveCompatibility.java 1292521 trunk/src/test/org/apache/hcatalog/pig/TestHCatLoaderComplexSchema.java 1292521 Diff: https://reviews.apache.org/r/3945/diff Testing ------- Thanks, Vikram
          Hide
          Vikram Dixit K added a comment -

          Updated with Francis/Sushanth's comments.

          Show
          Vikram Dixit K added a comment - Updated with Francis/Sushanth's comments.
          Hide
          jiraposter@reviews.apache.org added a comment -

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

          (Updated 2012-02-24 21:00:23.771166)

          Review request for Sushanth Sowmyan and Francis Liu.

          Changes
          -------

          Incorporated Francis'/Sushanth's comments.

          Summary
          -------

          Incorporated Francis' comments.

          This addresses bug HCATALOG-239.
          https://issues.apache.org/jira/browse/HCATALOG-239

          Diffs (updated)


          trunk/src/java/org/apache/hcatalog/common/ErrorType.java 1293367
          trunk/src/java/org/apache/hcatalog/common/HCatUtil.java 1293367
          trunk/src/java/org/apache/hcatalog/data/DataType.java 1293367
          trunk/src/java/org/apache/hcatalog/data/HCatRecordObjectInspectorFactory.java 1293367
          trunk/src/java/org/apache/hcatalog/data/LazyHCatRecord.java 1293367
          trunk/src/java/org/apache/hcatalog/mapreduce/DefaultOutputCommitterContainer.java 1293367
          trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputCommitterContainer.java 1293367
          trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputFormatContainer.java 1293367
          trunk/src/java/org/apache/hcatalog/mapreduce/FosterStorageHandler.java 1293367
          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java 1293367
          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseOutputFormat.java 1293367
          trunk/src/java/org/apache/hcatalog/mapreduce/HCatOutputFormat.java 1293367
          trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java 1293367
          trunk/src/java/org/apache/hcatalog/mapreduce/HCatSplit.java 1293367
          trunk/src/java/org/apache/hcatalog/mapreduce/HCatTableInfo.java 1293367
          trunk/src/java/org/apache/hcatalog/mapreduce/InitializeInput.java 1293367
          trunk/src/java/org/apache/hcatalog/mapreduce/InputJobInfo.java 1293367
          trunk/src/java/org/apache/hcatalog/mapreduce/InternalUtil.java 1293367
          trunk/src/java/org/apache/hcatalog/mapreduce/PartInfo.java 1293367
          trunk/src/java/org/apache/hcatalog/pig/HCatLoader.java 1293367
          trunk/src/test/org/apache/hcatalog/data/TestHCatRecordSerDe.java 1293367
          trunk/src/test/org/apache/hcatalog/data/TestLazyHCatRecord.java 1293367
          trunk/src/test/org/apache/hcatalog/mapreduce/TestHCatHiveCompatibility.java 1293367
          trunk/src/test/org/apache/hcatalog/mapreduce/TestHCatOutputFormat.java 1293367
          trunk/src/test/org/apache/hcatalog/pig/TestHCatLoaderComplexSchema.java 1293367

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

          Testing
          -------

          Thanks,

          Vikram

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3945/ ----------------------------------------------------------- (Updated 2012-02-24 21:00:23.771166) Review request for Sushanth Sowmyan and Francis Liu. Changes ------- Incorporated Francis'/Sushanth's comments. Summary ------- Incorporated Francis' comments. This addresses bug HCATALOG-239 . https://issues.apache.org/jira/browse/HCATALOG-239 Diffs (updated) trunk/src/java/org/apache/hcatalog/common/ErrorType.java 1293367 trunk/src/java/org/apache/hcatalog/common/HCatUtil.java 1293367 trunk/src/java/org/apache/hcatalog/data/DataType.java 1293367 trunk/src/java/org/apache/hcatalog/data/HCatRecordObjectInspectorFactory.java 1293367 trunk/src/java/org/apache/hcatalog/data/LazyHCatRecord.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/DefaultOutputCommitterContainer.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputCommitterContainer.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputFormatContainer.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/FosterStorageHandler.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseOutputFormat.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/HCatOutputFormat.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/HCatSplit.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/HCatTableInfo.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/InitializeInput.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/InputJobInfo.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/InternalUtil.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/PartInfo.java 1293367 trunk/src/java/org/apache/hcatalog/pig/HCatLoader.java 1293367 trunk/src/test/org/apache/hcatalog/data/TestHCatRecordSerDe.java 1293367 trunk/src/test/org/apache/hcatalog/data/TestLazyHCatRecord.java 1293367 trunk/src/test/org/apache/hcatalog/mapreduce/TestHCatHiveCompatibility.java 1293367 trunk/src/test/org/apache/hcatalog/mapreduce/TestHCatOutputFormat.java 1293367 trunk/src/test/org/apache/hcatalog/pig/TestHCatLoaderComplexSchema.java 1293367 Diff: https://reviews.apache.org/r/3945/diff Testing ------- Thanks, Vikram
          Hide
          Sushanth Sowmyan added a comment -

          +1

          There's a bit more cleaning up/refactoring that's possible, but so as to not further complicate an already big patch, that can go in to another patch.

          I'm good to go for committing this.

          Show
          Sushanth Sowmyan added a comment - +1 There's a bit more cleaning up/refactoring that's possible, but so as to not further complicate an already big patch, that can go in to another patch. I'm good to go for committing this.
          Hide
          jiraposter@reviews.apache.org added a comment -

          On 2012-02-24 17:52:26, Francis Liu wrote:

          > trunk/src/java/org/apache/hcatalog/mapreduce/InternalUtil.java, line 39

          > <https://reviews.apache.org/r/3945/diff/2-3/?file=85630#file85630line39>

          >

          > you can already clean this up. the output part is gone.

          Sushanth Sowmyan wrote:

          This will be cleaned up as part of Daniel's patch removing ISD/OSD info. It's best to leave this here for now, since further tests that check for its existence still need to be changed.

          This code doesn't really do anything. If Daniel's working on removing this I don't mind, I'm just worried it's gonna be forgotten.

          • Francis

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

          On 2012-02-24 21:00:23, Vikram Dixit Kumaraswamy wrote:

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

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

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

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

          (Updated 2012-02-24 21:00:23)

          Review request for Sushanth Sowmyan and Francis Liu.

          Summary

          -------

          Incorporated Francis' comments.

          This addresses bug HCATALOG-239.

          https://issues.apache.org/jira/browse/HCATALOG-239

          Diffs

          -----

          trunk/src/java/org/apache/hcatalog/common/ErrorType.java 1293367

          trunk/src/java/org/apache/hcatalog/common/HCatUtil.java 1293367

          trunk/src/java/org/apache/hcatalog/data/DataType.java 1293367

          trunk/src/java/org/apache/hcatalog/data/HCatRecordObjectInspectorFactory.java 1293367

          trunk/src/java/org/apache/hcatalog/data/LazyHCatRecord.java 1293367

          trunk/src/java/org/apache/hcatalog/mapreduce/DefaultOutputCommitterContainer.java 1293367

          trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputCommitterContainer.java 1293367

          trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputFormatContainer.java 1293367

          trunk/src/java/org/apache/hcatalog/mapreduce/FosterStorageHandler.java 1293367

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java 1293367

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseOutputFormat.java 1293367

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatOutputFormat.java 1293367

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java 1293367

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatSplit.java 1293367

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatTableInfo.java 1293367

          trunk/src/java/org/apache/hcatalog/mapreduce/InitializeInput.java 1293367

          trunk/src/java/org/apache/hcatalog/mapreduce/InputJobInfo.java 1293367

          trunk/src/java/org/apache/hcatalog/mapreduce/InternalUtil.java 1293367

          trunk/src/java/org/apache/hcatalog/mapreduce/PartInfo.java 1293367

          trunk/src/java/org/apache/hcatalog/pig/HCatLoader.java 1293367

          trunk/src/test/org/apache/hcatalog/data/TestHCatRecordSerDe.java 1293367

          trunk/src/test/org/apache/hcatalog/data/TestLazyHCatRecord.java 1293367

          trunk/src/test/org/apache/hcatalog/mapreduce/TestHCatHiveCompatibility.java 1293367

          trunk/src/test/org/apache/hcatalog/mapreduce/TestHCatOutputFormat.java 1293367

          trunk/src/test/org/apache/hcatalog/pig/TestHCatLoaderComplexSchema.java 1293367

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

          Testing

          -------

          Thanks,

          Vikram

          Show
          jiraposter@reviews.apache.org added a comment - On 2012-02-24 17:52:26, Francis Liu wrote: > trunk/src/java/org/apache/hcatalog/mapreduce/InternalUtil.java, line 39 > < https://reviews.apache.org/r/3945/diff/2-3/?file=85630#file85630line39 > > > you can already clean this up. the output part is gone. Sushanth Sowmyan wrote: This will be cleaned up as part of Daniel's patch removing ISD/OSD info. It's best to leave this here for now, since further tests that check for its existence still need to be changed. This code doesn't really do anything. If Daniel's working on removing this I don't mind, I'm just worried it's gonna be forgotten. Francis ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3945/#review5327 ----------------------------------------------------------- On 2012-02-24 21:00:23, Vikram Dixit Kumaraswamy wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3945/ ----------------------------------------------------------- (Updated 2012-02-24 21:00:23) Review request for Sushanth Sowmyan and Francis Liu. Summary ------- Incorporated Francis' comments. This addresses bug HCATALOG-239 . https://issues.apache.org/jira/browse/HCATALOG-239 Diffs ----- trunk/src/java/org/apache/hcatalog/common/ErrorType.java 1293367 trunk/src/java/org/apache/hcatalog/common/HCatUtil.java 1293367 trunk/src/java/org/apache/hcatalog/data/DataType.java 1293367 trunk/src/java/org/apache/hcatalog/data/HCatRecordObjectInspectorFactory.java 1293367 trunk/src/java/org/apache/hcatalog/data/LazyHCatRecord.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/DefaultOutputCommitterContainer.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputCommitterContainer.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputFormatContainer.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/FosterStorageHandler.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseOutputFormat.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/HCatOutputFormat.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/HCatSplit.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/HCatTableInfo.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/InitializeInput.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/InputJobInfo.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/InternalUtil.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/PartInfo.java 1293367 trunk/src/java/org/apache/hcatalog/pig/HCatLoader.java 1293367 trunk/src/test/org/apache/hcatalog/data/TestHCatRecordSerDe.java 1293367 trunk/src/test/org/apache/hcatalog/data/TestLazyHCatRecord.java 1293367 trunk/src/test/org/apache/hcatalog/mapreduce/TestHCatHiveCompatibility.java 1293367 trunk/src/test/org/apache/hcatalog/mapreduce/TestHCatOutputFormat.java 1293367 trunk/src/test/org/apache/hcatalog/pig/TestHCatLoaderComplexSchema.java 1293367 Diff: https://reviews.apache.org/r/3945/diff Testing ------- Thanks, Vikram
          Hide
          jiraposter@reviews.apache.org added a comment -

          On 2012-02-24 19:59:27, Vikram Dixit Kumaraswamy wrote:

          > trunk/src/java/org/apache/hcatalog/mapreduce/FosterStorageHandler.java, line 34

          > <https://reviews.apache.org/r/3945/diff/2/?file=85621#file85621line34>

          >

          > Actually needed it for unit-test to pass.

          which test?

          • Francis

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

          On 2012-02-24 21:00:23, Vikram Dixit Kumaraswamy wrote:

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

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

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

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

          (Updated 2012-02-24 21:00:23)

          Review request for Sushanth Sowmyan and Francis Liu.

          Summary

          -------

          Incorporated Francis' comments.

          This addresses bug HCATALOG-239.

          https://issues.apache.org/jira/browse/HCATALOG-239

          Diffs

          -----

          trunk/src/java/org/apache/hcatalog/common/ErrorType.java 1293367

          trunk/src/java/org/apache/hcatalog/common/HCatUtil.java 1293367

          trunk/src/java/org/apache/hcatalog/data/DataType.java 1293367

          trunk/src/java/org/apache/hcatalog/data/HCatRecordObjectInspectorFactory.java 1293367

          trunk/src/java/org/apache/hcatalog/data/LazyHCatRecord.java 1293367

          trunk/src/java/org/apache/hcatalog/mapreduce/DefaultOutputCommitterContainer.java 1293367

          trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputCommitterContainer.java 1293367

          trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputFormatContainer.java 1293367

          trunk/src/java/org/apache/hcatalog/mapreduce/FosterStorageHandler.java 1293367

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java 1293367

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseOutputFormat.java 1293367

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatOutputFormat.java 1293367

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java 1293367

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatSplit.java 1293367

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatTableInfo.java 1293367

          trunk/src/java/org/apache/hcatalog/mapreduce/InitializeInput.java 1293367

          trunk/src/java/org/apache/hcatalog/mapreduce/InputJobInfo.java 1293367

          trunk/src/java/org/apache/hcatalog/mapreduce/InternalUtil.java 1293367

          trunk/src/java/org/apache/hcatalog/mapreduce/PartInfo.java 1293367

          trunk/src/java/org/apache/hcatalog/pig/HCatLoader.java 1293367

          trunk/src/test/org/apache/hcatalog/data/TestHCatRecordSerDe.java 1293367

          trunk/src/test/org/apache/hcatalog/data/TestLazyHCatRecord.java 1293367

          trunk/src/test/org/apache/hcatalog/mapreduce/TestHCatHiveCompatibility.java 1293367

          trunk/src/test/org/apache/hcatalog/mapreduce/TestHCatOutputFormat.java 1293367

          trunk/src/test/org/apache/hcatalog/pig/TestHCatLoaderComplexSchema.java 1293367

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

          Testing

          -------

          Thanks,

          Vikram

          Show
          jiraposter@reviews.apache.org added a comment - On 2012-02-24 19:59:27, Vikram Dixit Kumaraswamy wrote: > trunk/src/java/org/apache/hcatalog/mapreduce/FosterStorageHandler.java, line 34 > < https://reviews.apache.org/r/3945/diff/2/?file=85621#file85621line34 > > > Actually needed it for unit-test to pass. which test? Francis ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3945/#review5329 ----------------------------------------------------------- On 2012-02-24 21:00:23, Vikram Dixit Kumaraswamy wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3945/ ----------------------------------------------------------- (Updated 2012-02-24 21:00:23) Review request for Sushanth Sowmyan and Francis Liu. Summary ------- Incorporated Francis' comments. This addresses bug HCATALOG-239 . https://issues.apache.org/jira/browse/HCATALOG-239 Diffs ----- trunk/src/java/org/apache/hcatalog/common/ErrorType.java 1293367 trunk/src/java/org/apache/hcatalog/common/HCatUtil.java 1293367 trunk/src/java/org/apache/hcatalog/data/DataType.java 1293367 trunk/src/java/org/apache/hcatalog/data/HCatRecordObjectInspectorFactory.java 1293367 trunk/src/java/org/apache/hcatalog/data/LazyHCatRecord.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/DefaultOutputCommitterContainer.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputCommitterContainer.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputFormatContainer.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/FosterStorageHandler.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseOutputFormat.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/HCatOutputFormat.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/HCatSplit.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/HCatTableInfo.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/InitializeInput.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/InputJobInfo.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/InternalUtil.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/PartInfo.java 1293367 trunk/src/java/org/apache/hcatalog/pig/HCatLoader.java 1293367 trunk/src/test/org/apache/hcatalog/data/TestHCatRecordSerDe.java 1293367 trunk/src/test/org/apache/hcatalog/data/TestLazyHCatRecord.java 1293367 trunk/src/test/org/apache/hcatalog/mapreduce/TestHCatHiveCompatibility.java 1293367 trunk/src/test/org/apache/hcatalog/mapreduce/TestHCatOutputFormat.java 1293367 trunk/src/test/org/apache/hcatalog/pig/TestHCatLoaderComplexSchema.java 1293367 Diff: https://reviews.apache.org/r/3945/diff Testing ------- Thanks, Vikram
          Hide
          jiraposter@reviews.apache.org added a comment -

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

          trunk/src/java/org/apache/hcatalog/data/LazyHCatRecord.java
          <https://reviews.apache.org/r/3945/#comment11642>

          Yeah sounds better, in general throwing Exception is not good since you're forcing the user to capture RuntimeException, etc as well.

          • Francis

          On 2012-02-24 21:00:23, Vikram Dixit Kumaraswamy wrote:

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

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

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

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

          (Updated 2012-02-24 21:00:23)

          Review request for Sushanth Sowmyan and Francis Liu.

          Summary

          -------

          Incorporated Francis' comments.

          This addresses bug HCATALOG-239.

          https://issues.apache.org/jira/browse/HCATALOG-239

          Diffs

          -----

          trunk/src/java/org/apache/hcatalog/common/ErrorType.java 1293367

          trunk/src/java/org/apache/hcatalog/common/HCatUtil.java 1293367

          trunk/src/java/org/apache/hcatalog/data/DataType.java 1293367

          trunk/src/java/org/apache/hcatalog/data/HCatRecordObjectInspectorFactory.java 1293367

          trunk/src/java/org/apache/hcatalog/data/LazyHCatRecord.java 1293367

          trunk/src/java/org/apache/hcatalog/mapreduce/DefaultOutputCommitterContainer.java 1293367

          trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputCommitterContainer.java 1293367

          trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputFormatContainer.java 1293367

          trunk/src/java/org/apache/hcatalog/mapreduce/FosterStorageHandler.java 1293367

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java 1293367

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseOutputFormat.java 1293367

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatOutputFormat.java 1293367

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java 1293367

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatSplit.java 1293367

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatTableInfo.java 1293367

          trunk/src/java/org/apache/hcatalog/mapreduce/InitializeInput.java 1293367

          trunk/src/java/org/apache/hcatalog/mapreduce/InputJobInfo.java 1293367

          trunk/src/java/org/apache/hcatalog/mapreduce/InternalUtil.java 1293367

          trunk/src/java/org/apache/hcatalog/mapreduce/PartInfo.java 1293367

          trunk/src/java/org/apache/hcatalog/pig/HCatLoader.java 1293367

          trunk/src/test/org/apache/hcatalog/data/TestHCatRecordSerDe.java 1293367

          trunk/src/test/org/apache/hcatalog/data/TestLazyHCatRecord.java 1293367

          trunk/src/test/org/apache/hcatalog/mapreduce/TestHCatHiveCompatibility.java 1293367

          trunk/src/test/org/apache/hcatalog/mapreduce/TestHCatOutputFormat.java 1293367

          trunk/src/test/org/apache/hcatalog/pig/TestHCatLoaderComplexSchema.java 1293367

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

          Testing

          -------

          Thanks,

          Vikram

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3945/#review5338 ----------------------------------------------------------- trunk/src/java/org/apache/hcatalog/data/LazyHCatRecord.java < https://reviews.apache.org/r/3945/#comment11642 > Yeah sounds better, in general throwing Exception is not good since you're forcing the user to capture RuntimeException, etc as well. Francis On 2012-02-24 21:00:23, Vikram Dixit Kumaraswamy wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3945/ ----------------------------------------------------------- (Updated 2012-02-24 21:00:23) Review request for Sushanth Sowmyan and Francis Liu. Summary ------- Incorporated Francis' comments. This addresses bug HCATALOG-239 . https://issues.apache.org/jira/browse/HCATALOG-239 Diffs ----- trunk/src/java/org/apache/hcatalog/common/ErrorType.java 1293367 trunk/src/java/org/apache/hcatalog/common/HCatUtil.java 1293367 trunk/src/java/org/apache/hcatalog/data/DataType.java 1293367 trunk/src/java/org/apache/hcatalog/data/HCatRecordObjectInspectorFactory.java 1293367 trunk/src/java/org/apache/hcatalog/data/LazyHCatRecord.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/DefaultOutputCommitterContainer.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputCommitterContainer.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputFormatContainer.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/FosterStorageHandler.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseOutputFormat.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/HCatOutputFormat.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/HCatSplit.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/HCatTableInfo.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/InitializeInput.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/InputJobInfo.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/InternalUtil.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/PartInfo.java 1293367 trunk/src/java/org/apache/hcatalog/pig/HCatLoader.java 1293367 trunk/src/test/org/apache/hcatalog/data/TestHCatRecordSerDe.java 1293367 trunk/src/test/org/apache/hcatalog/data/TestLazyHCatRecord.java 1293367 trunk/src/test/org/apache/hcatalog/mapreduce/TestHCatHiveCompatibility.java 1293367 trunk/src/test/org/apache/hcatalog/mapreduce/TestHCatOutputFormat.java 1293367 trunk/src/test/org/apache/hcatalog/pig/TestHCatLoaderComplexSchema.java 1293367 Diff: https://reviews.apache.org/r/3945/diff Testing ------- Thanks, Vikram
          Hide
          jiraposter@reviews.apache.org added a comment -

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

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java
          <https://reviews.apache.org/r/3945/#comment11645>

          Daniel and I already had this discussion about getSplits() passing conf, see HCATALOG-226 where we reference jobclient source code proving that it does get passed downstream. Apart from some storageHandlers needing it HCATALOG-247, you are breaking mapreduce behavior.

          • Francis

          On 2012-02-24 21:00:23, Vikram Dixit Kumaraswamy wrote:

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

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

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

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

          (Updated 2012-02-24 21:00:23)

          Review request for Sushanth Sowmyan and Francis Liu.

          Summary

          -------

          Incorporated Francis' comments.

          This addresses bug HCATALOG-239.

          https://issues.apache.org/jira/browse/HCATALOG-239

          Diffs

          -----

          trunk/src/java/org/apache/hcatalog/common/ErrorType.java 1293367

          trunk/src/java/org/apache/hcatalog/common/HCatUtil.java 1293367

          trunk/src/java/org/apache/hcatalog/data/DataType.java 1293367

          trunk/src/java/org/apache/hcatalog/data/HCatRecordObjectInspectorFactory.java 1293367

          trunk/src/java/org/apache/hcatalog/data/LazyHCatRecord.java 1293367

          trunk/src/java/org/apache/hcatalog/mapreduce/DefaultOutputCommitterContainer.java 1293367

          trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputCommitterContainer.java 1293367

          trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputFormatContainer.java 1293367

          trunk/src/java/org/apache/hcatalog/mapreduce/FosterStorageHandler.java 1293367

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java 1293367

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseOutputFormat.java 1293367

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatOutputFormat.java 1293367

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java 1293367

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatSplit.java 1293367

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatTableInfo.java 1293367

          trunk/src/java/org/apache/hcatalog/mapreduce/InitializeInput.java 1293367

          trunk/src/java/org/apache/hcatalog/mapreduce/InputJobInfo.java 1293367

          trunk/src/java/org/apache/hcatalog/mapreduce/InternalUtil.java 1293367

          trunk/src/java/org/apache/hcatalog/mapreduce/PartInfo.java 1293367

          trunk/src/java/org/apache/hcatalog/pig/HCatLoader.java 1293367

          trunk/src/test/org/apache/hcatalog/data/TestHCatRecordSerDe.java 1293367

          trunk/src/test/org/apache/hcatalog/data/TestLazyHCatRecord.java 1293367

          trunk/src/test/org/apache/hcatalog/mapreduce/TestHCatHiveCompatibility.java 1293367

          trunk/src/test/org/apache/hcatalog/mapreduce/TestHCatOutputFormat.java 1293367

          trunk/src/test/org/apache/hcatalog/pig/TestHCatLoaderComplexSchema.java 1293367

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

          Testing

          -------

          Thanks,

          Vikram

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3945/#review5341 ----------------------------------------------------------- trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java < https://reviews.apache.org/r/3945/#comment11645 > Daniel and I already had this discussion about getSplits() passing conf, see HCATALOG-226 where we reference jobclient source code proving that it does get passed downstream. Apart from some storageHandlers needing it HCATALOG-247 , you are breaking mapreduce behavior. Francis On 2012-02-24 21:00:23, Vikram Dixit Kumaraswamy wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3945/ ----------------------------------------------------------- (Updated 2012-02-24 21:00:23) Review request for Sushanth Sowmyan and Francis Liu. Summary ------- Incorporated Francis' comments. This addresses bug HCATALOG-239 . https://issues.apache.org/jira/browse/HCATALOG-239 Diffs ----- trunk/src/java/org/apache/hcatalog/common/ErrorType.java 1293367 trunk/src/java/org/apache/hcatalog/common/HCatUtil.java 1293367 trunk/src/java/org/apache/hcatalog/data/DataType.java 1293367 trunk/src/java/org/apache/hcatalog/data/HCatRecordObjectInspectorFactory.java 1293367 trunk/src/java/org/apache/hcatalog/data/LazyHCatRecord.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/DefaultOutputCommitterContainer.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputCommitterContainer.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputFormatContainer.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/FosterStorageHandler.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseOutputFormat.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/HCatOutputFormat.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/HCatSplit.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/HCatTableInfo.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/InitializeInput.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/InputJobInfo.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/InternalUtil.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/PartInfo.java 1293367 trunk/src/java/org/apache/hcatalog/pig/HCatLoader.java 1293367 trunk/src/test/org/apache/hcatalog/data/TestHCatRecordSerDe.java 1293367 trunk/src/test/org/apache/hcatalog/data/TestLazyHCatRecord.java 1293367 trunk/src/test/org/apache/hcatalog/mapreduce/TestHCatHiveCompatibility.java 1293367 trunk/src/test/org/apache/hcatalog/mapreduce/TestHCatOutputFormat.java 1293367 trunk/src/test/org/apache/hcatalog/pig/TestHCatLoaderComplexSchema.java 1293367 Diff: https://reviews.apache.org/r/3945/diff Testing ------- Thanks, Vikram
          Hide
          Alan Gates added a comment -

          Patch 8 checked in. Thanks Vikram for your determined work on this. We will deal with Francis' review comments in separate JIRAs.

          Show
          Alan Gates added a comment - Patch 8 checked in. Thanks Vikram for your determined work on this. We will deal with Francis' review comments in separate JIRAs.
          Hide
          jiraposter@reviews.apache.org added a comment -

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

          Ship it!

          There are some issues that need to be addressed, but can be addressed later to unblock testing.

          • Francis

          On 2012-02-24 21:00:23, Vikram Dixit Kumaraswamy wrote:

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

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

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

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

          (Updated 2012-02-24 21:00:23)

          Review request for Sushanth Sowmyan and Francis Liu.

          Summary

          -------

          Incorporated Francis' comments.

          This addresses bug HCATALOG-239.

          https://issues.apache.org/jira/browse/HCATALOG-239

          Diffs

          -----

          trunk/src/java/org/apache/hcatalog/common/ErrorType.java 1293367

          trunk/src/java/org/apache/hcatalog/common/HCatUtil.java 1293367

          trunk/src/java/org/apache/hcatalog/data/DataType.java 1293367

          trunk/src/java/org/apache/hcatalog/data/HCatRecordObjectInspectorFactory.java 1293367

          trunk/src/java/org/apache/hcatalog/data/LazyHCatRecord.java 1293367

          trunk/src/java/org/apache/hcatalog/mapreduce/DefaultOutputCommitterContainer.java 1293367

          trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputCommitterContainer.java 1293367

          trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputFormatContainer.java 1293367

          trunk/src/java/org/apache/hcatalog/mapreduce/FosterStorageHandler.java 1293367

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java 1293367

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseOutputFormat.java 1293367

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatOutputFormat.java 1293367

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java 1293367

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatSplit.java 1293367

          trunk/src/java/org/apache/hcatalog/mapreduce/HCatTableInfo.java 1293367

          trunk/src/java/org/apache/hcatalog/mapreduce/InitializeInput.java 1293367

          trunk/src/java/org/apache/hcatalog/mapreduce/InputJobInfo.java 1293367

          trunk/src/java/org/apache/hcatalog/mapreduce/InternalUtil.java 1293367

          trunk/src/java/org/apache/hcatalog/mapreduce/PartInfo.java 1293367

          trunk/src/java/org/apache/hcatalog/pig/HCatLoader.java 1293367

          trunk/src/test/org/apache/hcatalog/data/TestHCatRecordSerDe.java 1293367

          trunk/src/test/org/apache/hcatalog/data/TestLazyHCatRecord.java 1293367

          trunk/src/test/org/apache/hcatalog/mapreduce/TestHCatHiveCompatibility.java 1293367

          trunk/src/test/org/apache/hcatalog/mapreduce/TestHCatOutputFormat.java 1293367

          trunk/src/test/org/apache/hcatalog/pig/TestHCatLoaderComplexSchema.java 1293367

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

          Testing

          -------

          Thanks,

          Vikram

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3945/#review5345 ----------------------------------------------------------- Ship it! There are some issues that need to be addressed, but can be addressed later to unblock testing. Francis On 2012-02-24 21:00:23, Vikram Dixit Kumaraswamy wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3945/ ----------------------------------------------------------- (Updated 2012-02-24 21:00:23) Review request for Sushanth Sowmyan and Francis Liu. Summary ------- Incorporated Francis' comments. This addresses bug HCATALOG-239 . https://issues.apache.org/jira/browse/HCATALOG-239 Diffs ----- trunk/src/java/org/apache/hcatalog/common/ErrorType.java 1293367 trunk/src/java/org/apache/hcatalog/common/HCatUtil.java 1293367 trunk/src/java/org/apache/hcatalog/data/DataType.java 1293367 trunk/src/java/org/apache/hcatalog/data/HCatRecordObjectInspectorFactory.java 1293367 trunk/src/java/org/apache/hcatalog/data/LazyHCatRecord.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/DefaultOutputCommitterContainer.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputCommitterContainer.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/FileOutputFormatContainer.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/FosterStorageHandler.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/HCatBaseOutputFormat.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/HCatOutputFormat.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/HCatSplit.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/HCatTableInfo.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/InitializeInput.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/InputJobInfo.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/InternalUtil.java 1293367 trunk/src/java/org/apache/hcatalog/mapreduce/PartInfo.java 1293367 trunk/src/java/org/apache/hcatalog/pig/HCatLoader.java 1293367 trunk/src/test/org/apache/hcatalog/data/TestHCatRecordSerDe.java 1293367 trunk/src/test/org/apache/hcatalog/data/TestLazyHCatRecord.java 1293367 trunk/src/test/org/apache/hcatalog/mapreduce/TestHCatHiveCompatibility.java 1293367 trunk/src/test/org/apache/hcatalog/mapreduce/TestHCatOutputFormat.java 1293367 trunk/src/test/org/apache/hcatalog/pig/TestHCatLoaderComplexSchema.java 1293367 Diff: https://reviews.apache.org/r/3945/diff Testing ------- Thanks, Vikram
          Hide
          Alan Gates added a comment -

          Issue closed with 0.4 release.

          Show
          Alan Gates added a comment - Issue closed with 0.4 release.

            People

            • Assignee:
              Vikram Dixit K
              Reporter:
              Alan Gates
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development