JacksonKjar commented on PR #59215:
URL: https://github.com/apache/spark/pull/59215#issuecomment-6330404884
## Records with JavaBean-style getters
Thanks for raising this. You're right that it changes the result for records
that declared `getX()`/`isX()` getters as a workaround. I think this is the
right call, with my reasoning explained below. Just want to confirm you're
aligned with making this change. If so, I'm happy to add a migration guide
entry and a PR description note.
---
### Why should we introduce a breaking change?
Adding bean-style getters doesn't make a record a bean. Records are
immutable, so they can't have setters. As far as I know, it's undocumented how
Spark should handle these invalid "beans". In practice, the workaround only
ever worked for encoding: decoding back into the record failed with
`EXPRESSION_DECODING_FAILED`. Users reached for it because Spark had no record
support. This PR adds that support, so they can use the record encoder
directly, which is likely what they wanted in the first place.
In order to keep existing code working unchanged, `Encoders.bean` would
have to opt-out of the new record component based encoder. This would
effectively make `Encoders.bean` a legacy API. Today it works as a general
entry point for Java types: it's built on the same type inference, so it also
handles types like `String`, `Integer` or `LocalDate`, nested beans, and now
records. Users of records would have to know to switch to `Encoders.record`,
and some cases still wouldn't work: a record nested inside a bean, or passed to
`createDataFrame(..., beanClass)`, would still produce no columns. That's a
broader change, affecting more users than those who relied on the workaround.
### Migration Options
For users who relied on the workaround, the simplest path is to make the
record's components match the schema they want, in the order they want the
columns, since the canonical constructor now defines it. For example, a derived
getter becomes a component, filled in by a convenience constructor:
```java
record Person(String first, String fullName, String last) {
Person(String first, String last) { this(first, first + " " + last, last);
}
}
```
As a bonus, these records can now also be decoded.
There are lighter options that avoid rewriting the records, but my opinion
is that they're not worth it:
- **An opt-in overload,** such as `Encoders.bean(cls, legacyRecords)`. Users
would still have to change every call site, so it saves little over fixing the
record, and it adds a mode flag to every bean entry point that we'd have to
support indefinitely.
- **A legacy SQL config,** such as `spark.sql.legacy.encodeRecordsAsBeans`,
that restores getter-based encoding without code changes and can be removed
later. The caveat is that encoders are inferred when they're created. In Spark
Connect that happens on the client, which doesn't see the server's SQL config,
so the flag would be ignored there unless we add plumbing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
