You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "bziobrowski (via GitHub)" <gi...@apache.org> on 2024/11/19 13:03:36 UTC
[PR] Add test to check data type/cardinality/encoding compatibility with i… [pinot]
bziobrowski opened a new pull request, #14493:
URL: https://github.com/apache/pinot/pull/14493
PR adds integration test (disabled by default) that checks compatibility between combinations of:
- data type - INT, LONG, FLOAT, DOUBLE, BIG_DECIMAL, BOOLEAN, TIMESTAMP, STRING, JSON, BYTES, MAP (all variants)
- cardinality - single value, multiple values
- encoding - raw or dictionary
- index types - timestamp, bloom, fst, geo, inverted, json, native_text, text, range, vector
and reports the outcome .
--
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-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Add test to check data and index types compatibility [pinot]
Posted by "bziobrowski (via GitHub)" <gi...@apache.org>.
bziobrowski commented on PR #14493:
URL: https://github.com/apache/pinot/pull/14493#issuecomment-2491836627
It is rather slow when reload fails in the background, there's no feedback and test waits 5 seconds before marking test case as failed.
--
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-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Add test to check data and index types compatibility [pinot]
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on PR #14493:
URL: https://github.com/apache/pinot/pull/14493#issuecomment-2491971332
Nice! This can give us big confidence!
How long does it take to run the test? Both in your local environment and in Github Actions?
--
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-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Add test to check data and index types compatibility [pinot]
Posted by "bziobrowski (via GitHub)" <gi...@apache.org>.
bziobrowski commented on PR #14493:
URL: https://github.com/apache/pinot/pull/14493#issuecomment-2506827402
@Jackie-Jiang
I rewrote test at unit level. It takes about 5 seconds now (vs 5+ minutes before).
@gortiz
As discussed, I rewrote test to assert test cases against .csv file.
--
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-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Add test to check data and index types compatibility [pinot]
Posted by "gortiz (via GitHub)" <gi...@apache.org>.
gortiz commented on code in PR #14493:
URL: https://github.com/apache/pinot/pull/14493#discussion_r1853420154
##########
pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/TableIndexingTest.java:
##########
@@ -0,0 +1,776 @@
+/**
+ * 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.integration.tests;
+
+import com.fasterxml.jackson.databind.node.ArrayNode;
+import com.fasterxml.jackson.databind.node.JsonNodeFactory;
+import com.fasterxml.jackson.databind.node.ObjectNode;
+import java.io.File;
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.text.SimpleDateFormat;
+import java.time.Duration;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Date;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Random;
+import java.util.stream.Collectors;
+import org.apache.avro.file.DataFileWriter;
+import org.apache.avro.generic.GenericData;
+import org.apache.avro.generic.GenericDatumWriter;
+import org.apache.commons.io.FileUtils;
+import org.apache.pinot.common.metrics.ServerMetrics;
+import org.apache.pinot.spi.config.table.FieldConfig;
+import org.apache.pinot.spi.config.table.IndexingConfig;
+import org.apache.pinot.spi.config.table.StarTreeIndexConfig;
+import org.apache.pinot.spi.config.table.TableConfig;
+import org.apache.pinot.spi.config.table.TableType;
+import org.apache.pinot.spi.config.table.TimestampConfig;
+import org.apache.pinot.spi.config.table.TimestampIndexGranularity;
+import org.apache.pinot.spi.data.ComplexFieldSpec;
+import org.apache.pinot.spi.data.DimensionFieldSpec;
+import org.apache.pinot.spi.data.FieldSpec;
+import org.apache.pinot.spi.data.FieldSpec.DataType;
+import org.apache.pinot.spi.data.MetricFieldSpec;
+import org.apache.pinot.spi.data.Schema;
+import org.apache.pinot.spi.metrics.PinotMetricName;
+import org.apache.pinot.spi.metrics.PinotMetricUtils;
+import org.apache.pinot.spi.utils.builder.TableConfigBuilder;
+import org.apache.pinot.util.TestUtils;
+import org.testng.Assert;
+import org.testng.annotations.AfterClass;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.DataProvider;
+import org.testng.annotations.Ignore;
+import org.testng.annotations.Test;
+
+
+// Try to create various index types for all data type/cardinality/encoding combinations and report outcome.
+// NOTES: There is no multi-value type for BigDecimal, JSON or MAP.
+// see PinotDataType.getPinotDataTypeForIngestion()
+@Test(enabled = false)
+public class TableIndexingTest extends BaseClusterIntegrationTestSet {
+
+ private final ArrayList<String> _tableNames = new ArrayList<>();
+ private final int _allDocs = 3000;
+ private final SimpleDateFormat _format = new SimpleDateFormat("HH:mm:ss.SSS");
+ private final List<TestCase> _allResults = new ArrayList<>();
+
+ static class TestCase {
+ String _tableName;
+ String _indexType;
+ Throwable _error;
+
+ public TestCase(String tableName, String indexType) {
+ _tableName = tableName;
+ _indexType = indexType;
+ }
+
+ @Override
+ public String toString() {
+ return _tableName + "," + _indexType;
+ }
+ }
+
+ @BeforeClass
+ public void setUp()
+ throws Exception {
+ TestUtils.ensureDirectoriesExistAndEmpty(_tempDir, _segmentDir, _tarDir);
+
+ // Start the Pinot cluster
+ startZk();
+ startController();
+ startBrokers(1);
+ startServers(1);
+
+ // Create and upload the schema and table config
+ List<Schema> schemas = createSchemas();
+ addSchemas(schemas);
+ List<TableConfig> tableConfigs = createOfflineTableConfigs(schemas);
+ addTableConfigs(tableConfigs);
+
+ List<List<File>> avroFiles = createAvroFile(schemas);
+
+ for (int i = 0; i < schemas.size(); i++) {
+ // we've to use separate directories because segment tar files must exist for the duration of test
+ File schemaSegmentDir = new File(_segmentDir, "schema_" + i);
+ File schemaTarDir = new File(_tarDir, "schema_" + i);
+ TestUtils.ensureDirectoriesExistAndEmpty(schemaSegmentDir, schemaTarDir);
+ ClusterIntegrationTestUtils.buildSegmentsFromAvro(avroFiles.get(i), tableConfigs.get(i), schemas.get(i), 0,
+ schemaSegmentDir, schemaTarDir);
+ uploadSegments(schemas.get(i).getSchemaName(), schemaTarDir);
+ }
+
+ waitForAllDocsLoaded(schemas);
+ }
+
+ private void addTableConfigs(List<TableConfig> tableConfigs)
+ throws IOException {
+ for (TableConfig config : tableConfigs) {
+ super.addTableConfig(config);
+ }
+ }
+
+ private List<TableConfig> createOfflineTableConfigs(List<Schema> schemas) {
+ return
+ schemas.stream().map(s -> new TableConfigBuilder(TableType.OFFLINE)
+ .setTableName(s.getSchemaName())
+ .build())
+ .collect(Collectors.toList());
+ }
+
+ private void waitForAllDocsLoaded(final List<Schema> schemas) {
+ HashSet<String> incompleteTables = new HashSet<>();
+ for (Schema schema : schemas) {
+ incompleteTables.add(schema.getSchemaName());
+ }
+ List<String> toRemove = new ArrayList<>();
+
+ TestUtils.waitForCondition(() -> {
+ toRemove.clear();
+ for (String table : incompleteTables) {
+ if (getCurrentCountStarResult(table) == _allDocs) {
+ toRemove.add(table);
+ }
+ }
+ incompleteTables.removeAll(toRemove);
+ return incompleteTables.isEmpty();
+ }, 100L, 60_000L,
+ "Failed to load " + _allDocs + " documents", true, Duration.ofMillis(60_000L / 10));
+ }
+
+ @AfterClass
+ public void tearDown()
+ throws Exception {
+ stopServer();
+ stopBroker();
+ stopController();
+ stopZk();
+ FileUtils.deleteDirectory(_tempDir);
+ }
+
+ @Ignore
+ @Test(dataProvider = "fieldsAndIndexTypes")
+ public void testAddIndex(TestCase testCase)
+ throws Throwable {
+ try {
+ String schemaName = testCase._tableName;
+ String indexType = testCase._indexType;
+
+ System.out.println(
+ _format.format(new Date()) + " Starting check for column: " + schemaName + " index type: " + indexType);
+ Schema schema = getSchema(schemaName);
+ FieldSpec field = schema.getFieldSpecFor("col");
+
+ // These exceptions are thrown during segment reload (and not table config update) and appear in logs only
+ // We're throwing them here to make test faster and improve output.
+ if ("geo".equals(indexType) && field.getDataType() != DataType.BYTES) {
+ throw new RuntimeException("Geo/H3 index can only be applied to column of BYTES data type!");
+ }
+
+ if ("json".equals(indexType) && ((field.getDataType() != DataType.STRING && field.getDataType() != DataType.JSON)
+ || !field.isSingleValueField())) {
+ throw new RuntimeException(
+ "JSON index can only be applied to single value column of STRING or JSON data type!");
+ }
+
+ if ("vector".equals(indexType) && (field.getDataType() != DataType.FLOAT || field.isSingleValueField())) {
+ throw new RuntimeException("VECTOR index can only be applied to Float Array columns");
+ }
+
+ if (("text".equals(indexType) || "native_text".equals(indexType)) && field.getDataType() != DataType.STRING) {
+ throw new RuntimeException("Text index is currently only supported on STRING columns");
+ }
+
+ TableConfig tableConfig = getOfflineTableConfig(schemaName);
+ IndexingConfig idxCfg = tableConfig.getIndexingConfig();
+
+ FieldConfig.EncodingType encoding =
+ field.getName().startsWith("raw") ? FieldConfig.EncodingType.RAW : FieldConfig.EncodingType.DICTIONARY;
+
+ List<FieldConfig.IndexType> indexTypes = new ArrayList<>();
+ Map<String, String> properties = new HashMap<>();
+ ObjectNode indexes = new ObjectNode(JsonNodeFactory.instance);
+ TimestampConfig tstmpConfig = null;
+ FieldConfig config = getByName(tableConfig, field.getName());
+ // ignore existing config and overwrite it, otherwise it will fail on earlier errors
+ boolean isNew = config == null;
+
+ switch (indexType) {
+ case "bloom":
+ /* bloom filter. Maybe we should call it bloom filter index to be consistent ?
+ {
+ "tableName": "somePinotTable",
+ "fieldConfigList": [
+ {
+ "name": "playerID",
+ "indexes": {
+ "bloom": {}
+ }
+ },
+ ...
+ ],
+ ...
+ } */
+ // no params
+ indexes.put("bloom", new ObjectNode(JsonNodeFactory.instance));
Review Comment:
```suggestion
indexes.put("bloom", JsonUtils.newObjectNode());
```
--
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-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Add test to check data and index types compatibility [pinot]
Posted by "npawar (via GitHub)" <gi...@apache.org>.
npawar commented on PR #14493:
URL: https://github.com/apache/pinot/pull/14493#issuecomment-2491806358
Why disable by default? this would be a good thing to have running to catch combination regressions?
--
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-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Add test to check data and index types compatibility [pinot]
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang merged PR #14493:
URL: https://github.com/apache/pinot/pull/14493
--
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-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Add test to check data and index types compatibility [pinot]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #14493:
URL: https://github.com/apache/pinot/pull/14493#issuecomment-2485808842
## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/14493?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 55.37%. Comparing base [(`59551e4`)](https://app.codecov.io/gh/apache/pinot/commit/59551e45224f1535c4863fd577622b37366ccc97?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`fc767ca`)](https://app.codecov.io/gh/apache/pinot/commit/fc767caf0dceb211733472be9f7ddc02ef289aad?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
> Report is 1358 commits behind head on master.
> :exclamation: There is a different number of reports uploaded between BASE (59551e4) and HEAD (fc767ca). Click for more details.
>
> <details><summary>HEAD has 53 uploads less than BASE</summary>
>
>| Flag | BASE (59551e4) | HEAD (fc767ca) |
>|------|------|------|
>|integration|7|0|
>|integration2|3|0|
>|temurin|12|1|
>|java-21|7|1|
>|skip-bytebuffers-true|3|1|
>|skip-bytebuffers-false|7|0|
>|unittests|5|1|
>|unittests1|2|1|
>|java-11|5|0|
>|unittests2|3|0|
>|integration1|2|0|
>|custom-integration1|2|0|
></details>
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #14493 +/- ##
============================================
- Coverage 61.75% 55.37% -6.38%
- Complexity 207 698 +491
============================================
Files 2436 2100 -336
Lines 133233 110660 -22573
Branches 20636 17582 -3054
============================================
- Hits 82274 61276 -20998
+ Misses 44911 44486 -425
+ Partials 6048 4898 -1150
```
| [Flag](https://app.codecov.io/gh/apache/pinot/pull/14493/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [custom-integration1](https://app.codecov.io/gh/apache/pinot/pull/14493/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration](https://app.codecov.io/gh/apache/pinot/pull/14493/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration1](https://app.codecov.io/gh/apache/pinot/pull/14493/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration2](https://app.codecov.io/gh/apache/pinot/pull/14493/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [java-11](https://app.codecov.io/gh/apache/pinot/pull/14493/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [java-21](https://app.codecov.io/gh/apache/pinot/pull/14493/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `55.37% <ø> (-6.26%)` | :arrow_down: |
| [skip-bytebuffers-false](https://app.codecov.io/gh/apache/pinot/pull/14493/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [skip-bytebuffers-true](https://app.codecov.io/gh/apache/pinot/pull/14493/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `55.37% <ø> (+27.64%)` | :arrow_up: |
| [temurin](https://app.codecov.io/gh/apache/pinot/pull/14493/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `55.37% <ø> (-6.38%)` | :arrow_down: |
| [unittests](https://app.codecov.io/gh/apache/pinot/pull/14493/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `55.37% <ø> (-6.38%)` | :arrow_down: |
| [unittests1](https://app.codecov.io/gh/apache/pinot/pull/14493/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `55.37% <ø> (+8.48%)` | :arrow_up: |
| [unittests2](https://app.codecov.io/gh/apache/pinot/pull/14493/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/pinot/pull/14493?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
----
🚨 Try these New Features:
- [Flaky Tests Detection](https://docs.codecov.com/docs/test-result-ingestion-beta) - Detect and resolve failed and flaky tests
- [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis) - Avoid shipping oversized bundles
--
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-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Add test to check data and index types compatibility [pinot]
Posted by "gortiz (via GitHub)" <gi...@apache.org>.
gortiz commented on code in PR #14493:
URL: https://github.com/apache/pinot/pull/14493#discussion_r1853422448
##########
pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/TableIndexingTest.java:
##########
@@ -0,0 +1,776 @@
+/**
+ * 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.integration.tests;
+
+import com.fasterxml.jackson.databind.node.ArrayNode;
+import com.fasterxml.jackson.databind.node.JsonNodeFactory;
+import com.fasterxml.jackson.databind.node.ObjectNode;
+import java.io.File;
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.text.SimpleDateFormat;
+import java.time.Duration;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Date;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Random;
+import java.util.stream.Collectors;
+import org.apache.avro.file.DataFileWriter;
+import org.apache.avro.generic.GenericData;
+import org.apache.avro.generic.GenericDatumWriter;
+import org.apache.commons.io.FileUtils;
+import org.apache.pinot.common.metrics.ServerMetrics;
+import org.apache.pinot.spi.config.table.FieldConfig;
+import org.apache.pinot.spi.config.table.IndexingConfig;
+import org.apache.pinot.spi.config.table.StarTreeIndexConfig;
+import org.apache.pinot.spi.config.table.TableConfig;
+import org.apache.pinot.spi.config.table.TableType;
+import org.apache.pinot.spi.config.table.TimestampConfig;
+import org.apache.pinot.spi.config.table.TimestampIndexGranularity;
+import org.apache.pinot.spi.data.ComplexFieldSpec;
+import org.apache.pinot.spi.data.DimensionFieldSpec;
+import org.apache.pinot.spi.data.FieldSpec;
+import org.apache.pinot.spi.data.FieldSpec.DataType;
+import org.apache.pinot.spi.data.MetricFieldSpec;
+import org.apache.pinot.spi.data.Schema;
+import org.apache.pinot.spi.metrics.PinotMetricName;
+import org.apache.pinot.spi.metrics.PinotMetricUtils;
+import org.apache.pinot.spi.utils.builder.TableConfigBuilder;
+import org.apache.pinot.util.TestUtils;
+import org.testng.Assert;
+import org.testng.annotations.AfterClass;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.DataProvider;
+import org.testng.annotations.Ignore;
+import org.testng.annotations.Test;
+
+
+// Try to create various index types for all data type/cardinality/encoding combinations and report outcome.
+// NOTES: There is no multi-value type for BigDecimal, JSON or MAP.
+// see PinotDataType.getPinotDataTypeForIngestion()
+@Test(enabled = false)
+public class TableIndexingTest extends BaseClusterIntegrationTestSet {
+
+ private final ArrayList<String> _tableNames = new ArrayList<>();
+ private final int _allDocs = 3000;
+ private final SimpleDateFormat _format = new SimpleDateFormat("HH:mm:ss.SSS");
+ private final List<TestCase> _allResults = new ArrayList<>();
+
+ static class TestCase {
+ String _tableName;
+ String _indexType;
+ Throwable _error;
+
+ public TestCase(String tableName, String indexType) {
+ _tableName = tableName;
+ _indexType = indexType;
+ }
+
+ @Override
+ public String toString() {
+ return _tableName + "," + _indexType;
+ }
+ }
+
+ @BeforeClass
+ public void setUp()
+ throws Exception {
+ TestUtils.ensureDirectoriesExistAndEmpty(_tempDir, _segmentDir, _tarDir);
+
+ // Start the Pinot cluster
+ startZk();
+ startController();
+ startBrokers(1);
+ startServers(1);
+
+ // Create and upload the schema and table config
+ List<Schema> schemas = createSchemas();
+ addSchemas(schemas);
+ List<TableConfig> tableConfigs = createOfflineTableConfigs(schemas);
+ addTableConfigs(tableConfigs);
+
+ List<List<File>> avroFiles = createAvroFile(schemas);
+
+ for (int i = 0; i < schemas.size(); i++) {
+ // we've to use separate directories because segment tar files must exist for the duration of test
+ File schemaSegmentDir = new File(_segmentDir, "schema_" + i);
+ File schemaTarDir = new File(_tarDir, "schema_" + i);
+ TestUtils.ensureDirectoriesExistAndEmpty(schemaSegmentDir, schemaTarDir);
+ ClusterIntegrationTestUtils.buildSegmentsFromAvro(avroFiles.get(i), tableConfigs.get(i), schemas.get(i), 0,
+ schemaSegmentDir, schemaTarDir);
+ uploadSegments(schemas.get(i).getSchemaName(), schemaTarDir);
+ }
+
+ waitForAllDocsLoaded(schemas);
+ }
+
+ private void addTableConfigs(List<TableConfig> tableConfigs)
+ throws IOException {
+ for (TableConfig config : tableConfigs) {
+ super.addTableConfig(config);
+ }
+ }
+
+ private List<TableConfig> createOfflineTableConfigs(List<Schema> schemas) {
+ return
+ schemas.stream().map(s -> new TableConfigBuilder(TableType.OFFLINE)
+ .setTableName(s.getSchemaName())
+ .build())
+ .collect(Collectors.toList());
+ }
+
+ private void waitForAllDocsLoaded(final List<Schema> schemas) {
+ HashSet<String> incompleteTables = new HashSet<>();
+ for (Schema schema : schemas) {
+ incompleteTables.add(schema.getSchemaName());
+ }
+ List<String> toRemove = new ArrayList<>();
+
+ TestUtils.waitForCondition(() -> {
+ toRemove.clear();
+ for (String table : incompleteTables) {
+ if (getCurrentCountStarResult(table) == _allDocs) {
+ toRemove.add(table);
+ }
+ }
+ incompleteTables.removeAll(toRemove);
+ return incompleteTables.isEmpty();
+ }, 100L, 60_000L,
+ "Failed to load " + _allDocs + " documents", true, Duration.ofMillis(60_000L / 10));
+ }
+
+ @AfterClass
+ public void tearDown()
+ throws Exception {
+ stopServer();
+ stopBroker();
+ stopController();
+ stopZk();
+ FileUtils.deleteDirectory(_tempDir);
+ }
+
+ @Ignore
+ @Test(dataProvider = "fieldsAndIndexTypes")
+ public void testAddIndex(TestCase testCase)
+ throws Throwable {
+ try {
+ String schemaName = testCase._tableName;
+ String indexType = testCase._indexType;
+
+ System.out.println(
+ _format.format(new Date()) + " Starting check for column: " + schemaName + " index type: " + indexType);
+ Schema schema = getSchema(schemaName);
+ FieldSpec field = schema.getFieldSpecFor("col");
+
+ // These exceptions are thrown during segment reload (and not table config update) and appear in logs only
+ // We're throwing them here to make test faster and improve output.
+ if ("geo".equals(indexType) && field.getDataType() != DataType.BYTES) {
+ throw new RuntimeException("Geo/H3 index can only be applied to column of BYTES data type!");
+ }
+
+ if ("json".equals(indexType) && ((field.getDataType() != DataType.STRING && field.getDataType() != DataType.JSON)
+ || !field.isSingleValueField())) {
+ throw new RuntimeException(
+ "JSON index can only be applied to single value column of STRING or JSON data type!");
+ }
+
+ if ("vector".equals(indexType) && (field.getDataType() != DataType.FLOAT || field.isSingleValueField())) {
+ throw new RuntimeException("VECTOR index can only be applied to Float Array columns");
+ }
+
+ if (("text".equals(indexType) || "native_text".equals(indexType)) && field.getDataType() != DataType.STRING) {
+ throw new RuntimeException("Text index is currently only supported on STRING columns");
+ }
+
+ TableConfig tableConfig = getOfflineTableConfig(schemaName);
+ IndexingConfig idxCfg = tableConfig.getIndexingConfig();
+
+ FieldConfig.EncodingType encoding =
+ field.getName().startsWith("raw") ? FieldConfig.EncodingType.RAW : FieldConfig.EncodingType.DICTIONARY;
+
+ List<FieldConfig.IndexType> indexTypes = new ArrayList<>();
+ Map<String, String> properties = new HashMap<>();
+ ObjectNode indexes = new ObjectNode(JsonNodeFactory.instance);
+ TimestampConfig tstmpConfig = null;
+ FieldConfig config = getByName(tableConfig, field.getName());
+ // ignore existing config and overwrite it, otherwise it will fail on earlier errors
+ boolean isNew = config == null;
+
+ switch (indexType) {
+ case "bloom":
+ /* bloom filter. Maybe we should call it bloom filter index to be consistent ?
+ {
+ "tableName": "somePinotTable",
+ "fieldConfigList": [
+ {
+ "name": "playerID",
+ "indexes": {
+ "bloom": {}
+ }
+ },
+ ...
+ ],
+ ...
+ } */
+ // no params
+ indexes.put("bloom", new ObjectNode(JsonNodeFactory.instance));
+
+ break;
+ case "fst":
+ /* fst index / text index
+ "fieldConfigList":[
+ {
+ "name":"text_col_1",
+ "encodingType":"DICTIONARY",
+ "indexType":"FST"
+ }
+ ]
+ */
+ indexTypes.add(FieldConfig.IndexType.FST);
+ break;
+ case "geo":
+ /* geospatial - requires dictionary be disabled
+ {
+ "fieldConfigList": [
+ {
+ "name": "location_st_point",
+ "encodingType":"RAW", // this actually disables the dictionary
+ "indexes": {
+ "h3": {
+ "resolutions": [13, 5, 6]
+ }
+ }
+ }
+ ],
+ ...
+ }
+ */
+ ObjectNode resolutions = new ObjectNode(JsonNodeFactory.instance);
+ ArrayNode res = new ArrayNode(JsonNodeFactory.instance);
+ res.add(13).add(5).add(6);
+ resolutions.put("resolutions", res);
Review Comment:
```suggestion
ObjectNode resolutions = JsonUtils.stringToJsonNode("{\"resolutions\": [13, 5, 6]}");
```
--
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-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Add test to check data and index types compatibility [pinot]
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on code in PR #14493:
URL: https://github.com/apache/pinot/pull/14493#discussion_r1868225424
##########
pinot-core/src/test/java/org/apache/pinot/core/data/manager/TableIndexingTest.java:
##########
@@ -0,0 +1,717 @@
+/**
+ * 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.core.data.manager;
+
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.node.JsonNodeFactory;
+import com.fasterxml.jackson.databind.node.ObjectNode;
+import java.io.File;
+import java.io.IOException;
+import java.net.URL;
+import java.nio.ByteBuffer;
+import java.nio.file.Files;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Random;
+import org.apache.commons.io.FileUtils;
+import org.apache.pinot.core.data.manager.offline.ImmutableSegmentDataManager;
+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.local.segment.readers.GenericRowRecordReader;
+import org.apache.pinot.segment.local.utils.TableConfigUtils;
+import org.apache.pinot.segment.spi.ImmutableSegment;
+import org.apache.pinot.segment.spi.creator.SegmentGeneratorConfig;
+import org.apache.pinot.segment.spi.creator.SegmentVersion;
+import org.apache.pinot.segment.spi.datasource.DataSource;
+import org.apache.pinot.segment.spi.index.IndexService;
+import org.apache.pinot.segment.spi.index.startree.StarTreeV2;
+import org.apache.pinot.spi.config.table.FieldConfig;
+import org.apache.pinot.spi.config.table.IndexingConfig;
+import org.apache.pinot.spi.config.table.StarTreeIndexConfig;
+import org.apache.pinot.spi.config.table.TableConfig;
+import org.apache.pinot.spi.config.table.TableType;
+import org.apache.pinot.spi.config.table.TimestampConfig;
+import org.apache.pinot.spi.config.table.TimestampIndexGranularity;
+import org.apache.pinot.spi.data.ComplexFieldSpec;
+import org.apache.pinot.spi.data.DimensionFieldSpec;
+import org.apache.pinot.spi.data.FieldSpec;
+import org.apache.pinot.spi.data.FieldSpec.DataType;
+import org.apache.pinot.spi.data.MetricFieldSpec;
+import org.apache.pinot.spi.data.Schema;
+import org.apache.pinot.spi.data.readers.GenericRow;
+import org.apache.pinot.spi.utils.JsonUtils;
+import org.apache.pinot.spi.utils.builder.TableConfigBuilder;
+import org.apache.pinot.spi.utils.builder.TableNameBuilder;
+import org.apache.pinot.util.TestUtils;
+import org.jetbrains.annotations.NotNull;
+import org.testng.Assert;
+import org.testng.annotations.AfterClass;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.DataProvider;
+import org.testng.annotations.Test;
+
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+
+// Tests various combinations of field type, encoding, single/multi-value and index type
+// and compares test case results with TableIndexingTest.csv
+public class TableIndexingTest {
+ private static final File TEMP_DIR = new File(FileUtils.getTempDirectory(), "TableIndexingTest");
+ private static final String TABLE_NAME = "mytable";
+ private static final String OFFLINE_TABLE_NAME = TableNameBuilder.OFFLINE.tableNameWithType(TABLE_NAME);
+ public static final String COLUMN_NAME = "col";
+ public static final String COLUMN_DAY_NAME = "$col$DAY";
+ public static final String COLUMN_MONTH_NAME = "$col$MONTH";
+ public static final String COLUMN_WEEK_NAME = "$col$WEEK";
+
+ private final ArrayList<Schema> _schemas = new ArrayList<>();
+ private TestCase[] _testCases;
+ private Map<TestCase, TestCase> _testCaseMap;
+ private final List<TestCase> _allResults = new ArrayList<>();
+
+ @BeforeClass
+ public void setUp()
+ throws Exception {
+ TestUtils.ensureDirectoriesExistAndEmpty(TEMP_DIR);
+ createSchemas();
+ createTestCases();
+ readExpectedResults();
+ }
+
+ private void createTestCases() {
+ String[] indexTypes =
+ {
+ "timestamp_index", "bloom_filter", "fst_index", "h3_index", "inverted_index", "json_index",
+ "native_text_index", "text_index", "range_index", "startree_index", "vector_index"
+ };
Review Comment:
(nit) We usually put `{` in the previous line
```suggestion
String[] indexTypes = {
"timestamp_index", "bloom_filter", "fst_index", "h3_index", "inverted_index", "json_index",
"native_text_index", "text_index", "range_index", "startree_index", "vector_index"
};
```
##########
pinot-core/src/test/java/org/apache/pinot/core/data/manager/TableIndexingTest.java:
##########
@@ -0,0 +1,717 @@
+/**
+ * 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.core.data.manager;
+
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.node.JsonNodeFactory;
+import com.fasterxml.jackson.databind.node.ObjectNode;
+import java.io.File;
+import java.io.IOException;
+import java.net.URL;
+import java.nio.ByteBuffer;
+import java.nio.file.Files;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Random;
+import org.apache.commons.io.FileUtils;
+import org.apache.pinot.core.data.manager.offline.ImmutableSegmentDataManager;
+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.local.segment.readers.GenericRowRecordReader;
+import org.apache.pinot.segment.local.utils.TableConfigUtils;
+import org.apache.pinot.segment.spi.ImmutableSegment;
+import org.apache.pinot.segment.spi.creator.SegmentGeneratorConfig;
+import org.apache.pinot.segment.spi.creator.SegmentVersion;
+import org.apache.pinot.segment.spi.datasource.DataSource;
+import org.apache.pinot.segment.spi.index.IndexService;
+import org.apache.pinot.segment.spi.index.startree.StarTreeV2;
+import org.apache.pinot.spi.config.table.FieldConfig;
+import org.apache.pinot.spi.config.table.IndexingConfig;
+import org.apache.pinot.spi.config.table.StarTreeIndexConfig;
+import org.apache.pinot.spi.config.table.TableConfig;
+import org.apache.pinot.spi.config.table.TableType;
+import org.apache.pinot.spi.config.table.TimestampConfig;
+import org.apache.pinot.spi.config.table.TimestampIndexGranularity;
+import org.apache.pinot.spi.data.ComplexFieldSpec;
+import org.apache.pinot.spi.data.DimensionFieldSpec;
+import org.apache.pinot.spi.data.FieldSpec;
+import org.apache.pinot.spi.data.FieldSpec.DataType;
+import org.apache.pinot.spi.data.MetricFieldSpec;
+import org.apache.pinot.spi.data.Schema;
+import org.apache.pinot.spi.data.readers.GenericRow;
+import org.apache.pinot.spi.utils.JsonUtils;
+import org.apache.pinot.spi.utils.builder.TableConfigBuilder;
+import org.apache.pinot.spi.utils.builder.TableNameBuilder;
+import org.apache.pinot.util.TestUtils;
+import org.jetbrains.annotations.NotNull;
+import org.testng.Assert;
+import org.testng.annotations.AfterClass;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.DataProvider;
+import org.testng.annotations.Test;
+
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+
+// Tests various combinations of field type, encoding, single/multi-value and index type
+// and compares test case results with TableIndexingTest.csv
+public class TableIndexingTest {
+ private static final File TEMP_DIR = new File(FileUtils.getTempDirectory(), "TableIndexingTest");
+ private static final String TABLE_NAME = "mytable";
+ private static final String OFFLINE_TABLE_NAME = TableNameBuilder.OFFLINE.tableNameWithType(TABLE_NAME);
+ public static final String COLUMN_NAME = "col";
+ public static final String COLUMN_DAY_NAME = "$col$DAY";
+ public static final String COLUMN_MONTH_NAME = "$col$MONTH";
+ public static final String COLUMN_WEEK_NAME = "$col$WEEK";
+
+ private final ArrayList<Schema> _schemas = new ArrayList<>();
+ private TestCase[] _testCases;
+ private Map<TestCase, TestCase> _testCaseMap;
+ private final List<TestCase> _allResults = new ArrayList<>();
+
+ @BeforeClass
+ public void setUp()
+ throws Exception {
+ TestUtils.ensureDirectoriesExistAndEmpty(TEMP_DIR);
+ createSchemas();
+ createTestCases();
+ readExpectedResults();
+ }
+
+ private void createTestCases() {
+ String[] indexTypes =
+ {
+ "timestamp_index", "bloom_filter", "fst_index", "h3_index", "inverted_index", "json_index",
+ "native_text_index", "text_index", "range_index", "startree_index", "vector_index"
+ };
+
+ _testCases = new TestCase[_schemas.size() * indexTypes.length];
+ _testCaseMap = new HashMap<>();
+
+ for (int i = 0; i < _schemas.size(); i++) {
+ for (int j = 0; j < indexTypes.length; j++) {
+ TestCase testCase = new TestCase(_schemas.get(i).getSchemaName(), i, indexTypes[j]);
+ _testCases[i * indexTypes.length + j] = testCase;
+ _testCaseMap.put(testCase, testCase);
+ }
+ }
+ }
+
+ private void readExpectedResults()
+ throws IOException {
+ List<String> expected = readExpectedFromFile();
+ // parse csv lines, e.g. INT;sv;raw;timestamp_index;true;
+ for (int i = 1; i < expected.size(); i++) {
+ String line = expected.get(i);
+ if (line.length() == 0) {
Review Comment:
(nit) My IDE usually gives warning on this
```suggestion
if (line.isEmpty()) {
```
--
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-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Add test to check data and index types compatibility [pinot]
Posted by "gortiz (via GitHub)" <gi...@apache.org>.
gortiz commented on code in PR #14493:
URL: https://github.com/apache/pinot/pull/14493#discussion_r1853418182
##########
pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/TableIndexingTest.java:
##########
@@ -0,0 +1,776 @@
+/**
+ * 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.integration.tests;
+
+import com.fasterxml.jackson.databind.node.ArrayNode;
+import com.fasterxml.jackson.databind.node.JsonNodeFactory;
+import com.fasterxml.jackson.databind.node.ObjectNode;
+import java.io.File;
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.text.SimpleDateFormat;
+import java.time.Duration;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Date;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Random;
+import java.util.stream.Collectors;
+import org.apache.avro.file.DataFileWriter;
+import org.apache.avro.generic.GenericData;
+import org.apache.avro.generic.GenericDatumWriter;
+import org.apache.commons.io.FileUtils;
+import org.apache.pinot.common.metrics.ServerMetrics;
+import org.apache.pinot.spi.config.table.FieldConfig;
+import org.apache.pinot.spi.config.table.IndexingConfig;
+import org.apache.pinot.spi.config.table.StarTreeIndexConfig;
+import org.apache.pinot.spi.config.table.TableConfig;
+import org.apache.pinot.spi.config.table.TableType;
+import org.apache.pinot.spi.config.table.TimestampConfig;
+import org.apache.pinot.spi.config.table.TimestampIndexGranularity;
+import org.apache.pinot.spi.data.ComplexFieldSpec;
+import org.apache.pinot.spi.data.DimensionFieldSpec;
+import org.apache.pinot.spi.data.FieldSpec;
+import org.apache.pinot.spi.data.FieldSpec.DataType;
+import org.apache.pinot.spi.data.MetricFieldSpec;
+import org.apache.pinot.spi.data.Schema;
+import org.apache.pinot.spi.metrics.PinotMetricName;
+import org.apache.pinot.spi.metrics.PinotMetricUtils;
+import org.apache.pinot.spi.utils.builder.TableConfigBuilder;
+import org.apache.pinot.util.TestUtils;
+import org.testng.Assert;
+import org.testng.annotations.AfterClass;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.DataProvider;
+import org.testng.annotations.Ignore;
+import org.testng.annotations.Test;
+
+
+// Try to create various index types for all data type/cardinality/encoding combinations and report outcome.
+// NOTES: There is no multi-value type for BigDecimal, JSON or MAP.
+// see PinotDataType.getPinotDataTypeForIngestion()
+@Test(enabled = false)
+public class TableIndexingTest extends BaseClusterIntegrationTestSet {
+
+ private final ArrayList<String> _tableNames = new ArrayList<>();
+ private final int _allDocs = 3000;
+ private final SimpleDateFormat _format = new SimpleDateFormat("HH:mm:ss.SSS");
+ private final List<TestCase> _allResults = new ArrayList<>();
+
+ static class TestCase {
+ String _tableName;
+ String _indexType;
+ Throwable _error;
+
+ public TestCase(String tableName, String indexType) {
+ _tableName = tableName;
+ _indexType = indexType;
+ }
+
+ @Override
+ public String toString() {
+ return _tableName + "," + _indexType;
+ }
+ }
+
+ @BeforeClass
+ public void setUp()
+ throws Exception {
+ TestUtils.ensureDirectoriesExistAndEmpty(_tempDir, _segmentDir, _tarDir);
+
+ // Start the Pinot cluster
+ startZk();
+ startController();
+ startBrokers(1);
+ startServers(1);
+
+ // Create and upload the schema and table config
+ List<Schema> schemas = createSchemas();
+ addSchemas(schemas);
+ List<TableConfig> tableConfigs = createOfflineTableConfigs(schemas);
+ addTableConfigs(tableConfigs);
+
+ List<List<File>> avroFiles = createAvroFile(schemas);
+
+ for (int i = 0; i < schemas.size(); i++) {
+ // we've to use separate directories because segment tar files must exist for the duration of test
+ File schemaSegmentDir = new File(_segmentDir, "schema_" + i);
+ File schemaTarDir = new File(_tarDir, "schema_" + i);
+ TestUtils.ensureDirectoriesExistAndEmpty(schemaSegmentDir, schemaTarDir);
+ ClusterIntegrationTestUtils.buildSegmentsFromAvro(avroFiles.get(i), tableConfigs.get(i), schemas.get(i), 0,
+ schemaSegmentDir, schemaTarDir);
+ uploadSegments(schemas.get(i).getSchemaName(), schemaTarDir);
+ }
+
+ waitForAllDocsLoaded(schemas);
+ }
+
+ private void addTableConfigs(List<TableConfig> tableConfigs)
+ throws IOException {
+ for (TableConfig config : tableConfigs) {
+ super.addTableConfig(config);
+ }
+ }
+
+ private List<TableConfig> createOfflineTableConfigs(List<Schema> schemas) {
+ return
+ schemas.stream().map(s -> new TableConfigBuilder(TableType.OFFLINE)
+ .setTableName(s.getSchemaName())
+ .build())
+ .collect(Collectors.toList());
+ }
+
+ private void waitForAllDocsLoaded(final List<Schema> schemas) {
+ HashSet<String> incompleteTables = new HashSet<>();
+ for (Schema schema : schemas) {
+ incompleteTables.add(schema.getSchemaName());
+ }
+ List<String> toRemove = new ArrayList<>();
+
+ TestUtils.waitForCondition(() -> {
+ toRemove.clear();
+ for (String table : incompleteTables) {
+ if (getCurrentCountStarResult(table) == _allDocs) {
+ toRemove.add(table);
+ }
+ }
+ incompleteTables.removeAll(toRemove);
+ return incompleteTables.isEmpty();
+ }, 100L, 60_000L,
+ "Failed to load " + _allDocs + " documents", true, Duration.ofMillis(60_000L / 10));
+ }
+
+ @AfterClass
+ public void tearDown()
+ throws Exception {
+ stopServer();
+ stopBroker();
+ stopController();
+ stopZk();
+ FileUtils.deleteDirectory(_tempDir);
+ }
+
+ @Ignore
+ @Test(dataProvider = "fieldsAndIndexTypes")
+ public void testAddIndex(TestCase testCase)
+ throws Throwable {
+ try {
+ String schemaName = testCase._tableName;
+ String indexType = testCase._indexType;
+
+ System.out.println(
+ _format.format(new Date()) + " Starting check for column: " + schemaName + " index type: " + indexType);
+ Schema schema = getSchema(schemaName);
+ FieldSpec field = schema.getFieldSpecFor("col");
+
+ // These exceptions are thrown during segment reload (and not table config update) and appear in logs only
+ // We're throwing them here to make test faster and improve output.
+ if ("geo".equals(indexType) && field.getDataType() != DataType.BYTES) {
+ throw new RuntimeException("Geo/H3 index can only be applied to column of BYTES data type!");
+ }
+
+ if ("json".equals(indexType) && ((field.getDataType() != DataType.STRING && field.getDataType() != DataType.JSON)
+ || !field.isSingleValueField())) {
+ throw new RuntimeException(
+ "JSON index can only be applied to single value column of STRING or JSON data type!");
+ }
+
+ if ("vector".equals(indexType) && (field.getDataType() != DataType.FLOAT || field.isSingleValueField())) {
+ throw new RuntimeException("VECTOR index can only be applied to Float Array columns");
+ }
+
+ if (("text".equals(indexType) || "native_text".equals(indexType)) && field.getDataType() != DataType.STRING) {
+ throw new RuntimeException("Text index is currently only supported on STRING columns");
+ }
Review Comment:
I don't understand this. What does this failure means? If we want to skip these cases we can either:
1. Preferred option: Filter them out in `fieldsAndIndexTypes`
2. Alternatively: We can throw SkipException, which makes TestNG to skip the test.
--
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-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org