Skip to content

[SPARK-23291][R][FOLLOWUP] Update SparkR migration note for SPARK-23291 - #21249

Closed
HyukjinKwon wants to merge 3 commits into
apache:masterfrom
HyukjinKwon:SPARK-23291
Closed

[SPARK-23291][R][FOLLOWUP] Update SparkR migration note for SPARK-23291#21249
HyukjinKwon wants to merge 3 commits into
apache:masterfrom
HyukjinKwon:SPARK-23291

Conversation

@HyukjinKwon

@HyukjinKwonHyukjinKwon commented May 6, 2018

Copy link
Copy Markdown
Member

What changes were proposed in this pull request?

This PR fixes the migration note for SPARK-23291 since it's going to backport to 2.3.1. See the discussion in https://issues.apache.org/jira/browse/SPARK-23291

How was this patch tested?

N/A

@HyukjinKwonHyukjinKwon changed the title [SPARK-23291][R][FOLLOWUPUpdate SparkR migration note for SPARK-23291[SPARK-23291][R][FOLLOWUP] Update SparkR migration note for SPARK-23291May 6, 2018
@HyukjinKwon

Copy link
Copy Markdown
MemberAuthor

@viiryaviirya left a comment

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.

LGTM

@SparkQA

Copy link
Copy Markdown

Test build #90271 has finished for PR 21249 at commit 1c6c9a2.

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

Comment threaddocs/sparkr.md Outdated
## Upgrading to SparkR 2.3.1 and above

- The `start` parameter of `substr` method was wrongly subtracted by one, previously. In other words, the index specified by `start` parameter was considered as 0-base. This can lead to inconsistent substring results and also does not match with the behaviour with `substr` in R. It has been fixed so the `start` parameter of `substr` method is now 1-base, e.g., therefore to get the same result as `substr(df$a, 2, 5)`, it should be changed to `substr(df$a, 1, 4)`.
- In SparkR 2.3.0 and earlier, the `start` parameter of `substr` method was wrongly subtracted by one, previously. In other words, the index specified by `start` parameter was considered as 0-base. This can lead to inconsistent substring results and also does not match with the behaviour with `substr` in R. In version 2.3.1 and later, it has been fixed so the `start` parameter of `substr` method is now 1-base. As an example, `substr(lit('abcdef'), 2, 4))` would result to `abc` in SparkR 2.3.0, and the result would be `bcd` in SparkR 2.3.1.

@viiryaviiryaMay 7, 2018

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.

nit: ...in SparkR 2.3.0 and earlier, and the result would be `bcd` in SparkR 2.3.1 and later.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

I think it's fine since it's an example ...

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.

ok. :)

@SparkQA

Copy link
Copy Markdown

Test build #90309 has finished for PR 21249 at commit 6c4743a.

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

@cloud-fan

Copy link
Copy Markdown
Contributor

LGTM

@SparkQA

Copy link
Copy Markdown

Test build #90326 has finished for PR 21249 at commit 04e042a.

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

asfgit pushed a commit that referenced this pull request May 7, 2018
…ng position by 1 when calling Scala API
## What changes were proposed in this pull request?
This PR backports 24b5c69 and #21249
There's no conflict but I opened this just to run the test and for sure.
See the discussion in https://issues.apache.org/jira/browse/SPARK-23291
## How was this patch tested?
Jenkins tests.
Author: hyukjinkwon <gurwls223@apache.org>
Author: Liang-Chi Hsieh <viirya@gmail.com>
Closes#21250 from HyukjinKwon/SPARK-23291-backport.
@yanboliang

Copy link
Copy Markdown
Contributor

Merged into master, thanks.

@asfgitasfgit closed this in 1c9c5deMay 7, 2018
@HyukjinKwon

Copy link
Copy Markdown
MemberAuthor

Thanks, @cloud-fan, @yanboliang, @felixcheung and @viirya.

@HyukjinKwon
HyukjinKwon deleted the SPARK-23291 branch October 16, 2018 12:43
Sign up for freeto 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.

5 participants

@HyukjinKwon@SparkQA@cloud-fan@yanboliang@viirya