Skip to content
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.

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

[SPARK-8264][SQL]add substring_index function #7843

Closed
wants to merge 11 commits into from

Conversation

davies
Copy link
Contributor

@davies davies commented Jul 31, 2015

This PR is based on #7533 , thanks to @zhichao-li

Closes #7533

@SparkQA
Copy link

SparkQA commented Aug 1, 2015

Test build #39285 has finished for PR 7843 at commit 3ce7802.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • case class SubstringIndex(strExpr: Expression, delimExpr: Expression, countExpr: Expression)

@SparkQA
Copy link

SparkQA commented Aug 1, 2015

Test build #39288 has finished for PR 7843 at commit 391347b.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • case class SubstringIndex(strExpr: Expression, delimExpr: Expression, countExpr: Expression)

@davies
Copy link
Contributor Author

davies commented Aug 1, 2015

@rxin This is ready to review

@@ -154,6 +154,63 @@ class StringFunctionsSuite extends QueryTest {
Row(1))
}

test("string substring_index function") {
val df = Seq(("www.apache.org", ".", "zz")).toDF("a", "b", "c")
checkAnswer(
Copy link
Contributor

Choose a reason for hiding this comment

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

we can remove some of these tests since here we are mostly testing whether things work end to end, rather than unit testing all corner cases.

@rxin
Copy link
Contributor

rxin commented Aug 1, 2015

@davies is the behavior to return the entire string if there is not enough number of occurrence of the delimiter?

LGTM other than this question.

@davies
Copy link
Contributor Author

davies commented Aug 1, 2015

@rxin I think so.

@rxin
Copy link
Contributor

rxin commented Aug 1, 2015

Merging in master. When you submit your next patch for something else, just delete some of the tests in StringFunctionsSuite.

@asfgit asfgit closed this in 6996bd2 Aug 1, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants