Jackie-Jiang commented on code in PR #16149:
URL: https://github.com/apache/pinot/pull/16149#discussion_r2164888912
##########
pinot-spi/src/main/java/org/apache/pinot/spi/data/FieldSpec.java:
##########
@@ -94,6 +92,26 @@ public abstract class FieldSpec implements
Comparable<FieldSpec>, Serializable {
public static final Map DEFAULT_COMPLEX_NULL_VALUE_OF_MAP = Map.of();
public static final List DEFAULT_COMPLEX_NULL_VALUE_OF_LIST = List.of();
+ private static final int DEFAULT_MAX_LENGTH = 512;
+
+ private static MaxLengthExceedStrategy _defaultJsonSanitizationStrategy =
MaxLengthExceedStrategy.NO_ACTION;
Review Comment:
Keep the name and config consistent
```suggestion
private static MaxLengthExceedStrategy _defaultJsonMaxLengthExceedStrategy
= MaxLengthExceedStrategy.NO_ACTION;
```
##########
pinot-spi/src/main/java/org/apache/pinot/spi/data/FieldSpec.java:
##########
@@ -233,17 +251,39 @@ public void setNotNull(boolean notNull) {
}
public int getMaxLength() {
- return _maxLength;
+ // If explicitly set, return that value
+ if (_maxLength != null) {
+ return _maxLength;
+ }
+
+ // For JSON fields, use configurable default
+ if (_dataType == DataType.JSON) {
+ return getDefaultJsonMaxLength();
+ }
+
+ // For all other fields, use the standard default
+ return DEFAULT_MAX_LENGTH;
}
// Required by JSON de-serializer. DO NOT REMOVE.
- public void setMaxLength(int maxLength) {
+ public void setMaxLength(Integer maxLength) {
_maxLength = maxLength;
}
- @Nullable
public MaxLengthExceedStrategy getMaxLengthExceedStrategy() {
Review Comment:
Same here. The handling should be from the caller side
##########
pinot-spi/src/main/java/org/apache/pinot/spi/data/FieldSpec.java:
##########
@@ -233,17 +251,39 @@ public void setNotNull(boolean notNull) {
}
public int getMaxLength() {
Review Comment:
I think we should return `Integer` here, and handle it at the caller side
##########
pinot-spi/src/main/java/org/apache/pinot/spi/utils/CommonConstants.java:
##########
@@ -1805,6 +1805,13 @@ public static class ForwardIndexConfigs {
"pinot.forward.index.default.target.docs.per.chunk";
}
+ public static class FieldSpecConfigs {
+ public static final String CONFIG_OF_DEFAULT_JSON_SANITIZATION_STRATEGY =
Review Comment:
Keep the name consistent with the config key:
`pinot.field.spec.default.json.max.length.exceed.strategy`
--
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
