Jesshuan opened a new issue, #10905:
URL: https://github.com/apache/gravitino/issues/10905

   ### 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)
   
   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. 


-- 
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]

Reply via email to