Skip to content

[SPARK-17091] Add rule to convert IN predicate to equivalent Parquet filter. - #18424

Closed
ptkool wants to merge 1 commit into
apache:masterfrom
ptkool:convert_in_predicate_for_parquet
Closed

[SPARK-17091] Add rule to convert IN predicate to equivalent Parquet filter.#18424
ptkool wants to merge 1 commit into
apache:masterfrom
ptkool:convert_in_predicate_for_parquet

Conversation

@ptkool

@ptkoolptkool commented Jun 26, 2017

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

Add a new optimizer rule to convert an IN predicate to an equivalent Parquet filter.

How was this patch tested?

Tested using unit tests, integration tests, and manual tests.

@ptkool
ptkoolforce-pushed the convert_in_predicate_for_parquet branch from e98fbd8 to 8c443a7CompareJune 26, 2017 19:06
@HyukjinKwon

HyukjinKwon commented Jun 26, 2017

Copy link
Copy Markdown
Member

Is it a duplicate of #14671? cc @a10y.

@a10y

a10y commented Jun 26, 2017

Copy link
Copy Markdown

Indeed it appears to be. The resolution from my previous PR was that per @HyukjinKwon's benchmarks, performing the disjunction in Spark was slightly more performant than pushing it down to Parquet.

I haven't been following Spark closely these past 12 months so things may have changed.

@ptkool did you do any profiling that would lead you to believe pushing the filter to Parquet leads to perf improvements?

@ptkool

ptkool commented Jun 27, 2017

Copy link
Copy Markdown
ContributorAuthor

@a10y Yes. Please have a look at my comments in https://issues.apache.org/jira/browse/SPARK-21218.

@HyukjinKwon

HyukjinKwon commented Jun 28, 2017

Copy link
Copy Markdown
Member

(I would like to suggest fix the JIRA in the PR title to point out SPARK-17091)

@ptkoolptkool changed the title [SPARK-21218] Add rule to convert IN predicate to equivalent Parquet filter.[SPARK-17091] Add rule to convert IN predicate to equivalent Parquet filter.Jun 28, 2017
@rxin

rxin commented Jun 30, 2017

Copy link
Copy Markdown
Contributor

Have you done actual benchmarks to validate that this is a perf improvement?

@ptkool

ptkool commented Jul 5, 2017

Copy link
Copy Markdown
ContributorAuthor

@rxin Yes, I have.

https://issues.apache.org/jira/browse/SPARK-21218?focusedCommentId=16064608&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16064608

gatorsmile
gatorsmile reviewed Jul 8, 2017
View reviewed changes
Comment threadsql/core/src/main/scala/org/apache/spark/sql/execution/datasources/parquet/ParquetFilters.scala Outdated

@gatorsmilegatorsmileJul 8, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Always push-down? Should we also consider the number of elements in values? What is the performance impact when the number of values is around 10 or more?

a10y
a10y reviewed Jul 16, 2017
View reviewed changes
Comment threadsql/core/src/main/scala/org/apache/spark/sql/execution/datasources/parquet/ParquetFilters.scala Outdated

@a10ya10yJul 16, 2017

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You can eliminate the var by using reduceLeft

@HyukjinKwon

HyukjinKwon commented Nov 14, 2017

Copy link
Copy Markdown
Member

ok to test

@HyukjinKwon

HyukjinKwon commented Nov 14, 2017

Copy link
Copy Markdown
Member

Now we will have this for row group filtering in most cases after #15049. I believe it makes sense in this case.

@SparkQA

SparkQA commented Nov 14, 2017

Copy link
Copy Markdown

Test build #83844 has finished for PR 18424 at commit 8c443a7.

  • This patch fails some tests.
  • This patch does not merge cleanly.
  • This patch adds no public classes.

@HyukjinKwon

HyukjinKwon commented Nov 14, 2017

Copy link
Copy Markdown
Member

I believe I need to cc @jiangxb1987 and @viirya too who activiely reviewed my PR.

@HyukjinKwon

HyukjinKwon commented Nov 14, 2017

Copy link
Copy Markdown
Member

cc @liancheng too who I know is insightful in this.

@jiangxb1987

jiangxb1987 commented Nov 14, 2017

Copy link
Copy Markdown
Contributor

Please rebase this PR to the latest master. Thanks!

@viirya

viirya commented Nov 16, 2017

Copy link
Copy Markdown
Member

I guess this is inactive now.

@a10y

a10y commented Dec 1, 2017

Copy link
Copy Markdown

@ptkool are you still tracking this at all?

@ptkool

ptkool commented Dec 4, 2017

Copy link
Copy Markdown
ContributorAuthor

@a10y Yes, I'm still tracking this.

@ptkool
ptkoolforce-pushed the convert_in_predicate_for_parquet branch from 8c443a7 to 62f273bCompareDecember 4, 2017 13:25
@HyukjinKwon

HyukjinKwon commented Jan 1, 2018

Copy link
Copy Markdown
Member

ok to test

@SparkQA

SparkQA commented Jan 1, 2018

Copy link
Copy Markdown

Test build #85573 has finished for PR 18424 at commit 62f273b.

  • This patch fails Scala style tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@HyukjinKwon

HyukjinKwon commented Jun 9, 2018

Copy link
Copy Markdown
Member

ok to test

@SparkQA

SparkQA commented Jun 9, 2018

Copy link
Copy Markdown

Test build #91614 has finished for PR 18424 at commit 62f273b.

  • This patch fails Scala style tests.
  • This patch does not merge cleanly.
  • This patch adds no public classes.

@wangyum

wangyum commented Jun 19, 2018

Copy link
Copy Markdown
Member

@ptkool Are you still working on?

@HyukjinKwon

HyukjinKwon commented Jun 20, 2018

Copy link
Copy Markdown
Member

@wangyum, can you take over this? Seems it's been inactive long time.

@wangyumwangyum mentioned this pull request Jun 21, 2018
Closed
HyukjinKwon pushed a commit to HyukjinKwon/spark that referenced this pull request Jul 14, 2018
@wangyum@HyukjinKwon
[SPARK-17091][SQL] Add rule to convert IN predicate to equivalent Par…
e1de341
…quet filter
## What changes were proposed in this pull request?
The original pr is: apache#18424
Add a new optimizer rule to convert an IN predicate to an equivalent Parquet filter and add `spark.sql.parquet.pushdown.inFilterThreshold` to control limit thresholds. Different data types have different limit thresholds, this is a copy of data for reference:
Type | limit threshold
-- | --
string | 370
int | 210
long | 285
double | 270
float | 220
decimal | Won't provide better performance before [SPARK-24549](https://issues.apache.org/jira/browse/SPARK-24549)
## How was this patch tested?
unit tests and manual tests
Author: Yuming Wang <yumwang@ebay.com>
Closesapache#21603 from wangyum/SPARK-17091.
@wangyumwangyum mentioned this pull request Aug 21, 2018
Closed
@ptkool
ptkool deleted the convert_in_predicate_for_parquet branch January 18, 2020 12:14
zifeif2 pushed a commit to zifeif2/spark that referenced this pull request Nov 22, 2025
@srowen@zifeif2
[BUILD] Close stale PRs
75ff05e
Closesapache#16411Closesapache#21870Closesapache#21794Closesapache#21610Closesapache#21961Closesapache#21940Closesapache#21870Closesapache#22118Closesapache#21624Closesapache#19528Closesapache#18424Closesapache#22159 from srowen/Stale.
Authored-by: Sean Owen <sean.owen@databricks.com>
Signed-off-by: Sean Owen <sean.owen@databricks.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gatorsmilegatorsmilegatorsmile left review comments
+1 more reviewer
@a10ya10ya10y left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

9 participants

@ptkool@HyukjinKwon@a10y@rxin@SparkQA@jiangxb1987@viirya@wangyum@gatorsmile