Skip to content

Conversation

@LuciferYang
Copy link
Contributor

@LuciferYang LuciferYang commented Mar 7, 2023

What changes were proposed in this pull request?

This pr aims to implement Dataset.toJSON.

Why are the changes needed?

Add Spark connect jvm client api coverage.

Does this PR introduce any user-facing change?

No

How was this patch tested?

  • Add new test
  • Manually checked Scala 2.13

@LuciferYang LuciferYang changed the title [SPARK-42692][CONNECT] [SPARK-42692][CONNECT] Implement Dataset.toJSON Mar 7, 2023

def toJSON: Dataset[String] = {
throw new UnsupportedOperationException("toJSON is not implemented.")
sparkSession.newDataset(StringEncoder) { builder =>
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this better over project(to_json(col(*))).as(StringEncoder)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry ... what is project?

Copy link
Contributor

Choose a reason for hiding this comment

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

lol, I mean select...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks . Sorry I didn't understand what project is before ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed


test("toJSON") {
val expected = Array(
"{\"b\":0.0,\"id\":0,\"d\":\"world\",\"a\":0}",
Copy link
Contributor

@hvanhovell hvanhovell Mar 7, 2023

Choose a reason for hiding this comment

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

Use raw strings here. That is easier to read.

val expected = Array(
  """{"b":0.0,"id":0,"d":"world","a":0}""",
  ...
)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

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

@amaliujia
Copy link
Contributor

LGTM

@hvanhovell hvanhovell closed this in 51504e4 Mar 7, 2023
hvanhovell pushed a commit that referenced this pull request Mar 7, 2023
### What changes were proposed in this pull request?
This pr aims to implement Dataset.toJSON.

### Why are the changes needed?
Add Spark connect jvm client api coverage.

### Does this PR introduce _any_ user-facing change?
No

### How was this patch tested?
- Add new test
- Manually checked Scala 2.13

Closes #40319 from LuciferYang/SPARK-42692.

Authored-by: yangjie01 <[email protected]>
Signed-off-by: Herman van Hovell <[email protected]>
(cherry picked from commit 51504e4)
Signed-off-by: Herman van Hovell <[email protected]>
@LuciferYang
Copy link
Contributor Author

Thanks @hvanhovell @amaliujia

snmvaughan pushed a commit to snmvaughan/spark that referenced this pull request Jun 20, 2023
### What changes were proposed in this pull request?
This pr aims to implement Dataset.toJSON.

### Why are the changes needed?
Add Spark connect jvm client api coverage.

### Does this PR introduce _any_ user-facing change?
No

### How was this patch tested?
- Add new test
- Manually checked Scala 2.13

Closes apache#40319 from LuciferYang/SPARK-42692.

Authored-by: yangjie01 <[email protected]>
Signed-off-by: Herman van Hovell <[email protected]>
(cherry picked from commit 51504e4)
Signed-off-by: Herman van Hovell <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants