Skip to content

[SPARK-16693][SPARKR] Remove methods deprecated #22843

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
wants to merge 4 commits into from
Closed

[SPARK-16693][SPARKR] Remove methods deprecated #22843

wants to merge 4 commits into from

Conversation

@felixcheung
Copy link
Member

@felixcheung felixcheung commented Oct 26, 2018

What changes were proposed in this pull request?

Remove deprecated functions which includes:
SQLContext/HiveContext stuff
sparkR.init
jsonFile
parquetFile
registerTempTable
saveAsParquetFile
unionAll
createExternalTable
dropTempTable

How was this patch tested?

jenkins

@SparkQA
Copy link

SparkQA commented Oct 26, 2018

Test build #98069 has finished for PR 22843 at commit 3864490.

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

@SparkQA
Copy link

SparkQA commented Oct 26, 2018

Test build #98070 has finished for PR 22843 at commit 60c5808.

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

@srowen
Copy link
Member

srowen commented Oct 26, 2018

I personally am OK with removing anything deprecated before 2.4. Even things deprecated in 2.4 are technically fair game. If any of these were deprecated pretty recently, might be debatable, but I am in favor, myself.

dongjoon-hyun
dongjoon-hyun reviewed Oct 26, 2018
View reviewed changes
R/pkg/tests/fulltests/test_sparkSQL.R Outdated
Copy link
Member

@dongjoon-hyun dongjoon-hyun Oct 26, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Hi, @felixcheung . Mostly, this PR looks good, but I have one question for this line.
It seems that we are losing a test coverage for SPARK-16538.
SparkR::sql is also deprecated, too?

Copy link
Member Author

@felixcheung felixcheung Oct 27, 2018

Choose a reason for hiding this comment

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

thx, but this is no longer relevant - the issue was with the param mapping in dispatchFunc and is completely removed

Copy link
Member Author

@felixcheung felixcheung Oct 27, 2018

Choose a reason for hiding this comment

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

I added back some calls with SparkR:: but really this is now standard R call

@viirya
Copy link
Member

viirya commented Oct 27, 2018

Some deprecated functions (e.g., createExternalTable, registerTempTable, dropTempTable) are mentioned in Migration Guide of Spark R document, but seems some are not. Should we explicitly declare in the document these deprecated functions are removed?

felixcheung
felixcheung commented Oct 27, 2018
View reviewed changes
Copy link
Member Author

@felixcheung felixcheung left a comment

Choose a reason for hiding this comment

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

these are deprecated back in 2.0.0 at least

R/pkg/tests/fulltests/test_sparkSQL.R Outdated
Copy link
Member Author

@felixcheung felixcheung Oct 27, 2018

Choose a reason for hiding this comment

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

thx, but this is no longer relevant - the issue was with the param mapping in dispatchFunc and is completely removed

R/pkg/tests/fulltests/test_sparkSQL.R Outdated
Copy link
Member Author

@felixcheung felixcheung Oct 27, 2018

Choose a reason for hiding this comment

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

I added back some calls with SparkR:: but really this is now standard R call

felixcheung
felixcheung commented Oct 27, 2018
View reviewed changes
docs/sparkr.md Outdated

- The deprecated methods `parquetFile`, `jsonRDD` and `jsonFile` in `SQLContext` have been removed. Use `read.parquet` and `read.json`.
- The deprecated methods `sparkR.init`, `sparkRSQL.init`, `sparkRHive.init` have been removed. Use `sparkR.session` instead.
- The deprecated methods `parquetFile`, `saveAsParquetFile`, `jsonFile`, `registerTempTable`, `createExternalTable`, `dropTempTable`, `unionAll` have been removed. Use `read.parquet`, `write.parquet`, `read.json`, `createOrReplaceTempView`, , `createTable`, `dropTempView`, `union` instead.
Copy link
Member Author

@felixcheung felixcheung Oct 27, 2018

Choose a reason for hiding this comment

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

there's no "SQLContext" in R.... also I'm going remove RDD methods next - they are not actual public API

@SparkQA
Copy link

SparkQA commented Oct 27, 2018

Test build #98113 has finished for PR 22843 at commit 053af5b.

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

@SparkQA
Copy link

SparkQA commented Oct 27, 2018

Test build #98114 has finished for PR 22843 at commit d79765a.

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

@felixcheung
Copy link
Member Author

felixcheung commented Oct 27, 2018

merged to master

@asfgit asfgit closed this in 41e1416 Oct 27, 2018
jackylee-ch pushed a commit to jackylee-ch/spark that referenced this pull request Feb 18, 2019
@felixcheung
[SPARK-16693][SPARKR] Remove methods deprecated
28ce296 
## What changes were proposed in this pull request?

Remove deprecated functions which includes:
SQLContext/HiveContext stuff
sparkR.init
jsonFile
parquetFile
registerTempTable
saveAsParquetFile
unionAll
createExternalTable
dropTempTable

## How was this patch tested?

jenkins

Author: Felix Cheung <[email protected]>

Closes apache#22843 from felixcheung/rrddapi.
@huaxingao huaxingao mentioned this pull request Mar 28, 2020
Closed
HyukjinKwon pushed a commit that referenced this pull request Apr 1, 2020
@huaxingao @HyukjinKwon
[SPARK-31290][R] Add back the deprecated R APIs
fd0b228 
### What changes were proposed in this pull request?
Add back the deprecated R APIs removed by #22843 and #22815.

These APIs are

- `sparkR.init`
- `sparkRSQL.init`
- `sparkRHive.init`
- `registerTempTable`
- `createExternalTable`
- `dropTempTable`

No need to port the function such as
```r
createExternalTable <- function(x, ...) {
  dispatchFunc("createExternalTable(tableName, path = NULL, source = NULL, ...)", x, ...)
}
```
because this was for the backward compatibility when SQLContext exists before assuming from #9192,  but seems we don't need it anymore since SparkR replaced SQLContext with Spark Session at #13635.

### Why are the changes needed?
Amend Spark's Semantic Versioning Policy

### Does this PR introduce any user-facing change?
Yes
The removed R APIs are put back.

### How was this patch tested?
Add back the removed tests

Closes #28058 from huaxingao/r.

Authored-by: Huaxin Gao <[email protected]>
Signed-off-by: HyukjinKwon <[email protected]>
HyukjinKwon pushed a commit that referenced this pull request Apr 1, 2020
@huaxingao @HyukjinKwon
[SPARK-31290][R] Add back the deprecated R APIs
233dc12 
### What changes were proposed in this pull request?
Add back the deprecated R APIs removed by #22843 and #22815.

These APIs are

- `sparkR.init`
- `sparkRSQL.init`
- `sparkRHive.init`
- `registerTempTable`
- `createExternalTable`
- `dropTempTable`

No need to port the function such as
```r
createExternalTable <- function(x, ...) {
  dispatchFunc("createExternalTable(tableName, path = NULL, source = NULL, ...)", x, ...)
}
```
because this was for the backward compatibility when SQLContext exists before assuming from #9192,  but seems we don't need it anymore since SparkR replaced SQLContext with Spark Session at #13635.

### Why are the changes needed?
Amend Spark's Semantic Versioning Policy

### Does this PR introduce any user-facing change?
Yes
The removed R APIs are put back.

### How was this patch tested?
Add back the removed tests

Closes #28058 from huaxingao/r.

Authored-by: Huaxin Gao <[email protected]>
Signed-off-by: HyukjinKwon <[email protected]>
(cherry picked from commit fd0b228)
Signed-off-by: HyukjinKwon <[email protected]>
sjincho pushed a commit to sjincho/spark that referenced this pull request Apr 15, 2020
@huaxingao
[SPARK-31290][R] Add back the deprecated R APIs
f54a5a1 
### What changes were proposed in this pull request?
Add back the deprecated R APIs removed by apache#22843 and apache#22815.

These APIs are

- `sparkR.init`
- `sparkRSQL.init`
- `sparkRHive.init`
- `registerTempTable`
- `createExternalTable`
- `dropTempTable`

No need to port the function such as
```r
createExternalTable <- function(x, ...) {
  dispatchFunc("createExternalTable(tableName, path = NULL, source = NULL, ...)", x, ...)
}
```
because this was for the backward compatibility when SQLContext exists before assuming from apache#9192,  but seems we don't need it anymore since SparkR replaced SQLContext with Spark Session at apache#13635.

### Why are the changes needed?
Amend Spark's Semantic Versioning Policy

### Does this PR introduce any user-facing change?
Yes
The removed R APIs are put back.

### How was this patch tested?
Add back the removed tests

Closes apache#28058 from huaxingao/r.

Authored-by: Huaxin Gao <[email protected]>
Signed-off-by: HyukjinKwon <[email protected]>
@HyukjinKwon HyukjinKwon mentioned this pull request Aug 7, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dongjoon-hyun dongjoon-hyun dongjoon-hyun left review comments

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

@felixcheung @SparkQA @srowen @viirya @dongjoon-hyun