Hive
  1. Hive
  2. HIVE-2908

Hive: Extend ALTER TABLE DROP PARTITION syntax to use all comparators

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 0.9.0
    • Fix Version/s: 0.9.0
    • Component/s: Query Processor
    • Labels:
      None

      Description

      To drop a partition from a Hive table, this works:

      ALTER TABLE foo DROP PARTITION(ds = 'date')

      ...but it should also work to drop all partitions prior to date.

      ALTER TABLE foo DROP PARTITION(ds < 'date')

      This task is to implement ALTER TABLE DROP PARTITION for all of the comparators, < > <= >= <> = != instead of just for =.

      1. HIVE-2908.final.patch.txt
        34 kB
        Sambavi Muthukrishnan
      2. ASF.LICENSE.NOT.GRANTED--HIVE-2908.D2523.2.patch
        27 kB
        Phabricator
      3. ASF.LICENSE.NOT.GRANTED--HIVE-2908.D2523.1.patch
        27 kB
        Phabricator

        Issue Links

          Activity

          Hide
          Hudson added a comment -

          Integrated in Hive-trunk-hadoop2 #54 (See https://builds.apache.org/job/Hive-trunk-hadoop2/54/)
          HIVE-2908. Hive: Extend ALTER TABLE DROP PARTITION syntax to use all comparators. (sambavim via kevinwilfong) (Revision 1308427)

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

          • /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java
          • /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java
          • /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/Hive.g
          • /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/plan/DropTableDesc.java
          • /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/plan/PartitionSpec.java
          • /hive/trunk/ql/src/test/queries/clientnegative/drop_partition_filter_failure.q
          • /hive/trunk/ql/src/test/queries/clientpositive/drop_partitions_filter.q
          • /hive/trunk/ql/src/test/results/clientnegative/drop_partition_failure.q.out
          • /hive/trunk/ql/src/test/results/clientnegative/drop_partition_filter_failure.q.out
          • /hive/trunk/ql/src/test/results/clientpositive/drop_multi_partitions.q.out
          • /hive/trunk/ql/src/test/results/clientpositive/drop_partitions_filter.q.out
          • /hive/trunk/ql/src/test/results/clientpositive/escape1.q.out
          Show
          Hudson added a comment - Integrated in Hive-trunk-hadoop2 #54 (See https://builds.apache.org/job/Hive-trunk-hadoop2/54/ ) HIVE-2908 . Hive: Extend ALTER TABLE DROP PARTITION syntax to use all comparators. (sambavim via kevinwilfong) (Revision 1308427) Result = ABORTED kevinwilfong : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1308427 Files : /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/Hive.g /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/plan/DropTableDesc.java /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/plan/PartitionSpec.java /hive/trunk/ql/src/test/queries/clientnegative/drop_partition_filter_failure.q /hive/trunk/ql/src/test/queries/clientpositive/drop_partitions_filter.q /hive/trunk/ql/src/test/results/clientnegative/drop_partition_failure.q.out /hive/trunk/ql/src/test/results/clientnegative/drop_partition_filter_failure.q.out /hive/trunk/ql/src/test/results/clientpositive/drop_multi_partitions.q.out /hive/trunk/ql/src/test/results/clientpositive/drop_partitions_filter.q.out /hive/trunk/ql/src/test/results/clientpositive/escape1.q.out
          Hide
          Ashutosh Chauhan added a comment -

          This issue is closed now. It was released with the fix in 0.9.0. If there is a problem, please open a new jira and link this one with that.

          Show
          Ashutosh Chauhan added a comment - This issue is closed now. It was released with the fix in 0.9.0. If there is a problem, please open a new jira and link this one with that.
          Hide
          Hudson added a comment -

          Integrated in Hive-trunk-h0.21 #1348 (See https://builds.apache.org/job/Hive-trunk-h0.21/1348/)
          HIVE-2908. Hive: Extend ALTER TABLE DROP PARTITION syntax to use all comparators. (sambavim via kevinwilfong) (Revision 1308427)

          Result = SUCCESS
          kevinwilfong : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1308427
          Files :

          • /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java
          • /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java
          • /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/Hive.g
          • /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/plan/DropTableDesc.java
          • /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/plan/PartitionSpec.java
          • /hive/trunk/ql/src/test/queries/clientnegative/drop_partition_filter_failure.q
          • /hive/trunk/ql/src/test/queries/clientpositive/drop_partitions_filter.q
          • /hive/trunk/ql/src/test/results/clientnegative/drop_partition_failure.q.out
          • /hive/trunk/ql/src/test/results/clientnegative/drop_partition_filter_failure.q.out
          • /hive/trunk/ql/src/test/results/clientpositive/drop_multi_partitions.q.out
          • /hive/trunk/ql/src/test/results/clientpositive/drop_partitions_filter.q.out
          • /hive/trunk/ql/src/test/results/clientpositive/escape1.q.out
          Show
          Hudson added a comment - Integrated in Hive-trunk-h0.21 #1348 (See https://builds.apache.org/job/Hive-trunk-h0.21/1348/ ) HIVE-2908 . Hive: Extend ALTER TABLE DROP PARTITION syntax to use all comparators. (sambavim via kevinwilfong) (Revision 1308427) Result = SUCCESS kevinwilfong : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1308427 Files : /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/Hive.g /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/plan/DropTableDesc.java /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/plan/PartitionSpec.java /hive/trunk/ql/src/test/queries/clientnegative/drop_partition_filter_failure.q /hive/trunk/ql/src/test/queries/clientpositive/drop_partitions_filter.q /hive/trunk/ql/src/test/results/clientnegative/drop_partition_failure.q.out /hive/trunk/ql/src/test/results/clientnegative/drop_partition_filter_failure.q.out /hive/trunk/ql/src/test/results/clientpositive/drop_multi_partitions.q.out /hive/trunk/ql/src/test/results/clientpositive/drop_partitions_filter.q.out /hive/trunk/ql/src/test/results/clientpositive/escape1.q.out
          Hide
          Kevin Wilfong added a comment -

          Committed. Thanks, Sambavi.

          Show
          Kevin Wilfong added a comment - Committed. Thanks, Sambavi.
          Hide
          Phabricator added a comment -

          kevinwilfong has accepted the revision "HIVE-2908 [jira] Hive: Extend ALTER TABLE DROP PARTITION syntax to use all comparators".

          +1 Looks good, and thanks for uploading the patch with all the files to the JIRA.

          Will commit if tests pass.

          Will let you know about the documentation.

          REVISION DETAIL
          https://reviews.facebook.net/D2523

          BRANCH
          svn

          Show
          Phabricator added a comment - kevinwilfong has accepted the revision " HIVE-2908 [jira] Hive: Extend ALTER TABLE DROP PARTITION syntax to use all comparators". +1 Looks good, and thanks for uploading the patch with all the files to the JIRA. Will commit if tests pass. Will let you know about the documentation. REVISION DETAIL https://reviews.facebook.net/D2523 BRANCH svn
          Hide
          Sambavi Muthukrishnan added a comment -

          This patch includes the fix to the test log for escape1.q.out. Per discussion with Kevin included that fix and uploaded the svn patch.

          Show
          Sambavi Muthukrishnan added a comment - This patch includes the fix to the test log for escape1.q.out. Per discussion with Kevin included that fix and uploaded the svn patch.
          Hide
          Sambavi Muthukrishnan added a comment -

          Final patch after code review for HIVE-2908

          Show
          Sambavi Muthukrishnan added a comment - Final patch after code review for HIVE-2908
          Hide
          Phabricator added a comment -

          sambavim has commented on the revision "HIVE-2908 [jira] Hive: Extend ALTER TABLE DROP PARTITION syntax to use all comparators".

          Note: The committer will need to run the below test with -Doverwrite=true due to a problem with arc diff loading the differential on the test results.

          ant test -Dtestcase=TestCliDriver -Dqfile=escape1.q -Doverwrite=true

          REVISION DETAIL
          https://reviews.facebook.net/D2523

          Show
          Phabricator added a comment - sambavim has commented on the revision " HIVE-2908 [jira] Hive: Extend ALTER TABLE DROP PARTITION syntax to use all comparators". Note: The committer will need to run the below test with -Doverwrite=true due to a problem with arc diff loading the differential on the test results. ant test -Dtestcase=TestCliDriver -Dqfile=escape1.q -Doverwrite=true REVISION DETAIL https://reviews.facebook.net/D2523
          Hide
          Phabricator added a comment -

          sambavim updated the revision "HIVE-2908 [jira] Hive: Extend ALTER TABLE DROP PARTITION syntax to use all comparators".
          Reviewers: JIRA, njain, kevinwilfong

          Updated the error message generated in DDLSemanticAnalyzer.java when the drop partition filter does not generate any partitions to drop.

          Tested the drop_partitions_filter test with a filter d < 2 instead of d < '2' - the former does not work, and the implementation of getPartitionsWithFilter says that it only works on string partition keys. Somehow providing a string value for the integer key seems to work although this seems hacky. The documentation should mention that this is only for string keys.

          When and how should I update the documentation?

          REVISION DETAIL
          https://reviews.facebook.net/D2523

          AFFECTED FILES
          ql/src/test/results/clientpositive/drop_partitions_filter.q.out
          ql/src/test/results/clientpositive/drop_multi_partitions.q.out
          ql/src/test/results/clientnegative/drop_partition_filter_failure.q.out
          ql/src/test/results/clientnegative/drop_partition_failure.q.out
          ql/src/test/queries/clientpositive/drop_partitions_filter.q
          ql/src/test/queries/clientnegative/drop_partition_filter_failure.q
          ql/src/java/org/apache/hadoop/hive/ql/plan/DropTableDesc.java
          ql/src/java/org/apache/hadoop/hive/ql/plan/PartitionSpec.java
          ql/src/java/org/apache/hadoop/hive/ql/parse/Hive.g
          ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java
          ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java

          Show
          Phabricator added a comment - sambavim updated the revision " HIVE-2908 [jira] Hive: Extend ALTER TABLE DROP PARTITION syntax to use all comparators". Reviewers: JIRA, njain, kevinwilfong Updated the error message generated in DDLSemanticAnalyzer.java when the drop partition filter does not generate any partitions to drop. Tested the drop_partitions_filter test with a filter d < 2 instead of d < '2' - the former does not work, and the implementation of getPartitionsWithFilter says that it only works on string partition keys. Somehow providing a string value for the integer key seems to work although this seems hacky. The documentation should mention that this is only for string keys. When and how should I update the documentation? REVISION DETAIL https://reviews.facebook.net/D2523 AFFECTED FILES ql/src/test/results/clientpositive/drop_partitions_filter.q.out ql/src/test/results/clientpositive/drop_multi_partitions.q.out ql/src/test/results/clientnegative/drop_partition_filter_failure.q.out ql/src/test/results/clientnegative/drop_partition_failure.q.out ql/src/test/queries/clientpositive/drop_partitions_filter.q ql/src/test/queries/clientnegative/drop_partition_filter_failure.q ql/src/java/org/apache/hadoop/hive/ql/plan/DropTableDesc.java ql/src/java/org/apache/hadoop/hive/ql/plan/PartitionSpec.java ql/src/java/org/apache/hadoop/hive/ql/parse/Hive.g ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java
          Hide
          Phabricator added a comment -

          sambavim has commented on the revision "HIVE-2908 [jira] Hive: Extend ALTER TABLE DROP PARTITION syntax to use all comparators".

          Will update review once I make the change to the error message.

          INLINE COMMENTS
          ql/src/java/org/apache/hadoop/hive/ql/plan/PartitionSpec.java:61 I found the != produced the unexpected side effect that it actually behaved like an =. That was obviously a problem, so I decided to just convert it to a <> for safety.

          I didn't debug why I got that behavior with a !=, but I suspect it is because != is not a valid operator on a filter against the database. What was surprising is the outcome of deleting the partition that was specified in the !=.
          ql/src/test/queries/clientpositive/drop_partitions_filter.q:18 I'll test it and report back.
          ql/src/test/results/clientnegative/drop_partition_filter_failure.q.out:18 Will fix. Good catch.

          REVISION DETAIL
          https://reviews.facebook.net/D2523

          Show
          Phabricator added a comment - sambavim has commented on the revision " HIVE-2908 [jira] Hive: Extend ALTER TABLE DROP PARTITION syntax to use all comparators". Will update review once I make the change to the error message. INLINE COMMENTS ql/src/java/org/apache/hadoop/hive/ql/plan/PartitionSpec.java:61 I found the != produced the unexpected side effect that it actually behaved like an =. That was obviously a problem, so I decided to just convert it to a <> for safety. I didn't debug why I got that behavior with a !=, but I suspect it is because != is not a valid operator on a filter against the database. What was surprising is the outcome of deleting the partition that was specified in the !=. ql/src/test/queries/clientpositive/drop_partitions_filter.q:18 I'll test it and report back. ql/src/test/results/clientnegative/drop_partition_filter_failure.q.out:18 Will fix. Good catch. REVISION DETAIL https://reviews.facebook.net/D2523
          Hide
          Phabricator added a comment -

          kevinwilfong has commented on the revision "HIVE-2908 [jira] Hive: Extend ALTER TABLE DROP PARTITION syntax to use all comparators".

          Looks good, just some cosmetic things.

          INLINE COMMENTS
          ql/src/java/org/apache/hadoop/hive/ql/plan/PartitionSpec.java:61 Why do you change the != to <>, why not use whichever the user chose to use?
          ql/src/test/results/clientnegative/drop_partition_filter_failure.q.out:18 This error message seems confusing, the filter was < 1, so it should not be surprising that the partition '1' was not found. (I know why this exception was thrown, I'm just saying the message could be improved)
          ql/src/test/queries/clientpositive/drop_partitions_filter.q:18 Out of curiosity, does the behavior change if you use d<=2 vs. d<='2'?

          REVISION DETAIL
          https://reviews.facebook.net/D2523

          Show
          Phabricator added a comment - kevinwilfong has commented on the revision " HIVE-2908 [jira] Hive: Extend ALTER TABLE DROP PARTITION syntax to use all comparators". Looks good, just some cosmetic things. INLINE COMMENTS ql/src/java/org/apache/hadoop/hive/ql/plan/PartitionSpec.java:61 Why do you change the != to <>, why not use whichever the user chose to use? ql/src/test/results/clientnegative/drop_partition_filter_failure.q.out:18 This error message seems confusing, the filter was < 1, so it should not be surprising that the partition '1' was not found. (I know why this exception was thrown, I'm just saying the message could be improved) ql/src/test/queries/clientpositive/drop_partitions_filter.q:18 Out of curiosity, does the behavior change if you use d<=2 vs. d<='2'? REVISION DETAIL https://reviews.facebook.net/D2523
          Hide
          Phabricator added a comment -

          sambavim requested code review of "HIVE-2908 [jira] Hive: Extend ALTER TABLE DROP PARTITION syntax to use all comparators".
          Reviewers: JIRA

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

          This diff introduces drop partition with non-equality comparisons
          on the partition key. You can now issue commands of the form:
          alter table <tbl> drop partition (key<value)
          to drop all partitions which satisfy the predicate.

          Changes were introduced to hive.g to allow the new operators:
          <, >, <>, !=, <=, >=
          in alter table drop partition statements.

          The change is implemented by using the method getPartitionsByFilter on
          the Hive object. In order to cleanup the passing around of the partition
          spec information, I introduced a new class called PartitionSpec which
          encapsulates one complete partition spec (multiple key value comparisons).
          The drop statement thus has a list of the PartitionSpec objects.

          Unit tests have been added to test the new functionality. Some existing
          test results needed an update due to the change in the AST for the
          partition spec.

          All unit tests passed with this change. The test results of the escape1.q
          test need an update as well but arc diff complains since it concludes the file
          is binary. I will work with the committer to make sure that change is captured
          before commit.

          To drop a partition from a Hive table, this works:

          ALTER TABLE foo DROP PARTITION(ds = 'date')

          ...but it should also work to drop all partitions prior to date.

          ALTER TABLE foo DROP PARTITION(ds < 'date')

          This task is to implement ALTER TABLE DROP PARTITION for all of the comparators, < > <= >= <> = != instead of just for =.

          TEST PLAN
          EMPTY

          REVISION DETAIL
          https://reviews.facebook.net/D2523

          AFFECTED FILES
          ql/src/test/results/clientpositive/drop_partitions_filter.q.out
          ql/src/test/results/clientpositive/drop_multi_partitions.q.out
          ql/src/test/results/clientnegative/drop_partition_filter_failure.q.out
          ql/src/test/queries/clientpositive/drop_partitions_filter.q
          ql/src/test/queries/clientnegative/drop_partition_filter_failure.q
          ql/src/java/org/apache/hadoop/hive/ql/plan/DropTableDesc.java
          ql/src/java/org/apache/hadoop/hive/ql/plan/PartitionSpec.java
          ql/src/java/org/apache/hadoop/hive/ql/parse/Hive.g
          ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java
          ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java

          MANAGE HERALD DIFFERENTIAL RULES
          https://reviews.facebook.net/herald/view/differential/

          WHY DID I GET THIS EMAIL?
          https://reviews.facebook.net/herald/transcript/5685/

          Tip: use the X-Herald-Rules header to filter Herald messages in your client.

          Show
          Phabricator added a comment - sambavim requested code review of " HIVE-2908 [jira] Hive: Extend ALTER TABLE DROP PARTITION syntax to use all comparators". Reviewers: JIRA https://issues.apache.org/jira/browse/HIVE-2908 This diff introduces drop partition with non-equality comparisons on the partition key. You can now issue commands of the form: alter table <tbl> drop partition (key<value) to drop all partitions which satisfy the predicate. Changes were introduced to hive.g to allow the new operators: <, >, <>, !=, <=, >= in alter table drop partition statements. The change is implemented by using the method getPartitionsByFilter on the Hive object. In order to cleanup the passing around of the partition spec information, I introduced a new class called PartitionSpec which encapsulates one complete partition spec (multiple key value comparisons). The drop statement thus has a list of the PartitionSpec objects. Unit tests have been added to test the new functionality. Some existing test results needed an update due to the change in the AST for the partition spec. All unit tests passed with this change. The test results of the escape1.q test need an update as well but arc diff complains since it concludes the file is binary. I will work with the committer to make sure that change is captured before commit. To drop a partition from a Hive table, this works: ALTER TABLE foo DROP PARTITION(ds = 'date') ...but it should also work to drop all partitions prior to date. ALTER TABLE foo DROP PARTITION(ds < 'date') This task is to implement ALTER TABLE DROP PARTITION for all of the comparators, < > <= >= <> = != instead of just for =. TEST PLAN EMPTY REVISION DETAIL https://reviews.facebook.net/D2523 AFFECTED FILES ql/src/test/results/clientpositive/drop_partitions_filter.q.out ql/src/test/results/clientpositive/drop_multi_partitions.q.out ql/src/test/results/clientnegative/drop_partition_filter_failure.q.out ql/src/test/queries/clientpositive/drop_partitions_filter.q ql/src/test/queries/clientnegative/drop_partition_filter_failure.q ql/src/java/org/apache/hadoop/hive/ql/plan/DropTableDesc.java ql/src/java/org/apache/hadoop/hive/ql/plan/PartitionSpec.java ql/src/java/org/apache/hadoop/hive/ql/parse/Hive.g ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java MANAGE HERALD DIFFERENTIAL RULES https://reviews.facebook.net/herald/view/differential/ WHY DID I GET THIS EMAIL? https://reviews.facebook.net/herald/transcript/5685/ Tip: use the X-Herald-Rules header to filter Herald messages in your client.

            People

            • Assignee:
              Sambavi Muthukrishnan
              Reporter:
              Sambavi Muthukrishnan
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Time Tracking

                Estimated:
                Original Estimate - 96h
                96h
                Remaining:
                Remaining Estimate - 96h
                96h
                Logged:
                Time Spent - Not Specified
                Not Specified

                  Development