somandal commented on code in PR #9740: URL: https://github.com/apache/pinot/pull/9740#discussion_r1018627043
########## pinot-core/src/test/java/org/apache/pinot/queries/ForwardIndexDisabledMultiValueQueriesTestWithReload.java: ########## @@ -0,0 +1,525 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.pinot.queries; + +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.Set; +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; +import org.apache.pinot.common.utils.DataSchema; +import org.apache.pinot.segment.local.indexsegment.immutable.ImmutableSegmentLoader; +import org.apache.pinot.segment.local.segment.creator.impl.SegmentIndexCreationDriverImpl; +import org.apache.pinot.segment.local.segment.index.loader.IndexLoadingConfig; +import org.apache.pinot.segment.spi.ColumnMetadata; +import org.apache.pinot.segment.spi.ImmutableSegment; +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.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; +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.Test; + +import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertFalse; +import static org.testng.Assert.assertNotNull; +import static org.testng.Assert.assertNull; +import static org.testng.Assert.assertTrue; + + +/** + * The <code>ForwardIndexDisabledMultiValueQueriesTestWithReload</code> class sets up the index segment for the + * no forward index multi-value queries test with reload. + * <p>There are totally 14 columns, 100000 records inside the original Avro file where 10 columns are selected to build + * the index segment. Selected columns information are as following: + * <ul> + * ColumnName, FieldType, DataType, Cardinality, IsSorted, HasInvertedIndex, IsMultiValue, FwdIndexDisabled: S1, S2 + * <li>column1, METRIC, INT, 51594, F, F, F, F, F</li> + * <li>column2, METRIC, INT, 42242, F, F, F, F, F</li> + * <li>column3, DIMENSION, STRING, 5, F, T, F, F, F</li> + * <li>column5, DIMENSION, STRING, 9, F, F, F, F, F</li> + * <li>column6, DIMENSION, INT, 18499, F, T, T, T, T</li> + * <li>column7, DIMENSION, INT, 359, F, F, T, F, F</li> + * <li>column8, DIMENSION, INT, 850, F, T, F, F, F</li> + * <li>column9, METRIC, INT, 146, F, T, F, F, F</li> + * <li>column10, METRIC, INT, 3960, F, F, F, F, F</li> + * <li>daysSinceEpoch, TIME, INT, 1, T, F, F, F, F</li> + * </ul> + */ +public class ForwardIndexDisabledMultiValueQueriesTestWithReload extends BaseQueriesTest { + 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(), + "ForwardIndexDisabledMultiValueQueriesTestWithReload"); + + private static final String SELECT_STAR_QUERY = "SELECT * FROM testTable"; + + // Build the segment schema. + private static final 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(); + + // 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"; + + private IndexSegment _indexSegment; + // Contains 2 identical index segments. + private List<IndexSegment> _indexSegments; + + private TableConfig _tableConfig; + private List<String> _invertedIndexColumns; + private List<String> _forwardIndexDisabledColumns; + private List<String> _noDictionaryColumns; + + @BeforeMethod + public void buildSegment() + throws Exception { + FileUtils.deleteQuietly(INDEX_DIR); + + // Get resource file path. + URL resource = getClass().getClassLoader().getResource(AVRO_DATA); + assertNotNull(resource); + String filePath = resource.getFile(); + + createSegment(filePath, SEGMENT_NAME_1); + createSegment(filePath, SEGMENT_NAME_2); + + 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) + 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()))); + // 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").setNoDictionaryColumns(Arrays.asList("column7")) + .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", "column8", "column9"); + segmentGeneratorConfig.setInvertedIndexCreationColumns(_invertedIndexColumns); + _forwardIndexDisabledColumns = new ArrayList<>(Arrays.asList("column6")); + segmentGeneratorConfig.setForwardIndexDisabledColumns(_forwardIndexDisabledColumns); + _noDictionaryColumns = new ArrayList<>(Arrays.asList("column7")); Review Comment: done ########## pinot-core/src/test/java/org/apache/pinot/queries/ForwardIndexDisabledMultiValueQueriesTestWithReload.java: ########## @@ -0,0 +1,525 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.pinot.queries; + +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.Set; +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; +import org.apache.pinot.common.utils.DataSchema; +import org.apache.pinot.segment.local.indexsegment.immutable.ImmutableSegmentLoader; +import org.apache.pinot.segment.local.segment.creator.impl.SegmentIndexCreationDriverImpl; +import org.apache.pinot.segment.local.segment.index.loader.IndexLoadingConfig; +import org.apache.pinot.segment.spi.ColumnMetadata; +import org.apache.pinot.segment.spi.ImmutableSegment; +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.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; +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.Test; + +import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertFalse; +import static org.testng.Assert.assertNotNull; +import static org.testng.Assert.assertNull; +import static org.testng.Assert.assertTrue; + + +/** + * The <code>ForwardIndexDisabledMultiValueQueriesTestWithReload</code> class sets up the index segment for the + * no forward index multi-value queries test with reload. + * <p>There are totally 14 columns, 100000 records inside the original Avro file where 10 columns are selected to build + * the index segment. Selected columns information are as following: + * <ul> + * ColumnName, FieldType, DataType, Cardinality, IsSorted, HasInvertedIndex, IsMultiValue, FwdIndexDisabled: S1, S2 + * <li>column1, METRIC, INT, 51594, F, F, F, F, F</li> + * <li>column2, METRIC, INT, 42242, F, F, F, F, F</li> + * <li>column3, DIMENSION, STRING, 5, F, T, F, F, F</li> + * <li>column5, DIMENSION, STRING, 9, F, F, F, F, F</li> + * <li>column6, DIMENSION, INT, 18499, F, T, T, T, T</li> + * <li>column7, DIMENSION, INT, 359, F, F, T, F, F</li> + * <li>column8, DIMENSION, INT, 850, F, T, F, F, F</li> + * <li>column9, METRIC, INT, 146, F, T, F, F, F</li> + * <li>column10, METRIC, INT, 3960, F, F, F, F, F</li> + * <li>daysSinceEpoch, TIME, INT, 1, T, F, F, F, F</li> + * </ul> + */ +public class ForwardIndexDisabledMultiValueQueriesTestWithReload extends BaseQueriesTest { + 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(), + "ForwardIndexDisabledMultiValueQueriesTestWithReload"); + + private static final String SELECT_STAR_QUERY = "SELECT * FROM testTable"; + + // Build the segment schema. + private static final 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(); + + // 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"; + + private IndexSegment _indexSegment; + // Contains 2 identical index segments. + private List<IndexSegment> _indexSegments; + + private TableConfig _tableConfig; + private List<String> _invertedIndexColumns; + private List<String> _forwardIndexDisabledColumns; + private List<String> _noDictionaryColumns; + + @BeforeMethod + public void buildSegment() + throws Exception { + FileUtils.deleteQuietly(INDEX_DIR); + + // Get resource file path. + URL resource = getClass().getClassLoader().getResource(AVRO_DATA); + assertNotNull(resource); + String filePath = resource.getFile(); + + createSegment(filePath, SEGMENT_NAME_1); + createSegment(filePath, SEGMENT_NAME_2); + + 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) + 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()))); + // 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").setNoDictionaryColumns(Arrays.asList("column7")) + .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", "column8", "column9"); + segmentGeneratorConfig.setInvertedIndexCreationColumns(_invertedIndexColumns); + _forwardIndexDisabledColumns = new ArrayList<>(Arrays.asList("column6")); + segmentGeneratorConfig.setForwardIndexDisabledColumns(_forwardIndexDisabledColumns); + _noDictionaryColumns = new ArrayList<>(Arrays.asList("column7")); + segmentGeneratorConfig.setRawIndexCreationColumns(_noDictionaryColumns); + // 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. + SegmentIndexCreationDriver driver = new SegmentIndexCreationDriverImpl(); + driver.init(segmentGeneratorConfig); + 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.setNoDictionaryColumns(new HashSet<>(_noDictionaryColumns)); + 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) -> { + if (column.equals("column6")) { + assertTrue(metadata.hasDictionary()); + assertFalse(metadata.isSingleValue()); + assertNull(immutableSegment.getForwardIndex(column)); + } else { + assertNotNull(immutableSegment.getForwardIndex(column)); + } + }); + + return immutableSegment; + } + + @AfterMethod + public void deleteAndDestroySegment() { + FileUtils.deleteQuietly(INDEX_DIR); + _indexSegments.forEach((IndexSegment::destroy)); + } + + @Override + protected String getFilter() { + return FILTER; + } + + @Override + protected IndexSegment getIndexSegment() { + return _indexSegment; + } + + @Override + protected List<IndexSegment> getIndexSegments() { + return _indexSegments; + } + + @Test + public void testSelectQueriesWithReload() + throws Exception { + // Selection query without filters including column7 + // This is just a sanity check to ensure the query works when forward index is enabled + String query = "SELECT column1, column5, column7 FROM testTable WHERE column7 != 201 ORDER BY column1"; + BrokerResponseNative brokerResponseNative = getBrokerResponse(query); + assertTrue(brokerResponseNative.getProcessingExceptions() == null + || brokerResponseNative.getProcessingExceptions().size() == 0); + ResultTable resultTable = brokerResponseNative.getResultTable(); + assertEquals(brokerResponseNative.getNumRowsResultSet(), 10); + assertEquals(brokerResponseNative.getTotalDocs(), 400_000L); + assertEquals(brokerResponseNative.getNumDocsScanned(), 399_896L); + assertEquals(brokerResponseNative.getNumSegmentsProcessed(), 4L); + assertEquals(brokerResponseNative.getNumSegmentsMatched(), 4L); + assertEquals(brokerResponseNative.getNumEntriesScannedPostFilter(), 399_976L); + assertEquals(brokerResponseNative.getNumEntriesScannedInFilter(), 536_360L); + assertNotNull(brokerResponseNative.getProcessingExceptions()); Review Comment: done - removed -- 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: commits-unsubscr...@pinot.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
