Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Do not allow setting TableConfig in IndexLoadingConfig after construction #14098

Merged
merged 1 commit into from
Sep 27, 2024
Merged

Do not allow setting TableConfig in IndexLoadingConfig after construction #14098

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 4 additions & 11 deletions pinot-core/src/test/java/org/apache/pinot/queries/BaseJsonQueryTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
import java.io.File;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashSet;
import java.util.List;
import org.apache.commons.io.FileUtils;
import org.apache.pinot.common.response.broker.BrokerResponseNative;
Expand All @@ -35,7 +34,6 @@
import org.apache.pinot.spi.config.table.TableConfig;
import org.apache.pinot.spi.data.Schema;
import org.apache.pinot.spi.data.readers.GenericRow;
import org.apache.pinot.spi.utils.ReadMode;
import org.testng.annotations.BeforeClass;
import org.testng.annotations.DataProvider;

Expand Down Expand Up @@ -99,6 +97,7 @@ public void setUp()
FileUtils.deleteDirectory(indexDir);

TableConfig tableConfig = tableConfig();
Schema schema = schema();

List<GenericRow> records = new ArrayList<>(numRecords());
records.add(createRecord(1, 1, "daffy duck",
Expand Down Expand Up @@ -142,10 +141,8 @@ public void setUp()
"{\"name\": {\"first\": \"multi-dimensional-1\",\"last\": \"array\"},\"days\": 111}"));
records.add(createRecord(14, 14, "top level array", "[{\"i1\":1,\"i2\":2}, {\"i1\":3,\"i2\":4}]"));

List<String> jsonIndexColumns = new ArrayList<>();
jsonIndexColumns.add("jsonColumn");
tableConfig.getIndexingConfig().setJsonIndexColumns(jsonIndexColumns);
SegmentGeneratorConfig segmentGeneratorConfig = new SegmentGeneratorConfig(tableConfig, schema());
tableConfig.getIndexingConfig().setJsonIndexColumns(List.of("jsonColumn"));
SegmentGeneratorConfig segmentGeneratorConfig = new SegmentGeneratorConfig(tableConfig, schema);
segmentGeneratorConfig.setTableName(RAW_TABLE_NAME);
segmentGeneratorConfig.setSegmentName(SEGMENT_NAME);
segmentGeneratorConfig.setOutDir(indexDir.getPath());
Expand All @@ -154,11 +151,7 @@ public void setUp()
driver.init(segmentGeneratorConfig, new GenericRowRecordReader(records));
driver.build();

IndexLoadingConfig indexLoadingConfig = new IndexLoadingConfig();
indexLoadingConfig.setTableConfig(tableConfig);
indexLoadingConfig.setJsonIndexColumns(new HashSet<>(jsonIndexColumns));
indexLoadingConfig.setReadMode(ReadMode.mmap);

IndexLoadingConfig indexLoadingConfig = new IndexLoadingConfig(tableConfig, schema);
ImmutableSegment immutableSegment =
ImmutableSegmentLoader.load(new File(indexDir, SEGMENT_NAME), indexLoadingConfig);
_indexSegment = immutableSegment;
Expand Down
45 changes: 13 additions & 32 deletions pinot-core/src/test/java/org/apache/pinot/queries/ExplainPlanQueriesTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import java.net.URL;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -59,7 +58,7 @@
import org.apache.pinot.segment.spi.IndexSegment;
import org.apache.pinot.segment.spi.creator.SegmentGeneratorConfig;
import org.apache.pinot.spi.config.instance.InstanceDataManagerConfig;
import org.apache.pinot.spi.config.table.IndexingConfig;
import org.apache.pinot.spi.config.table.FieldConfig;
import org.apache.pinot.spi.config.table.TableConfig;
import org.apache.pinot.spi.config.table.TableType;
import org.apache.pinot.spi.data.FieldSpec.DataType;
Expand All @@ -68,7 +67,6 @@
import org.apache.pinot.spi.env.CommonsConfigurationUtils;
import org.apache.pinot.spi.env.PinotConfiguration;
import org.apache.pinot.spi.utils.CommonConstants.Broker;
import org.apache.pinot.spi.utils.ReadMode;
import org.apache.pinot.spi.utils.builder.TableConfigBuilder;
import org.apache.pinot.spi.utils.builder.TableNameBuilder;
import org.apache.pinot.sql.parsers.CalciteSqlCompiler;
Expand Down Expand Up @@ -131,16 +129,23 @@ public class ExplainPlanQueriesTest extends BaseQueriesTest {
.addMultiValueDimension(MV_COL1_NO_INDEX, DataType.INT)
.build();

private static final TableConfig TABLE_CONFIG = new TableConfigBuilder(TableType.OFFLINE).setTableName(RAW_TABLE_NAME)
.setNoDictionaryColumns(List.of(COL1_RAW, MV_COL1_RAW))
.setSortedColumn(COL1_SORTED_INDEX)
.setInvertedIndexColumns(List.of(COL1_INVERTED_INDEX, COL2_INVERTED_INDEX, COL3_INVERTED_INDEX))
.setRangeIndexColumns(List.of(COL1_RANGE_INDEX, COL2_RANGE_INDEX, COL3_RANGE_INDEX))
.setJsonIndexColumns(List.of(COL1_JSON_INDEX))
.setFieldConfigList(List.of(
new FieldConfig(COL1_TEXT_INDEX, FieldConfig.EncodingType.DICTIONARY, List.of(FieldConfig.IndexType.TEXT),
null, null)))
.build();

private static final DataSchema DATA_SCHEMA = new DataSchema(
new String[]{"Operator", "Operator_Id", "Parent_Id"},
new ColumnDataType[]{ColumnDataType.STRING, ColumnDataType.INT, ColumnDataType.INT}
);
//@formatter:on

private static final TableConfig TABLE_CONFIG =
new TableConfigBuilder(TableType.OFFLINE).setNoDictionaryColumns(List.of(COL1_RAW, MV_COL1_RAW))
.setTableName(RAW_TABLE_NAME).build();

private IndexSegment _indexSegment;
private List<IndexSegment> _indexSegments;
private List<String> _segmentNames;
Expand Down Expand Up @@ -198,22 +203,6 @@ GenericRow createMockRecord(int noIndexCol1, int noIndexCol2, int noIndexCol3, b

ImmutableSegment createImmutableSegment(List<GenericRow> records, String segmentName)
throws Exception {
IndexingConfig indexingConfig = TABLE_CONFIG.getIndexingConfig();

List<String> invertedIndexColumns = List.of(COL1_INVERTED_INDEX, COL2_INVERTED_INDEX, COL3_INVERTED_INDEX);
indexingConfig.setInvertedIndexColumns(invertedIndexColumns);

List<String> rangeIndexColumns = List.of(COL1_RANGE_INDEX, COL2_RANGE_INDEX, COL3_RANGE_INDEX);
indexingConfig.setRangeIndexColumns(rangeIndexColumns);

List<String> sortedIndexColumns = List.of(COL1_SORTED_INDEX);
indexingConfig.setSortedColumn(sortedIndexColumns);

List<String> jsonIndexColumns = List.of(COL1_JSON_INDEX);
indexingConfig.setJsonIndexColumns(jsonIndexColumns);

List<String> textIndexColumns = List.of(COL1_TEXT_INDEX);

File tableDataDir = new File(TEMP_DIR, OFFLINE_TABLE_NAME);
SegmentGeneratorConfig segmentGeneratorConfig = new SegmentGeneratorConfig(TABLE_CONFIG, SCHEMA);
segmentGeneratorConfig.setSegmentName(segmentName);
Expand All @@ -223,16 +212,8 @@ ImmutableSegment createImmutableSegment(List<GenericRow> records, String segment
driver.init(segmentGeneratorConfig, new GenericRowRecordReader(records));
driver.build();

IndexLoadingConfig indexLoadingConfig = new IndexLoadingConfig();
indexLoadingConfig.setTableConfig(TABLE_CONFIG);
indexLoadingConfig.setInvertedIndexColumns(new HashSet<>(invertedIndexColumns));
indexLoadingConfig.setRangeIndexColumns(new HashSet<>(rangeIndexColumns));
indexLoadingConfig.setJsonIndexColumns(new HashSet<>(jsonIndexColumns));
indexLoadingConfig.setTextIndexColumns(new HashSet<>(textIndexColumns));
indexLoadingConfig.setReadMode(ReadMode.mmap);

IndexLoadingConfig indexLoadingConfig = new IndexLoadingConfig(TABLE_CONFIG, SCHEMA);
_segmentNames.add(segmentName);

return ImmutableSegmentLoader.load(new File(tableDataDir, segmentName), indexLoadingConfig);
}

Expand Down
6 changes: 2 additions & 4 deletions pinot-core/src/test/java/org/apache/pinot/queries/ExprMinMaxTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,14 +45,13 @@
import org.apache.pinot.spi.utils.ReadMode;
import org.apache.pinot.spi.utils.builder.TableConfigBuilder;
import org.apache.pinot.sql.parsers.rewriter.QueryRewriterFactory;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.testng.Assert;
import org.testng.annotations.AfterClass;
import org.testng.annotations.BeforeClass;
import org.testng.annotations.Test;

import static org.apache.pinot.spi.utils.CommonConstants.RewriterConstants.*;
import static org.apache.pinot.spi.utils.CommonConstants.RewriterConstants.CHILD_AGGREGATION_NAME_PREFIX;
import static org.apache.pinot.spi.utils.CommonConstants.RewriterConstants.PARENT_AGGREGATION_NAME_PREFIX;
import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertNull;
import static org.testng.Assert.assertTrue;
Expand All @@ -63,7 +62,6 @@
* Queries test for exprmin/exprmax functions.
*/
public class ExprMinMaxTest extends BaseQueriesTest {
private static final Logger LOGGER = LoggerFactory.getLogger(ExprMinMaxTest.class);
private static final File INDEX_DIR = new File(FileUtils.getTempDirectory(), "ExprMinMaxTest");
private static final String RAW_TABLE_NAME = "testTable";
private static final String SEGMENT_NAME = "testSegment";
Expand Down
163 changes: 61 additions & 102 deletions ...ore/src/test/java/org/apache/pinot/queries/ForwardIndexDisabledMultiValueQueriesTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,8 @@

import java.io.File;
import java.net.URL;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.concurrent.TimeUnit;
import org.apache.commons.io.FileUtils;
import org.apache.pinot.common.response.broker.BrokerResponseNative;
import org.apache.pinot.common.response.broker.ResultTable;
Expand All @@ -39,20 +34,15 @@
import org.apache.pinot.segment.spi.IndexSegment;
import org.apache.pinot.segment.spi.creator.SegmentGeneratorConfig;
import org.apache.pinot.segment.spi.creator.SegmentIndexCreationDriver;
import org.apache.pinot.segment.spi.index.ForwardIndexConfig;
import org.apache.pinot.segment.spi.index.StandardIndexes;
import org.apache.pinot.spi.config.table.FieldConfig;
import org.apache.pinot.spi.config.table.IndexConfig;
import org.apache.pinot.spi.config.table.TableConfig;
import org.apache.pinot.spi.config.table.TableType;
import org.apache.pinot.spi.data.FieldSpec;
import org.apache.pinot.spi.data.FieldSpec.DataType;
import org.apache.pinot.spi.data.Schema;
import org.apache.pinot.spi.data.TimeGranularitySpec;
import org.apache.pinot.spi.utils.ReadMode;
import org.apache.pinot.spi.utils.builder.TableConfigBuilder;
import org.testng.Assert;
import org.testng.annotations.AfterMethod;
import org.testng.annotations.BeforeMethod;
import org.testng.annotations.AfterClass;
import org.testng.annotations.BeforeClass;
import org.testng.annotations.Test;

import static org.testng.Assert.*;
Expand All @@ -78,127 +68,96 @@
* </ul>
*/
public class ForwardIndexDisabledMultiValueQueriesTest extends BaseQueriesTest {
private static final File INDEX_DIR =
new File(FileUtils.getTempDirectory(), ForwardIndexDisabledMultiValueQueriesTest.class.getSimpleName());
private static final String AVRO_DATA = "data" + File.separator + "test_data-mv.avro";
private static final String SEGMENT_NAME_1 = "testTable_1756015688_1756015688";
private static final String SEGMENT_NAME_2 = "testTable_1756015689_1756015689";
private static final File INDEX_DIR = new File(FileUtils.getTempDirectory(),
"ForwardIndexDisabledMultiValueQueriesTest");
private static final String RAW_TABLE_NAME = "testTable";
private static final String SEGMENT_NAME = "testSegment";

private static final String SELECT_STAR_QUERY = "SELECT * FROM testTable";

// Hard-coded query filter.
//@formatter:off
// Hard-coded query filter
protected static final String FILTER = " WHERE column1 > 100000000"
+ " AND column2 BETWEEN 20000000 AND 1000000000"
+ " AND column3 <> 'w'"
+ " AND (column6 < 500000 OR column7 NOT IN (225, 407))"
+ " AND daysSinceEpoch = 1756015683";
//@formatter:on

private IndexSegment _indexSegment;
// Contains 2 identical index segments.
private List<IndexSegment> _indexSegments;

private TableConfig _tableConfig;
private List<String> _invertedIndexColumns;
private List<String> _forwardIndexDisabledColumns;

@BeforeMethod
public void buildSegment()
@BeforeClass
public void setUp()
throws Exception {
FileUtils.deleteQuietly(INDEX_DIR);

// Get resource file path.
//@formatter:off
Schema schema = new Schema.SchemaBuilder().setSchemaName(RAW_TABLE_NAME)
.addMetric("column1", DataType.INT)
.addMetric("column2", DataType.INT)
.addSingleValueDimension("column3", DataType.STRING)
.addSingleValueDimension("column5", DataType.STRING)
.addMultiValueDimension("column6", DataType.INT)
.addMultiValueDimension("column7", DataType.INT)
.addSingleValueDimension("column8", DataType.INT)
.addMetric("column9", DataType.INT)
.addMetric("column10", DataType.INT)
.addDateTime("daysSinceEpoch", DataType.INT, "EPOCH|DAYS", "1:DAYS")
.build();

List<FieldConfig> fieldConfigs = List.of(
new FieldConfig("column6", FieldConfig.EncodingType.DICTIONARY, List.of(), null,
Map.of(FieldConfig.FORWARD_INDEX_DISABLED, "true")),
new FieldConfig("column7", FieldConfig.EncodingType.DICTIONARY, List.of(), null,
Map.of(FieldConfig.FORWARD_INDEX_DISABLED, "true")));
TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE).setTableName(RAW_TABLE_NAME)
.setTimeColumnName("daysSinceEpoch")
.setNoDictionaryColumns(List.of("column5"))
.setInvertedIndexColumns(List.of("column3", "column6", "column7", "column8", "column9"))
.setCreateInvertedIndexDuringSegmentGeneration(true)
.setFieldConfigList(fieldConfigs)
.build();
//@formatter:on

URL resource = getClass().getClassLoader().getResource(AVRO_DATA);
assertNotNull(resource);
String filePath = resource.getFile();

// Build the segment schema.
Schema schema = new Schema.SchemaBuilder().setSchemaName("testTable").addMetric("column1", FieldSpec.DataType.INT)
.addMetric("column2", FieldSpec.DataType.INT).addSingleValueDimension("column3", FieldSpec.DataType.STRING)
.addSingleValueDimension("column5", FieldSpec.DataType.STRING)
.addMultiValueDimension("column6", FieldSpec.DataType.INT)
.addMultiValueDimension("column7", FieldSpec.DataType.INT)
.addSingleValueDimension("column8", FieldSpec.DataType.INT).addMetric("column9", FieldSpec.DataType.INT)
.addMetric("column10", FieldSpec.DataType.INT)
.addTime(new TimeGranularitySpec(FieldSpec.DataType.INT, TimeUnit.DAYS, "daysSinceEpoch"), null).build();

createSegment(filePath, SEGMENT_NAME_1, schema);
createSegment(filePath, SEGMENT_NAME_2, schema);

ImmutableSegment immutableSegment1 = loadSegmentWithMetadataChecks(SEGMENT_NAME_1);
ImmutableSegment immutableSegment2 = loadSegmentWithMetadataChecks(SEGMENT_NAME_2);

_indexSegment = immutableSegment1;
_indexSegments = Arrays.asList(immutableSegment1, immutableSegment2);
}

private void createSegment(String filePath, String segmentName, Schema schema)
throws Exception {
// Create field configs for the no forward index columns
List<FieldConfig> fieldConfigList = new ArrayList<>();
fieldConfigList.add(new FieldConfig("column6", FieldConfig.EncodingType.DICTIONARY, Collections.emptyList(), null,
Collections.singletonMap(FieldConfig.FORWARD_INDEX_DISABLED, Boolean.TRUE.toString())));
if (segmentName.equals(SEGMENT_NAME_1)) {
fieldConfigList.add(new FieldConfig("column7", FieldConfig.EncodingType.DICTIONARY, Collections.emptyList(),
null, Collections.singletonMap(FieldConfig.FORWARD_INDEX_DISABLED, Boolean.TRUE.toString())));
String avroFile = resource.getFile();

// Build table config based on segment 1 as it contains both columns under no forward index
_tableConfig = new TableConfigBuilder(TableType.OFFLINE).setNoDictionaryColumns(Arrays.asList("column5"))
.setTableName("testTable").setTimeColumnName("daysSinceEpoch").setFieldConfigList(fieldConfigList).build();
}

// Create the segment generator config.
SegmentGeneratorConfig segmentGeneratorConfig = new SegmentGeneratorConfig(_tableConfig, schema);
segmentGeneratorConfig.setInputFilePath(filePath);
segmentGeneratorConfig.setTableName("testTable");
segmentGeneratorConfig.setOutDir(INDEX_DIR.getAbsolutePath());
segmentGeneratorConfig.setSegmentName(segmentName);
_invertedIndexColumns = Arrays.asList("column3", "column6", "column7", "column8", "column9");
segmentGeneratorConfig.setIndexOn(StandardIndexes.inverted(), IndexConfig.ENABLED, _invertedIndexColumns);
_forwardIndexDisabledColumns = new ArrayList<>(Arrays.asList("column6", "column7"));
segmentGeneratorConfig.setIndexOn(StandardIndexes.forward(), ForwardIndexConfig.DISABLED,
_forwardIndexDisabledColumns);
// 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
segmentGeneratorConfig.setSkipTimeValueCheck(true);

// Build the index segment.
SegmentGeneratorConfig generatorConfig = new SegmentGeneratorConfig(tableConfig, schema);
generatorConfig.setInputFilePath(avroFile);
generatorConfig.setOutDir(INDEX_DIR.getAbsolutePath());
generatorConfig.setSegmentName(SEGMENT_NAME);
generatorConfig.setSkipTimeValueCheck(true);
SegmentIndexCreationDriver driver = new SegmentIndexCreationDriverImpl();
driver.init(segmentGeneratorConfig);
driver.init(generatorConfig);
driver.build();
}

private ImmutableSegment loadSegmentWithMetadataChecks(String segmentName)
throws Exception {
IndexLoadingConfig indexLoadingConfig = new IndexLoadingConfig();
indexLoadingConfig.setTableConfig(_tableConfig);
indexLoadingConfig.setInvertedIndexColumns(new HashSet<>(_invertedIndexColumns));
indexLoadingConfig.setForwardIndexDisabledColumns(new HashSet<>(_forwardIndexDisabledColumns));
indexLoadingConfig.setReadMode(ReadMode.heap);

ImmutableSegment immutableSegment = ImmutableSegmentLoader.load(new File(INDEX_DIR, segmentName),
indexLoadingConfig);

Map<String, ColumnMetadata> columnMetadataMap1 = immutableSegment.getSegmentMetadata().getColumnMetadataMap();
columnMetadataMap1.forEach((column, metadata) -> {
ImmutableSegment segment = ImmutableSegmentLoader.load(new File(INDEX_DIR, SEGMENT_NAME),
new IndexLoadingConfig(tableConfig, schema));
Map<String, ColumnMetadata> columnMetadataMap = segment.getSegmentMetadata().getColumnMetadataMap();
for (Map.Entry<String, ColumnMetadata> entry : columnMetadataMap.entrySet()) {
String column = entry.getKey();
ColumnMetadata metadata = entry.getValue();
if (column.equals("column6") || column.equals("column7")) {
assertTrue(metadata.hasDictionary());
assertFalse(metadata.isSingleValue());
assertNull(immutableSegment.getForwardIndex(column));
assertNull(segment.getForwardIndex(column));
} else {
assertNotNull(immutableSegment.getForwardIndex(column));
assertNotNull(segment.getForwardIndex(column));
}
});
}

return immutableSegment;
_indexSegment = segment;
_indexSegments = List.of(segment, segment);
}

@AfterMethod
public void deleteAndDestroySegment() {
@AfterClass
public void tearDown() {
_indexSegment.destroy();
FileUtils.deleteQuietly(INDEX_DIR);
_indexSegments.forEach((IndexSegment::destroy));
}

@Override
Expand Down
Loading
Loading