Jackie-Jiang commented on code in PR #9320:
URL: https://github.com/apache/pinot/pull/9320#discussion_r961333301
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/recordtransformer/DataTypeTransformer.java:
##########
@@ -40,14 +47,33 @@
*/
@SuppressWarnings("rawtypes")
public class DataTypeTransformer implements RecordTransformer {
+ private static final Logger LOGGER =
LoggerFactory.getLogger(DataTypeTransformer.class);
+
private final Map<String, PinotDataType> _dataTypes = new HashMap<>();
+ private final boolean _continueOnError;
+ private final boolean _validateTimeValues;
+ private final String _timeColumnName;
+ private final DateTimeFormatSpec _timeColumnSpec;
Review Comment:
(nit)
```suggestion
private final DateTimeFormatSpec _timeFormatSpec;
```
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/recordtransformer/DataTypeTransformer.java:
##########
@@ -40,14 +47,33 @@
*/
@SuppressWarnings("rawtypes")
public class DataTypeTransformer implements RecordTransformer {
+ private static final Logger LOGGER =
LoggerFactory.getLogger(DataTypeTransformer.class);
+
private final Map<String, PinotDataType> _dataTypes = new HashMap<>();
+ private final boolean _continueOnError;
+ private final boolean _validateTimeValues;
+ private final String _timeColumnName;
+ private final DateTimeFormatSpec _timeColumnSpec;
- public DataTypeTransformer(Schema schema) {
+ public DataTypeTransformer(TableConfig tableConfig, Schema schema) {
for (FieldSpec fieldSpec : schema.getAllFieldSpecs()) {
if (!fieldSpec.isVirtualColumn()) {
_dataTypes.put(fieldSpec.getName(),
PinotDataType.getPinotDataTypeForIngestion(fieldSpec));
}
}
+
+ _continueOnError = tableConfig.getIndexingConfig().isContinueOnError();
+ _validateTimeValues =
tableConfig.getIndexingConfig().isValidateTimeValue();
+ _timeColumnName = tableConfig.getValidationConfig().getTimeColumnName();
+
+ DateTimeFormatSpec timeColumnSpec = null;
+ if (StringUtils.isNotEmpty(_timeColumnName)) {
+ DateTimeFieldSpec dateTimeFieldSpec =
schema.getSpecForTimeColumn(_timeColumnName);
+ if (dateTimeFieldSpec != null) {
+ timeColumnSpec = dateTimeFieldSpec.getFormatSpec();
Review Comment:
We should throw exception here
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/recordtransformer/DataTypeTransformer.java:
##########
@@ -59,6 +85,17 @@ public GenericRow transform(GenericRow record) {
if (value == null) {
continue;
}
+
+ if (_validateTimeValues && _timeColumnSpec != null &&
column.equals(_timeColumnName)) {
+ long timeInMs = _timeColumnSpec.fromFormatToMillis(value.toString());
+ if (!TimeUtils.timeValueInValidRange(timeInMs)) {
+ LOGGER.debug("Time value {} is not in valid range for column: {},
must be between: {}",
Review Comment:
We should check if `continueOnError` is on, and throw exception when it is
off
--
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]
