Skip to content

Commit 6002f46

Browse files
Jackie-JiangJackie-Jiang
committed
Do not allow setting TableConfig in IndexLoadingConfig after construction
1 parent 7668b21 commit 6002f46

File tree

14 files changed

+549
-918
lines changed

14 files changed

+549
-918
lines changed

‎pinot-core/src/test/java/org/apache/pinot/queries/BaseJsonQueryTest.java‎

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121
import java.io.File;
2222
import java.util.ArrayList;
2323
import java.util.Arrays;
24-
import java.util.HashSet;
2524
import java.util.List;
2625
import org.apache.commons.io.FileUtils;
2726
import org.apache.pinot.common.response.broker.BrokerResponseNative;
@@ -35,7 +34,6 @@
3534
import org.apache.pinot.spi.config.table.TableConfig;
3635
import org.apache.pinot.spi.data.Schema;
3736
import org.apache.pinot.spi.data.readers.GenericRow;
38-
import org.apache.pinot.spi.utils.ReadMode;
3937
import org.testng.annotations.BeforeClass;
4038
import org.testng.annotations.DataProvider;
4139

@@ -99,6 +97,7 @@ public void setUp()
9997
FileUtils.deleteDirectory(indexDir);
10098

10199
TableConfig tableConfig = tableConfig();
100+
Schema schema = schema();
102101

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

145-
List<String> jsonIndexColumns = new ArrayList<>();
146-
jsonIndexColumns.add("jsonColumn");
147-
tableConfig.getIndexingConfig().setJsonIndexColumns(jsonIndexColumns);
148-
SegmentGeneratorConfig segmentGeneratorConfig = new SegmentGeneratorConfig(tableConfig, schema());
144+
tableConfig.getIndexingConfig().setJsonIndexColumns(List.of("jsonColumn"));
145+
SegmentGeneratorConfig segmentGeneratorConfig = new SegmentGeneratorConfig(tableConfig, schema);
149146
segmentGeneratorConfig.setTableName(RAW_TABLE_NAME);
150147
segmentGeneratorConfig.setSegmentName(SEGMENT_NAME);
151148
segmentGeneratorConfig.setOutDir(indexDir.getPath());
@@ -154,11 +151,7 @@ public void setUp()
154151
driver.init(segmentGeneratorConfig, new GenericRowRecordReader(records));
155152
driver.build();
156153

157-
IndexLoadingConfig indexLoadingConfig = new IndexLoadingConfig();
158-
indexLoadingConfig.setTableConfig(tableConfig);
159-
indexLoadingConfig.setJsonIndexColumns(new HashSet<>(jsonIndexColumns));
160-
indexLoadingConfig.setReadMode(ReadMode.mmap);
161-
154+
IndexLoadingConfig indexLoadingConfig = new IndexLoadingConfig(tableConfig, schema);
162155
ImmutableSegment immutableSegment =
163156
ImmutableSegmentLoader.load(new File(indexDir, SEGMENT_NAME), indexLoadingConfig);
164157
_indexSegment = immutableSegment;

‎pinot-core/src/test/java/org/apache/pinot/queries/ExplainPlanQueriesTest.java‎

Lines changed: 13 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222
import java.net.URL;
2323
import java.util.ArrayList;
2424
import java.util.HashMap;
25-
import java.util.HashSet;
2625
import java.util.Iterator;
2726
import java.util.List;
2827
import java.util.Map;
@@ -59,7 +58,7 @@
5958
import org.apache.pinot.segment.spi.IndexSegment;
6059
import org.apache.pinot.segment.spi.creator.SegmentGeneratorConfig;
6160
import org.apache.pinot.spi.config.instance.InstanceDataManagerConfig;
62-
import org.apache.pinot.spi.config.table.IndexingConfig;
61+
import org.apache.pinot.spi.config.table.FieldConfig;
6362
import org.apache.pinot.spi.config.table.TableConfig;
6463
import org.apache.pinot.spi.config.table.TableType;
6564
import org.apache.pinot.spi.data.FieldSpec.DataType;
@@ -68,7 +67,6 @@
6867
import org.apache.pinot.spi.env.CommonsConfigurationUtils;
6968
import org.apache.pinot.spi.env.PinotConfiguration;
7069
import org.apache.pinot.spi.utils.CommonConstants.Broker;
71-
import org.apache.pinot.spi.utils.ReadMode;
7270
import org.apache.pinot.spi.utils.builder.TableConfigBuilder;
7371
import org.apache.pinot.spi.utils.builder.TableNameBuilder;
7472
import org.apache.pinot.sql.parsers.CalciteSqlCompiler;
@@ -131,16 +129,23 @@ public class ExplainPlanQueriesTest extends BaseQueriesTest {
131129
.addMultiValueDimension(MV_COL1_NO_INDEX, DataType.INT)
132130
.build();
133131

132+
private static final TableConfig TABLE_CONFIG = new TableConfigBuilder(TableType.OFFLINE).setTableName(RAW_TABLE_NAME)
133+
.setNoDictionaryColumns(List.of(COL1_RAW, MV_COL1_RAW))
134+
.setSortedColumn(COL1_SORTED_INDEX)
135+
.setInvertedIndexColumns(List.of(COL1_INVERTED_INDEX, COL2_INVERTED_INDEX, COL3_INVERTED_INDEX))
136+
.setRangeIndexColumns(List.of(COL1_RANGE_INDEX, COL2_RANGE_INDEX, COL3_RANGE_INDEX))
137+
.setJsonIndexColumns(List.of(COL1_JSON_INDEX))
138+
.setFieldConfigList(List.of(
139+
new FieldConfig(COL1_TEXT_INDEX, FieldConfig.EncodingType.DICTIONARY, List.of(FieldConfig.IndexType.TEXT),
140+
null, null)))
141+
.build();
142+
134143
private static final DataSchema DATA_SCHEMA = new DataSchema(
135144
new String[]{"Operator", "Operator_Id", "Parent_Id"},
136145
new ColumnDataType[]{ColumnDataType.STRING, ColumnDataType.INT, ColumnDataType.INT}
137146
);
138147
//@formatter:on
139148

140-
private static final TableConfig TABLE_CONFIG =
141-
new TableConfigBuilder(TableType.OFFLINE).setNoDictionaryColumns(List.of(COL1_RAW, MV_COL1_RAW))
142-
.setTableName(RAW_TABLE_NAME).build();
143-
144149
private IndexSegment _indexSegment;
145150
private List<IndexSegment> _indexSegments;
146151
private List<String> _segmentNames;
@@ -198,22 +203,6 @@ GenericRow createMockRecord(int noIndexCol1, int noIndexCol2, int noIndexCol3, b
198203

199204
ImmutableSegment createImmutableSegment(List<GenericRow> records, String segmentName)
200205
throws Exception {
201-
IndexingConfig indexingConfig = TABLE_CONFIG.getIndexingConfig();
202-
203-
List<String> invertedIndexColumns = List.of(COL1_INVERTED_INDEX, COL2_INVERTED_INDEX, COL3_INVERTED_INDEX);
204-
indexingConfig.setInvertedIndexColumns(invertedIndexColumns);
205-
206-
List<String> rangeIndexColumns = List.of(COL1_RANGE_INDEX, COL2_RANGE_INDEX, COL3_RANGE_INDEX);
207-
indexingConfig.setRangeIndexColumns(rangeIndexColumns);
208-
209-
List<String> sortedIndexColumns = List.of(COL1_SORTED_INDEX);
210-
indexingConfig.setSortedColumn(sortedIndexColumns);
211-
212-
List<String> jsonIndexColumns = List.of(COL1_JSON_INDEX);
213-
indexingConfig.setJsonIndexColumns(jsonIndexColumns);
214-
215-
List<String> textIndexColumns = List.of(COL1_TEXT_INDEX);
216-
217206
File tableDataDir = new File(TEMP_DIR, OFFLINE_TABLE_NAME);
218207
SegmentGeneratorConfig segmentGeneratorConfig = new SegmentGeneratorConfig(TABLE_CONFIG, SCHEMA);
219208
segmentGeneratorConfig.setSegmentName(segmentName);
@@ -223,16 +212,8 @@ ImmutableSegment createImmutableSegment(List<GenericRow> records, String segment
223212
driver.init(segmentGeneratorConfig, new GenericRowRecordReader(records));
224213
driver.build();
225214

226-
IndexLoadingConfig indexLoadingConfig = new IndexLoadingConfig();
227-
indexLoadingConfig.setTableConfig(TABLE_CONFIG);
228-
indexLoadingConfig.setInvertedIndexColumns(new HashSet<>(invertedIndexColumns));
229-
indexLoadingConfig.setRangeIndexColumns(new HashSet<>(rangeIndexColumns));
230-
indexLoadingConfig.setJsonIndexColumns(new HashSet<>(jsonIndexColumns));
231-
indexLoadingConfig.setTextIndexColumns(new HashSet<>(textIndexColumns));
232-
indexLoadingConfig.setReadMode(ReadMode.mmap);
233-
215+
IndexLoadingConfig indexLoadingConfig = new IndexLoadingConfig(TABLE_CONFIG, SCHEMA);
234216
_segmentNames.add(segmentName);
235-
236217
return ImmutableSegmentLoader.load(new File(tableDataDir, segmentName), indexLoadingConfig);
237218
}
238219

‎pinot-core/src/test/java/org/apache/pinot/queries/ExprMinMaxTest.java‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,6 @@
6363
* Queries test for exprmin/exprmax functions.
6464
*/
6565
public class ExprMinMaxTest extends BaseQueriesTest {
66-
private static final Logger LOGGER = LoggerFactory.getLogger(ExprMinMaxTest.class);
6766
private static final File INDEX_DIR = new File(FileUtils.getTempDirectory(), "ExprMinMaxTest");
6867
private static final String RAW_TABLE_NAME = "testTable";
6968
private static final String SEGMENT_NAME = "testSegment";

‎pinot-core/src/test/java/org/apache/pinot/queries/ForwardIndexDisabledMultiValueQueriesTest.java‎

Lines changed: 61 additions & 102 deletions
Original file line numberDiff line numberDiff line change
@@ -20,13 +20,8 @@
2020

2121
import java.io.File;
2222
import java.net.URL;
23-
import java.util.ArrayList;
24-
import java.util.Arrays;
25-
import java.util.Collections;
26-
import java.util.HashSet;
2723
import java.util.List;
2824
import java.util.Map;
29-
import java.util.concurrent.TimeUnit;
3025
import org.apache.commons.io.FileUtils;
3126
import org.apache.pinot.common.response.broker.BrokerResponseNative;
3227
import org.apache.pinot.common.response.broker.ResultTable;
@@ -39,20 +34,15 @@
3934
import org.apache.pinot.segment.spi.IndexSegment;
4035
import org.apache.pinot.segment.spi.creator.SegmentGeneratorConfig;
4136
import org.apache.pinot.segment.spi.creator.SegmentIndexCreationDriver;
42-
import org.apache.pinot.segment.spi.index.ForwardIndexConfig;
43-
import org.apache.pinot.segment.spi.index.StandardIndexes;
4437
import org.apache.pinot.spi.config.table.FieldConfig;
45-
import org.apache.pinot.spi.config.table.IndexConfig;
4638
import org.apache.pinot.spi.config.table.TableConfig;
4739
import org.apache.pinot.spi.config.table.TableType;
48-
import org.apache.pinot.spi.data.FieldSpec;
40+
import org.apache.pinot.spi.data.FieldSpec.DataType;
4941
import org.apache.pinot.spi.data.Schema;
50-
import org.apache.pinot.spi.data.TimeGranularitySpec;
51-
import org.apache.pinot.spi.utils.ReadMode;
5242
import org.apache.pinot.spi.utils.builder.TableConfigBuilder;
5343
import org.testng.Assert;
54-
import org.testng.annotations.AfterMethod;
55-
import org.testng.annotations.BeforeMethod;
44+
import org.testng.annotations.AfterClass;
45+
import org.testng.annotations.BeforeClass;
5646
import org.testng.annotations.Test;
5747

5848
import static org.testng.Assert.*;
@@ -78,127 +68,96 @@
7868
* </ul>
7969
*/
8070
public class ForwardIndexDisabledMultiValueQueriesTest extends BaseQueriesTest {
71+
private static final File INDEX_DIR =
72+
new File(FileUtils.getTempDirectory(), ForwardIndexDisabledMultiValueQueriesTest.class.getSimpleName());
8173
private static final String AVRO_DATA = "data" + File.separator + "test_data-mv.avro";
82-
private static final String SEGMENT_NAME_1 = "testTable_1756015688_1756015688";
83-
private static final String SEGMENT_NAME_2 = "testTable_1756015689_1756015689";
84-
private static final File INDEX_DIR = new File(FileUtils.getTempDirectory(),
85-
"ForwardIndexDisabledMultiValueQueriesTest");
74+
private static final String RAW_TABLE_NAME = "testTable";
75+
private static final String SEGMENT_NAME = "testSegment";
8676

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

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

9688
private IndexSegment _indexSegment;
9789
// Contains 2 identical index segments.
9890
private List<IndexSegment> _indexSegments;
9991

100-
private TableConfig _tableConfig;
101-
private List<String> _invertedIndexColumns;
102-
private List<String> _forwardIndexDisabledColumns;
103-
104-
@BeforeMethod
105-
public void buildSegment()
92+
@BeforeClass
93+
public void setUp()
10694
throws Exception {
10795
FileUtils.deleteQuietly(INDEX_DIR);
10896

109-
// Get resource file path.
97+
//@formatter:off
98+
Schema schema = new Schema.SchemaBuilder().setSchemaName(RAW_TABLE_NAME)
99+
.addMetric("column1", DataType.INT)
100+
.addMetric("column2", DataType.INT)
101+
.addSingleValueDimension("column3", DataType.STRING)
102+
.addSingleValueDimension("column5", DataType.STRING)
103+
.addMultiValueDimension("column6", DataType.INT)
104+
.addMultiValueDimension("column7", DataType.INT)
105+
.addSingleValueDimension("column8", DataType.INT)
106+
.addMetric("column9", DataType.INT)
107+
.addMetric("column10", DataType.INT)
108+
.addDateTime("daysSinceEpoch", DataType.INT, "EPOCH|DAYS", "1:DAYS")
109+
.build();
110+
111+
List<FieldConfig> fieldConfigs = List.of(
112+
new FieldConfig("column6", FieldConfig.EncodingType.DICTIONARY, List.of(), null,
113+
Map.of(FieldConfig.FORWARD_INDEX_DISABLED, "true")),
114+
new FieldConfig("column7", FieldConfig.EncodingType.DICTIONARY, List.of(), null,
115+
Map.of(FieldConfig.FORWARD_INDEX_DISABLED, "true")));
116+
TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE).setTableName(RAW_TABLE_NAME)
117+
.setTimeColumnName("daysSinceEpoch")
118+
.setNoDictionaryColumns(List.of("column5"))
119+
.setInvertedIndexColumns(List.of("column3", "column6", "column7", "column8", "column9"))
120+
.setCreateInvertedIndexDuringSegmentGeneration(true)
121+
.setFieldConfigList(fieldConfigs)
122+
.build();
123+
//@formatter:on
124+
110125
URL resource = getClass().getClassLoader().getResource(AVRO_DATA);
111126
assertNotNull(resource);
112-
String filePath = resource.getFile();
113-
114-
// Build the segment schema.
115-
Schema schema = new Schema.SchemaBuilder().setSchemaName("testTable").addMetric("column1", FieldSpec.DataType.INT)
116-
.addMetric("column2", FieldSpec.DataType.INT).addSingleValueDimension("column3", FieldSpec.DataType.STRING)
117-
.addSingleValueDimension("column5", FieldSpec.DataType.STRING)
118-
.addMultiValueDimension("column6", FieldSpec.DataType.INT)
119-
.addMultiValueDimension("column7", FieldSpec.DataType.INT)
120-
.addSingleValueDimension("column8", FieldSpec.DataType.INT).addMetric("column9", FieldSpec.DataType.INT)
121-
.addMetric("column10", FieldSpec.DataType.INT)
122-
.addTime(new TimeGranularitySpec(FieldSpec.DataType.INT, TimeUnit.DAYS, "daysSinceEpoch"), null).build();
123-
124-
createSegment(filePath, SEGMENT_NAME_1, schema);
125-
createSegment(filePath, SEGMENT_NAME_2, schema);
126-
127-
ImmutableSegment immutableSegment1 = loadSegmentWithMetadataChecks(SEGMENT_NAME_1);
128-
ImmutableSegment immutableSegment2 = loadSegmentWithMetadataChecks(SEGMENT_NAME_2);
129-
130-
_indexSegment = immutableSegment1;
131-
_indexSegments = Arrays.asList(immutableSegment1, immutableSegment2);
132-
}
133-
134-
private void createSegment(String filePath, String segmentName, Schema schema)
135-
throws Exception {
136-
// Create field configs for the no forward index columns
137-
List<FieldConfig> fieldConfigList = new ArrayList<>();
138-
fieldConfigList.add(new FieldConfig("column6", FieldConfig.EncodingType.DICTIONARY, Collections.emptyList(), null,
139-
Collections.singletonMap(FieldConfig.FORWARD_INDEX_DISABLED, Boolean.TRUE.toString())));
140-
if (segmentName.equals(SEGMENT_NAME_1)) {
141-
fieldConfigList.add(new FieldConfig("column7", FieldConfig.EncodingType.DICTIONARY, Collections.emptyList(),
142-
null, Collections.singletonMap(FieldConfig.FORWARD_INDEX_DISABLED, Boolean.TRUE.toString())));
127+
String avroFile = resource.getFile();
143128

144-
// Build table config based on segment 1 as it contains both columns under no forward index
145-
_tableConfig = new TableConfigBuilder(TableType.OFFLINE).setNoDictionaryColumns(Arrays.asList("column5"))
146-
.setTableName("testTable").setTimeColumnName("daysSinceEpoch").setFieldConfigList(fieldConfigList).build();
147-
}
148-
149-
// Create the segment generator config.
150-
SegmentGeneratorConfig segmentGeneratorConfig = new SegmentGeneratorConfig(_tableConfig, schema);
151-
segmentGeneratorConfig.setInputFilePath(filePath);
152-
segmentGeneratorConfig.setTableName("testTable");
153-
segmentGeneratorConfig.setOutDir(INDEX_DIR.getAbsolutePath());
154-
segmentGeneratorConfig.setSegmentName(segmentName);
155-
_invertedIndexColumns = Arrays.asList("column3", "column6", "column7", "column8", "column9");
156-
segmentGeneratorConfig.setIndexOn(StandardIndexes.inverted(), IndexConfig.ENABLED, _invertedIndexColumns);
157-
_forwardIndexDisabledColumns = new ArrayList<>(Arrays.asList("column6", "column7"));
158-
segmentGeneratorConfig.setIndexOn(StandardIndexes.forward(), ForwardIndexConfig.DISABLED,
159-
_forwardIndexDisabledColumns);
160-
// The segment generation code in SegmentColumnarIndexCreator will throw
161-
// exception if start and end time in time column are not in acceptable
162-
// range. For this test, we first need to fix the input avro data
163-
// to have the time column values in allowed range. Until then, the check
164-
// is explicitly disabled
165-
segmentGeneratorConfig.setSkipTimeValueCheck(true);
166-
167-
// Build the index segment.
129+
SegmentGeneratorConfig generatorConfig = new SegmentGeneratorConfig(tableConfig, schema);
130+
generatorConfig.setInputFilePath(avroFile);
131+
generatorConfig.setOutDir(INDEX_DIR.getAbsolutePath());
132+
generatorConfig.setSegmentName(SEGMENT_NAME);
133+
generatorConfig.setSkipTimeValueCheck(true);
168134
SegmentIndexCreationDriver driver = new SegmentIndexCreationDriverImpl();
169-
driver.init(segmentGeneratorConfig);
135+
driver.init(generatorConfig);
170136
driver.build();
171-
}
172137

173-
private ImmutableSegment loadSegmentWithMetadataChecks(String segmentName)
174-
throws Exception {
175-
IndexLoadingConfig indexLoadingConfig = new IndexLoadingConfig();
176-
indexLoadingConfig.setTableConfig(_tableConfig);
177-
indexLoadingConfig.setInvertedIndexColumns(new HashSet<>(_invertedIndexColumns));
178-
indexLoadingConfig.setForwardIndexDisabledColumns(new HashSet<>(_forwardIndexDisabledColumns));
179-
indexLoadingConfig.setReadMode(ReadMode.heap);
180-
181-
ImmutableSegment immutableSegment = ImmutableSegmentLoader.load(new File(INDEX_DIR, segmentName),
182-
indexLoadingConfig);
183-
184-
Map<String, ColumnMetadata> columnMetadataMap1 = immutableSegment.getSegmentMetadata().getColumnMetadataMap();
185-
columnMetadataMap1.forEach((column, metadata) -> {
138+
ImmutableSegment segment = ImmutableSegmentLoader.load(new File(INDEX_DIR, SEGMENT_NAME),
139+
new IndexLoadingConfig(tableConfig, schema));
140+
Map<String, ColumnMetadata> columnMetadataMap = segment.getSegmentMetadata().getColumnMetadataMap();
141+
for (Map.Entry<String, ColumnMetadata> entry : columnMetadataMap.entrySet()) {
142+
String column = entry.getKey();
143+
ColumnMetadata metadata = entry.getValue();
186144
if (column.equals("column6") || column.equals("column7")) {
187145
assertTrue(metadata.hasDictionary());
188146
assertFalse(metadata.isSingleValue());
189-
assertNull(immutableSegment.getForwardIndex(column));
147+
assertNull(segment.getForwardIndex(column));
190148
} else {
191-
assertNotNull(immutableSegment.getForwardIndex(column));
149+
assertNotNull(segment.getForwardIndex(column));
192150
}
193-
});
151+
}
194152

195-
return immutableSegment;
153+
_indexSegment = segment;
154+
_indexSegments = List.of(segment, segment);
196155
}
197156

198-
@AfterMethod
199-
public void deleteAndDestroySegment() {
157+
@AfterClass
158+
public void tearDown() {
159+
_indexSegment.destroy();
200160
FileUtils.deleteQuietly(INDEX_DIR);
201-
_indexSegments.forEach((IndexSegment::destroy));
202161
}
203162

204163
@Override

0 commit comments

Comments
 (0)