gianm commented on PR #18181: URL: https://github.com/apache/druid/pull/18181#issuecomment-3023123758
> I wonder if `ROW_BASED_V2` should the default after say 2 releases ? I was thinking about this earlier....there are things which can't be done on-the-spot; but should be (much) later....is there an annotation or some other marking to ensure that will be considered? (this is now kinda out-of-scope) It should be the default at some point, although I was thinking more like 4 releases. That would mean the Broker no longer needs to set `rowBasedFrameType`, which would clean things up a bit. However, making this change would mean that rolling upgrade from a version < 34 isn't possible, and I tend to favor longer compatibility windows, since it simplifies upgrading when people don't do it every single release. There isn't an annotation really to help find this stuff. Could be useful. In the case of this PR, to help with remembering to change things later, I've tried to leave plenty of comments about what the purpose of V1 is, and why the default is currently V1. > In the recent days I've bumped into a [similar issue](https://github.com/apache/druid/issues/13951) for which I feel like the fix might be closely related; if a `row_version` is added - it might be great to also cover that in the same version bump...what do you think? I think that could probably be fixed similarily inside `StringFieldReader` / `StringFieldWriter` I just commented on this one; IMO if the discrepancy is to be fixed, the best place is in the `topN` engine (meaning no frame format change would be needed). -- 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]
