Skip to content

Conversation

@gatorsmile
Copy link
Member

What changes were proposed in this pull request?

This is a follow-up PR of #17633.

This PR is to add a conf spark.sql.hive.advancedPartitionPredicatePushdown.enabled, which can be used to turn the enhancement off.

How was this patch tested?

Add a test case

@SparkQA
Copy link

SparkQA commented Oct 21, 2017

Test build #82944 has finished for PR 19547 at commit 4057602.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

convertComplexFilters(table, filters)
} else {
convertBasicFilters(table, filters)
}
Copy link
Member

@dongjoon-hyun dongjoon-hyun Oct 21, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit, Can we remove the duplication of varcharKeys logic by moving into convertFilters like the following? table was removed from convertBasicFilters and convertCompexFilters, too.

def convertFilters(table: Table, filters: Seq[Expression]): String = {
    // hive varchar is treated as catalyst string, but hive varchar can't be pushed down.
    lazy val varcharKeys = table.getPartitionKeys.asScala
      .filter(col => col.getType.startsWith(serdeConstants.VARCHAR_TYPE_NAME) ||
        col.getType.startsWith(serdeConstants.CHAR_TYPE_NAME))
      .map(col => col.getName).toSet

    if (SQLConf.get.advancedPartitionPredicatePushdownEnabled) {
      convertComplexFilters(filters, varcharKeys)
    } else {
      convertBasicFilters(filters, varcharKeys)
    }
  }

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! We can do it when enhancing convertComplexFilters . Here, it is just to keep the original codes (from Spark 2.2) untouched.

Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1, LGTM.
This is really great to have this option. Thank you so much!

@gatorsmile
Copy link
Member Author

Thanks! Merged to master.

@asfgit asfgit closed this in d8cada8 Oct 21, 2017
rdblue pushed a commit to rdblue/spark that referenced this pull request Apr 3, 2019
…on pruning predicate pushdown

## What changes were proposed in this pull request?
This is a follow-up PR of apache#17633.

This PR is to add a conf `spark.sql.hive.advancedPartitionPredicatePushdown.enabled`, which can be used to turn the enhancement off.

## How was this patch tested?
Add a test case

Author: gatorsmile <gatorsmile@gmail.com>

Closes apache#19547 from gatorsmile/Spark20331FollowUp.

(cherry picked from commit d8cada8)
rdblue pushed a commit to rdblue/spark that referenced this pull request Apr 3, 2019
…on pruning predicate pushdown

This is a follow-up PR of apache#17633.

This PR is to add a conf `spark.sql.hive.advancedPartitionPredicatePushdown.enabled`, which can be used to turn the enhancement off.

Add a test case

Author: gatorsmile <gatorsmile@gmail.com>

Closes apache#19547 from gatorsmile/Spark20331FollowUp.

(cherry picked from commit d8cada8)

Conflicts:
	sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala
JsonLiuUp pushed a commit to JsonLiuUp/spark that referenced this pull request Dec 30, 2020
…on pruning predicate pushdown

## What changes were proposed in this pull request?
This is a follow-up PR of apache#17633.

This PR is to add a conf `spark.sql.hive.advancedPartitionPredicatePushdown.enabled`, which can be used to turn the enhancement off.

## How was this patch tested?
Add a test case

Author: gatorsmile <gatorsmile@gmail.com>

Closes apache#19547 from gatorsmile/Spark20331FollowUp.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants