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-23291][R][FOLLOWUP] Update SparkR migration note for SPARK-23291 #21249

Closed
wants to merge 3 commits into from

Conversation

HyukjinKwon
Copy link
Member

@HyukjinKwon HyukjinKwon commented May 6, 2018

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

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

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

Copy link
Member

@viirya viirya left a comment

Choose a reason for hiding this comment

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

LGTM

@SparkQA
Copy link

SparkQA commented May 6, 2018

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.

docs/sparkr.md Outdated

- 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.
Copy link
Member

@viirya viirya May 7, 2018

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
Member Author

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
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

SparkQA commented May 7, 2018

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
Contributor

LGTM

@SparkQA
Copy link

SparkQA commented May 7, 2018

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 <[email protected]>
Author: Liang-Chi Hsieh <[email protected]>

Closes #21250 from HyukjinKwon/SPARK-23291-backport.
@yanboliang
Copy link
Contributor

Merged into master, thanks.

@asfgit asfgit closed this in 1c9c5de May 7, 2018
@HyukjinKwon
Copy link
Member Author

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

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

5 participants