siddharthteotia commented on code in PR #9678:
URL: https://github.com/apache/pinot/pull/9678#discussion_r1012298557
##########
pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/loader/ForwardIndexHandlerTest.java:
##########
@@ -469,18 +553,131 @@ public void
testRewriteRawForwardIndexForMultipleColumns()
IndexLoadingConfig indexLoadingConfig = new IndexLoadingConfig(null,
tableConfig);
IndexCreatorProvider indexCreatorProvider =
IndexingOverrides.getIndexCreatorProvider();
- ForwardIndexHandler fwdIndexHandler = new
ForwardIndexHandler(existingSegmentMetadata, indexLoadingConfig);
+ ForwardIndexHandler fwdIndexHandler = new
ForwardIndexHandler(existingSegmentMetadata, indexLoadingConfig, null);
fwdIndexHandler.updateIndices(writer, indexCreatorProvider);
// Tear down before validation. Because columns.psf and index map cleanup
happens at segmentDirectory.close()
segmentLocalFSDirectory.close();
- validateIndexMap();
+ validateIndexMap(column1, false);
validateForwardIndex(column1, newCompressionType);
+ // Validate metadata properties. Nothing should change when a forwardIndex
is rewritten for compressionType
+ // change.
+ ColumnMetadata metadata =
existingSegmentMetadata.getColumnMetadataFor(column1);
+ validateMetadataProperties(column1, metadata.hasDictionary(),
metadata.getColumnMaxLength(),
+ metadata.getCardinality(), metadata.getTotalDocs(),
metadata.getDataType(), metadata.getFieldType(),
+ metadata.isSorted(), metadata.isSingleValue(),
metadata.getMaxNumberOfMultiValues(),
+ metadata.getTotalNumberOfEntries(), metadata.isAutoGenerated(),
metadata.getMinValue(), metadata.getMaxValue());
+
+ validateIndexMap(column2, false);
validateForwardIndex(column2, newCompressionType);
+ metadata = existingSegmentMetadata.getColumnMetadataFor(column2);
+ validateMetadataProperties(column2, metadata.hasDictionary(),
metadata.getColumnMaxLength(),
+ metadata.getCardinality(), metadata.getTotalDocs(),
metadata.getDataType(), metadata.getFieldType(),
+ metadata.isSorted(), metadata.isSingleValue(),
metadata.getMaxNumberOfMultiValues(),
+ metadata.getTotalNumberOfEntries(), metadata.isAutoGenerated(),
metadata.getMinValue(), metadata.getMaxValue());
+ }
+
+ @Test(priority = 3)
+ public void testEnableDictionaryForMultipleColumns()
+ throws Exception {
+ SegmentMetadataImpl existingSegmentMetadata = new
SegmentMetadataImpl(_segmentDirectory);
+ SegmentDirectory segmentLocalFSDirectory =
+ new SegmentLocalFSDirectory(_segmentDirectory,
existingSegmentMetadata, ReadMode.mmap);
+ SegmentDirectory.Writer writer = segmentLocalFSDirectory.createWriter();
+ IndexLoadingConfig indexLoadingConfig = new IndexLoadingConfig(null,
_tableConfig);
+
+ Random rand = new Random();
+ String col1 =
_noDictionaryColumns.get(rand.nextInt(_noDictionaryColumns.size()));
+ indexLoadingConfig.getNoDictionaryColumns().remove(col1);
+ String col2 =
_noDictionaryColumns.get(rand.nextInt(_noDictionaryColumns.size()));
+ indexLoadingConfig.getNoDictionaryColumns().remove(col2);
+
+ ForwardIndexHandler fwdIndexHandler = new
ForwardIndexHandler(existingSegmentMetadata, indexLoadingConfig, _schema);
+ IndexCreatorProvider indexCreatorProvider =
IndexingOverrides.getIndexCreatorProvider();
+ fwdIndexHandler.updateIndices(writer, indexCreatorProvider);
+
+ // Tear down before validation. Because columns.psf and index map cleanup
happens at segmentDirectory.close()
+ segmentLocalFSDirectory.close();
+
+ // Col1 validation.
+ validateIndexMap(col1, true);
+ validateForwardIndex(col1, null);
+ // In column metadata, nothing other than hasDictionary and
dictionaryElementSize should change.
+ int dictionaryElementSize = 0;
+ ColumnMetadata metadata =
existingSegmentMetadata.getColumnMetadataFor(col1);
+ FieldSpec.DataType dataType = metadata.getDataType();
+ if (dataType == FieldSpec.DataType.STRING || dataType ==
FieldSpec.DataType.BYTES) {
+ // This value is based on the rows in createTestData().
+ dictionaryElementSize = 7;
+ } else if (dataType == FieldSpec.DataType.BIG_DECIMAL) {
+ dictionaryElementSize = 11;
+ }
+ validateMetadataProperties(col1, true, dictionaryElementSize,
metadata.getCardinality(), metadata.getTotalDocs(),
+ dataType, metadata.getFieldType(), metadata.isSorted(),
metadata.isSingleValue(),
+ metadata.getMaxNumberOfMultiValues(),
metadata.getTotalNumberOfEntries(), metadata.isAutoGenerated(),
+ metadata.getMinValue(), metadata.getMaxValue());
+
+ // Col2 validation.
+ validateIndexMap(col2, true);
+ validateForwardIndex(col2, null);
+ // In column metadata, nothing other than hasDictionary and
dictionaryElementSize should change.
+ dictionaryElementSize = 0;
+ metadata = existingSegmentMetadata.getColumnMetadataFor(col2);
+ dataType = metadata.getDataType();
+ if (dataType == FieldSpec.DataType.STRING || dataType ==
FieldSpec.DataType.BYTES) {
+ // This value is based on the rows in createTestData().
+ dictionaryElementSize = 7;
+ } else if (dataType == FieldSpec.DataType.BIG_DECIMAL) {
+ dictionaryElementSize = 11;
+ }
+ validateMetadataProperties(col2, true, dictionaryElementSize,
metadata.getCardinality(), metadata.getTotalDocs(),
+ dataType, metadata.getFieldType(), metadata.isSorted(),
metadata.isSingleValue(),
+ metadata.getMaxNumberOfMultiValues(),
metadata.getTotalNumberOfEntries(), metadata.isAutoGenerated(),
+ metadata.getMinValue(), metadata.getMaxValue());
+ }
+
+ @Test(priority = 4)
+ public void testEnableDictionaryForSingleColumn()
+ throws Exception {
+ for (int i = 0; i < _noDictionaryColumns.size(); i++) {
+ SegmentMetadataImpl existingSegmentMetadata = new
SegmentMetadataImpl(_segmentDirectory);
+ SegmentDirectory segmentLocalFSDirectory =
+ new SegmentLocalFSDirectory(_segmentDirectory,
existingSegmentMetadata, ReadMode.mmap);
+ SegmentDirectory.Writer writer = segmentLocalFSDirectory.createWriter();
+
+ IndexLoadingConfig indexLoadingConfig = new IndexLoadingConfig(null,
_tableConfig);
+ String column = _noDictionaryColumns.get(i);
+ indexLoadingConfig.getNoDictionaryColumns().remove(column);
+ ForwardIndexHandler fwdIndexHandler =
+ new ForwardIndexHandler(existingSegmentMetadata, indexLoadingConfig,
_schema);
+ IndexCreatorProvider indexCreatorProvider =
IndexingOverrides.getIndexCreatorProvider();
+ fwdIndexHandler.updateIndices(writer, indexCreatorProvider);
+
+ // Tear down before validation. Because columns.psf and index map
cleanup happens at segmentDirectory.close()
+ segmentLocalFSDirectory.close();
+
+ validateIndexMap(column, true);
+ validateForwardIndex(column, null);
Review Comment:
Can we also check if ColumnIndexEntry exists / dictionary index buffer
exists by calling `hasIndex()` ?
##########
pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/loader/ForwardIndexHandlerTest.java:
##########
@@ -469,18 +553,131 @@ public void
testRewriteRawForwardIndexForMultipleColumns()
IndexLoadingConfig indexLoadingConfig = new IndexLoadingConfig(null,
tableConfig);
IndexCreatorProvider indexCreatorProvider =
IndexingOverrides.getIndexCreatorProvider();
- ForwardIndexHandler fwdIndexHandler = new
ForwardIndexHandler(existingSegmentMetadata, indexLoadingConfig);
+ ForwardIndexHandler fwdIndexHandler = new
ForwardIndexHandler(existingSegmentMetadata, indexLoadingConfig, null);
fwdIndexHandler.updateIndices(writer, indexCreatorProvider);
// Tear down before validation. Because columns.psf and index map cleanup
happens at segmentDirectory.close()
segmentLocalFSDirectory.close();
- validateIndexMap();
+ validateIndexMap(column1, false);
validateForwardIndex(column1, newCompressionType);
+ // Validate metadata properties. Nothing should change when a forwardIndex
is rewritten for compressionType
+ // change.
+ ColumnMetadata metadata =
existingSegmentMetadata.getColumnMetadataFor(column1);
+ validateMetadataProperties(column1, metadata.hasDictionary(),
metadata.getColumnMaxLength(),
+ metadata.getCardinality(), metadata.getTotalDocs(),
metadata.getDataType(), metadata.getFieldType(),
+ metadata.isSorted(), metadata.isSingleValue(),
metadata.getMaxNumberOfMultiValues(),
+ metadata.getTotalNumberOfEntries(), metadata.isAutoGenerated(),
metadata.getMinValue(), metadata.getMaxValue());
+
+ validateIndexMap(column2, false);
validateForwardIndex(column2, newCompressionType);
+ metadata = existingSegmentMetadata.getColumnMetadataFor(column2);
+ validateMetadataProperties(column2, metadata.hasDictionary(),
metadata.getColumnMaxLength(),
+ metadata.getCardinality(), metadata.getTotalDocs(),
metadata.getDataType(), metadata.getFieldType(),
+ metadata.isSorted(), metadata.isSingleValue(),
metadata.getMaxNumberOfMultiValues(),
+ metadata.getTotalNumberOfEntries(), metadata.isAutoGenerated(),
metadata.getMinValue(), metadata.getMaxValue());
+ }
+
+ @Test(priority = 3)
+ public void testEnableDictionaryForMultipleColumns()
+ throws Exception {
+ SegmentMetadataImpl existingSegmentMetadata = new
SegmentMetadataImpl(_segmentDirectory);
+ SegmentDirectory segmentLocalFSDirectory =
+ new SegmentLocalFSDirectory(_segmentDirectory,
existingSegmentMetadata, ReadMode.mmap);
+ SegmentDirectory.Writer writer = segmentLocalFSDirectory.createWriter();
+ IndexLoadingConfig indexLoadingConfig = new IndexLoadingConfig(null,
_tableConfig);
+
+ Random rand = new Random();
+ String col1 =
_noDictionaryColumns.get(rand.nextInt(_noDictionaryColumns.size()));
+ indexLoadingConfig.getNoDictionaryColumns().remove(col1);
+ String col2 =
_noDictionaryColumns.get(rand.nextInt(_noDictionaryColumns.size()));
+ indexLoadingConfig.getNoDictionaryColumns().remove(col2);
+
+ ForwardIndexHandler fwdIndexHandler = new
ForwardIndexHandler(existingSegmentMetadata, indexLoadingConfig, _schema);
+ IndexCreatorProvider indexCreatorProvider =
IndexingOverrides.getIndexCreatorProvider();
+ fwdIndexHandler.updateIndices(writer, indexCreatorProvider);
+
+ // Tear down before validation. Because columns.psf and index map cleanup
happens at segmentDirectory.close()
+ segmentLocalFSDirectory.close();
+
+ // Col1 validation.
+ validateIndexMap(col1, true);
+ validateForwardIndex(col1, null);
+ // In column metadata, nothing other than hasDictionary and
dictionaryElementSize should change.
+ int dictionaryElementSize = 0;
+ ColumnMetadata metadata =
existingSegmentMetadata.getColumnMetadataFor(col1);
+ FieldSpec.DataType dataType = metadata.getDataType();
+ if (dataType == FieldSpec.DataType.STRING || dataType ==
FieldSpec.DataType.BYTES) {
+ // This value is based on the rows in createTestData().
+ dictionaryElementSize = 7;
+ } else if (dataType == FieldSpec.DataType.BIG_DECIMAL) {
+ dictionaryElementSize = 11;
+ }
+ validateMetadataProperties(col1, true, dictionaryElementSize,
metadata.getCardinality(), metadata.getTotalDocs(),
+ dataType, metadata.getFieldType(), metadata.isSorted(),
metadata.isSingleValue(),
+ metadata.getMaxNumberOfMultiValues(),
metadata.getTotalNumberOfEntries(), metadata.isAutoGenerated(),
+ metadata.getMinValue(), metadata.getMaxValue());
+
+ // Col2 validation.
+ validateIndexMap(col2, true);
+ validateForwardIndex(col2, null);
+ // In column metadata, nothing other than hasDictionary and
dictionaryElementSize should change.
+ dictionaryElementSize = 0;
+ metadata = existingSegmentMetadata.getColumnMetadataFor(col2);
+ dataType = metadata.getDataType();
+ if (dataType == FieldSpec.DataType.STRING || dataType ==
FieldSpec.DataType.BYTES) {
+ // This value is based on the rows in createTestData().
+ dictionaryElementSize = 7;
+ } else if (dataType == FieldSpec.DataType.BIG_DECIMAL) {
+ dictionaryElementSize = 11;
+ }
+ validateMetadataProperties(col2, true, dictionaryElementSize,
metadata.getCardinality(), metadata.getTotalDocs(),
+ dataType, metadata.getFieldType(), metadata.isSorted(),
metadata.isSingleValue(),
+ metadata.getMaxNumberOfMultiValues(),
metadata.getTotalNumberOfEntries(), metadata.isAutoGenerated(),
+ metadata.getMinValue(), metadata.getMaxValue());
+ }
+
+ @Test(priority = 4)
+ public void testEnableDictionaryForSingleColumn()
+ throws Exception {
+ for (int i = 0; i < _noDictionaryColumns.size(); i++) {
+ SegmentMetadataImpl existingSegmentMetadata = new
SegmentMetadataImpl(_segmentDirectory);
+ SegmentDirectory segmentLocalFSDirectory =
+ new SegmentLocalFSDirectory(_segmentDirectory,
existingSegmentMetadata, ReadMode.mmap);
+ SegmentDirectory.Writer writer = segmentLocalFSDirectory.createWriter();
+
+ IndexLoadingConfig indexLoadingConfig = new IndexLoadingConfig(null,
_tableConfig);
+ String column = _noDictionaryColumns.get(i);
+ indexLoadingConfig.getNoDictionaryColumns().remove(column);
+ ForwardIndexHandler fwdIndexHandler =
+ new ForwardIndexHandler(existingSegmentMetadata, indexLoadingConfig,
_schema);
+ IndexCreatorProvider indexCreatorProvider =
IndexingOverrides.getIndexCreatorProvider();
+ fwdIndexHandler.updateIndices(writer, indexCreatorProvider);
+
+ // Tear down before validation. Because columns.psf and index map
cleanup happens at segmentDirectory.close()
+ segmentLocalFSDirectory.close();
+
+ validateIndexMap(column, true);
+ validateForwardIndex(column, null);
Review Comment:
Can we also check if `ColumnIndexEntry` exists / dictionary index buffer
exists by calling `hasIndex()` ?
--
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]
