Jackie-Jiang commented on code in PR #16149:
URL: https://github.com/apache/pinot/pull/16149#discussion_r2176058291
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/creator/impl/SegmentColumnarIndexCreator.java:
##########
@@ -599,10 +599,14 @@ public static void
addColumnMetadataInfo(PropertiesConfiguration properties, Str
properties.setProperty(getKeyFor(column, IS_AUTO_GENERATED),
String.valueOf(columnIndexCreationInfo.isAutoGenerated()));
if (dataType.equals(DataType.STRING) || dataType.equals(DataType.BYTES) ||
dataType.equals(DataType.JSON)) {
- properties.setProperty(getKeyFor(column, SCHEMA_MAX_LENGTH),
fieldSpec.getMaxLength());
- FieldSpec.MaxLengthExceedStrategy maxLengthExceedStrategy =
fieldSpec.getMaxLengthExceedStrategy();
- if (maxLengthExceedStrategy != null) {
- properties.setProperty(getKeyFor(column,
SCHEMA_MAX_LENGTH_EXCEED_STRATEGY), maxLengthExceedStrategy);
+ Integer maxLength = fieldSpec.getMaxLength();
Review Comment:
We can consider adding a TODO to revisit the
`SCHEMA_MAX_LENGTH_EXCEED_STRATEGY`. Changing segment CRC has side effect: we
can no longer detect segment with same data. We should avoid coupling that into
this PR.
In the `FieldSpec.equals()`, IMO we should compare the configured one.
Explicitly configured and implicitly derived are different. The caller should
decide how to compare, not the `FieldSpec` impl
--
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: commits-unsubscr...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
