presto
presto copied to clipboard
Add support for REST catalog in Iceberg connector
Description
This PR is currently three commits:
- one that adds support for REST, which necessitates updating
com.fasterxml.jackson.core:jackson-coreand:jackson-databindto be >2.11 in the root pom - one that re-factors the Iceberg catalog implementation to be more modular (contains the bulk of the code changes)
- one that updates the
io.jsonwebtoken:jjwtpackage in order to be compatible with Java 11 builds, which necessitates changing artifacts to includeio.jsonwebtoken:jjwt-api,:jjwt-impl, and:jjwt-jackson. This commit will be removed upon merge of #22762, and the iceberg-specific changes incorporated into the first commit
Motivation and Context
Closes #20422
Impact
- Adds support for using the Iceberg REST catalog
Test Plan
Tests have been added that spin up a testing HTTP server to create an Iceberg REST catalog with JDBC backing. Currently, two of the distributed smoke tests fail when using this set up. I'm skipping them for now until I can look into why this is, though it's looking like it may be an Iceberg bug.
Contributor checklist
- [x] Please make sure your submission complies with our development, formatting, commit message, and attribution guidelines.
- [x] PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced.
- [x] Documented new properties (with its default value), SQL syntax, functions, or other functionality.
- [x] If release notes are required, they follow the release notes guidelines.
- [x] Adequate tests were added if applicable.
- [x] CI passed.
Release Notes
Please follow release notes guidelines and fill in the release notes below.
== RELEASE NOTES ==
General Changes
* Upgrades fasterxml.jackson artifacts to 2.11 :pr:`22417`
Iceberg Connector Changes
* Add support for Iceberg REST catalog :pr:`22417`
For reference, I agree with @ZacBlanco, use a separate commit for refactoring IcebergResourceFactory in the same PR would make the code more clearer.
@ZacBlanco @hantangwangd love the idea of doing two separate commits. That would be the best of both worlds since I can keep it separate from the functionality but also get the refactor done sooner than later
As for the re-factor, there are certainly several ways to go about it. I'm currently (at least once I'm back from vacation) considering two methods. First is passing a newSetBinder to the IcebergResourceFactory and accessing the relevant methods from there. This may be a bit easier. The second method is probably more involved, separating out each type of Iceberg catalog into it's own module and extending all the relevant classes from there (I think just config and resource factory's getCatalogProperties, though I haven't delved deep into this yet).
Please feel free to suggest additional ways and to weigh in on the above! As mentioned, I won't be back to this in another day or two 🙂
I took a brief look. Re-factor overall looks good! I will perform a detailed review once out of draft stage
Hi @kiersten-stokes, can you rebase your branch onto the newest master branch? it's a bit too far behind :-).
nit, suggest adding PR # into each item of the release note entry:
== RELEASE NOTES ==
General Changes
* Upgrades fasterxml.jackson artifacts to 2.11 :pr:`22417`
Iceberg Connector Changes
* Add support for Iceberg REST catalog :pr:`22417`