Copilot commented on code in PR #18668:
URL: https://github.com/apache/pinot/pull/18668#discussion_r3450291104
##########
pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/FieldIndexConfigsUtil.java:
##########
@@ -80,6 +80,24 @@ public static FieldIndexConfigs fromFieldConfig(@Nullable
FieldConfig fieldConfi
return builder.build();
}
+ @SuppressWarnings("deprecation")
+ private static boolean isRawForwardEncoded(@Nullable FieldConfig
fieldConfig) {
+ if (fieldConfig == null) {
+ return false;
+ }
+ JsonNode indexes = fieldConfig.getIndexes();
+ JsonNode forward = indexes != null ? indexes.get("forward") : null;
+ if (forward != null && forward.isObject()) {
+ try {
+ return JsonUtils.jsonNodeToObject(forward,
ForwardIndexConfig.class).getEncodingType()
+ == FieldConfig.EncodingType.RAW;
+ } catch (IOException e) {
+ throw new IllegalArgumentException("Failed to parse forward index
config from FieldConfig", e);
+ }
+ }
+ return fieldConfig.getEncodingType() == FieldConfig.EncodingType.RAW;
+ }
Review Comment:
`FieldIndexConfigsUtil.fromFieldConfig()` is documented as deriving the
dictionary setting from `fieldConfig.indexes.forward.encodingType` and using
legacy signals only as a fallback. However, `isRawForwardEncoded()` currently
parses the whole `forward` block into `ForwardIndexConfig`, which defaults
`encodingType` to `DICTIONARY` when absent. This means a config that has a
`forward` block (e.g. for compression) but omits `encodingType` will *never*
fall back to the deprecated field-level `encodingType` (or other legacy
signals) and will be treated as DICTIONARY-encoded even if the legacy
field-level value is `RAW`.
This can break the stated backward-compat behavior for legacy inputs where
`indexes.forward.encodingType` is missing. Consider checking the raw
`forward.encodingType` field directly and only using it when explicitly
present; otherwise fall back to `fieldConfig.getEncodingType()`.
--
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]
