Skip to content

[SPARK-23909][SQL] Add filter function. #21965

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
ueshin wants to merge 5 commits into from
Closed

[SPARK-23909][SQL] Add filter function. #21965

ueshin wants to merge 5 commits into from

Conversation

@ueshin
Copy link
Member

@ueshin ueshin commented Aug 2, 2018 •
edited
Loading

What changes were proposed in this pull request?

This pr adds filter function which filters the input array using the given predicate.

> SELECT filter(array(1, 2, 3), x -> x % 2 == 1);
 array(1, 3)

How was this patch tested?

Added tests.

@ueshin
Copy link
Member Author

ueshin commented Aug 2, 2018

This is based on #21954. I'll update after it is merged.

@SparkQA
Copy link

SparkQA commented Aug 2, 2018

Test build #93983 has finished for PR 21965 at commit fe48510.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
  • case class ArrayFilter(

@gatorsmile
Copy link
Member

gatorsmile commented Aug 2, 2018

@ueshin Please rebase it. Thanks!

@ueshin ueshin force-pushed the issues/SPARK-23909/filter branch from fe48510 to ace19dd Compare August 3, 2018 02:56
@ueshin ueshin changed the title [WIP][SPARK-23909][SQL] Add filter function. [SPARK-23909][SQL] Add filter function. Aug 3, 2018
@gatorsmile
Copy link
Member

gatorsmile commented Aug 3, 2018

cc @hvanhovell

@SparkQA
Copy link

SparkQA commented Aug 3, 2018

Test build #94088 has finished for PR 21965 at commit ace19dd.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
  • case class ArrayFilter(

@ueshin
Copy link
Member Author

ueshin commented Aug 3, 2018

Jenkins, retest this please.

@SparkQA
Copy link

SparkQA commented Aug 3, 2018

Test build #94107 has finished for PR 21965 at commit ace19dd.

  • This patch fails due to an unknown error code, -9.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
  • case class ArrayFilter(

@kiszk
Copy link
Member

kiszk commented Aug 3, 2018

retest this please

hvanhovell
hvanhovell reviewed Aug 3, 2018
View reviewed changes
...catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/higherOrderFunctions.scala Outdated
null
} else {
val f = functionForEval
val buffer = new mutable.ArrayBuffer[Any]
Copy link
Contributor

@hvanhovell hvanhovell Aug 3, 2018

Choose a reason for hiding this comment

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

I am wondering if we should use the buffer builder with a size hint here? Or, alternatively manage the array ourself.

Copy link
Member Author

@ueshin ueshin Aug 3, 2018

Choose a reason for hiding this comment

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

Sounds good. I'll use the original length as a size hint.

hvanhovell
hvanhovell reviewed Aug 3, 2018
View reviewed changes
...catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/higherOrderFunctions.scala
}
}

override def prettyName: String = "filter"
Copy link
Contributor

@hvanhovell hvanhovell Aug 3, 2018

Choose a reason for hiding this comment

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

Is filter too generic? wdyt?

Copy link
Member Author

@ueshin ueshin Aug 3, 2018

Choose a reason for hiding this comment

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

Yeah, it might be. How about array_filter?

hvanhovell
hvanhovell approved these changes Aug 3, 2018
View reviewed changes
Copy link
Contributor

@hvanhovell hvanhovell left a comment

Choose a reason for hiding this comment

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

LGTM - two small nits, that can be addressed later.

@ueshin ueshin changed the title [SPARK-23909][SQL] Add filter function. [SPARK-23909][SQL] Add array_filter function. Aug 3, 2018
@ueshin ueshin changed the title [SPARK-23909][SQL] Add array_filter function. [SPARK-23909][SQL] Add filter function. Aug 3, 2018
@SparkQA
Copy link

SparkQA commented Aug 3, 2018

Test build #94119 has finished for PR 21965 at commit e9af094.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@kiszk
Copy link
Member

kiszk commented Aug 3, 2018

retest this please

@SparkQA
Copy link

SparkQA commented Aug 3, 2018

Test build #94113 has finished for PR 21965 at commit ace19dd.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
  • case class ArrayFilter(

@SparkQA
Copy link

SparkQA commented Aug 3, 2018

Test build #94124 has finished for PR 21965 at commit f6aaa90.

  • This patch fails PySpark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Aug 3, 2018

Test build #94122 has finished for PR 21965 at commit e79ebbd.

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

@SparkQA
Copy link

SparkQA commented Aug 3, 2018

Test build #94128 has finished for PR 21965 at commit f6aaa90.

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

@ueshin
Copy link
Member Author

ueshin commented Aug 4, 2018

Thanks! merging to master.

@asfgit asfgit closed this in 0ecc132 Aug 4, 2018
@ueshin ueshin mentioned this pull request Aug 7, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hvanhovell hvanhovell hvanhovell approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@ueshin @SparkQA @gatorsmile @kiszk @hvanhovell