zanmato1984 commented on PR #40237: URL: https://github.com/apache/arrow/pull/40237#issuecomment-2029709056
Sorry for coming back to this late. I'm pushing a new approach into this PR as previously discussed (thus the title change). It is still WIP, with some further refactoring to be done and a little more tests to be added. But I think it is enough to show the essential of the approach. So I'd like to start the discussion now before further coding, to prevent any last minute surprise. There are two things: 1. The change in `scalar.h` may seem a little overkill, specifically the `FillScalarScratchSpace` and subclasses. This is because I want to mandate the initialization of the scratch space in the subclasses' constructors, meanwhile do it in a polymorphic fashion. Do you think this is proper? 2. So far I have only made the `value`s immutable for subclasses of `ArraySpanFillFromScalarScratchSpace`. This is sufficient to ensure the validity of the scratch space content. But this doesn't fully achieve the goal of "making scalar absolutely immutable by all means" - which is doable (partially done in my other branch, not shown in this PR), but just requires a lot more changes. Should I move on towards the full-immutability? I appreciate your feedback @pitrou @bkietz . Thanks. -- 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]
