Hive
  1. Hive
  2. HIVE-2702

Enhance listPartitionsByFilter to add support for integral types both for equality and non-equality

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.8.1
    • Fix Version/s: 0.12.0
    • Component/s: None
    • Labels:

      Description

      listPartitionsByFilter supports only non-string partitions. This is because its explicitly specified in generateJDOFilterOverPartitions in ExpressionTree.java.

      //Can only support partitions whose types are string
      if( ! table.getPartitionKeys().get(partitionColumnIndex).
      getType().equals(org.apache.hadoop.hive.serde.Constants.STRING_TYPE_NAME) )

      { throw new MetaException ("Filtering is supported only on partition keys of type string"); }
      1. HIVE-2702.1.patch
        5 kB
        Aniket Mokashi
      2. ASF.LICENSE.NOT.GRANTED--HIVE-2702.D2043.1.patch
        5 kB
        Phabricator
      3. HIVE-2702-v0.patch
        19 kB
        Sergey Shelukhin
      4. HIVE-2702.D11715.1.patch
        24 kB
        Phabricator
      5. HIVE-2702.D11715.2.patch
        27 kB
        Phabricator
      6. HIVE-2702.patch
        27 kB
        Sergey Shelukhin
      7. HIVE-2702.D11715.3.patch
        28 kB
        Phabricator
      8. HIVE-2702.D11847.1.patch
        26 kB
        Phabricator
      9. HIVE-2702.D11847.2.patch
        26 kB
        Phabricator

        Issue Links

          Activity

          Hide
          Lefty Leverenz added a comment -

          Doc note: See HIVE-7164 comment for information about setting hive.metastore.integral.jdo.pushdown.

          Show
          Lefty Leverenz added a comment - Doc note: See HIVE-7164 comment for information about setting hive.metastore.integral.jdo.pushdown . HIVE-7164 comment
          Hide
          Ashutosh Chauhan added a comment -

          This issue has been fixed and released as part of 0.12 release. If you find further issues, please create a new jira and link it to this one.

          Show
          Ashutosh Chauhan added a comment - This issue has been fixed and released as part of 0.12 release. If you find further issues, please create a new jira and link it to this one.
          Hide
          Hudson added a comment -

          ABORTED: Integrated in Hive-trunk-hadoop2 #319 (See https://builds.apache.org/job/Hive-trunk-hadoop2/319/)
          HIVE-2702 : Enhance listPartitionsByFilter to add support for integral types both for equality and non-equality (Sergey Shelukhin via Ashutosh Chauhan) (hashutosh: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1508539)

          • /hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/parser/ExpressionTree.java
          • /hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/parser/Filter.g
          • /hive/trunk/metastore/src/test/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java
          • /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java
          • /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/optimizer/ppr/PartitionPruner.java
          • /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java
          • /hive/trunk/ql/src/test/results/clientpositive/alter_partition_coltype.q.out
          Show
          Hudson added a comment - ABORTED: Integrated in Hive-trunk-hadoop2 #319 (See https://builds.apache.org/job/Hive-trunk-hadoop2/319/ ) HIVE-2702 : Enhance listPartitionsByFilter to add support for integral types both for equality and non-equality (Sergey Shelukhin via Ashutosh Chauhan) (hashutosh: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1508539 ) /hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/parser/ExpressionTree.java /hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/parser/Filter.g /hive/trunk/metastore/src/test/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/optimizer/ppr/PartitionPruner.java /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java /hive/trunk/ql/src/test/results/clientpositive/alter_partition_coltype.q.out
          Hide
          Hudson added a comment -

          FAILURE: Integrated in Hive-trunk-h0.21 #2234 (See https://builds.apache.org/job/Hive-trunk-h0.21/2234/)
          HIVE-2702 : Enhance listPartitionsByFilter to add support for integral types both for equality and non-equality (Sergey Shelukhin via Ashutosh Chauhan) (hashutosh: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1508539)

          • /hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/parser/ExpressionTree.java
          • /hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/parser/Filter.g
          • /hive/trunk/metastore/src/test/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java
          • /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java
          • /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/optimizer/ppr/PartitionPruner.java
          • /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java
          • /hive/trunk/ql/src/test/results/clientpositive/alter_partition_coltype.q.out
          Show
          Hudson added a comment - FAILURE: Integrated in Hive-trunk-h0.21 #2234 (See https://builds.apache.org/job/Hive-trunk-h0.21/2234/ ) HIVE-2702 : Enhance listPartitionsByFilter to add support for integral types both for equality and non-equality (Sergey Shelukhin via Ashutosh Chauhan) (hashutosh: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1508539 ) /hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/parser/ExpressionTree.java /hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/parser/Filter.g /hive/trunk/metastore/src/test/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/optimizer/ppr/PartitionPruner.java /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java /hive/trunk/ql/src/test/results/clientpositive/alter_partition_coltype.q.out
          Hide
          Hudson added a comment -

          SUCCESS: Integrated in Hive-trunk-hadoop1-ptest #109 (See https://builds.apache.org/job/Hive-trunk-hadoop1-ptest/109/)
          HIVE-2702 : Enhance listPartitionsByFilter to add support for integral types both for equality and non-equality (Sergey Shelukhin via Ashutosh Chauhan) (hashutosh: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1508539)

          • /hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/parser/ExpressionTree.java
          • /hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/parser/Filter.g
          • /hive/trunk/metastore/src/test/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java
          • /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java
          • /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/optimizer/ppr/PartitionPruner.java
          • /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java
          • /hive/trunk/ql/src/test/results/clientpositive/alter_partition_coltype.q.out
          Show
          Hudson added a comment - SUCCESS: Integrated in Hive-trunk-hadoop1-ptest #109 (See https://builds.apache.org/job/Hive-trunk-hadoop1-ptest/109/ ) HIVE-2702 : Enhance listPartitionsByFilter to add support for integral types both for equality and non-equality (Sergey Shelukhin via Ashutosh Chauhan) (hashutosh: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1508539 ) /hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/parser/ExpressionTree.java /hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/parser/Filter.g /hive/trunk/metastore/src/test/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/optimizer/ppr/PartitionPruner.java /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java /hive/trunk/ql/src/test/results/clientpositive/alter_partition_coltype.q.out
          Hide
          Hudson added a comment -

          FAILURE: Integrated in Hive-trunk-hadoop2-ptest #37 (See https://builds.apache.org/job/Hive-trunk-hadoop2-ptest/37/)
          HIVE-2702 : Enhance listPartitionsByFilter to add support for integral types both for equality and non-equality (Sergey Shelukhin via Ashutosh Chauhan) (hashutosh: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1508539)

          • /hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/parser/ExpressionTree.java
          • /hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/parser/Filter.g
          • /hive/trunk/metastore/src/test/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java
          • /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java
          • /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/optimizer/ppr/PartitionPruner.java
          • /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java
          • /hive/trunk/ql/src/test/results/clientpositive/alter_partition_coltype.q.out
          Show
          Hudson added a comment - FAILURE: Integrated in Hive-trunk-hadoop2-ptest #37 (See https://builds.apache.org/job/Hive-trunk-hadoop2-ptest/37/ ) HIVE-2702 : Enhance listPartitionsByFilter to add support for integral types both for equality and non-equality (Sergey Shelukhin via Ashutosh Chauhan) (hashutosh: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1508539 ) /hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/parser/ExpressionTree.java /hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/parser/Filter.g /hive/trunk/metastore/src/test/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/optimizer/ppr/PartitionPruner.java /hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java /hive/trunk/ql/src/test/results/clientpositive/alter_partition_coltype.q.out
          Hide
          Ashutosh Chauhan added a comment -

          Committed to trunk. Thanks, Sergey!

          Show
          Ashutosh Chauhan added a comment - Committed to trunk. Thanks, Sergey!
          Hide
          Phabricator added a comment -

          sershe updated the revision "HIVE-2702 [jira] listPartitionsByFilter only supports string partitions for equals".

          Adding the query change. Fetching partition "dt=100x" for query dt = 100 seems incorrect

          Reviewers: ashutoshc, JIRA

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

          CHANGE SINCE LAST DIFF
          https://reviews.facebook.net/D11847?vs=36303&id=36483#toc

          BRANCH
          HIVE-2702-2

          ARCANIST PROJECT
          hive

          AFFECTED FILES
          metastore/src/java/org/apache/hadoop/hive/metastore/parser/ExpressionTree.java
          metastore/src/java/org/apache/hadoop/hive/metastore/parser/Filter.g
          metastore/src/test/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java
          ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java
          ql/src/java/org/apache/hadoop/hive/ql/optimizer/ppr/PartitionPruner.java
          ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java
          ql/src/test/results/clientpositive/alter_partition_coltype.q.out

          To: JIRA, ashutoshc, sershe

          Show
          Phabricator added a comment - sershe updated the revision " HIVE-2702 [jira] listPartitionsByFilter only supports string partitions for equals". Adding the query change. Fetching partition "dt=100x" for query dt = 100 seems incorrect Reviewers: ashutoshc, JIRA REVISION DETAIL https://reviews.facebook.net/D11847 CHANGE SINCE LAST DIFF https://reviews.facebook.net/D11847?vs=36303&id=36483#toc BRANCH HIVE-2702 -2 ARCANIST PROJECT hive AFFECTED FILES metastore/src/java/org/apache/hadoop/hive/metastore/parser/ExpressionTree.java metastore/src/java/org/apache/hadoop/hive/metastore/parser/Filter.g metastore/src/test/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java ql/src/java/org/apache/hadoop/hive/ql/optimizer/ppr/PartitionPruner.java ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java ql/src/test/results/clientpositive/alter_partition_coltype.q.out To: JIRA, ashutoshc, sershe
          Hide
          Ashutosh Chauhan added a comment -

          I ran the tests on current patch on latest trunk. All tests passed except alter_partition_coltype.q for which you just need to update .q.out file. Can you also include that update in your next refresh of the patch?

          Show
          Ashutosh Chauhan added a comment - I ran the tests on current patch on latest trunk. All tests passed except alter_partition_coltype.q for which you just need to update .q.out file. Can you also include that update in your next refresh of the patch?
          Hide
          Phabricator added a comment -

          ashutoshc has accepted the revision "HIVE-2702 [jira] listPartitionsByFilter only supports string partitions for equals".

          +1 Looks good.
          Few minor nits.

          INLINE COMMENTS
          metastore/src/java/org/apache/hadoop/hive/metastore/parser/Filter.g:160 Do u want to name this as IntegralLiteral now ?
          metastore/src/java/org/apache/hadoop/hive/metastore/parser/ExpressionTree.java:312 Do you instead want to say in this TODO that this will be dealt in HIVE-4888
          ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java:2188 Do you want to say for 2nd TODO that this will be dealt in HIVE-4888

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

          BRANCH
          HIVE-2702-2

          ARCANIST PROJECT
          hive

          To: JIRA, ashutoshc, sershe

          Show
          Phabricator added a comment - ashutoshc has accepted the revision " HIVE-2702 [jira] listPartitionsByFilter only supports string partitions for equals". +1 Looks good. Few minor nits. INLINE COMMENTS metastore/src/java/org/apache/hadoop/hive/metastore/parser/Filter.g:160 Do u want to name this as IntegralLiteral now ? metastore/src/java/org/apache/hadoop/hive/metastore/parser/ExpressionTree.java:312 Do you instead want to say in this TODO that this will be dealt in HIVE-4888 ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java:2188 Do you want to say for 2nd TODO that this will be dealt in HIVE-4888 REVISION DETAIL https://reviews.facebook.net/D11847 BRANCH HIVE-2702 -2 ARCANIST PROJECT hive To: JIRA, ashutoshc, sershe
          Hide
          Ashutosh Chauhan added a comment -

          Do you want to improve the description of ticket ... something like
          Enhance listPartitionsByFilter to add support for integral types both for equality and non-equality

          Show
          Ashutosh Chauhan added a comment - Do you want to improve the description of ticket ... something like Enhance listPartitionsByFilter to add support for integral types both for equality and non-equality
          Hide
          Phabricator added a comment -

          sershe requested code review of "HIVE-2702 [jira] listPartitionsByFilter only supports string partitions for equals".

          Reviewers: JIRA

          Rebase on top of HIVE-4929. It should still compile/pass server tests, but won't work properly before HIVE-4929

          listPartitionsByFilter supports only non-string partitions. This is because its explicitly specified in generateJDOFilterOverPartitions in ExpressionTree.java.

          //Can only support partitions whose types are string
          if( ! table.getPartitionKeys().get(partitionColumnIndex).
          getType().equals(org.apache.hadoop.hive.serde.Constants.STRING_TYPE_NAME) )

          { throw new MetaException ("Filtering is supported only on partition keys of type string"); }

          TEST PLAN
          EMPTY

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

          AFFECTED FILES
          metastore/src/java/org/apache/hadoop/hive/metastore/parser/ExpressionTree.java
          metastore/src/java/org/apache/hadoop/hive/metastore/parser/Filter.g
          metastore/src/test/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java
          ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java
          ql/src/java/org/apache/hadoop/hive/ql/optimizer/ppr/PartitionPruner.java
          ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java

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

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

          To: JIRA, sershe

          Show
          Phabricator added a comment - sershe requested code review of " HIVE-2702 [jira] listPartitionsByFilter only supports string partitions for equals". Reviewers: JIRA Rebase on top of HIVE-4929 . It should still compile/pass server tests, but won't work properly before HIVE-4929 listPartitionsByFilter supports only non-string partitions. This is because its explicitly specified in generateJDOFilterOverPartitions in ExpressionTree.java. //Can only support partitions whose types are string if( ! table.getPartitionKeys().get(partitionColumnIndex). getType().equals(org.apache.hadoop.hive.serde.Constants.STRING_TYPE_NAME) ) { throw new MetaException ("Filtering is supported only on partition keys of type string"); } TEST PLAN EMPTY REVISION DETAIL https://reviews.facebook.net/D11847 AFFECTED FILES metastore/src/java/org/apache/hadoop/hive/metastore/parser/ExpressionTree.java metastore/src/java/org/apache/hadoop/hive/metastore/parser/Filter.g metastore/src/test/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java ql/src/java/org/apache/hadoop/hive/ql/optimizer/ppr/PartitionPruner.java ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java MANAGE HERALD RULES https://reviews.facebook.net/herald/view/differential/ WHY DID I GET THIS EMAIL? https://reviews.facebook.net/herald/transcript/28167/ To: JIRA, sershe
          Hide
          Sergey Shelukhin added a comment -

          Large number of tests failed due to removal of the conversion of everything to double. I will fix them and update.

          Show
          Sergey Shelukhin added a comment - Large number of tests failed due to removal of the conversion of everything to double. I will fix them and update.
          Hide
          Phabricator added a comment -

          ashutoshc has accepted the revision "HIVE-2702 [jira] listPartitionsByFilter only supports string partitions".

          +1.
          Sergey, How did the tests go?

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

          BRANCH
          HIVE-2702

          ARCANIST PROJECT
          hive

          To: JIRA, ashutoshc, sershe

          Show
          Phabricator added a comment - ashutoshc has accepted the revision " HIVE-2702 [jira] listPartitionsByFilter only supports string partitions". +1. Sergey, How did the tests go? REVISION DETAIL https://reviews.facebook.net/D11715 BRANCH HIVE-2702 ARCANIST PROJECT hive To: JIRA, ashutoshc, sershe
          Hide
          Phabricator added a comment -

          sershe updated the revision "HIVE-2702 [jira] listPartitionsByFilter only supports string partitions".

          CR feedback. Hive QA appears to be sleeping, I will run the tests overnight.

          Reviewers: ashutoshc, JIRA

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

          CHANGE SINCE LAST DIFF
          https://reviews.facebook.net/D11715?vs=35919&id=35991#toc

          AFFECTED FILES
          metastore/src/java/org/apache/hadoop/hive/metastore/parser/ExpressionTree.java
          metastore/src/java/org/apache/hadoop/hive/metastore/parser/Filter.g
          metastore/src/test/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java
          ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java
          ql/src/java/org/apache/hadoop/hive/ql/optimizer/ppr/PartitionPruner.java
          ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java
          ql/src/java/org/apache/hadoop/hive/ql/parse/TypeCheckProcFactory.java

          To: JIRA, ashutoshc, sershe

          Show
          Phabricator added a comment - sershe updated the revision " HIVE-2702 [jira] listPartitionsByFilter only supports string partitions". CR feedback. Hive QA appears to be sleeping, I will run the tests overnight. Reviewers: ashutoshc, JIRA REVISION DETAIL https://reviews.facebook.net/D11715 CHANGE SINCE LAST DIFF https://reviews.facebook.net/D11715?vs=35919&id=35991#toc AFFECTED FILES metastore/src/java/org/apache/hadoop/hive/metastore/parser/ExpressionTree.java metastore/src/java/org/apache/hadoop/hive/metastore/parser/Filter.g metastore/src/test/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java ql/src/java/org/apache/hadoop/hive/ql/optimizer/ppr/PartitionPruner.java ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java ql/src/java/org/apache/hadoop/hive/ql/parse/TypeCheckProcFactory.java To: JIRA, ashutoshc, sershe
          Hide
          Sergey Shelukhin added a comment -

          attaching patch for hiveqa.

          Show
          Sergey Shelukhin added a comment - attaching patch for hiveqa.
          Hide
          Phabricator added a comment -

          ashutoshc has requested changes to the revision "HIVE-2702 [jira] listPartitionsByFilter only supports string partitions".

          Some comments.

          INLINE COMMENTS
          metastore/src/java/org/apache/hadoop/hive/metastore/parser/ExpressionTree.java:161 Its better to say numeric type here, since we can also support byte and short (both of which are valid hive types) as well.
          metastore/src/java/org/apache/hadoop/hive/metastore/parser/ExpressionTree.java:262 As stated earlier, lets rename this method and variable as doesOpSupportNumeric()
          metastore/src/java/org/apache/hadoop/hive/metastore/parser/ExpressionTree.java:272 Why are you checking value instanceof Long here ? This parsing is not done via grammar described in Filter.g
          metastore/src/java/org/apache/hadoop/hive/metastore/parser/ExpressionTree.java:299 I presume this depends on DN upgrade, lets postpone it till than.
          ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java:7699 HIVE-3059, I assume you want to say.

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

          BRANCH
          HIVE-2702

          ARCANIST PROJECT
          hive

          To: JIRA, ashutoshc, sershe

          Show
          Phabricator added a comment - ashutoshc has requested changes to the revision " HIVE-2702 [jira] listPartitionsByFilter only supports string partitions". Some comments. INLINE COMMENTS metastore/src/java/org/apache/hadoop/hive/metastore/parser/ExpressionTree.java:161 Its better to say numeric type here, since we can also support byte and short (both of which are valid hive types) as well. metastore/src/java/org/apache/hadoop/hive/metastore/parser/ExpressionTree.java:262 As stated earlier, lets rename this method and variable as doesOpSupportNumeric() metastore/src/java/org/apache/hadoop/hive/metastore/parser/ExpressionTree.java:272 Why are you checking value instanceof Long here ? This parsing is not done via grammar described in Filter.g metastore/src/java/org/apache/hadoop/hive/metastore/parser/ExpressionTree.java:299 I presume this depends on DN upgrade, lets postpone it till than. ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java:7699 HIVE-3059 , I assume you want to say. REVISION DETAIL https://reviews.facebook.net/D11715 BRANCH HIVE-2702 ARCANIST PROJECT hive To: JIRA, ashutoshc, sershe
          Hide
          Sergey Shelukhin added a comment -

          ran some perf tests, filter pushdown can cut about 300ms for getting partitions on 16k-partition table on my machine, with standalone metastore on the same machine (for equivalent queries e.g. n=0 vs "n=0 and n>=0")

          Show
          Sergey Shelukhin added a comment - ran some perf tests, filter pushdown can cut about 300ms for getting partitions on 16k-partition table on my machine, with standalone metastore on the same machine (for equivalent queries e.g. n=0 vs "n=0 and n>=0")
          Hide
          Phabricator added a comment -

          sershe updated the revision "HIVE-2702 [jira] listPartitionsByFilter only supports string partitions".

          Some minor fixes to the patch, remove client support for most compare ops since it cannot easily be supported due to DN bugs/missing features in 2.0, fix the number type scrambling that was causing pushdown to not work for integers.

          Reviewers: JIRA, ashutoshc

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

          CHANGE SINCE LAST DIFF
          https://reviews.facebook.net/D11715?vs=35793&id=35919#toc

          AFFECTED FILES
          metastore/src/java/org/apache/hadoop/hive/metastore/parser/ExpressionTree.java
          metastore/src/java/org/apache/hadoop/hive/metastore/parser/Filter.g
          metastore/src/test/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java
          ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java
          ql/src/java/org/apache/hadoop/hive/ql/optimizer/ppr/PartitionPruner.java
          ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java
          ql/src/java/org/apache/hadoop/hive/ql/parse/TypeCheckProcFactory.java

          To: JIRA, ashutoshc, sershe

          Show
          Phabricator added a comment - sershe updated the revision " HIVE-2702 [jira] listPartitionsByFilter only supports string partitions". Some minor fixes to the patch, remove client support for most compare ops since it cannot easily be supported due to DN bugs/missing features in 2.0, fix the number type scrambling that was causing pushdown to not work for integers. Reviewers: JIRA, ashutoshc REVISION DETAIL https://reviews.facebook.net/D11715 CHANGE SINCE LAST DIFF https://reviews.facebook.net/D11715?vs=35793&id=35919#toc AFFECTED FILES metastore/src/java/org/apache/hadoop/hive/metastore/parser/ExpressionTree.java metastore/src/java/org/apache/hadoop/hive/metastore/parser/Filter.g metastore/src/test/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java ql/src/java/org/apache/hadoop/hive/ql/optimizer/ppr/PartitionPruner.java ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java ql/src/java/org/apache/hadoop/hive/ql/parse/TypeCheckProcFactory.java To: JIRA, ashutoshc, sershe
          Hide
          Sergey Shelukhin added a comment -

          Actually, as a quick workaround I can change the part of HIVE-2249 that changes the legitimately derived integer type to double if the other column is string. I don't think it makes sense... I feel like an archeologist by now

          Show
          Sergey Shelukhin added a comment - Actually, as a quick workaround I can change the part of HIVE-2249 that changes the legitimately derived integer type to double if the other column is string. I don't think it makes sense... I feel like an archeologist by now
          Hide
          Sergey Shelukhin added a comment -

          Well, this is also blocked (kind of) by the hack where during query parsing we set the types of all partitioning columns to string. This causes the constant type to be changed to double thru a complex chain of events, unless it's explicitly set to bigint.
          This causes the validation to fail and filter to not be pushed down.
          I am not sure why HIVE-3059 reverted it, there are no details...

          Show
          Sergey Shelukhin added a comment - Well, this is also blocked (kind of) by the hack where during query parsing we set the types of all partitioning columns to string. This causes the constant type to be changed to double thru a complex chain of events, unless it's explicitly set to bigint. This causes the validation to fail and filter to not be pushed down. I am not sure why HIVE-3059 reverted it, there are no details...
          Hide
          Sergey Shelukhin added a comment -

          gt/lt/etc. hit HIVE-2609 unfortunately; I simplified valString to avoid nested substrings but that didn't help either, and Collection get method is not supported on DN 2.0.
          Making blocked on DN upgrade

          Show
          Sergey Shelukhin added a comment - gt/lt/etc. hit HIVE-2609 unfortunately; I simplified valString to avoid nested substrings but that didn't help either, and Collection get method is not supported on DN 2.0. Making blocked on DN upgrade
          Hide
          Phabricator added a comment -

          sershe requested code review of "HIVE-2702 [jira] listPartitionsByFilter only supports string partitions".

          Reviewers: JIRA

          initial changes and tests, this time posting to Phabricator; compared to previous patch in the JIRA, added testing, as well as support for negative numbers.
          I will run some perf tests, locally on derby it seems to work well.

          listPartitionsByFilter supports only non-string partitions. This is because its explicitly specified in generateJDOFilterOverPartitions in ExpressionTree.java.

          TEST PLAN
          EMPTY

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

          AFFECTED FILES
          metastore/src/java/org/apache/hadoop/hive/metastore/parser/ExpressionTree.java
          metastore/src/java/org/apache/hadoop/hive/metastore/parser/Filter.g
          metastore/src/test/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java
          ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java
          ql/src/java/org/apache/hadoop/hive/ql/optimizer/ppr/PartitionPruner.java

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

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

          To: JIRA, sershe

          Show
          Phabricator added a comment - sershe requested code review of " HIVE-2702 [jira] listPartitionsByFilter only supports string partitions". Reviewers: JIRA initial changes and tests, this time posting to Phabricator; compared to previous patch in the JIRA, added testing, as well as support for negative numbers. I will run some perf tests, locally on derby it seems to work well. listPartitionsByFilter supports only non-string partitions. This is because its explicitly specified in generateJDOFilterOverPartitions in ExpressionTree.java. TEST PLAN EMPTY REVISION DETAIL https://reviews.facebook.net/D11715 AFFECTED FILES metastore/src/java/org/apache/hadoop/hive/metastore/parser/ExpressionTree.java metastore/src/java/org/apache/hadoop/hive/metastore/parser/Filter.g metastore/src/test/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java ql/src/java/org/apache/hadoop/hive/ql/optimizer/ppr/PartitionPruner.java MANAGE HERALD RULES https://reviews.facebook.net/herald/view/differential/ WHY DID I GET THIS EMAIL? https://reviews.facebook.net/herald/transcript/27771/ To: JIRA, sershe
          Hide
          Sergey Shelukhin added a comment -

          Preliminary patch. No tests so far, in case something is wrong in the approach, this is my first hive patch.
          Existing tests pass.

          Patch summary: for filter pushdown,

          • add support for equals for integral partition types;
          • add support for not-equals for both;
          • allow the client to push down other compare ops for string partitions - server already supports that;
          • change Filter.g integral type handling to use Long instead of Integer, to support bigger numbers;
          • refactor generateJDOFilterOverPartitions a bit.

          Adding other compare operators for integral types is next. I added a TODO in the patch where it should be done. The approach will need to be considered more carefully and may involve some refactoring.

          Show
          Sergey Shelukhin added a comment - Preliminary patch. No tests so far, in case something is wrong in the approach, this is my first hive patch. Existing tests pass. Patch summary: for filter pushdown, add support for equals for integral partition types; add support for not-equals for both; allow the client to push down other compare ops for string partitions - server already supports that; change Filter.g integral type handling to use Long instead of Integer, to support bigger numbers; refactor generateJDOFilterOverPartitions a bit. Adding other compare operators for integral types is next. I added a TODO in the patch where it should be done. The approach will need to be considered more carefully and may involve some refactoring.
          Hide
          Sergey Shelukhin added a comment -

          There hasn't been activity for a while, I will take over if you don't mind

          Show
          Sergey Shelukhin added a comment - There hasn't been activity for a while, I will take over if you don't mind
          Hide
          cyril liao added a comment -

          Drop partition get org.apache.hadoop.hive.ql.parse.SemanticException because of this design.
          Drop partition is a base function,why this design exist if a base function can not work!

          Show
          cyril liao added a comment - Drop partition get org.apache.hadoop.hive.ql.parse.SemanticException because of this design. Drop partition is a base function,why this design exist if a base function can not work!
          Hide
          Jakob Homan added a comment -

          This is very limiting for HCat as it means HCat clients cannot access non-string partitions from Pig or MR, whereas they are available via Hive directly. The JDOQL that's used in generateJDOFilter() to generate the query is not powerful enough to support stripping out the value, casting it to a number, and then sorting it thusly. The best solution is probably to re-write ObjectStore.listPartitionNamesByFilter() to not use the Partitions table (which stores the values as "PARTITION=FOO" but rather take advantage of PARTITION_KEY_VALS, which stores the partition values by themselves. Any objections to this approach?

          Show
          Jakob Homan added a comment - This is very limiting for HCat as it means HCat clients cannot access non-string partitions from Pig or MR, whereas they are available via Hive directly. The JDOQL that's used in generateJDOFilter() to generate the query is not powerful enough to support stripping out the value, casting it to a number, and then sorting it thusly. The best solution is probably to re-write ObjectStore.listPartitionNamesByFilter() to not use the Partitions table (which stores the values as "PARTITION=FOO" but rather take advantage of PARTITION_KEY_VALS, which stores the partition values by themselves. Any objections to this approach?
          Hide
          Aniket Mokashi added a comment -

          I agree on your comment, I didn't catch it earlier. I wonder if there is an elegant workaround for this.

          Show
          Aniket Mokashi added a comment - I agree on your comment, I didn't catch it earlier. I wonder if there is an elegant workaround for this.
          Hide
          Ashutosh Chauhan added a comment -

          @Aniket,
          This is by design. Partition values are stored as strings in backend db (mysql) so pushing filters into db where partition column is of numeric types won't work, since then comparison will happen lexicographically. You should be able to catch this with rigorous tests. e.g., with your patch on, create table with partition key of int type, add partitions 1-11 and then do filter p < 2 and you will get partitions 1,10,11 instead of 1. Though, you can still push equality predicate.
          Enabling this feature requires mysql table schema updates which can retain type information for partition keys.

          Show
          Ashutosh Chauhan added a comment - @Aniket, This is by design. Partition values are stored as strings in backend db (mysql) so pushing filters into db where partition column is of numeric types won't work, since then comparison will happen lexicographically. You should be able to catch this with rigorous tests. e.g., with your patch on, create table with partition key of int type, add partitions 1-11 and then do filter p < 2 and you will get partitions 1,10,11 instead of 1. Though, you can still push equality predicate. Enabling this feature requires mysql table schema updates which can retain type information for partition keys.
          Hide
          Phabricator added a comment -

          aniket486 requested code review of "HIVE-2702 [jira] listPartitionsByFilter only supports non-string partitions".
          Reviewers: JIRA

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

          Added support for integer partitions in listPartitionByFilter method

          listPartitionsByFilter supports only non-string partitions. This is because its explicitly specified in generateJDOFilterOverPartitions in ExpressionTree.java.

          //Can only support partitions whose types are string
          if( ! table.getPartitionKeys().get(partitionColumnIndex).
          getType().equals(org.apache.hadoop.hive.serde.Constants.STRING_TYPE_NAME) )

          { throw new MetaException ("Filtering is supported only on partition keys of type string"); }

          TEST PLAN
          EMPTY

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

          AFFECTED FILES
          metastore/src/test/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java
          metastore/src/java/org/apache/hadoop/hive/metastore/parser/ExpressionTree.java

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

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

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

          Show
          Phabricator added a comment - aniket486 requested code review of " HIVE-2702 [jira] listPartitionsByFilter only supports non-string partitions". Reviewers: JIRA https://issues.apache.org/jira/browse/HIVE-2702 Added support for integer partitions in listPartitionByFilter method listPartitionsByFilter supports only non-string partitions. This is because its explicitly specified in generateJDOFilterOverPartitions in ExpressionTree.java. //Can only support partitions whose types are string if( ! table.getPartitionKeys().get(partitionColumnIndex). getType().equals(org.apache.hadoop.hive.serde.Constants.STRING_TYPE_NAME) ) { throw new MetaException ("Filtering is supported only on partition keys of type string"); } TEST PLAN EMPTY REVISION DETAIL https://reviews.facebook.net/D2043 AFFECTED FILES metastore/src/test/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java metastore/src/java/org/apache/hadoop/hive/metastore/parser/ExpressionTree.java MANAGE HERALD DIFFERENTIAL RULES https://reviews.facebook.net/herald/view/differential/ WHY DID I GET THIS EMAIL? https://reviews.facebook.net/herald/transcript/4395/ Tip: use the X-Herald-Rules header to filter Herald messages in your client.

            People

            • Assignee:
              Sergey Shelukhin
              Reporter:
              Aniket Mokashi
            • Votes:
              0 Vote for this issue
              Watchers:
              11 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development