Jackie-Jiang commented on a change in pull request #3484: Fix the bug where
time conversion is skipped when incoming and outgoing time column name are the
same
URL: https://github.com/apache/incubator-pinot/pull/3484#discussion_r234835978
##########
File path:
pinot-core/src/main/java/com/linkedin/pinot/core/data/recordtransformer/TimeTransformer.java
##########
@@ -29,37 +29,64 @@
* column for other record transformers (incoming time column can be ignored).
*/
public class TimeTransformer implements RecordTransformer {
- private final String _incomingTimeColumn;
- private final String _outgoingTimeColumn;
- private final TimeConverter _timeConverter;
+ private String _incomingTimeColumn;
+ private String _outgoingTimeColumn;
+ private TimeConverter _incomingTimeConverter;
+ private TimeConverter _outgoingTimeConverter;
+ private boolean _isFirstRecord = true;
public TimeTransformer(Schema schema) {
TimeFieldSpec timeFieldSpec = schema.getTimeFieldSpec();
if (timeFieldSpec != null) {
TimeGranularitySpec incomingGranularitySpec =
timeFieldSpec.getIncomingGranularitySpec();
TimeGranularitySpec outgoingGranularitySpec =
timeFieldSpec.getOutgoingGranularitySpec();
+
+ // Perform time conversion only if incoming and outgoing granularity
spec are different
if (!incomingGranularitySpec.equals(outgoingGranularitySpec)) {
_incomingTimeColumn = incomingGranularitySpec.getName();
_outgoingTimeColumn = outgoingGranularitySpec.getName();
- _timeConverter =
TimeConverterProvider.getTimeConverter(incomingGranularitySpec,
outgoingGranularitySpec);
- return;
+ _incomingTimeConverter = new TimeConverter(incomingGranularitySpec);
+ _outgoingTimeConverter = new TimeConverter(outgoingGranularitySpec);
}
}
- _incomingTimeColumn = null;
- _outgoingTimeColumn = null;
- _timeConverter = null;
}
@Override
public GenericRow transform(GenericRow record) {
- if (_timeConverter == null) {
+ if (_incomingTimeColumn == null) {
return record;
}
- // Skip transformation if outgoing value already exist
- // NOTE: outgoing value might already exist for OFFLINE data
- if (record.getValue(_outgoingTimeColumn) == null) {
- record.putField(_outgoingTimeColumn,
_timeConverter.convert(record.getValue(_incomingTimeColumn)));
+
+ // Use the first record for sanity check and determine whether the
conversion is needed
+ Object incomingTimeValue = record.getValue(_incomingTimeColumn);
+ if (_isFirstRecord) {
+ _isFirstRecord = false;
Review comment:
Make sense when use just ignore the exception and keep transforming. Will
make the change, thanks.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
