[Improvement] Make the gravitino-trino-connector compatible with Starburst ? (fast crash method & integration questions)
- Dominant language
- Java
- Stars
- 3.2k
- Forks
- 935
- Avg merge
- 1d 17h
- Merged PRs (30d)
- 339
Description
### What would you like to be improved?
Hey, the gravitino community,
I'm wonderning if it could be a good idea to make the gravitino-trino-connector compatible with Starburst, version by version...
For business reasons,
I've just forked & commited a simple "fast" and "crash method" solution that works with Starburst, after few tries.
(Gravitino seems to be the only way for me to federate an old hive legacy catalog)
I know Starburst is not an open-source product, and perhaps the good way could be that each starburst user have to fork and maintain this compatibility (and i'm agree with that...) but among the differents 4 commits, you will see that a good part of the changes are some little changes that are fairly "agnostics" and perhaps benefits for future trino versions too.
So I'd like to hear your thoughts on these topics and see if it's worth coordinating these little changes that could be enought for this Starburst compatibility
My forked gravitino repo project
(i've work on the 1.2.0-hotfix branch for now, to ensure that i was working with the 1.2.0 version with the current fix...):
[https://github.com/Jesshuan/gravitino/tree/1.2.0-hotfix]
(https://github.com/Jesshuan/gravitino/tree/1.2.0-hotfix)
The details with the 'merge diff view', for a best visibility:
[https://github.com/apache/gravitino/compare/1.2.0-hotfix...Jesshuan:gravitino:1.2.0-hotfix?expand=1]
(https://github.com/apache/gravitino/compare/1.2.0-hotfix...Jesshuan:gravitino:1.2.0-hotfix?expand=1)
### How should we improve?
Here the "crash method" that i've used (and my comments), fix by fix, to achieve a final good result for now:
- first, i made a little patch for the little **checkTrinoSpiVersion** method within the **GravitinoConnectorFactory** class, just for the string version compatibility. Yes, the Starburst versions are not integer but string.
=> could it be beneficial for the gravitino trino connector project too ?
- secondly, i have replaced the **io.trino:trino-spi:$trinoVersion** dependency with the 'private' starburst jar dependency equivalent, to prevent, i think, a lot of issues.. Of course, here, i suppose that each straburst user will have to take care of this themselves. We won't push a starburst jar within a gravitino trino project !... The solution for the user (or the starburst team ?) might be o maintain a hybrid version of the gravitino project with the starburst project.
Despite of this, perhaps these other changes could be benefit after this replacement...
- with Starburst, the implementation of a **DynamicFilter** object like **GravitinoDynamicFilter** need to have a **getPreferredDynamicFilterTimeout** method. I rewrote it with an empty Long return...
=> perhaps too interdependent with Starburst here to share this change with the gravitino project. I don't really know, because it seems the getPreferredDynamicFilterTimeout method is very linked to starburst. (?)
- i also needed to build a new implementation of **createBlockEncodingSerde** for the **JsonCodec** class, with a versatile approach. The problem was that the initial method create a new empty instantiation without any parameter, according to the **io.trino.metadata.BlockEncodingManager** class and starburst have a FeaturesConfig parameter for this class. It was linked to my starburst usage too, of course... but i think a similar method, very agnostic, and with a versatile approach is perhaps beneficial for any trino variants with Guice multibindings, etc. The current "non-args" implementation perhaps could make some issues with other trino versions too. I let you see that.
Contributor guide
Research direction
Start by reviewing the linked fork diff for the gravitino-trino-connector, especially GravitinoConnectorFactory.checkTrinoSpiVersion, GravitinoDynamicFilter, and JsonCodec.createBlockEncodingSerde. Determine which proposed changes are compatible with supported Trino versions and which depend on Starburst; done means the project has a decided compatibility scope and integration approach.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- java
- Domain
- databases
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Needs clarification
- Newbie friendliness
- 35/100