Jefffrey commented on PR #18192:
URL: https://github.com/apache/datafusion/pull/18192#issuecomment-3434801204

   > > > I think this makes sense to me. Main concern (other than the pointer 
bits) is the introduction of `DecodeContext`; I guess it wasn't easily possible 
to do via codec (I think you mentioned this already)?
   > > 
   > > 
   > > I don't see a better way to add the mutable context necessary for this 
to work. One olive branch I can extend to ease backwards compatibility is to 
`impl From<&'a TaskContext> for DecodeContext<'a>` and make `DecodeContext<'a>` 
`Clone` and then change the public signature of the functions that now require 
a `DecodeContext` to accept `impl Into<DecodeContext<'a>>`. But that is quite a 
bit of added complexity, personally I don't think it's worth it but I can 
implement that if reviewers feel it is required.
   > 
   > Something like this: https://github.com/pydantic/datafusion/pull/41/files
   
   Hmm yeah I don't this is is any better, since we might as well go all the 
way instead of a halfway solution 😅 
   
   I'll cc @timsaucer too as they also changed the signatures recently for 
proto physical plan in #18123


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


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to