feat: remove serialisation of built-in file formats from ballista - #2348
feat: remove serialisation of built-in file formats from ballista #2348milenkovicm wants to merge 1 commit into
Conversation
milenkovicm
changed the title
| &self, | ||
| mut f: impl FnMut(&dyn LogicalExtensionCodec) -> Result<R>, | ||
| ) -> Result<(u32, R)> { | ||
| let mut last_err = None; |
There was a problem hiding this comment.
I think this changes the protobuf encoding wire numbers? If so, we should bump BALLISTA_PROTOCOL_VERSION
There was a problem hiding this comment.
#2337 ( you 🫵🏻 ) update it two days ago 😀. i believe, we as we haven't released it yet, no need to update it. do i miss something @avantgardnerio ?
There was a problem hiding this comment.
I was not sure what convention we wanted to support here. I'm not sure if folks are tracking ballista/main in their own projects. It seemed like monotonic bumps couldn't hurt. @phillipleblanc ?
There was a problem hiding this comment.
I have no problem to increment it, id argue that we should have API checkpoint on release
There was a problem hiding this comment.
Makes me wonder if it should just be the hash of the protobuf, or a git SHA. Not advocating for either, just thinking out loud.
There was a problem hiding this comment.
we could do it, it would be easy to automate, it would be hard to capture backward compatible changes, though.
i believe this change should produce same binary output (as the logic has been moved from here to datafusion), i would not put my money on that claim 😀
3 participants
Which issue does this PR close?
Closes #.
Rationale for this change
datafusion
DefaultLogicalExtensionCodecsupport serde of build in file formats since apache/datafusion#16944 there is no need to have duplicated code in ballista.What changes are included in this PR?
remove handling of build in file formats from ballista logical code, and delegate it to datafusion
Are there any user-facing changes?
No