clintropolis commented on code in PR #17625:
URL: https://github.com/apache/druid/pull/17625#discussion_r1919524856
##########
processing/src/main/java/org/apache/druid/guice/BuiltInTypesModule.java:
##########
@@ -72,12 +71,7 @@ public void configure(Binder binder)
@LazySingleton
public SideEffectRegisterer
initDimensionHandlerAndMvHandlingMode(DefaultColumnFormatConfig formatsConfig)
{
- if (formatsConfig.getNestedColumnFormatVersion() != null &&
formatsConfig.getNestedColumnFormatVersion() == 4) {
- DimensionHandlerUtils.registerDimensionHandlerProvider(
- NestedDataComplexTypeSerde.TYPE_NAME,
- new NestedColumnV4HandlerProvider()
- );
- } else {
+ if (formatsConfig.getNestedColumnFormatVersion() == null ||
formatsConfig.getNestedColumnFormatVersion() == 5) {
Review Comment:
A failure would happen in `DefaultColumnFormatConfig` which throws an
exception if the format is not 5.
I guess I could think about switching this over to use the
`PropertiesValidator` introduced in #17634, though this config itself isn't
really deprecated, I did intend that thing to be used for validation of
runtime.properties.
The more I think about it, I'm not actually sure failing is that useful,
since v5 isn't really much of a behavior change, the risk is that with v5
segments you cannot roll back to Druid versions older than 26.0.0, which was
covered in prior release notes and doesn't seem to be worth exploding over. The
main difference of v5 format is that there are nested array fields which means
you can use json_value function to get arrays of primitives out more
efficiently and there are optimizations for the cases where there are no actual
objects in a json column (e.g. it writes a scalar column instead if there is no
nesting), so there isn't really much of a behavioral risk to using the new
format.
I think we could also consider just log.warn about having a bad config value
that is no longer supported and just ignoring it, though I suppose older druid
versions also fail if you didn't put the value of 4 or 5 into the config, so
:shrug:
--
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]
