You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by ja...@apache.org on 2019/10/29 01:42:24 UTC
[incubator-pinot] branch master updated: [SegmentGeneratorConfig
Cleanup] Replace checkTimeColumnValidityDuringGeneration with
skipTimeValueCheck (#4745)
This is an automated email from the ASF dual-hosted git repository.
jackie pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git
The following commit(s) were added to refs/heads/master by this push:
new 74aea85 [SegmentGeneratorConfig Cleanup] Replace checkTimeColumnValidityDuringGeneration with skipTimeValueCheck (#4745)
74aea85 is described below
commit 74aea8564e6b8d289b2946247fa63e892c75453f
Author: Xiaotian (Jackie) Jiang <17...@users.noreply.github.com>
AuthorDate: Mon Oct 28 18:42:13 2019 -0700
[SegmentGeneratorConfig Cleanup] Replace checkTimeColumnValidityDuringGeneration with skipTimeValueCheck (#4745)
- Reverse the config so that the default is false (easier to use)
- Simplify the logic of time value check
- Fix the invalid time value in some tests
NOTE: This commit has backward-incompatible change on the config introduced in #4368 merged in 07/16/2019. In SegmentGeneratorConfig, checkTimeColumnValidityDuringGeneration (default true) is replaced with skipTimeValueCheck (default false).
---
.../SegmentsValidationAndRetentionConfig.java | 23 +--
.../apache/pinot/common/utils/time/TimeUtils.java | 35 ++---
.../controller/api/upload/SegmentValidator.java | 49 ++-----
.../controller/util/SegmentIntervalUtils.java | 12 +-
.../validation/OfflineSegmentIntervalChecker.java | 3 +-
.../generator/SegmentGeneratorConfig.java | 12 +-
.../apache/pinot/core/minion/SegmentConverter.java | 29 ++--
.../converter/RealtimeSegmentConverter.java | 2 +-
.../creator/impl/SegmentColumnarIndexCreator.java | 163 ++++-----------------
.../core/segment/index/SegmentMetadataImpl.java | 4 +-
.../pinot/core/minion/SegmentConverterTest.java | 78 ++++------
...adataAndDictionaryAggregationPlanMakerTest.java | 9 +-
.../core/segment/index/ColumnMetadataTest.java | 5 +-
.../segment/index/SegmentMetadataImplTest.java | 8 +-
.../SegmentV1V2ToV3FormatConverterTest.java | 3 +-
.../SegmentGenerationWithTimeColumnTest.java | 21 +--
.../index/loader/SegmentPreProcessorTest.java | 9 +-
.../startree/StarTreeIndexTestSegmentHelper.java | 44 ++----
.../pinot/core/startree/TestStarTreeMetadata.java | 33 ++---
.../hll/SegmentWithHllIndexCreateHelper.java | 6 +-
.../org/apache/pinot/core/util/CrcUtilsTest.java | 3 +-
.../pinot/queries/BaseMultiValueQueriesTest.java | 2 +-
.../pinot/queries/BaseSingleValueQueriesTest.java | 2 +-
.../apache/pinot/queries/DistinctQueriesTest.java | 1 -
.../apache/pinot/queries/FastHllQueriesTest.java | 2 +-
.../segments/v1/creator/DictionariesTest.java | 2 +-
.../pinot/segments/v1/creator/IntArraysTest.java | 7 +-
.../OfflineSegmentIntervalCheckerCommand.java | 3 +-
28 files changed, 178 insertions(+), 392 deletions(-)
diff --git a/pinot-common/src/main/java/org/apache/pinot/common/config/SegmentsValidationAndRetentionConfig.java b/pinot-common/src/main/java/org/apache/pinot/common/config/SegmentsValidationAndRetentionConfig.java
index 5f917da..f279ab9 100644
--- a/pinot-common/src/main/java/org/apache/pinot/common/config/SegmentsValidationAndRetentionConfig.java
+++ b/pinot-common/src/main/java/org/apache/pinot/common/config/SegmentsValidationAndRetentionConfig.java
@@ -83,6 +83,8 @@ public class SegmentsValidationAndRetentionConfig {
this.segmentAssignmentStrategy = segmentAssignmentStrategy;
}
+ // TODO: Use TimeFieldSpec in Schema
+ @Deprecated
public String getTimeColumnName() {
return timeColumnName;
}
@@ -91,6 +93,8 @@ public class SegmentsValidationAndRetentionConfig {
this.timeColumnName = timeColumnName;
}
+ // TODO: Use TimeFieldSpec in Schema
+ @Deprecated
public TimeUnit getTimeType() {
return _timeType;
}
@@ -233,15 +237,16 @@ public class SegmentsValidationAndRetentionConfig {
SegmentsValidationAndRetentionConfig that = (SegmentsValidationAndRetentionConfig) o;
- return EqualityUtils.isEqual(retentionTimeUnit, that.retentionTimeUnit) && EqualityUtils.isEqual(retentionTimeValue,
- that.retentionTimeValue) && EqualityUtils.isEqual(segmentPushFrequency, that.segmentPushFrequency)
- && EqualityUtils.isEqual(segmentPushType, that.segmentPushType) && EqualityUtils.isEqual(replication,
- that.replication) && EqualityUtils.isEqual(schemaName, that.schemaName) && EqualityUtils.isEqual(timeColumnName,
- that.timeColumnName) && EqualityUtils.isEqual(_timeType, that._timeType) && EqualityUtils.isEqual(
- segmentAssignmentStrategy, that.segmentAssignmentStrategy) && EqualityUtils.isEqual(replicaGroupStrategyConfig,
- that.replicaGroupStrategyConfig) && EqualityUtils.isEqual(_completionConfig, that._completionConfig)
- && EqualityUtils.isEqual(hllConfig, that.hllConfig) && EqualityUtils.isEqual(replicasPerPartition,
- that.replicasPerPartition);
+ return EqualityUtils.isEqual(retentionTimeUnit, that.retentionTimeUnit) && EqualityUtils
+ .isEqual(retentionTimeValue, that.retentionTimeValue) && EqualityUtils
+ .isEqual(segmentPushFrequency, that.segmentPushFrequency) && EqualityUtils
+ .isEqual(segmentPushType, that.segmentPushType) && EqualityUtils.isEqual(replication, that.replication)
+ && EqualityUtils.isEqual(schemaName, that.schemaName) && EqualityUtils
+ .isEqual(timeColumnName, that.timeColumnName) && EqualityUtils.isEqual(_timeType, that._timeType)
+ && EqualityUtils.isEqual(segmentAssignmentStrategy, that.segmentAssignmentStrategy) && EqualityUtils
+ .isEqual(replicaGroupStrategyConfig, that.replicaGroupStrategyConfig) && EqualityUtils
+ .isEqual(_completionConfig, that._completionConfig) && EqualityUtils.isEqual(hllConfig, that.hllConfig)
+ && EqualityUtils.isEqual(replicasPerPartition, that.replicasPerPartition);
}
@Override
diff --git a/pinot-common/src/main/java/org/apache/pinot/common/utils/time/TimeUtils.java b/pinot-common/src/main/java/org/apache/pinot/common/utils/time/TimeUtils.java
index 9ce6d44..86136d1 100644
--- a/pinot-common/src/main/java/org/apache/pinot/common/utils/time/TimeUtils.java
+++ b/pinot-common/src/main/java/org/apache/pinot/common/utils/time/TimeUtils.java
@@ -23,12 +23,18 @@ import javax.annotation.Nullable;
import org.joda.time.DateTime;
import org.joda.time.DateTimeZone;
import org.joda.time.Duration;
+import org.joda.time.Interval;
import org.joda.time.Period;
import org.joda.time.format.PeriodFormatter;
import org.joda.time.format.PeriodFormatterBuilder;
public class TimeUtils {
+ public static final long VALID_MIN_TIME_MILLIS = new DateTime(1971, 1, 1, 0, 0, 0, 0, DateTimeZone.UTC).getMillis();
+ public static final long VALID_MAX_TIME_MILLIS = new DateTime(2071, 1, 1, 0, 0, 0, 0, DateTimeZone.UTC).getMillis();
+ public static final Interval VALID_TIME_INTERVAL =
+ new Interval(VALID_MIN_TIME_MILLIS, VALID_MAX_TIME_MILLIS, DateTimeZone.UTC);
+
private static final String UPPER_CASE_DAYS = "DAYS";
private static final String UPPER_CASE_DAYS_SINCE_EPOCH = "DAYSSINCEEPOCH";
private static final String UPPER_CASE_HOURS = "HOURS";
@@ -47,9 +53,6 @@ public class TimeUtils {
private static final String UPPER_CASE_NANOS_SINCE_EPOCH = "NANOSSINCEEPOCH";
private static final String UPPER_CASE_NANOSECONDS_SINCE_EPOCH = "NANOSECONDSSINCEEPOCH";
- private static final long VALID_MIN_TIME_MILLIS = new DateTime(1971, 1, 1, 0, 0, 0, 0, DateTimeZone.UTC).getMillis();
- private static final long VALID_MAX_TIME_MILLIS = new DateTime(2071, 1, 1, 0, 0, 0, 0, DateTimeZone.UTC).getMillis();
-
/**
* Converts a time unit string into {@link TimeUnit}, ignoring case. For {@code null} or empty time unit string,
* returns {@code null}.
@@ -108,7 +111,16 @@ public class TimeUtils {
* <p>The current valid range used is between beginning of 1971 and beginning of 2071.
*/
public static boolean timeValueInValidRange(long timeValueInMillis) {
- return (VALID_MIN_TIME_MILLIS <= timeValueInMillis && timeValueInMillis <= VALID_MAX_TIME_MILLIS);
+ return timeValueInMillis >= VALID_MIN_TIME_MILLIS && timeValueInMillis <= VALID_MAX_TIME_MILLIS;
+ }
+
+ /**
+ * Given a time interval, returns true if the interval is between a valid range, false otherwise.
+ * <p>The current valid range used is between beginning of 1971 and beginning of 2071.
+ */
+ public static boolean isValidTimeInterval(Interval timeInterval) {
+ return timeInterval.getStartMillis() >= VALID_MIN_TIME_MILLIS
+ && timeInterval.getEndMillis() <= VALID_MAX_TIME_MILLIS;
}
/**
@@ -167,19 +179,4 @@ public class TimeUtils {
}
return periodStr;
}
-
- /**
- * Verify that start and end time (should be in milliseconds from epoch) of the segment
- * are in valid range.
- * @param startMillis start time (in milliseconds)
- * @param endMillis end time (in milliseconds)
- * @return true if start and end time are in range, false otherwise
- *
- * Note: this function assumes that given times are in milliseconds. The
- * caller should take care of converting to millis from epoch before
- * trying to validate the times.
- */
- public static boolean checkSegmentTimeValidity(final long startMillis, final long endMillis) {
- return timeValueInValidRange(startMillis) && timeValueInValidRange(endMillis);
- }
}
diff --git a/pinot-controller/src/main/java/org/apache/pinot/controller/api/upload/SegmentValidator.java b/pinot-controller/src/main/java/org/apache/pinot/controller/api/upload/SegmentValidator.java
index 8e0fcb6..db071a4 100644
--- a/pinot-controller/src/main/java/org/apache/pinot/controller/api/upload/SegmentValidator.java
+++ b/pinot-controller/src/main/java/org/apache/pinot/controller/api/upload/SegmentValidator.java
@@ -19,9 +19,7 @@
package org.apache.pinot.controller.api.upload;
import java.io.File;
-import java.util.Date;
import java.util.concurrent.Executor;
-import javax.annotation.Nonnull;
import javax.ws.rs.core.Response;
import org.apache.commons.httpclient.HttpConnectionManager;
import org.apache.pinot.common.config.TableConfig;
@@ -88,11 +86,14 @@ public class SegmentValidator {
+ quotaResponse.reason, Response.Status.FORBIDDEN);
}
- // Check time range
- if (!isSegmentTimeValid(segmentMetadata)) {
- throw new ControllerApplicationException(LOGGER,
- "Invalid segment start/end time for segment: " + segmentName + " of table: " + offlineTableName,
- Response.Status.NOT_ACCEPTABLE);
+ // Check time interval
+ // TODO: Pass in schema and check the existence of time interval when time field exists
+ Interval timeInterval = segmentMetadata.getTimeInterval();
+ if (timeInterval != null && !TimeUtils.isValidTimeInterval(timeInterval)) {
+ throw new ControllerApplicationException(LOGGER, String.format(
+ "Invalid segment start/end time: %s (in millis: %d/%d) for segment: %s of table: %s, must be between: %s",
+ timeInterval, timeInterval.getStartMillis(), timeInterval.getEndMillis(), segmentName, offlineTableName,
+ TimeUtils.VALID_TIME_INTERVAL), Response.Status.NOT_ACCEPTABLE);
}
}
@@ -103,8 +104,8 @@ public class SegmentValidator {
* @param metadata segment metadata. This should not be null.
* @param offlineTableConfig offline table configuration. This should not be null.
*/
- private StorageQuotaChecker.QuotaCheckerResponse checkStorageQuota(@Nonnull File segmentFile,
- @Nonnull SegmentMetadata metadata, @Nonnull TableConfig offlineTableConfig)
+ private StorageQuotaChecker.QuotaCheckerResponse checkStorageQuota(File segmentFile, SegmentMetadata metadata,
+ TableConfig offlineTableConfig)
throws InvalidConfigException {
if (!_controllerConf.getEnableStorageQuotaCheck()) {
return StorageQuotaChecker.success("Quota check is disabled");
@@ -116,34 +117,4 @@ public class SegmentValidator {
return quotaChecker.isSegmentStorageWithinQuota(segmentFile, metadata.getName(),
_controllerConf.getServerAdminRequestTimeoutSeconds() * 1000);
}
-
- /**
- * Returns true if:
- * - Segment does not have a start/end time, OR
- * - The start/end time are in a valid range (Jan 01 1971 - Jan 01, 2071)
- * @param metadata Segment metadata
- * @return
- */
- private boolean isSegmentTimeValid(SegmentMetadata metadata) {
- Interval interval = metadata.getTimeInterval();
- if (interval == null) {
- return true;
- }
-
- long startMillis = interval.getStartMillis();
- long endMillis = interval.getEndMillis();
-
- if (!TimeUtils.checkSegmentTimeValidity(startMillis, endMillis)) {
- Date minDate = new Date(TimeUtils.getValidMinTimeMillis());
- Date maxDate = new Date(TimeUtils.getValidMaxTimeMillis());
-
- LOGGER.error(
- "Invalid start time '{}ms' or end time '{}ms' for segment {}, must be between '{}' and '{}' (timecolumn {}, timeunit {})",
- interval.getStartMillis(), interval.getEndMillis(), metadata.getName(), minDate, maxDate,
- metadata.getTimeColumn(), metadata.getTimeUnit().toString());
- return false;
- }
-
- return true;
- }
}
diff --git a/pinot-controller/src/main/java/org/apache/pinot/controller/util/SegmentIntervalUtils.java b/pinot-controller/src/main/java/org/apache/pinot/controller/util/SegmentIntervalUtils.java
index b018689..b9cfc11 100644
--- a/pinot-controller/src/main/java/org/apache/pinot/controller/util/SegmentIntervalUtils.java
+++ b/pinot-controller/src/main/java/org/apache/pinot/controller/util/SegmentIntervalUtils.java
@@ -20,9 +20,7 @@ package org.apache.pinot.controller.util;
import org.apache.commons.lang3.StringUtils;
import org.apache.pinot.common.config.SegmentsValidationAndRetentionConfig;
-import org.apache.pinot.common.utils.time.TimeUtils;
import org.joda.time.Duration;
-import org.joda.time.Interval;
/**
@@ -31,15 +29,8 @@ import org.joda.time.Interval;
public class SegmentIntervalUtils {
/**
- * Checks if the given segment metadata time interval is valid
- */
- public static boolean isValidInterval(Interval timeInterval) {
- return timeInterval != null && TimeUtils.timeValueInValidRange(timeInterval.getStartMillis()) && TimeUtils
- .timeValueInValidRange(timeInterval.getEndMillis());
- }
-
- /**
* We only want to check missing segments if the table has at least 2 segments and a time column
+ * TODO: Use TimeFieldSpec in Schema
*/
public static boolean eligibleForMissingSegmentCheck(int numSegments,
SegmentsValidationAndRetentionConfig validationConfig) {
@@ -48,6 +39,7 @@ public class SegmentIntervalUtils {
/**
* We only want to check intervals if the table has a time column
+ * TODO: Use TimeFieldSpec in Schema
*/
public static boolean eligibleForSegmentIntervalCheck(SegmentsValidationAndRetentionConfig validationConfig) {
return StringUtils.isNotEmpty(validationConfig.getTimeColumnName());
diff --git a/pinot-controller/src/main/java/org/apache/pinot/controller/validation/OfflineSegmentIntervalChecker.java b/pinot-controller/src/main/java/org/apache/pinot/controller/validation/OfflineSegmentIntervalChecker.java
index 0eeadc7..9a43732 100644
--- a/pinot-controller/src/main/java/org/apache/pinot/controller/validation/OfflineSegmentIntervalChecker.java
+++ b/pinot-controller/src/main/java/org/apache/pinot/controller/validation/OfflineSegmentIntervalChecker.java
@@ -29,6 +29,7 @@ import org.apache.pinot.common.metadata.segment.OfflineSegmentZKMetadata;
import org.apache.pinot.common.metrics.ControllerMetrics;
import org.apache.pinot.common.metrics.ValidationMetrics;
import org.apache.pinot.common.utils.CommonConstants;
+import org.apache.pinot.common.utils.time.TimeUtils;
import org.apache.pinot.controller.ControllerConf;
import org.apache.pinot.controller.LeadControllerManager;
import org.apache.pinot.controller.helix.core.PinotHelixResourceManager;
@@ -88,7 +89,7 @@ public class OfflineSegmentIntervalChecker extends ControllerPeriodicTask<Void>
int numSegmentsWithInvalidIntervals = 0;
for (OfflineSegmentZKMetadata offlineSegmentZKMetadata : offlineSegmentZKMetadataList) {
Interval timeInterval = offlineSegmentZKMetadata.getTimeInterval();
- if (SegmentIntervalUtils.isValidInterval(timeInterval)) {
+ if (timeInterval != null && TimeUtils.isValidTimeInterval(timeInterval)) {
segmentIntervals.add(timeInterval);
} else {
numSegmentsWithInvalidIntervals++;
diff --git a/pinot-core/src/main/java/org/apache/pinot/core/indexsegment/generator/SegmentGeneratorConfig.java b/pinot-core/src/main/java/org/apache/pinot/core/indexsegment/generator/SegmentGeneratorConfig.java
index da728ad..fba03b2 100644
--- a/pinot-core/src/main/java/org/apache/pinot/core/indexsegment/generator/SegmentGeneratorConfig.java
+++ b/pinot-core/src/main/java/org/apache/pinot/core/indexsegment/generator/SegmentGeneratorConfig.java
@@ -112,7 +112,7 @@ public class SegmentGeneratorConfig {
private String _simpleDateFormat = null;
// Use on-heap or off-heap memory to generate index (currently only affect inverted index and star-tree v2)
private boolean _onHeap = false;
- private boolean _checkTimeColumnValidityDuringGeneration = true;
+ private boolean _skipTimeValueCheck = false;
private boolean _nullHandlingEnabled = false;
public SegmentGeneratorConfig() {
@@ -162,7 +162,7 @@ public class SegmentGeneratorConfig {
_simpleDateFormat = config._simpleDateFormat;
_onHeap = config._onHeap;
_recordReaderPath = config._recordReaderPath;
- _checkTimeColumnValidityDuringGeneration = config._checkTimeColumnValidityDuringGeneration;
+ _skipTimeValueCheck = config._skipTimeValueCheck;
_nullHandlingEnabled = config._nullHandlingEnabled;
}
@@ -599,12 +599,12 @@ public class SegmentGeneratorConfig {
_onHeap = onHeap;
}
- public boolean isCheckTimeColumnValidityDuringGeneration() {
- return _checkTimeColumnValidityDuringGeneration;
+ public boolean isSkipTimeValueCheck() {
+ return _skipTimeValueCheck;
}
- public void setCheckTimeColumnValidityDuringGeneration(boolean checkTimeColumnValidityDuringGeneration) {
- _checkTimeColumnValidityDuringGeneration = checkTimeColumnValidityDuringGeneration;
+ public void setSkipTimeValueCheck(boolean skipTimeValueCheck) {
+ _skipTimeValueCheck = skipTimeValueCheck;
}
public Map<String, ChunkCompressorFactory.CompressionType> getRawIndexCompressionType() {
diff --git a/pinot-core/src/main/java/org/apache/pinot/core/minion/SegmentConverter.java b/pinot-core/src/main/java/org/apache/pinot/core/minion/SegmentConverter.java
index 1723104..0e70747 100644
--- a/pinot-core/src/main/java/org/apache/pinot/core/minion/SegmentConverter.java
+++ b/pinot-core/src/main/java/org/apache/pinot/core/minion/SegmentConverter.java
@@ -22,7 +22,6 @@ import com.google.common.base.Preconditions;
import java.io.File;
import java.util.ArrayList;
import java.util.List;
-import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import org.apache.pinot.common.config.IndexingConfig;
import org.apache.pinot.common.data.StarTreeIndexSpec;
@@ -69,13 +68,12 @@ public class SegmentConverter {
private RecordAggregator _recordAggregator;
private List<String> _groupByColumns;
private IndexingConfig _indexingConfig;
- private boolean _checkTimeValidityDuringGeneration;
+ private boolean _skipTimeValueCheck;
- public SegmentConverter(@Nonnull List<File> inputIndexDirs, @Nonnull File workingDir, @Nonnull String tableName,
- @Nonnull String segmentName, int totalNumPartition, @Nonnull RecordTransformer recordTransformer,
- @Nullable RecordPartitioner recordPartitioner, @Nullable RecordAggregator recordAggregator,
- @Nullable List<String> groupByColumns, @Nullable IndexingConfig indexingConfig,
- boolean checkTimeValidityDuringGeneration) {
+ public SegmentConverter(List<File> inputIndexDirs, File workingDir, String tableName, String segmentName,
+ int totalNumPartition, RecordTransformer recordTransformer, @Nullable RecordPartitioner recordPartitioner,
+ @Nullable RecordAggregator recordAggregator, @Nullable List<String> groupByColumns,
+ @Nullable IndexingConfig indexingConfig, boolean skipTimeValueCheck) {
_inputIndexDirs = inputIndexDirs;
_workingDir = workingDir;
_recordTransformer = recordTransformer;
@@ -88,8 +86,7 @@ public class SegmentConverter {
_recordAggregator = recordAggregator;
_groupByColumns = groupByColumns;
_indexingConfig = indexingConfig;
-
- _checkTimeValidityDuringGeneration = checkTimeValidityDuringGeneration;
+ _skipTimeValueCheck = skipTimeValueCheck;
}
public List<File> convertSegment()
@@ -152,7 +149,7 @@ public class SegmentConverter {
segmentGeneratorConfig.setOutDir(outputPath);
segmentGeneratorConfig.setTableName(tableName);
segmentGeneratorConfig.setSegmentName(segmentName);
- segmentGeneratorConfig.setCheckTimeColumnValidityDuringGeneration(_checkTimeValidityDuringGeneration);
+ segmentGeneratorConfig.setSkipTimeValueCheck(_skipTimeValueCheck);
if (indexingConfig != null) {
segmentGeneratorConfig.setInvertedIndexCreationColumns(indexingConfig.getInvertedIndexColumns());
if (indexingConfig.getStarTreeIndexSpec() != null) {
@@ -178,9 +175,7 @@ public class SegmentConverter {
private RecordAggregator _recordAggregator;
private List<String> _groupByColumns;
private IndexingConfig _indexingConfig;
-
- // enabled by default
- private boolean _checkTimeValidityDuringGeneration = true;
+ private boolean _skipTimeValueCheck;
public Builder setInputIndexDirs(List<File> inputIndexDirs) {
_inputIndexDirs = inputIndexDirs;
@@ -232,8 +227,8 @@ public class SegmentConverter {
return this;
}
- public Builder setCheckTimeValidityDuringGeneration(final boolean checkTimeValidity) {
- _checkTimeValidityDuringGeneration = checkTimeValidity;
+ public Builder setSkipTimeValueCheck(boolean skipTimeValueCheck) {
+ _skipTimeValueCheck = skipTimeValueCheck;
return this;
}
@@ -241,7 +236,7 @@ public class SegmentConverter {
// Check that the group-by columns and record aggregator are configured together
if (_groupByColumns != null && _groupByColumns.size() > 0) {
Preconditions
- .checkNotNull(_groupByColumns, "If group-by columns are given, the record aggregator is required.");
+ .checkNotNull(_recordAggregator, "If group-by columns are given, the record aggregator is required.");
} else {
Preconditions.checkArgument(_recordAggregator == null,
"If group-by columns are not given, the record aggregator has to be null.");
@@ -249,7 +244,7 @@ public class SegmentConverter {
return new SegmentConverter(_inputIndexDirs, _workingDir, _tableName, _segmentName, _totalNumPartition,
_recordTransformer, _recordPartitioner, _recordAggregator, _groupByColumns, _indexingConfig,
- _checkTimeValidityDuringGeneration);
+ _skipTimeValueCheck);
}
}
}
diff --git a/pinot-core/src/main/java/org/apache/pinot/core/realtime/converter/RealtimeSegmentConverter.java b/pinot-core/src/main/java/org/apache/pinot/core/realtime/converter/RealtimeSegmentConverter.java
index 800aa0b..c59909e 100644
--- a/pinot-core/src/main/java/org/apache/pinot/core/realtime/converter/RealtimeSegmentConverter.java
+++ b/pinot-core/src/main/java/org/apache/pinot/core/realtime/converter/RealtimeSegmentConverter.java
@@ -102,7 +102,7 @@ public class RealtimeSegmentConverter {
// range. We don't want the realtime consumption to stop (if an exception
// is thrown) and thus the time validity check is explicitly disabled for
// realtime segment generation
- genConfig.setCheckTimeColumnValidityDuringGeneration(false);
+ genConfig.setSkipTimeValueCheck(true);
if (invertedIndexColumns != null && !invertedIndexColumns.isEmpty()) {
for (String column : invertedIndexColumns) {
genConfig.createInvertedIndexForColumn(column);
diff --git a/pinot-core/src/main/java/org/apache/pinot/core/segment/creator/impl/SegmentColumnarIndexCreator.java b/pinot-core/src/main/java/org/apache/pinot/core/segment/creator/impl/SegmentColumnarIndexCreator.java
index 165f564..dd98e0a 100644
--- a/pinot-core/src/main/java/org/apache/pinot/core/segment/creator/impl/SegmentColumnarIndexCreator.java
+++ b/pinot-core/src/main/java/org/apache/pinot/core/segment/creator/impl/SegmentColumnarIndexCreator.java
@@ -23,7 +23,6 @@ import com.google.common.collect.Iterables;
import java.io.File;
import java.io.IOException;
import java.util.Collection;
-import java.util.Date;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
@@ -61,6 +60,8 @@ import org.apache.pinot.core.segment.creator.impl.inv.OffHeapBitmapInvertedIndex
import org.apache.pinot.core.segment.creator.impl.inv.OnHeapBitmapInvertedIndexCreator;
import org.apache.pinot.core.segment.creator.impl.nullvalue.NullValueVectorCreator;
import org.apache.pinot.startree.hll.HllConfig;
+import org.joda.time.DateTimeZone;
+import org.joda.time.Interval;
import org.joda.time.format.DateTimeFormat;
import org.joda.time.format.DateTimeFormatter;
import org.slf4j.Logger;
@@ -359,33 +360,44 @@ public class SegmentColumnarIndexCreator implements SegmentCreator {
String timeColumn = config.getTimeColumnName();
ColumnIndexCreationInfo timeColumnIndexCreationInfo = indexCreationInfoMap.get(timeColumn);
if (timeColumnIndexCreationInfo != null) {
+ long startTime;
+ long endTime;
+ TimeUnit timeUnit;
+
// Use start/end time in config if defined
if (config.getStartTime() != null) {
- checkTime(config, config.getStartTime(), config.getEndTime(), segmentName);
- properties.setProperty(SEGMENT_START_TIME, config.getStartTime());
- properties.setProperty(SEGMENT_END_TIME, Preconditions.checkNotNull(config.getEndTime()));
- properties.setProperty(TIME_UNIT, Preconditions.checkNotNull(config.getSegmentTimeUnit()));
+ startTime = Long.parseLong(config.getStartTime());
+ endTime = Long.parseLong(config.getEndTime());
+ timeUnit = Preconditions.checkNotNull(config.getSegmentTimeUnit());
} else {
- Object minTime = Preconditions.checkNotNull(timeColumnIndexCreationInfo.getMin());
- Object maxTime = Preconditions.checkNotNull(timeColumnIndexCreationInfo.getMax());
+ String startTimeStr = timeColumnIndexCreationInfo.getMin().toString();
+ String endTimeStr = timeColumnIndexCreationInfo.getMax().toString();
if (config.getTimeColumnType() == SegmentGeneratorConfig.TimeColumnType.SIMPLE_DATE) {
// For TimeColumnType.SIMPLE_DATE_FORMAT, convert time value into millis since epoch
DateTimeFormatter dateTimeFormatter = DateTimeFormat.forPattern(config.getSimpleDateFormat());
- final long minTimeMillis = dateTimeFormatter.parseMillis(minTime.toString());
- final long maxTimeMillis = dateTimeFormatter.parseMillis(maxTime.toString());
- checkTime(config, minTimeMillis, maxTimeMillis, segmentName);
- properties.setProperty(SEGMENT_START_TIME, minTimeMillis);
- properties.setProperty(SEGMENT_END_TIME, maxTimeMillis);
- properties.setProperty(TIME_UNIT, TimeUnit.MILLISECONDS);
+ startTime = dateTimeFormatter.parseMillis(startTimeStr);
+ endTime = dateTimeFormatter.parseMillis(endTimeStr);
+ timeUnit = TimeUnit.MILLISECONDS;
} else {
// by default, time column type is TimeColumnType.EPOCH
- checkTime(config, minTime, maxTime, segmentName);
- properties.setProperty(SEGMENT_START_TIME, minTime);
- properties.setProperty(SEGMENT_END_TIME, maxTime);
- properties.setProperty(TIME_UNIT, Preconditions.checkNotNull(config.getSegmentTimeUnit()));
+ startTime = Long.parseLong(startTimeStr);
+ endTime = Long.parseLong(endTimeStr);
+ timeUnit = Preconditions.checkNotNull(config.getSegmentTimeUnit());
}
}
+
+ if (!config.isSkipTimeValueCheck()) {
+ Interval timeInterval = new Interval(timeUnit.toMillis(startTime), timeUnit.toMillis(endTime), DateTimeZone.UTC);
+ Preconditions.checkState(TimeUtils.isValidTimeInterval(timeInterval),
+ "Invalid segment start/end time: %s (in millis: %s/%s) for time column: %s, must be between: %s",
+ timeInterval, timeInterval.getStartMillis(), timeInterval.getEndMillis(), timeColumn,
+ TimeUtils.VALID_TIME_INTERVAL);
+ }
+
+ properties.setProperty(SEGMENT_START_TIME, startTime);
+ properties.setProperty(SEGMENT_END_TIME, endTime);
+ properties.setProperty(TIME_UNIT, timeUnit);
}
for (Map.Entry<String, String> entry : config.getCustomProperties().entrySet()) {
@@ -420,123 +432,6 @@ public class SegmentColumnarIndexCreator implements SegmentCreator {
properties.save();
}
- /**
- * Check for the validity of segment start and end time
- * @param startTime segment start time
- * @param endTime segment end time
- * @param segmentName segment name
- */
- private void checkTime(final SegmentGeneratorConfig config, final Object startTime, final Object endTime,
- final String segmentName) {
- if (!config.isCheckTimeColumnValidityDuringGeneration()) {
- return;
- }
-
- if (startTime == null || endTime == null) {
- throw new RuntimeException("Expecting non-null start/end time for segment: " + segmentName);
- }
-
- if (!(startTime.getClass().equals(endTime.getClass()))) {
- final StringBuilder err = new StringBuilder();
- err.append("Start and end time of segment should be of same type.").append(" segment name: ").append(segmentName)
- .append(" start time: ").append(startTime).append(" end time: ").append(endTime).append(" start time class: ")
- .append(startTime.getClass()).append(" end time class: ").append(endTime.getClass());
- throw new RuntimeException(err.toString());
- }
-
- long start;
- long end;
-
- final String cl = startTime.getClass().getSimpleName();
-
- switch (cl) {
- case "Long":
- start = (long) startTime;
- end = (long) endTime;
- break;
- case "String":
- start = Long.parseLong((String) startTime);
- end = Long.parseLong((String) endTime);
- break;
- case "Integer":
- start = ((Integer) startTime).longValue();
- end = ((Integer) endTime).longValue();
- break;
- default:
- final StringBuilder err = new StringBuilder();
- err.append("Unable to interpret type of time column value. Failed to validate start and end time of segment")
- .append(" uninterpreted type: ").append(startTime.getClass()).append(" start time: ").append(startTime)
- .append(" end time: ").append(endTime).append(" time column name: ").append(config.getTimeColumnName())
- .append(" segment name: ").append(segmentName).append(" segment time column unit: ")
- .append(config.getSegmentTimeUnit().toString()).append(" segment time column type: ")
- .append(config.getTimeColumnType().toString()).append(" time field spec data type: ")
- .append(config.getSchema().getTimeFieldSpec().getDataType().toString());
- LOGGER.error(err.toString());
- throw new RuntimeException(err.toString());
- }
-
- // note that handling of SimpleDateFormat (TimeColumnType.SIMPLE)
- // is done by the caller of this function that converts the simple format
- // into millis since epoch before calling this function for validation.
- // For TimeColumnType.EPOCH, the time field spec could still have unit
- // as any of the following and we need to convert to millis for doing the
- // min-max comparison against TimeUtils.getValidMinTimeMillis() and
- // TimeUtils.getValidMaxTimeMillis()
- if (config.getTimeColumnType() == SegmentGeneratorConfig.TimeColumnType.EPOCH) {
- switch (config.getSegmentTimeUnit()) {
- case DAYS:
- start = TimeUnit.DAYS.toMillis(start);
- end = TimeUnit.DAYS.toMillis(end);
- break;
- case HOURS:
- start = TimeUnit.HOURS.toMillis(start);
- end = TimeUnit.HOURS.toMillis(end);
- break;
- case MINUTES:
- start = TimeUnit.MINUTES.toMillis(start);
- end = TimeUnit.MINUTES.toMillis(end);
- break;
- case SECONDS:
- start = TimeUnit.SECONDS.toMillis(start);
- end = TimeUnit.SECONDS.toMillis(end);
- break;
- case MICROSECONDS:
- start = TimeUnit.MICROSECONDS.toMillis(start);
- end = TimeUnit.MICROSECONDS.toMillis(end);
- break;
- case NANOSECONDS:
- start = TimeUnit.NANOSECONDS.toMillis(start);
- end = TimeUnit.NANOSECONDS.toMillis(end);
- break;
- default:
- if (config.getSegmentTimeUnit() != TimeUnit.MILLISECONDS) {
- // we should never be here
- final StringBuilder err = new StringBuilder();
- err.append("Unexpected time unit: ").append(config.getSegmentTimeUnit().toString())
- .append(" for time column: ").append(config.getTimeColumnName()).append(" for segment: ")
- .append(segmentName);
- LOGGER.error(err.toString());
- throw new RuntimeException(err.toString());
- }
- }
- }
-
- if (!TimeUtils.checkSegmentTimeValidity(start, end)) {
- final Date minDate = new Date(TimeUtils.getValidMinTimeMillis());
- final Date maxDate = new Date(TimeUtils.getValidMaxTimeMillis());
- final StringBuilder err = new StringBuilder();
- err.append("Invalid start/end time.").append(" segment name: ").append(segmentName).append(" time column name: ")
- .append(config.getTimeColumnName()).append(" given start time: ").append(start).append("ms")
- .append(" given end time: ").append(end).append("ms").append(" start and end time must be between ")
- .append(minDate).append(" and ").append(maxDate).append(" segment time column unit: ")
- .append(config.getSegmentTimeUnit().toString()).append(" segment time column type: ")
- .append(config.getTimeColumnType().toString()).append(" time field spec data type: ")
- .append(config.getSchema().getTimeFieldSpec().getDataType().toString());
- LOGGER.error(err.toString());
- throw new RuntimeException(err.toString());
- }
- }
-
public static void addColumnMetadataInfo(PropertiesConfiguration properties, String column,
ColumnIndexCreationInfo columnIndexCreationInfo, int totalDocs, int totalRawDocs, int totalAggDocs,
FieldSpec fieldSpec, boolean hasDictionary, int dictionaryElementSize, boolean hasInvertedIndex,
diff --git a/pinot-core/src/main/java/org/apache/pinot/core/segment/index/SegmentMetadataImpl.java b/pinot-core/src/main/java/org/apache/pinot/core/segment/index/SegmentMetadataImpl.java
index f59590d..14051c0 100644
--- a/pinot-core/src/main/java/org/apache/pinot/core/segment/index/SegmentMetadataImpl.java
+++ b/pinot-core/src/main/java/org/apache/pinot/core/segment/index/SegmentMetadataImpl.java
@@ -57,6 +57,7 @@ import org.apache.pinot.core.segment.store.SegmentDirectoryPaths;
import org.apache.pinot.core.startree.v2.StarTreeV2Constants;
import org.apache.pinot.core.startree.v2.StarTreeV2Metadata;
import org.apache.pinot.startree.hll.HllConstants;
+import org.joda.time.DateTimeZone;
import org.joda.time.Duration;
import org.joda.time.Interval;
import org.slf4j.Logger;
@@ -190,7 +191,8 @@ public class SegmentMetadataImpl implements SegmentMetadata {
String endTimeString = segmentMetadataPropertiesConfiguration.getString(SEGMENT_END_TIME);
_segmentStartTime = Long.parseLong(startTimeString);
_segmentEndTime = Long.parseLong(endTimeString);
- _timeInterval = new Interval(_timeUnit.toMillis(_segmentStartTime), _timeUnit.toMillis(_segmentEndTime));
+ _timeInterval =
+ new Interval(_timeUnit.toMillis(_segmentStartTime), _timeUnit.toMillis(_segmentEndTime), DateTimeZone.UTC);
} catch (Exception e) {
LOGGER.warn("Caught exception while setting time interval and granularity", e);
_timeInterval = null;
diff --git a/pinot-core/src/test/java/org/apache/pinot/core/minion/SegmentConverterTest.java b/pinot-core/src/test/java/org/apache/pinot/core/minion/SegmentConverterTest.java
index e10e980..bf38b7e 100644
--- a/pinot-core/src/test/java/org/apache/pinot/core/minion/SegmentConverterTest.java
+++ b/pinot-core/src/test/java/org/apache/pinot/core/minion/SegmentConverterTest.java
@@ -29,20 +29,19 @@ import org.apache.pinot.common.data.FieldSpec;
import org.apache.pinot.common.data.MetricFieldSpec;
import org.apache.pinot.common.data.Schema;
import org.apache.pinot.common.data.TimeFieldSpec;
-import org.apache.pinot.common.utils.time.TimeUtils;
import org.apache.pinot.core.data.GenericRow;
import org.apache.pinot.core.data.readers.GenericRowRecordReader;
import org.apache.pinot.core.data.readers.PinotSegmentRecordReader;
import org.apache.pinot.core.data.readers.RecordReader;
import org.apache.pinot.core.indexsegment.generator.SegmentGeneratorConfig;
-import org.apache.pinot.core.operator.transform.transformer.datetime.BaseDateTimeTransformer;
-import org.apache.pinot.core.operator.transform.transformer.datetime.DateTimeTransformerFactory;
import org.apache.pinot.core.segment.creator.impl.SegmentIndexCreationDriverImpl;
-import org.testng.Assert;
+import org.joda.time.DateTime;
import org.testng.annotations.AfterClass;
import org.testng.annotations.BeforeClass;
import org.testng.annotations.Test;
+import static org.testng.Assert.assertEquals;
+
public class SegmentConverterTest {
private static final File WORKING_DIR = new File(FileUtils.getTempDirectory(), "SegmentConverterTest");
@@ -57,9 +56,9 @@ public class SegmentConverterTest {
private static final String D2 = "d2";
private static final String M1 = "m1";
private static final String T = "t";
+ private static final long BASE_TIMESTAMP = new DateTime(2018, 9, 5, 0, 0).getMillis();
private List<File> _segmentIndexDirList;
- private final long _referenceTimestamp = TimeUtils.getValidMinTimeMillis();
@BeforeClass
public void setUp()
@@ -74,14 +73,13 @@ public class SegmentConverterTest {
schema.addField(new TimeFieldSpec(T, FieldSpec.DataType.LONG, TimeUnit.MILLISECONDS));
List<GenericRow> rows = new ArrayList<>(NUM_ROWS);
- long timestamp = _referenceTimestamp;
for (int i = 0; i < NUM_ROWS; i++) {
int dimensionValue = i % (NUM_ROWS / REPEAT_ROWS);
GenericRow row = new GenericRow();
- row.putField(D1, dimensionValue);
- row.putField(D2, Integer.toString(dimensionValue));
- row.putField(M1, dimensionValue);
- row.putField(T, timestamp++);
+ row.putValue(D1, dimensionValue);
+ row.putValue(D2, Integer.toString(dimensionValue));
+ row.putValue(M1, dimensionValue);
+ row.putValue(T, BASE_TIMESTAMP + i);
rows.add(row);
}
@@ -111,7 +109,7 @@ public class SegmentConverterTest {
List<File> result = segmentConverter.convertSegment();
- Assert.assertEquals(result.size(), 1);
+ assertEquals(result.size(), 1);
List<GenericRow> outputRows = new ArrayList<>();
try (PinotSegmentRecordReader pinotSegmentRecordReader = new PinotSegmentRecordReader(result.get(0))) {
while (pinotSegmentRecordReader.hasNext()) {
@@ -119,20 +117,18 @@ public class SegmentConverterTest {
}
}
// Check that the segment is correctly rolled up on the time column
- Assert.assertEquals(outputRows.size(), NUM_ROWS * NUM_SEGMENTS,
+ assertEquals(outputRows.size(), NUM_ROWS * NUM_SEGMENTS,
"Number of rows returned by segment converter is incorrect");
// Check the value
for (int i = 0; i < NUM_SEGMENTS; i++) {
- int rowCount = 0;
- long timestamp = _referenceTimestamp;
for (int j = 0; j < NUM_ROWS; j++) {
- int expectedValue = rowCount % (NUM_ROWS / REPEAT_ROWS);
- Assert.assertEquals(outputRows.get(rowCount).getValue(D1), expectedValue);
- Assert.assertEquals(outputRows.get(rowCount).getValue(D2), Integer.toString(expectedValue));
- Assert.assertEquals(outputRows.get(rowCount).getValue(M1), expectedValue);
- Assert.assertEquals(outputRows.get(rowCount).getValue(T), timestamp++);
- rowCount++;
+ GenericRow row = outputRows.get(i * NUM_ROWS + j);
+ int expectedValue = j % (NUM_ROWS / REPEAT_ROWS);
+ assertEquals(row.getValue(D1), expectedValue);
+ assertEquals(row.getValue(D2), Integer.toString(expectedValue));
+ assertEquals(row.getValue(M1), expectedValue);
+ assertEquals(row.getValue(T), BASE_TIMESTAMP + j);
}
}
}
@@ -140,43 +136,26 @@ public class SegmentConverterTest {
@Test
public void testSegmentRollupWithTimeConversion()
throws Exception {
- final BaseDateTimeTransformer dateTimeTransformer =
- DateTimeTransformerFactory.getDateTimeTransformer("1:MILLISECONDS:EPOCH", "1:DAYS:EPOCH", "1:DAYS");
-
- // The segment generation code in SegmentColumnarIndexCreator will throw
- // exception if start and end time in time column are not in acceptable
- // range. For this test, we have explicitly disabled the check since the segment
- // conversion is happening along with transforming each input record by converting
- // the time column value (from millis since epoch) into days since epoch. While the
- // source data is under range, the transformed data (days since epoch) goes out of range
- // since segment conversion follows the same schema -- it does not create new schema. This
- // means that time column spec doesn't change and still carries the time unit as milliseconds
- // for the converted segment even though values we are writing are "days since epoch" and
- // not "millis since epoch". Thus SegmentColumnarIndexCreator throws exception.
SegmentConverter segmentConverter =
new SegmentConverter.Builder().setTableName(TABLE_NAME).setSegmentName("segmentRollupWithTimeConversion")
- .setInputIndexDirs(_segmentIndexDirList).setWorkingDir(WORKING_DIR).setCheckTimeValidityDuringGeneration(false)
+ .setInputIndexDirs(_segmentIndexDirList).setWorkingDir(WORKING_DIR).setSkipTimeValueCheck(false)
.setRecordTransformer((row) -> {
- long[] input = new long[1];
- long[] output = new long[1];
- input[0] = (Long) row.getValue(T);
- dateTimeTransformer.transform(input, output, 1);
- row.putField(T, output[0]);
- return row;
- }).setGroupByColumns(Arrays.asList(new String[]{D1, D2, T})).setRecordAggregator((rows) -> {
+ row.putValue(T, BASE_TIMESTAMP);
+ return row;
+ }).setGroupByColumns(Arrays.asList(D1, D2, T)).setRecordAggregator((rows) -> {
GenericRow result = rows.get(0);
for (int i = 1; i < rows.size(); i++) {
GenericRow current = rows.get(i);
Object aggregatedValue =
((Number) result.getValue(M1)).intValue() + ((Number) current.getValue(M1)).intValue();
- result.putField(M1, aggregatedValue);
+ result.putValue(M1, aggregatedValue);
}
return result;
}).setTotalNumPartition(1).build();
List<File> result = segmentConverter.convertSegment();
- Assert.assertEquals(result.size(), 1);
+ assertEquals(result.size(), 1);
List<GenericRow> outputRows = new ArrayList<>();
try (PinotSegmentRecordReader pinotSegmentRecordReader = new PinotSegmentRecordReader(result.get(0))) {
while (pinotSegmentRecordReader.hasNext()) {
@@ -184,15 +163,16 @@ public class SegmentConverterTest {
}
}
// Check that the segment is correctly rolled up on the time column
- Assert.assertEquals(outputRows.size(), NUM_ROWS / REPEAT_ROWS,
+ assertEquals(outputRows.size(), NUM_ROWS / REPEAT_ROWS,
"Number of rows returned by segment converter is incorrect");
// Check the value
int expectedValue = 0;
for (GenericRow row : outputRows) {
- Assert.assertEquals(row.getValue(D1), expectedValue);
- Assert.assertEquals(row.getValue(D2), Integer.toString(expectedValue));
- Assert.assertEquals(row.getValue(M1), expectedValue * NUM_SEGMENTS * REPEAT_ROWS);
+ assertEquals(row.getValue(D1), expectedValue);
+ assertEquals(row.getValue(D2), Integer.toString(expectedValue));
+ assertEquals(row.getValue(M1), expectedValue * NUM_SEGMENTS * REPEAT_ROWS);
+ assertEquals(row.getValue(T), BASE_TIMESTAMP);
expectedValue++;
}
}
@@ -207,7 +187,7 @@ public class SegmentConverterTest {
List<File> result = segmentConverter.convertSegment();
- Assert.assertEquals(result.size(), 3);
+ assertEquals(result.size(), 3);
List<List<GenericRow>> outputRows = new ArrayList<>();
for (File resultFile : result) {
@@ -220,7 +200,7 @@ public class SegmentConverterTest {
}
}
- Assert.assertEquals(outputRows.stream().mapToInt(r -> r.size()).sum(), NUM_ROWS * NUM_SEGMENTS);
+ assertEquals(outputRows.stream().mapToInt(List::size).sum(), NUM_ROWS * NUM_SEGMENTS);
}
@AfterClass
diff --git a/pinot-core/src/test/java/org/apache/pinot/core/plan/maker/MetadataAndDictionaryAggregationPlanMakerTest.java b/pinot-core/src/test/java/org/apache/pinot/core/plan/maker/MetadataAndDictionaryAggregationPlanMakerTest.java
index b6ba799..2b433ff 100644
--- a/pinot-core/src/test/java/org/apache/pinot/core/plan/maker/MetadataAndDictionaryAggregationPlanMakerTest.java
+++ b/pinot-core/src/test/java/org/apache/pinot/core/plan/maker/MetadataAndDictionaryAggregationPlanMakerTest.java
@@ -99,7 +99,7 @@ public class MetadataAndDictionaryAggregationPlanMakerTest {
// range. For this test, we first need to fix the input avro data
// to have the time column values in allowed range. Until then, the check
// is explicitly disabled
- segmentGeneratorConfig.setCheckTimeColumnValidityDuringGeneration(false);
+ segmentGeneratorConfig.setSkipTimeValueCheck(true);
segmentGeneratorConfig
.setInvertedIndexCreationColumns(Arrays.asList("column6", "column7", "column11", "column17", "column18"));
@@ -132,7 +132,7 @@ public class MetadataAndDictionaryAggregationPlanMakerTest {
// range. For this test, we first need to fix the input avro data
// to have the time column values in allowed range. Until then, the check
// is explicitly disabled
- segmentGeneratorConfig.setCheckTimeColumnValidityDuringGeneration(false);
+ segmentGeneratorConfig.setSkipTimeValueCheck(true);
// Build the index segment.
driver = new SegmentIndexCreationDriverImpl();
@@ -214,8 +214,9 @@ public class MetadataAndDictionaryAggregationPlanMakerTest {
boolean expectedIsFitForDictionary) {
BrokerRequest brokerRequest = COMPILER.compileToBrokerRequest(query);
- boolean isFitForMetadataBasedPlan = PLAN_MAKER.isFitForMetadataBasedPlan(brokerRequest, indexSegment);
- boolean isFitForDictionaryBasedPlan = PLAN_MAKER.isFitForDictionaryBasedPlan(brokerRequest, indexSegment);
+ boolean isFitForMetadataBasedPlan = InstancePlanMakerImplV2.isFitForMetadataBasedPlan(brokerRequest, indexSegment);
+ boolean isFitForDictionaryBasedPlan =
+ InstancePlanMakerImplV2.isFitForDictionaryBasedPlan(brokerRequest, indexSegment);
Assert.assertEquals(isFitForMetadataBasedPlan, expectedIsFitForMetadata);
Assert.assertEquals(isFitForDictionaryBasedPlan, expectedIsFitForDictionary);
}
diff --git a/pinot-core/src/test/java/org/apache/pinot/core/segment/index/ColumnMetadataTest.java b/pinot-core/src/test/java/org/apache/pinot/core/segment/index/ColumnMetadataTest.java
index 54886fc..54e88ee 100644
--- a/pinot-core/src/test/java/org/apache/pinot/core/segment/index/ColumnMetadataTest.java
+++ b/pinot-core/src/test/java/org/apache/pinot/core/segment/index/ColumnMetadataTest.java
@@ -44,7 +44,7 @@ import org.testng.annotations.Test;
public class ColumnMetadataTest {
private static final String AVRO_DATA = "data/test_data-mv.avro";
- private static final File INDEX_DIR = new File(ColumnMetadataTest.class.toString());
+ private static final File INDEX_DIR = new File(FileUtils.getTempDirectory(), "ColumnMetadataTest");
private static final String CREATOR_VERSION = "TestHadoopJar.1.1.1";
@BeforeMethod
@@ -67,13 +67,12 @@ public class ColumnMetadataTest {
.getSegmentGenSpecWithSchemAndProjectedColumns(new File(filePath), INDEX_DIR, "daysSinceEpoch", TimeUnit.HOURS,
"testTable");
config.setSegmentNamePostfix("1");
- config.setTimeColumnName("daysSinceEpoch");
// The segment generation code in SegmentColumnarIndexCreator will throw
// exception if start and end time in time column are not in acceptable
// range. For this test, we first need to fix the input avro data
// to have the time column values in allowed range. Until then, the check
// is explicitly disabled
- config.setCheckTimeColumnValidityDuringGeneration(false);
+ config.setSkipTimeValueCheck(true);
return config;
}
diff --git a/pinot-core/src/test/java/org/apache/pinot/core/segment/index/SegmentMetadataImplTest.java b/pinot-core/src/test/java/org/apache/pinot/core/segment/index/SegmentMetadataImplTest.java
index 30656e3..18142b3 100644
--- a/pinot-core/src/test/java/org/apache/pinot/core/segment/index/SegmentMetadataImplTest.java
+++ b/pinot-core/src/test/java/org/apache/pinot/core/segment/index/SegmentMetadataImplTest.java
@@ -21,7 +21,6 @@ package org.apache.pinot.core.segment.index;
import com.fasterxml.jackson.databind.JsonNode;
import java.io.File;
import java.io.IOException;
-import java.nio.file.Files;
import java.util.concurrent.TimeUnit;
import org.apache.commons.io.FileUtils;
import org.apache.pinot.core.indexsegment.generator.SegmentGeneratorConfig;
@@ -40,14 +39,12 @@ import static org.testng.Assert.assertTrue;
public class SegmentMetadataImplTest {
private static final String AVRO_DATA = "data/test_data-mv.avro";
- private File INDEX_DIR;
+ private static final File INDEX_DIR = new File(FileUtils.getTempDirectory(), "SegmentMetadataImplTest");
private File segmentDirectory;
@BeforeMethod
public void setUp()
throws Exception {
- INDEX_DIR = Files.createTempDirectory(SegmentMetadataImplTest.class.getName() + "_segmentDir").toFile();
-
final String filePath =
TestUtils.getFileFromResourceUrl(SegmentMetadataImplTest.class.getClassLoader().getResource(AVRO_DATA));
@@ -56,13 +53,12 @@ public class SegmentMetadataImplTest {
.getSegmentGenSpecWithSchemAndProjectedColumns(new File(filePath), INDEX_DIR, "daysSinceEpoch", TimeUnit.HOURS,
"testTable");
config.setSegmentNamePostfix("1");
- config.setTimeColumnName("daysSinceEpoch");
// The segment generation code in SegmentColumnarIndexCreator will throw
// exception if start and end time in time column are not in acceptable
// range. For this test, we first need to fix the input avro data
// to have the time column values in allowed range. Until then, the check
// is explicitly disabled
- config.setCheckTimeColumnValidityDuringGeneration(false);
+ config.setSkipTimeValueCheck(true);
final SegmentIndexCreationDriver driver = SegmentCreationDriverFactory.get(null);
driver.init(config);
driver.build();
diff --git a/pinot-core/src/test/java/org/apache/pinot/core/segment/index/converter/SegmentV1V2ToV3FormatConverterTest.java b/pinot-core/src/test/java/org/apache/pinot/core/segment/index/converter/SegmentV1V2ToV3FormatConverterTest.java
index 08d4008..2e8c561 100644
--- a/pinot-core/src/test/java/org/apache/pinot/core/segment/index/converter/SegmentV1V2ToV3FormatConverterTest.java
+++ b/pinot-core/src/test/java/org/apache/pinot/core/segment/index/converter/SegmentV1V2ToV3FormatConverterTest.java
@@ -63,13 +63,12 @@ public class SegmentV1V2ToV3FormatConverterTest {
.getSegmentGenSpecWithSchemAndProjectedColumns(new File(filePath), _indexDir, "daysSinceEpoch", TimeUnit.HOURS,
"testTable");
config.setSegmentNamePostfix("1");
- config.setTimeColumnName("daysSinceEpoch");
// The segment generation code in SegmentColumnarIndexCreator will throw
// exception if start and end time in time column are not in acceptable
// range. For this test, we first need to fix the input avro data
// to have the time column values in allowed range. Until then, the check
// is explicitly disabled
- config.setCheckTimeColumnValidityDuringGeneration(false);
+ config.setSkipTimeValueCheck(true);
final SegmentIndexCreationDriver driver = SegmentCreationDriverFactory.get(null);
driver.init(config);
driver.build();
diff --git a/pinot-core/src/test/java/org/apache/pinot/core/segment/index/creator/SegmentGenerationWithTimeColumnTest.java b/pinot-core/src/test/java/org/apache/pinot/core/segment/index/creator/SegmentGenerationWithTimeColumnTest.java
index 67970dd..460e4a7 100644
--- a/pinot-core/src/test/java/org/apache/pinot/core/segment/index/creator/SegmentGenerationWithTimeColumnTest.java
+++ b/pinot-core/src/test/java/org/apache/pinot/core/segment/index/creator/SegmentGenerationWithTimeColumnTest.java
@@ -68,7 +68,7 @@ public class SegmentGenerationWithTimeColumnTest {
@BeforeClass
public void printSeed() {
- System.out.println("Seed is: "+ seed);
+ System.out.println("Seed is: " + seed);
}
@BeforeMethod
@@ -98,15 +98,11 @@ public class SegmentGenerationWithTimeColumnTest {
Assert.assertEquals(metadata.getEndTime(), maxTime);
}
- @Test
- public void testSegmentGenerationWithInvalidTime() {
+ @Test(expectedExceptions = IllegalStateException.class)
+ public void testSegmentGenerationWithInvalidTime()
+ throws Exception {
Schema schema = createSchema(false);
- try {
- buildSegment(schema, false, true);
- Assert.fail("Expecting exception from buildSegment for invalid start/end time of segment");
- } catch (Exception e) {
- Assert.assertTrue(e.getMessage().contains("Invalid start/end time. segment name: testSegment time column name: date"));
- }
+ buildSegment(schema, false, true);
}
private Schema createSchema(boolean isSimpleDate) {
@@ -120,8 +116,7 @@ public class SegmentGenerationWithTimeColumnTest {
return schema;
}
- private File buildSegment(final Schema schema, final boolean isSimpleDate,
- final boolean isInvalidDate)
+ private File buildSegment(final Schema schema, final boolean isSimpleDate, final boolean isInvalidDate)
throws Exception {
SegmentGeneratorConfig config = new SegmentGeneratorConfig(schema);
config.setRawIndexCreationColumns(schema.getDimensionNames());
@@ -177,8 +172,8 @@ public class SegmentGenerationWithTimeColumnTest {
}
private Object getRandomValueForTimeColumn(boolean isSimpleDate, boolean isInvalidDate) {
- long randomMs = validMinTime + (long)(_random.nextDouble() * (startTime - validMinTime));
- Preconditions.checkArgument(TimeUtils.timeValueInValidRange(randomMs), "Value " + randomMs +" out of range");
+ long randomMs = validMinTime + (long) (_random.nextDouble() * (startTime - validMinTime));
+ Preconditions.checkArgument(TimeUtils.timeValueInValidRange(randomMs), "Value " + randomMs + " out of range");
long dateColVal = randomMs;
Object result;
if (isInvalidDate) {
diff --git a/pinot-core/src/test/java/org/apache/pinot/core/segment/index/loader/SegmentPreProcessorTest.java b/pinot-core/src/test/java/org/apache/pinot/core/segment/index/loader/SegmentPreProcessorTest.java
index a1f4353..77d8909 100644
--- a/pinot-core/src/test/java/org/apache/pinot/core/segment/index/loader/SegmentPreProcessorTest.java
+++ b/pinot-core/src/test/java/org/apache/pinot/core/segment/index/loader/SegmentPreProcessorTest.java
@@ -126,7 +126,7 @@ public class SegmentPreProcessorTest {
// range. For this test, we first need to fix the input avro data
// to have the time column values in allowed range. Until then, the check
// is explicitly disabled
- segmentGeneratorConfig.setCheckTimeColumnValidityDuringGeneration(false);
+ segmentGeneratorConfig.setSkipTimeValueCheck(true);
SegmentIndexCreationDriver driver = SegmentCreationDriverFactory.get(null);
driver.init(segmentGeneratorConfig);
driver.build();
@@ -277,15 +277,16 @@ public class SegmentPreProcessorTest {
processor.process();
}
-
SegmentMetadataImpl segmentMetadata = new SegmentMetadataImpl(_indexDir);
ColumnMetadata hllMetricMetadata = segmentMetadata.getColumnMetadataFor(NEW_HLL_BYTE_METRIC_COLUMN_NAME);
Assert.assertEquals(hllMetricMetadata.getDataType(), FieldSpec.DataType.BYTES);
- Assert.assertEquals(hllMetricMetadata.getDefaultNullValueString(), "00000008000000ac00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000");
+ Assert.assertEquals(hllMetricMetadata.getDefaultNullValueString(),
+ "00000008000000ac00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000");
ColumnMetadata tDigestMetricMetadata = segmentMetadata.getColumnMetadataFor(NEW_TDIGEST_BYTE_METRIC_COLUMN_NAME);
Assert.assertEquals(tDigestMetricMetadata.getDataType(), FieldSpec.DataType.BYTES);
- Assert.assertEquals(tDigestMetricMetadata.getDefaultNullValueString(), "0000000141ba085ee15d2f3241ba085ee15d2f324059000000000000000000013ff000000000000041ba085ee15d2f32");
+ Assert.assertEquals(tDigestMetricMetadata.getDefaultNullValueString(),
+ "0000000141ba085ee15d2f3241ba085ee15d2f324059000000000000000000013ff000000000000041ba085ee15d2f32");
}
private void checkUpdateDefaultColumns()
diff --git a/pinot-core/src/test/java/org/apache/pinot/core/startree/StarTreeIndexTestSegmentHelper.java b/pinot-core/src/test/java/org/apache/pinot/core/startree/StarTreeIndexTestSegmentHelper.java
index 2e57398..c33bc6c 100644
--- a/pinot-core/src/test/java/org/apache/pinot/core/startree/StarTreeIndexTestSegmentHelper.java
+++ b/pinot-core/src/test/java/org/apache/pinot/core/startree/StarTreeIndexTestSegmentHelper.java
@@ -19,47 +19,40 @@
package org.apache.pinot.core.startree;
import java.util.ArrayList;
-import java.util.HashMap;
import java.util.List;
import java.util.Random;
-import java.util.concurrent.TimeUnit;
import org.apache.commons.math.util.MathUtils;
import org.apache.pinot.common.data.DimensionFieldSpec;
import org.apache.pinot.common.data.FieldSpec;
import org.apache.pinot.common.data.MetricFieldSpec;
import org.apache.pinot.common.data.Schema;
import org.apache.pinot.common.data.StarTreeIndexSpec;
-import org.apache.pinot.common.data.TimeFieldSpec;
import org.apache.pinot.core.data.GenericRow;
import org.apache.pinot.core.data.readers.FileFormat;
import org.apache.pinot.core.data.readers.GenericRowRecordReader;
import org.apache.pinot.core.indexsegment.generator.SegmentGeneratorConfig;
import org.apache.pinot.core.segment.creator.impl.SegmentIndexCreationDriverImpl;
import org.apache.pinot.startree.hll.HllConfig;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
public class StarTreeIndexTestSegmentHelper {
- private static final Logger LOGGER = LoggerFactory.getLogger(StarTreeIndexTestSegmentHelper.class);
private static final Random RANDOM = new Random();
- private static final String TIME_COLUMN_NAME = "daysSinceEpoch";
private static final int NUM_DIMENSIONS = 4;
private static final int NUM_METRICS = 2;
private static final int METRIC_MAX_VALUE = 10000;
- public static Schema buildSegment(String segmentDirName, String segmentName)
+ public static void buildSegment(String segmentDirName, String segmentName)
throws Exception {
- return buildSegment(segmentDirName, segmentName, null);
+ buildSegment(segmentDirName, segmentName, null);
}
- public static Schema buildSegmentWithHll(String segmentDirName, String segmentName, HllConfig hllConfig)
+ public static void buildSegmentWithHll(String segmentDirName, String segmentName, HllConfig hllConfig)
throws Exception {
- return buildSegment(segmentDirName, segmentName, hllConfig);
+ buildSegment(segmentDirName, segmentName, hllConfig);
}
- private static Schema buildSegment(String segmentDirName, String segmentName, HllConfig hllConfig)
+ private static void buildSegment(String segmentDirName, String segmentName, HllConfig hllConfig)
throws Exception {
int numRows = (int) MathUtils.factorial(NUM_DIMENSIONS) * 100;
Schema schema = new Schema();
@@ -70,7 +63,6 @@ public class StarTreeIndexTestSegmentHelper {
schema.addField(dimensionFieldSpec);
}
- schema.addField(new TimeFieldSpec(TIME_COLUMN_NAME, FieldSpec.DataType.INT, TimeUnit.DAYS));
for (int i = 0; i < NUM_METRICS; i++) {
String metricName = "m" + (i + 1);
MetricFieldSpec metricFieldSpec = new MetricFieldSpec(metricName, FieldSpec.DataType.INT);
@@ -85,46 +77,32 @@ public class StarTreeIndexTestSegmentHelper {
config.setFormat(FileFormat.AVRO);
config.setSegmentName(segmentName);
config.setHllConfig(hllConfig);
- // The segment generation code in SegmentColumnarIndexCreator will throw
- // exception if start and end time in time column are not in acceptable
- // range. For this test, we first need to fix the input avro data
- // to have the time column values in allowed range. Until then, the check
- // is explicitly disabled
- config.setCheckTimeColumnValidityDuringGeneration(false);
List<GenericRow> rows = new ArrayList<>(numRows);
for (int rowId = 0; rowId < numRows; rowId++) {
- HashMap<String, Object> map = new HashMap<>();
- // Dim columns.
+ GenericRow genericRow = new GenericRow();
+ // Dimensions
for (int i = 0; i < NUM_DIMENSIONS / 2; i++) {
String dimName = schema.getDimensionFieldSpecs().get(i).getName();
- map.put(dimName, dimName + "-v" + rowId % (NUM_DIMENSIONS - i));
+ genericRow.putValue(dimName, dimName + "-v" + rowId % (NUM_DIMENSIONS - i));
}
// Random values make cardinality of d3, d4 column values larger to better test hll
for (int i = NUM_DIMENSIONS / 2; i < NUM_DIMENSIONS; i++) {
String dimName = schema.getDimensionFieldSpecs().get(i).getName();
- map.put(dimName, dimName + "-v" + RANDOM.nextInt(i * 100));
+ genericRow.putValue(dimName, dimName + "-v" + RANDOM.nextInt(i * 100));
}
- // Metric columns.
+ // Metrics
for (int i = 0; i < NUM_METRICS; i++) {
String metName = schema.getMetricFieldSpecs().get(i).getName();
- map.put(metName, RANDOM.nextInt(METRIC_MAX_VALUE));
+ genericRow.putValue(metName, RANDOM.nextInt(METRIC_MAX_VALUE));
}
- // Time column.
- map.put(TIME_COLUMN_NAME, rowId % 7);
-
- GenericRow genericRow = new GenericRow();
- genericRow.init(map);
rows.add(genericRow);
}
SegmentIndexCreationDriverImpl driver = new SegmentIndexCreationDriverImpl();
driver.init(config, new GenericRowRecordReader(rows, schema));
driver.build();
-
- LOGGER.info("Built segment {} at {}", segmentName, segmentDirName);
- return schema;
}
}
diff --git a/pinot-core/src/test/java/org/apache/pinot/core/startree/TestStarTreeMetadata.java b/pinot-core/src/test/java/org/apache/pinot/core/startree/TestStarTreeMetadata.java
index 08ee7b7..8c12cd0 100644
--- a/pinot-core/src/test/java/org/apache/pinot/core/startree/TestStarTreeMetadata.java
+++ b/pinot-core/src/test/java/org/apache/pinot/core/startree/TestStarTreeMetadata.java
@@ -20,14 +20,12 @@ package org.apache.pinot.core.startree;
import java.io.File;
import java.util.Arrays;
-import java.util.HashSet;
+import java.util.Collections;
import java.util.List;
import java.util.Set;
-import java.util.concurrent.TimeUnit;
import org.apache.commons.io.FileUtils;
import org.apache.pinot.common.data.StarTreeIndexSpec;
import org.apache.pinot.common.segment.StarTreeMetadata;
-import org.apache.pinot.core.indexsegment.IndexSegment;
import org.apache.pinot.core.indexsegment.generator.SegmentGeneratorConfig;
import org.apache.pinot.core.segment.creator.SegmentIndexCreationDriver;
import org.apache.pinot.core.segment.creator.impl.SegmentCreationDriverFactory;
@@ -40,24 +38,18 @@ import org.testng.annotations.Test;
public class TestStarTreeMetadata {
- private final String AVRO_DATA = "data/test_sample_data.avro";
+ private static final String AVRO_DATA = "data/test_sample_data.avro";
private static final int MAX_LEAF_RECORDS = 99;
private static final int SKIP_CARDINALITY_THRESHOLD = 99999;
- private static final List<String> DIMENSIONS_SPLIT_ORDER = Arrays.asList(new String[]{"column3", "column4"});
-
- private static final Set<String> SKIP_STAR_NODE_CREATION_DIMENSTIONS =
- new HashSet<String>(Arrays.asList(new String[]{"column9"}));
-
- private static final Set<String> SKIP_MATERIALIZATION_DIMENSIONS =
- new HashSet<String>(Arrays.asList(new String[]{"column11"}));
-
+ private static final List<String> DIMENSIONS_SPLIT_ORDER = Arrays.asList("column3", "column4");
+ private static final Set<String> SKIP_STAR_NODE_CREATION_DIMENSIONS = Collections.singleton("column9");
+ private static final Set<String> SKIP_MATERIALIZATION_DIMENSIONS = Collections.singleton("column11");
private static final String TABLE_NAME = "starTreeTable";
private static final String SEGMENT_NAME = "starTreeSegment";
private static final String INDEX_DIR_NAME = FileUtils.getTempDirectory() + File.separator + "starTreeMetaData";
private static File INDEX_DIR = new File(INDEX_DIR_NAME);
- public static IndexSegment _indexSegment;
/**
* Build the StarTree segment
@@ -82,9 +74,8 @@ public class TestStarTreeMetadata {
FileUtils.deleteQuietly(segmentDir);
}
- final SegmentGeneratorConfig config = SegmentTestUtils
- .getSegmentGenSpecWithSchemAndProjectedColumns(new File(filePath), segmentDir, "time_day", TimeUnit.DAYS,
- TABLE_NAME);
+ final SegmentGeneratorConfig config =
+ SegmentTestUtils.getSegmentGeneratorConfigWithoutTimeColumn(new File(filePath), segmentDir, TABLE_NAME);
config.setTableName(TABLE_NAME);
config.setSegmentName(SEGMENT_NAME);
@@ -92,16 +83,10 @@ public class TestStarTreeMetadata {
starTreeIndexSpec.setDimensionsSplitOrder(DIMENSIONS_SPLIT_ORDER);
starTreeIndexSpec.setMaxLeafRecords(MAX_LEAF_RECORDS);
starTreeIndexSpec.setSkipMaterializationCardinalityThreshold(SKIP_CARDINALITY_THRESHOLD);
- starTreeIndexSpec.setSkipStarNodeCreationForDimensions(SKIP_STAR_NODE_CREATION_DIMENSTIONS);
+ starTreeIndexSpec.setSkipStarNodeCreationForDimensions(SKIP_STAR_NODE_CREATION_DIMENSIONS);
starTreeIndexSpec.setSkipMaterializationForDimensions(SKIP_MATERIALIZATION_DIMENSIONS);
config.enableStarTreeIndex(starTreeIndexSpec);
- // The segment generation code in SegmentColumnarIndexCreator will throw
- // exception if start and end time in time column are not in acceptable
- // range. For this test, we first need to fix the input avro data
- // to have the time column values in allowed range. Until then, the check
- // is explicitly disabled
- config.setCheckTimeColumnValidityDuringGeneration(false);
final SegmentIndexCreationDriver driver = SegmentCreationDriverFactory.get(null);
driver.init(config);
@@ -124,7 +109,7 @@ public class TestStarTreeMetadata {
Assert.assertEquals(starTreeMetadata.getDimensionsSplitOrder(), DIMENSIONS_SPLIT_ORDER);
Assert.assertEquals(starTreeMetadata.getMaxLeafRecords(), MAX_LEAF_RECORDS);
- Assert.assertEquals(starTreeMetadata.getSkipStarNodeCreationForDimensions(), SKIP_STAR_NODE_CREATION_DIMENSTIONS);
+ Assert.assertEquals(starTreeMetadata.getSkipStarNodeCreationForDimensions(), SKIP_STAR_NODE_CREATION_DIMENSIONS);
Assert.assertEquals(starTreeMetadata.getSkipMaterializationCardinality(), SKIP_CARDINALITY_THRESHOLD);
Assert.assertEquals(starTreeMetadata.getSkipMaterializationForDimensions(), SKIP_MATERIALIZATION_DIMENSIONS);
}
diff --git a/pinot-core/src/test/java/org/apache/pinot/core/startree/hll/SegmentWithHllIndexCreateHelper.java b/pinot-core/src/test/java/org/apache/pinot/core/startree/hll/SegmentWithHllIndexCreateHelper.java
index e36334d..9a5946f 100644
--- a/pinot-core/src/test/java/org/apache/pinot/core/startree/hll/SegmentWithHllIndexCreateHelper.java
+++ b/pinot-core/src/test/java/org/apache/pinot/core/startree/hll/SegmentWithHllIndexCreateHelper.java
@@ -36,7 +36,6 @@ import org.apache.pinot.core.segment.creator.impl.SegmentCreationDriverFactory;
import org.apache.pinot.core.segment.creator.impl.SegmentIndexCreationDriverImpl;
import org.apache.pinot.segments.v1.creator.SegmentTestUtils;
import org.apache.pinot.startree.hll.HllConfig;
-import org.apache.pinot.startree.hll.HllConstants;
import org.apache.pinot.util.TestUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -44,14 +43,13 @@ import org.slf4j.LoggerFactory;
public class SegmentWithHllIndexCreateHelper {
private static final Logger LOGGER = LoggerFactory.getLogger(SegmentWithHllIndexCreateHelper.class);
- private static final String hllDeriveColumnSuffix = HllConstants.DEFAULT_HLL_DERIVE_COLUMN_SUFFIX;
private final String tableName;
private final File INDEX_DIR;
private final File inputAvro;
private final String timeColumnName;
private final TimeUnit timeUnit;
- private String segmentName = "starTreeSegment";
+ private String segmentName;
private Schema schema;
public SegmentWithHllIndexCreateHelper(String tableName, URL avroUrl, String timeColumnName, TimeUnit timeUnit,
@@ -139,7 +137,7 @@ public class SegmentWithHllIndexCreateHelper {
// range. For this test, we first need to fix the input avro data
// to have the time column values in allowed range. Until then, the check
// is explicitly disabled
- segmentGenConfig.setCheckTimeColumnValidityDuringGeneration(false);
+ segmentGenConfig.setSkipTimeValueCheck(true);
if (enableStarTree) {
setupStarTreeConfig(segmentGenConfig);
diff --git a/pinot-core/src/test/java/org/apache/pinot/core/util/CrcUtilsTest.java b/pinot-core/src/test/java/org/apache/pinot/core/util/CrcUtilsTest.java
index af8a36a..2d96fdd 100644
--- a/pinot-core/src/test/java/org/apache/pinot/core/util/CrcUtilsTest.java
+++ b/pinot-core/src/test/java/org/apache/pinot/core/util/CrcUtilsTest.java
@@ -82,13 +82,12 @@ public class CrcUtilsTest {
.getSegmentGenSpecWithSchemAndProjectedColumns(new File(filePath), INDEX_DIR, "daysSinceEpoch", TimeUnit.DAYS,
"testTable");
config.setSegmentNamePostfix("1");
- config.setTimeColumnName("daysSinceEpoch");
// The segment generation code in SegmentColumnarIndexCreator will throw
// exception if start and end time in time column are not in acceptable
// range. For this test, we first need to fix the input avro data
// to have the time column values in allowed range. Until then, the check
// is explicitly disabled
- config.setCheckTimeColumnValidityDuringGeneration(false);
+ config.setSkipTimeValueCheck(true);
final SegmentIndexCreationDriver driver = SegmentCreationDriverFactory.get(null);
driver.init(config);
driver.build();
diff --git a/pinot-core/src/test/java/org/apache/pinot/queries/BaseMultiValueQueriesTest.java b/pinot-core/src/test/java/org/apache/pinot/queries/BaseMultiValueQueriesTest.java
index 7820861..da22bf8 100644
--- a/pinot-core/src/test/java/org/apache/pinot/queries/BaseMultiValueQueriesTest.java
+++ b/pinot-core/src/test/java/org/apache/pinot/queries/BaseMultiValueQueriesTest.java
@@ -105,7 +105,7 @@ public abstract class BaseMultiValueQueriesTest extends BaseQueriesTest {
// range. For this test, we first need to fix the input avro data
// to have the time column values in allowed range. Until then, the check
// is explicitly disabled
- segmentGeneratorConfig.setCheckTimeColumnValidityDuringGeneration(false);
+ segmentGeneratorConfig.setSkipTimeValueCheck(true);
// Build the index segment.
SegmentIndexCreationDriver driver = new SegmentIndexCreationDriverImpl();
diff --git a/pinot-core/src/test/java/org/apache/pinot/queries/BaseSingleValueQueriesTest.java b/pinot-core/src/test/java/org/apache/pinot/queries/BaseSingleValueQueriesTest.java
index 860d61a..33fdca4 100644
--- a/pinot-core/src/test/java/org/apache/pinot/queries/BaseSingleValueQueriesTest.java
+++ b/pinot-core/src/test/java/org/apache/pinot/queries/BaseSingleValueQueriesTest.java
@@ -106,7 +106,7 @@ public abstract class BaseSingleValueQueriesTest extends BaseQueriesTest {
// range. For this test, we first need to fix the input avro data
// to have the time column values in allowed range. Until then, the check
// is explicitly disabled
- segmentGeneratorConfig.setCheckTimeColumnValidityDuringGeneration(false);
+ segmentGeneratorConfig.setSkipTimeValueCheck(true);
segmentGeneratorConfig
.setInvertedIndexCreationColumns(Arrays.asList("column6", "column7", "column11", "column17", "column18"));
diff --git a/pinot-core/src/test/java/org/apache/pinot/queries/DistinctQueriesTest.java b/pinot-core/src/test/java/org/apache/pinot/queries/DistinctQueriesTest.java
index d87a354..b1ae428 100644
--- a/pinot-core/src/test/java/org/apache/pinot/queries/DistinctQueriesTest.java
+++ b/pinot-core/src/test/java/org/apache/pinot/queries/DistinctQueriesTest.java
@@ -242,7 +242,6 @@ public class DistinctQueriesTest extends BaseQueriesTest {
segmentGeneratorConfig.setTableName(tableName);
segmentGeneratorConfig.setOutDir(INDEX_DIR.getAbsolutePath());
segmentGeneratorConfig.setSegmentName(segmentName);
- segmentGeneratorConfig.setCheckTimeColumnValidityDuringGeneration(false);
SegmentIndexCreationDriverImpl driver = new SegmentIndexCreationDriverImpl();
driver.init(segmentGeneratorConfig, recordReader);
diff --git a/pinot-core/src/test/java/org/apache/pinot/queries/FastHllQueriesTest.java b/pinot-core/src/test/java/org/apache/pinot/queries/FastHllQueriesTest.java
index 218ed07..ea7e283 100644
--- a/pinot-core/src/test/java/org/apache/pinot/queries/FastHllQueriesTest.java
+++ b/pinot-core/src/test/java/org/apache/pinot/queries/FastHllQueriesTest.java
@@ -240,7 +240,7 @@ public class FastHllQueriesTest extends BaseQueriesTest {
// range. For this test, we first need to fix the input avro data
// to have the time column values in allowed range. Until then, the check
// is explicitly disabled
- segmentGeneratorConfig.setCheckTimeColumnValidityDuringGeneration(false);
+ segmentGeneratorConfig.setSkipTimeValueCheck(true);
segmentGeneratorConfig
.setInvertedIndexCreationColumns(Arrays.asList("column6", "column7", "column11", "column17", "column18"));
if (hasPreGeneratedHllColumns) {
diff --git a/pinot-core/src/test/java/org/apache/pinot/segments/v1/creator/DictionariesTest.java b/pinot-core/src/test/java/org/apache/pinot/segments/v1/creator/DictionariesTest.java
index 5c35f03..b82382a 100644
--- a/pinot-core/src/test/java/org/apache/pinot/segments/v1/creator/DictionariesTest.java
+++ b/pinot-core/src/test/java/org/apache/pinot/segments/v1/creator/DictionariesTest.java
@@ -94,7 +94,7 @@ public class DictionariesTest {
// range. For this test, we first need to fix the input avro data
// to have the time column values in allowed range. Until then, the check
// is explicitly disabled
- config.setCheckTimeColumnValidityDuringGeneration(false);
+ config.setSkipTimeValueCheck(true);
final SegmentIndexCreationDriver driver = SegmentCreationDriverFactory.get(null);
driver.init(config);
driver.build();
diff --git a/pinot-core/src/test/java/org/apache/pinot/segments/v1/creator/IntArraysTest.java b/pinot-core/src/test/java/org/apache/pinot/segments/v1/creator/IntArraysTest.java
index b4ccc2d..7587f60 100644
--- a/pinot-core/src/test/java/org/apache/pinot/segments/v1/creator/IntArraysTest.java
+++ b/pinot-core/src/test/java/org/apache/pinot/segments/v1/creator/IntArraysTest.java
@@ -46,8 +46,7 @@ import org.testng.annotations.Test;
public class IntArraysTest {
private static final String AVRO_DATA = "data/test_data-mv.avro";
- private static File INDEX_DIR =
- new File(FileUtils.getTempDirectory() + File.separator + IntArraysTest.class.getName());
+ private static final File INDEX_DIR = new File(FileUtils.getTempDirectory(), "IntArraysTest");
@AfterClass
public static void cleanup() {
@@ -63,19 +62,17 @@ public class IntArraysTest {
FileUtils.deleteQuietly(INDEX_DIR);
}
-// System.out.println(INDEX_DIR.getAbsolutePath());
final SegmentIndexCreationDriver driver = SegmentCreationDriverFactory.get(null);
final SegmentGeneratorConfig config = SegmentTestUtils
.getSegmentGenSpecWithSchemAndProjectedColumns(new File(filePath), INDEX_DIR, "weeksSinceEpochSunday",
TimeUnit.DAYS, "test");
- config.setTimeColumnName("weeksSinceEpochSunday");
// The segment generation code in SegmentColumnarIndexCreator will throw
// exception if start and end time in time column are not in acceptable
// range. For this test, we first need to fix the input avro data
// to have the time column values in allowed range. Until then, the check
// is explicitly disabled
- config.setCheckTimeColumnValidityDuringGeneration(false);
+ config.setSkipTimeValueCheck(true);
driver.init(config);
driver.build();
diff --git a/pinot-tools/src/main/java/org/apache/pinot/tools/admin/command/OfflineSegmentIntervalCheckerCommand.java b/pinot-tools/src/main/java/org/apache/pinot/tools/admin/command/OfflineSegmentIntervalCheckerCommand.java
index 8a3096a..7e83724 100644
--- a/pinot-tools/src/main/java/org/apache/pinot/tools/admin/command/OfflineSegmentIntervalCheckerCommand.java
+++ b/pinot-tools/src/main/java/org/apache/pinot/tools/admin/command/OfflineSegmentIntervalCheckerCommand.java
@@ -32,6 +32,7 @@ import org.apache.pinot.common.config.TableConfig;
import org.apache.pinot.common.config.TableNameBuilder;
import org.apache.pinot.common.metadata.ZKMetadataProvider;
import org.apache.pinot.common.metadata.segment.OfflineSegmentZKMetadata;
+import org.apache.pinot.common.utils.time.TimeUtils;
import org.apache.pinot.controller.util.SegmentIntervalUtils;
import org.apache.pinot.tools.Command;
import org.joda.time.Interval;
@@ -140,7 +141,7 @@ public class OfflineSegmentIntervalCheckerCommand extends AbstractBaseAdminComma
if (SegmentIntervalUtils.eligibleForSegmentIntervalCheck(tableConfig.getValidationConfig())) {
for (OfflineSegmentZKMetadata offlineSegmentZKMetadata : offlineSegmentZKMetadataList) {
Interval timeInterval = offlineSegmentZKMetadata.getTimeInterval();
- if (!SegmentIntervalUtils.isValidInterval(timeInterval)) {
+ if (timeInterval == null || !TimeUtils.isValidTimeInterval(timeInterval)) {
segmentsWithInvalidIntervals.add(offlineSegmentZKMetadata.getSegmentName());
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org