Hive
  1. Hive
  2. HIVE-4304

Remove unused builtins and pdk submodules

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.11.0
    • Component/s: None
    • Labels:
      None

      Description

      Moving from email. The builtins and pdk submodules are not believed to be in use and should be removed. The main benefits are simplification and maintainability of the Hive code base.

      Forwarded conversation
      Subject: builtins submodule - is it still needed?
      ------------------------

      From: Travis Crawford <traviscrawford@gmail.com>
      Date: Thu, Apr 4, 2013 at 2:01 PM
      To: user@hive.apache.org, dev@hive.apache.org

      Hey hive gurus -

      Is the "builtins" hive submodule in use? The submodule was added in
      HIVE-2523 as a location for builtin-UDFs, but it appears to not have
      taken off. Any objections to removing it?

      DETAILS

      For HIVE-4278 I'm making some build changes for the HCatalog
      integration. The "builtins" submodule causes issues because it delays
      building until the packaging phase - so HCatalog can't depend on
      builtins, which it does transitively.

      While investigating a path forward I discovered the "builtins"
      submodule contains very little code, and likely could either go away
      entirely or merge into "ql", simplifying things both for users and
      developers.

      Thoughts? Can anyone with context help me understand "builtins", both
      in general and around its non-standard build? For your trouble I'll
      either make the submodule go away/merge into another submodule, or
      update the docs with what we learn.

      Thanks!
      Travis

      ----------
      From: Ashutosh Chauhan <ashutosh.chauhan@gmail.com>
      Date: Fri, Apr 5, 2013 at 3:10 PM
      To: dev@hive.apache.org
      Cc: "user@hive.apache.org" <user@hive.apache.org>

      I haven't used it myself anytime till now. Neither have met anyone who used
      it or plan to use it.

      Ashutosh

      On Thu, Apr 4, 2013 at 2:01 PM, Travis Crawford <traviscrawford@gmail.com>wrote:

      ----------
      From: Gunther Hagleitner <ghagleitner@hortonworks.com>
      Date: Fri, Apr 5, 2013 at 3:11 PM
      To: dev@hive.apache.org
      Cc: user@hive.apache.org

      +1

      I would actually go a step further and propose to remove both PDK and
      builtins. I've went through the code for both and here is what I found:

      Builtins:

      • BuiltInUtils.java: Empty file
      • UDAFUnionMap: Merges maps. Doesn't seem to be useful by itself, but was
        intended as a building block for PDK

      PDK:

      • some helper build.xml/test setup + teardown scripts
      • Classes/annotations to help run unit tests
      • rot13 as an example

      From what I can tell it's a fair assessment that it hasn't taken off, last
      commits to it seem to have happened more than 1.5 years ago.

      Thanks,
      Gunther.

      On Thu, Apr 4, 2013 at 2:01 PM, Travis Crawford <traviscrawford@gmail.com>wrote:

      ----------
      From: Owen O'Malley <omalley@apache.org>
      Date: Fri, Apr 5, 2013 at 4:45 PM
      To: user@hive.apache.org

      +1 to removing them.

      We have a Rot13 example in ql/src/test/org/apache/hadoop/hive/ql/io/udf/Rot13

      {In,Out}

      putFormat.java anyways. smile

      – Owen

      1. HIVE-4304.1.patch
        60 kB
        Gunther Hagleitner
      2. HIVE-4304.patch
        65 kB
        Travis Crawford

        Issue Links

          Activity

          Travis Crawford created issue -
          Hide
          Gunther Hagleitner added a comment -

          Removed the code earlier today to understand dependencies etc. Here's the patch.

          Show
          Gunther Hagleitner added a comment - Removed the code earlier today to understand dependencies etc. Here's the patch.
          Gunther Hagleitner made changes -
          Field Original Value New Value
          Attachment HIVE-4304.1.patch [ 12577338 ]
          Hide
          Travis Crawford added a comment -

          Hey Gunther Hagleitner - ha, I just finished doing roughly the same thing. My branch is https://github.com/traviscrawford/hive/tree/HIVE-4304_rm_builtins_pdk. Using git grep I found a number of other references too, in particular as ivy dependencies, eclipse classpath entries, and in start scripts.

          Can you give my branch a quick look? If you think it looks good I can kick off tests and post the diff for review.

          Show
          Travis Crawford added a comment - Hey Gunther Hagleitner - ha, I just finished doing roughly the same thing. My branch is https://github.com/traviscrawford/hive/tree/HIVE-4304_rm_builtins_pdk . Using git grep I found a number of other references too, in particular as ivy dependencies, eclipse classpath entries, and in start scripts. Can you give my branch a quick look? If you think it looks good I can kick off tests and post the diff for review.
          Hide
          Gunther Hagleitner added a comment -

          Travis Crawford: Yours is definitely better. I did miss a few things - anyways, just wanted to share what I had since I spent some time this morning.

          Show
          Gunther Hagleitner added a comment - Travis Crawford : Yours is definitely better. I did miss a few things - anyways, just wanted to share what I had since I spent some time this morning.
          Hide
          Ashutosh Chauhan added a comment -

          Travis Crawford Are you planning to upload a patch for this soon? I believe this will accelerate HIVE-4278

          Show
          Ashutosh Chauhan added a comment - Travis Crawford Are you planning to upload a patch for this soon? I believe this will accelerate HIVE-4278
          Hide
          Travis Crawford added a comment -

          Just started tests at https://travis.ci.cloudbees.com/job/HIVE-4304_rm_builtins_pdk/ and will post if they pass.

          Show
          Travis Crawford added a comment - Just started tests at https://travis.ci.cloudbees.com/job/HIVE-4304_rm_builtins_pdk/ and will post if they pass.
          Hide
          Ashutosh Chauhan added a comment -

          Travis Crawford looks like build failed.

          Show
          Ashutosh Chauhan added a comment - Travis Crawford looks like build failed.
          Hide
          Travis Crawford added a comment -

          Ashutosh Chauhan - I actually spent a bit of time Friday looking at this, and am having issues getting tests to run. I'm hoping to look at this shortly, but if you'd like to look at this sooner feel free to checkout the branch.

          Show
          Travis Crawford added a comment - Ashutosh Chauhan - I actually spent a bit of time Friday looking at this, and am having issues getting tests to run. I'm hoping to look at this shortly, but if you'd like to look at this sooner feel free to checkout the branch.
          Hide
          Gunther Hagleitner added a comment -

          Travis Crawford looks like you ran into: HIVE-4339. Build didn't work for a bit after branching.

          Show
          Gunther Hagleitner added a comment - Travis Crawford looks like you ran into: HIVE-4339 . Build didn't work for a bit after branching.
          Hide
          Ashutosh Chauhan added a comment -

          Travis Crawford I pulled your branch rebased it on trunk and built it. It worked fine. Now, running tests. Can you upload the patch on jira, so I can test actual patch out.

          Show
          Ashutosh Chauhan added a comment - Travis Crawford I pulled your branch rebased it on trunk and built it. It worked fine. Now, running tests. Can you upload the patch on jira, so I can test actual patch out.
          Hide
          Ashutosh Chauhan added a comment -

          Travis Crawford All tests passed except show_functions.q which just needs to be run with -Doverwrite=true to update .q.out

          Show
          Ashutosh Chauhan added a comment - Travis Crawford All tests passed except show_functions.q which just needs to be run with -Doverwrite=true to update .q.out
          Hide
          Travis Crawford added a comment -

          Thanks for helping resolve the build issues Gunther Hagleitner and Ashutosh Chauhan. I'll rebase, update that test and start the tests again.

          Show
          Travis Crawford added a comment - Thanks for helping resolve the build issues Gunther Hagleitner and Ashutosh Chauhan . I'll rebase, update that test and start the tests again.
          Hide
          Ashutosh Chauhan added a comment -

          Travis Crawford Rebasing your branch after commit of HIVE-4278 and with few minor edits, I was able to build successfully.

          Show
          Ashutosh Chauhan added a comment - Travis Crawford Rebasing your branch after commit of HIVE-4278 and with few minor edits, I was able to build successfully.
          Travis Crawford made changes -
          Attachment HIVE-4304.patch [ 12579555 ]
          Travis Crawford made changes -
          Status Open [ 1 ] Patch Available [ 10002 ]
          Hide
          Ashutosh Chauhan added a comment -

          +1 will commit if tests pass

          Show
          Ashutosh Chauhan added a comment - +1 will commit if tests pass
          Hide
          Ashutosh Chauhan added a comment -

          Committed to trunk. Thanks, Travis!

          Show
          Ashutosh Chauhan added a comment - Committed to trunk. Thanks, Travis!
          Ashutosh Chauhan made changes -
          Status Patch Available [ 10002 ] Resolved [ 5 ]
          Fix Version/s 0.12.0 [ 12324312 ]
          Resolution Fixed [ 1 ]
          Hide
          Hudson added a comment -

          Integrated in Hive-trunk-hadoop2 #168 (See https://builds.apache.org/job/Hive-trunk-hadoop2/168/)
          HIVE-4304 : Remove unused builtins and pdk submodules (Travis Crawford via Ashutosh Chauhan) (Revision 1470203)

          Result = FAILURE
          hashutosh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1470203
          Files :

          • /hive/trunk/bin/hive
          • /hive/trunk/build.properties
          • /hive/trunk/build.xml
          • /hive/trunk/builtins
          • /hive/trunk/eclipse-templates/.classpath
          • /hive/trunk/hcatalog/build-support/ant/deploy.xml
          • /hive/trunk/pdk
          • /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java
          • /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java
          • /hive/trunk/ql/src/test/results/clientpositive/show_functions.q.out
          Show
          Hudson added a comment - Integrated in Hive-trunk-hadoop2 #168 (See https://builds.apache.org/job/Hive-trunk-hadoop2/168/ ) HIVE-4304 : Remove unused builtins and pdk submodules (Travis Crawford via Ashutosh Chauhan) (Revision 1470203) Result = FAILURE hashutosh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1470203 Files : /hive/trunk/bin/hive /hive/trunk/build.properties /hive/trunk/build.xml /hive/trunk/builtins /hive/trunk/eclipse-templates/.classpath /hive/trunk/hcatalog/build-support/ant/deploy.xml /hive/trunk/pdk /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java /hive/trunk/ql/src/test/results/clientpositive/show_functions.q.out
          Hide
          Hudson added a comment -

          Integrated in Hive-trunk-h0.21 #2072 (See https://builds.apache.org/job/Hive-trunk-h0.21/2072/)
          HIVE-4304 : Remove unused builtins and pdk submodules (Travis Crawford via Ashutosh Chauhan) (Revision 1470203)

          Result = ABORTED
          hashutosh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1470203
          Files :

          • /hive/trunk/bin/hive
          • /hive/trunk/build.properties
          • /hive/trunk/build.xml
          • /hive/trunk/builtins
          • /hive/trunk/eclipse-templates/.classpath
          • /hive/trunk/hcatalog/build-support/ant/deploy.xml
          • /hive/trunk/pdk
          • /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java
          • /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java
          • /hive/trunk/ql/src/test/results/clientpositive/show_functions.q.out
          Show
          Hudson added a comment - Integrated in Hive-trunk-h0.21 #2072 (See https://builds.apache.org/job/Hive-trunk-h0.21/2072/ ) HIVE-4304 : Remove unused builtins and pdk submodules (Travis Crawford via Ashutosh Chauhan) (Revision 1470203) Result = ABORTED hashutosh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1470203 Files : /hive/trunk/bin/hive /hive/trunk/build.properties /hive/trunk/build.xml /hive/trunk/builtins /hive/trunk/eclipse-templates/.classpath /hive/trunk/hcatalog/build-support/ant/deploy.xml /hive/trunk/pdk /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java /hive/trunk/ql/src/test/results/clientpositive/show_functions.q.out
          Ashutosh Chauhan made changes -
          Fix Version/s 0.11.0 [ 12323587 ]
          Fix Version/s 0.12.0 [ 12324312 ]
          Owen O'Malley made changes -
          Status Resolved [ 5 ] Closed [ 6 ]
          Lee Tucker made changes -
          Link This issue is related to OOZIE-1665 [ OOZIE-1665 ]

            People

            • Assignee:
              Travis Crawford
              Reporter:
              Travis Crawford
            • Votes:
              0 Vote for this issue
              Watchers:
              7 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development