Revert #25124 Write mapping support

[sebastianopeluso]

Motivation and Context

Summary: This PR reverts #25124 Write mapping support. #25124 changed JdbcClient interface and breaks facebook internal downstream dependencies blocking the internal release. A fix requires non-trivial changes to our internal implementations of the interface, and we need more time to work on a proper fix.

Original commit changeset: 9bb915e4d34b

Original Phabricator Diff: D76875662

Differential Revision: D76945079

Impact

Revert code

Test Plan

Build

Contributor checklist

• [ ] Please make sure your submission complies with our contributing guide, in particular code style and commit standards.
• [ ] PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced.
• [ ] Documented new properties (with its default value), SQL syntax, functions, or other functionality.
• [ ] If release notes are required, they follow the release notes guidelines.
• [ ] Adequate tests were added if applicable.
• [ ] CI passed.

Release Notes

Please follow release notes guidelines and fill in the release notes below.

== RELEASE NOTES ==

General Changes
* Fix: Revert the Write mapping support

[linux-foundation-easycla[bot]]

• :white_check_mark:login: sebastianopeluso / (07282d37167588b2d31ff4992e13bc09366f0d65)

The committers listed above are authorized under a signed CLA.

[facebook-github-bot]

This pull request was exported from Phabricator. Differential Revision: D76945079

[sebastianopeluso]

@infvg I was wondering whether we can proceed with some incremental changes to the interface so as to avoid breaking the existing implementations.

[infvg]

@sebastianopeluso sure that works

[infvg]

@sebastianopeluso could you please elaborate on what issues were faced with these changes & which parts of the PR caused breaking changes that were difficult to fix?

[sebastianopeluso]

@infvg, as the return type of toPrestoType changed from Optional<ReadMapping> to Optional<ColumnMapping>, that broke our internal implementation of toPrestoType. I tried to adapt that implementation. The main challenge was on the replacement of sliceReadMapping with ColumnMapping.sliceMapping, as I am not sure on the additional parameter of type SliceWriteFunction to use. Not sure if a lambda throwing an UnsupportedOperationException would be just enough in that case. Another challenge is on the implementation of the additional toWriteMapping. @gggrace14, whenever you get the chance, can you take a look as you have context on the existing implementation? Happy to discuss a path forward.

[sebastianopeluso]

Hi @infvg, would it be possible for you to have a chat on Slack? We can figure out how to move forward with this change, and I can also get more context. What's your name on Slack? CC: @amitkdutta

[sebastianopeluso]

We just had a meeting to discuss the details of this PR. Thanks @infvg for your time and the explanations! Attendees: @infvg, @gggrace14 , @rschlussel , @sebastianopeluso , @zation99

@infvg explained the details of the change, and how was able to adapt existing implementations to work with the new interface. @infvg also explained that it did not take more than few hours to adapt the existing implementations, and given that @infvg had full context about this PR. Internally, ibm has a qa team to implement test cases for this change, and going through testing several scenarios.

After the meeting, we believe that, adapting to the interface's change of this PR for Meta's internal implementation might be quick. However, we need extensive testing for correctness because we are touching every type. Therefore, we agreed to merge this revert #25369 PR, and then provide an ETA on when we are going to have the adaptation and testing ready to bring the changes from the original #25124 PR back.