Skip to content

Drop ConnectorSession from Type.getObjectValue #25945

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

Merged
merged 2 commits into from
Jun 8, 2025

Conversation

wendigo
Copy link
Contributor

@wendigo wendigo commented Jun 5, 2025

Type's object value retrieval should be independent of the session
but previously it was coupled to JSON serialization.

Making types independent of the serialization, moves the formatting logic
to protocol layer.

Description

Additional context and related issues

Release notes

( ) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
( ) Release notes are required, with the following suggested text:

## SPI
* Remove ConnectorSession from Type.getObjectValue ({issue}`issuenumber`)

@cla-bot cla-bot bot added the cla-signed label Jun 5, 2025
@github-actions github-actions bot added iceberg Iceberg connector delta-lake Delta Lake connector hive Hive connector mongodb MongoDB connector cassandra Cassandra connector kafka Kafka connector postgresql PostgreSQL connector labels Jun 5, 2025
@wendigo wendigo force-pushed the serafin/drop-connector-session branch from cc6bb77 to fc7a20e Compare June 5, 2025 17:52
@raunaqmorarka raunaqmorarka requested a review from dain June 5, 2025 19:00
@wendigo
Copy link
Contributor Author

wendigo commented Jun 6, 2025

@dain wdyt?

@wendigo wendigo force-pushed the serafin/drop-connector-session branch 2 times, most recently from d375a9a to d968a51 Compare June 6, 2025 14:36
wendigo added 2 commits June 6, 2025 16:44
Type's object value retrieval should be independent of the session
but previously it was coupled to JSON serialization.

Making types independent of the serialization, moves the formatting logic
to protocol layer.
@wendigo wendigo force-pushed the serafin/drop-connector-session branch from d968a51 to 171fd99 Compare June 6, 2025 14:44
@martint
Copy link
Member

martint commented Jun 6, 2025

If I recall, this was only needed for legacy timestamps, which relied on the session timezone to compute their representation. I think it's a good change.

The only thing I'd raise is that it removes the only way to customize serialization dynamically in case we need to introduce a change in behavior and want to allow users to roll it out gradually.

@wendigo
Copy link
Contributor Author

wendigo commented Jun 6, 2025

@martint legacy timestamps are still supported, it just happens in the serialization code, not in the Type SPI code.

@wendigo
Copy link
Contributor Author

wendigo commented Jun 6, 2025

@martint I'm still thinking about adding a SPI that allows a custom type (not a built-in) to describe how it wants to be represented over the wire (using built-in types)

@wendigo wendigo merged commit f71c515 into master Jun 8, 2025
195 of 202 checks passed
@wendigo wendigo deleted the serafin/drop-connector-session branch June 8, 2025 16:58
@github-actions github-actions bot added this to the 477 milestone Jun 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cassandra Cassandra connector cla-signed delta-lake Delta Lake connector hive Hive connector iceberg Iceberg connector kafka Kafka connector mongodb MongoDB connector postgresql PostgreSQL connector
Development

Successfully merging this pull request may close these issues.

3 participants