xiangfu0 commented on code in PR #18668:
URL: https://github.com/apache/pinot/pull/18668#discussion_r4418734876
##########
pinot-spi/src/main/java/org/apache/pinot/spi/config/table/FieldConfig.java:
##########
@@ -216,8 +222,32 @@ public String getName() {
return _name;
}
+ /// Returns the legacy field-level forward-index encoding.
+ ///
+ /// Prefer configuring and reading `encodingType` from `indexes.forward`.
This method keeps backward-compatible
+ /// defaulting for older configs: when the deprecated field-level value is
absent, it returns `DICTIONARY`.
+ @JsonIgnore
+ @Deprecated
Review Comment:
Addressed in dbf0005ffe: all new FieldConfig deprecations now specify since
= "1.7.0" and forRemoval = true. Existing deprecations retain their historical
versions (0.13.0 for the constructor and 0.9.0 for getIndexType).
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/utils/TableConfigUtils.java:
##########
@@ -2047,11 +2047,16 @@ private static void
validateIndexingConfigAndFieldConfigList(TableConfig tableCo
String column = fieldConfig.getName();
Preconditions.checkState(seenColumns.add(column), "Duplicate
FieldConfig for column: %s", column);
Preconditions.checkState(schema.hasColumn(column), "Failed to find
column: %s in schema", column);
-
- // Validate DELTA / DELTADELTA compression codecs compatibility
- validateGorillaCompressionCodecIfPresent(fieldConfig,
schema.getFieldSpecFor(column));
+ Preconditions.checkState(!fieldConfig.hasFieldLevelEncodingType(),
+ "FieldConfig.encodingType is deprecated for column: %s. Use
fieldConfigList[].indexes.forward.encodingType "
Review Comment:
Addressed in dbf0005ffe by choosing soft deprecation, as Jackie suggested.
Create/update validation accepts and preserves legacy encoding, compression,
and properties, including serializer-injected DICTIONARY. Automatic removal of
legacy RAW would make the config unreadable by older nodes, so it is deferred;
genuine RAW versus canonical DICTIONARY conflicts still fail. The controller
GET/edit-description/PUT regression passes for DICTIONARY, RAW plus SNAPPY, and
legacy properties.
##########
pinot-spi/src/main/java/org/apache/pinot/spi/config/table/FieldConfig.java:
##########
@@ -242,6 +272,7 @@ public JsonNode getTierOverwrites() {
/// Returns the raw forward-index compression codec, or `null` when using
`codecSpec` or the
/// default. Still the only way to express `MV_ENTRY_DICT` and the CLP
family — not deprecated.
@Nullable
+ @Deprecated
Review Comment:
Addressed in dbf0005ffe: the getter documentation now points to
indexes.forward.compressionCodec, including MV_ENTRY_DICT and the CLP family,
and distinguishes codecSpec pipeline compression. Removed the stale "not
deprecated" claim.
--
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]
