xiangfu0 commented on code in PR #18364:
URL: https://github.com/apache/pinot/pull/18364#discussion_r3159574939
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/creator/impl/BaseSegmentCreator.java:
##########
@@ -555,6 +555,13 @@ public static void
addColumnMetadataInfo(PropertiesConfiguration properties, Str
int cardinality = columnStatistics.getCardinality();
properties.setProperty(getKeyFor(column, CARDINALITY),
String.valueOf(cardinality));
properties.setProperty(getKeyFor(column, HAS_DICTIONARY),
String.valueOf(hasDictionary));
+ // Persist the forward-index encoding alongside HAS_DICTIONARY so future
readers can distinguish columns that
+ // keep a dictionary purely for secondary indexes (encoding=RAW,
hasDictionary=true) from the standard
+ // dictionary-encoded forward index (encoding=DICTIONARY,
hasDictionary=true). Until shared-dictionary segments
+ // are produced, encoding tracks hasDictionary 1:1 — readers fall back to
deriving from HAS_DICTIONARY when
+ // FORWARD_INDEX_ENCODING is absent (old segments).
+ properties.setProperty(getKeyFor(column, FORWARD_INDEX_ENCODING),
+ (hasDictionary ? ForwardIndexConfig.Encoding.DICTIONARY :
ForwardIndexConfig.Encoding.RAW).name());
Review Comment:
this is wrong, we should read from the forwardIndexConfig, fallback to use
`hasDictionary` to check
--
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]
