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 2025/06/13 13:08:22 UTC
[PR] Add Multi-column Text index [pinot]
bziobrowski opened a new pull request, #16103:
URL: https://github.com/apache/pinot/pull/16103
This PR adds new Multi-Column Text Index.
It it functionally equivalent to existing Lucene-based Text Index, but stores data for all indexed-columns together, in a single directory. The approach is especially beneficial when the number of text-indexed columns is large (tens or hundreds), because it consolidates lots of small files, reducing both disk space and memory usage (including e.g. file handles).
Apart from saving space on shared intra-column tokens within Lucene, the new index uses a single document id mapping.
Example configuration (within table config):
```json
"tableIndexConfig": {
"multiColumnTextIndexConfig": {
"columns": ["hobbies", "skills", "titles" ],
"properties": {
"caseSensitive": "false"
}
"perColumnProperties": {
"titles": {
"caseSensitive": "false"
}
}
},
```
As shown in example above, index configuration allows for both:
- setting shared index properties that apply to all columns with "properties".
Allowed keys are : `enableQueryCacheForTextIndex`, `luceneUseCompoundFile`, `luceneMaxBufferSizeMB`, `reuseMutableIndex` and all allowed in `perColumnProperties`.
- setting column-specific properties (overriding shared ones) with `perColumnProperties`.
Allowed keys: `useANDForMultiTermTextIndexQueries`, `enablePrefixSuffixMatchingInPhraseQueries`, `stopWordInclude`, `stopWordExclude`, `caseSensitive`, `luceneAnalyzerClass`, `luceneAnalyzerClassArgs`, `luceneAnalyzerClassArgTypes`, `luceneQueryParserClass`.
Note: this index doesn't handle `noRawDataForTextIndex` and `rawValueForTextIndex` properties.
Benchmarking with BenchmarkTextMatchQueriesSSQE class shows performance close to existing Lucene Text Index:
```
(_query) Mode Cnt Score Error Units
TEXT_MATCH(SC_STR_COL_0, 'pinot OR java') avgt 5 0.374 ± 0.065 ms/op
TEXT_MATCH(MC_STR_COL_0, 'pinot OR java') avgt 5 0.358 ± 0.038 ms/op
TEXT_MATCH(SC_STR_COL_0, 'pinot java') OR TEXT_MATCH(SC_STR_COL_1, 'distributed database') avgt 5 0.724 ± 0.066 ms/op
TEXT_MATCH(MC_STR_COL_0, 'pinot java') OR TEXT_MATCH(MC_STR_COL_1, 'distributed database') avgt 5 0.742 ± 0.070 ms/op
Q5 avgt 5 0.862 ± 0.095 ms/op
Q6 avgt 5 0.879 ± 0.154 ms/op
```
Where:
- each query starts with `SELECT count(*) from MyTable WHERE` .
- Q5 is
```sql
SELECT count(*)
FROM MyTable
WHERE TEXT_MATCH(SC_STR_COL_0, 'pinot')
OR TEXT_MATCH(SC_STR_COL_1, 'java')
OR TEXT_MATCH(SC_STR_COL_2, 'database')
OR TEXT_MATCH(SC_STR_COL_3, 'distributed')
OR TEXT_MATCH(SC_STR_COL_4, 'multi-tenant')
```
- Q6 is
```sql
SELECT count(*)
FROM MyTable
WHERE TEXT_MATCH(MC_STR_COL_0, 'pinot')
OR TEXT_MATCH(MC_STR_COL_1, 'java')
OR TEXT_MATCH(MC_STR_COL_2, 'database')
OR TEXT_MATCH(MC_STR_COL_3, 'distributed')
OR TEXT_MATCH(MC_STR_COL_4, 'multi-tenant')
```
--
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 Multi-column Text index [pinot]
Posted by "gortiz (via GitHub)" <gi...@apache.org>.
gortiz commented on code in PR #16103:
URL: https://github.com/apache/pinot/pull/16103#discussion_r2151654606
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/SegmentPreProcessor.java:
##########
@@ -158,8 +170,17 @@ public void process(@Nullable SegmentOperationsThrottler segmentOperationsThrott
// that the other required indices (e.g. forward index) are up-to-date.
try (SegmentDirectory.Writer segmentWriter = _segmentDirectory.createWriter()) {
if (processStarTrees(indexDir, segmentOperationsThrottler)) {
- // NOTE: When adding new steps after this, un-comment the next line.
Review Comment:
Should we keep this comment, moved to line 183 so we don't forget to reload metadata if we add more steps?
--
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 Multi-column Text index [pinot]
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on PR #16103:
URL: https://github.com/apache/pinot/pull/16103#issuecomment-2977555126
Could you please add this feature to pinot documentation?
Given this directly applies to multiple columns, I guess we cannot specify it in `FieldConfig`? What will happen if user specify the index via both `multiColumnTextIndexConfig` and `FieldConfig`? Will it create 2 separate text index?
IMO a more preferred way for this is to add a boolean `createMultiColumnTextIndex` into `tableIndexConfig` and still honor the index config from `FieldConfig`. When this flag is set to true, create a single index file; otherwise, keep the existing behavior
--
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 Multi-column Text index [pinot]
Posted by "bziobrowski (via GitHub)" <gi...@apache.org>.
bziobrowski commented on PR #16103:
URL: https://github.com/apache/pinot/pull/16103#issuecomment-2977626970
Sure, I'll add documentation tomorrow.
If column is included then in multi-col index and has regular text index defined, they'll both be created (though not at the same time) but the latter has higher priority (for now).
I disagree about config part because letting user configure via field-config:
- suggests that it's just another of the single-column indexes (it's not and e.g. rebuilding it is bound to take more time)
- ignores the fact that there's one index with shared core settings
- assumes all settings can be customized per-column (a number of settings can't be adjusted this way)
What I also wanted to avoid is wasting memory on fake per-column indexes that'd only call this index.
--
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 Multi-column Text index [pinot]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #16103:
URL: https://github.com/apache/pinot/pull/16103#issuecomment-2970662455
## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/16103?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
Attention: Patch coverage is `41.71429%` with `612 lines` in your changes missing coverage. Please review.
> Project coverage is 56.26%. Comparing base [(`1a476de`)](https://app.codecov.io/gh/apache/pinot/commit/1a476dea72ebf1c15e14ded28ae60c385f71b2c3?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`8ee80e3`)](https://app.codecov.io/gh/apache/pinot/commit/8ee80e3597acbd7284d5284c21ab28a283b06c68?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
> Report is 253 commits behind head on master.
| [Files with missing lines](https://app.codecov.io/gh/apache/pinot/pull/16103?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...index/multicolumntext/MultiColumnTextMetadata.java](https://app.codecov.io/gh/apache/pinot/pull/16103?src=pr&el=tree&filepath=pinot-segment-spi%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fsegment%2Fspi%2Findex%2Fmulticolumntext%2FMultiColumnTextMetadata.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc2VnbWVudC1zcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3NlZ21lbnQvc3BpL2luZGV4L211bHRpY29sdW1udGV4dC9NdWx0aUNvbHVtblRleHRNZXRhZGF0YS5qYXZh) | 0.00% | [142 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/16103?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...ertedindex/MultiColumnRealtimeLuceneTextIndex.java](https://app.codecov.io/gh/apache/pinot/pull/16103?src=pr&el=tree&filepath=pinot-segment-local%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fsegment%2Flocal%2Frealtime%2Fimpl%2Finvertedindex%2FMultiColumnRealtimeLuceneTextIndex.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9yZWFsdGltZS9pbXBsL2ludmVydGVkaW5kZXgvTXVsdGlDb2x1bW5SZWFsdGltZUx1Y2VuZVRleHRJbmRleC5qYXZh) | 0.00% | [130 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/16103?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...r/impl/text/MultiColumnLuceneTextIndexCreator.java](https://app.codecov.io/gh/apache/pinot/pull/16103?src=pr&el=tree&filepath=pinot-segment-local%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fsegment%2Flocal%2Fsegment%2Fcreator%2Fimpl%2Ftext%2FMultiColumnLuceneTextIndexCreator.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2NyZWF0b3IvaW1wbC90ZXh0L011bHRpQ29sdW1uTHVjZW5lVGV4dEluZGV4Q3JlYXRvci5qYXZh) | 27.51% | [97 Missing and 11 partials :warning: ](https://app.codecov.io/gh/apache/pinot/pull/16103?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...readers/text/MultiColumnLuceneTextIndexReader.java](https://app.codecov.io/gh/apache/pinot/pull/16103?src=pr&el=tree&filepath=pinot-segment-local%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fsegment%2Flocal%2Fsegment%2Findex%2Freaders%2Ftext%2FMultiColumnLuceneTextIndexReader.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L3JlYWRlcnMvdGV4dC9NdWx0aUNvbHVtbkx1Y2VuZVRleHRJbmRleFJlYWRlci5qYXZh) | 60.30% | [62 Missing and 17 partials :warning: ](https://app.codecov.io/gh/apache/pinot/pull/16103?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...local/indexsegment/mutable/MutableSegmentImpl.java](https://app.codecov.io/gh/apache/pinot/pull/16103?src=pr&el=tree&filepath=pinot-segment-local%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fsegment%2Flocal%2Findexsegment%2Fmutable%2FMutableSegmentImpl.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9pbmRleHNlZ21lbnQvbXV0YWJsZS9NdXRhYmxlU2VnbWVudEltcGwuamF2YQ==) | 21.56% | [36 Missing and 4 partials :warning: ](https://app.codecov.io/gh/apache/pinot/pull/16103?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...egment/spi/index/metadata/SegmentMetadataImpl.java](https://app.codecov.io/gh/apache/pinot/pull/16103?src=pr&el=tree&filepath=pinot-segment-spi%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fsegment%2Fspi%2Findex%2Fmetadata%2FSegmentMetadataImpl.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc2VnbWVudC1zcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3NlZ21lbnQvc3BpL2luZGV4L21ldGFkYXRhL1NlZ21lbnRNZXRhZGF0YUltcGwuamF2YQ==) | 0.00% | [22 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/16103?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...ocal/segment/index/loader/SegmentPreProcessor.java](https://app.codecov.io/gh/apache/pinot/pull/16103?src=pr&el=tree&filepath=pinot-segment-local%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fsegment%2Flocal%2Fsegment%2Findex%2Floader%2FSegmentPreProcessor.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L2xvYWRlci9TZWdtZW50UHJlUHJvY2Vzc29yLmphdmE=) | 64.15% | [10 Missing and 9 partials :warning: ](https://app.codecov.io/gh/apache/pinot/pull/16103?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...der/invertedindex/MultiColumnTextIndexHandler.java](https://app.codecov.io/gh/apache/pinot/pull/16103?src=pr&el=tree&filepath=pinot-segment-local%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fsegment%2Flocal%2Fsegment%2Findex%2Floader%2Finvertedindex%2FMultiColumnTextIndexHandler.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L2xvYWRlci9pbnZlcnRlZGluZGV4L011bHRpQ29sdW1uVGV4dEluZGV4SGFuZGxlci5qYXZh) | 84.55% | [18 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/pinot/pull/16103?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...s/SegmentMultiColTextIndexPreprocessThrottler.java](https://app.codecov.io/gh/apache/pinot/pull/16103?src=pr&el=tree&filepath=pinot-segment-local%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fsegment%2Flocal%2Futils%2FSegmentMultiColTextIndexPreprocessThrottler.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC91dGlscy9TZWdtZW50TXVsdGlDb2xUZXh0SW5kZXhQcmVwcm9jZXNzVGhyb3R0bGVyLmphdmE=) | 46.66% | [8 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/16103?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [.../core/operator/filter/TextMatchFilterOperator.java](https://app.codecov.io/gh/apache/pinot/pull/16103?src=pr&el=tree&filepath=pinot-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fcore%2Foperator%2Ffilter%2FTextMatchFilterOperator.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9maWx0ZXIvVGV4dE1hdGNoRmlsdGVyT3BlcmF0b3IuamF2YQ==) | 76.00% | [6 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/16103?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| ... and [18 more](https://app.codecov.io/gh/apache/pinot/pull/16103?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | |
> :exclamation: There is a different number of reports uploaded between BASE (1a476de) and HEAD (8ee80e3). Click for more details.
>
> <details><summary>HEAD has 68 uploads less than BASE</summary>
>
>| Flag | BASE (1a476de) | HEAD (8ee80e3) |
>|------|------|------|
>|java-21|10|1|
>|unittests|6|1|
>|unittests1|3|1|
>|skip-bytebuffers-true|4|0|
>|temurin|15|1|
>|unittests2|3|0|
>|skip-bytebuffers-false|8|0|
>|java-11|5|0|
>|integration|9|0|
>|integration2|3|0|
>|integration1|3|0|
>|custom-integration1|3|0|
></details>
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #16103 +/- ##
============================================
- Coverage 62.90% 56.26% -6.65%
+ Complexity 1386 690 -696
============================================
Files 2867 2289 -578
Lines 163354 125664 -37690
Branches 24952 20015 -4937
============================================
- Hits 102755 70702 -32053
+ Misses 52847 49109 -3738
+ Partials 7752 5853 -1899
```
| [Flag](https://app.codecov.io/gh/apache/pinot/pull/16103/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/16103/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/16103/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/16103/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/16103/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/16103/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/16103/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `56.26% <41.71%> (-6.56%)` | :arrow_down: |
| [skip-bytebuffers-false](https://app.codecov.io/gh/apache/pinot/pull/16103/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/16103/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [temurin](https://app.codecov.io/gh/apache/pinot/pull/16103/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `56.26% <41.71%> (-6.65%)` | :arrow_down: |
| [unittests](https://app.codecov.io/gh/apache/pinot/pull/16103/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `56.26% <41.71%> (-6.64%)` | :arrow_down: |
| [unittests1](https://app.codecov.io/gh/apache/pinot/pull/16103/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `56.26% <41.71%> (+0.44%)` | :arrow_up: |
| [unittests2](https://app.codecov.io/gh/apache/pinot/pull/16103/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/16103?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).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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 Multi-column Text index [pinot]
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on PR #16103:
URL: https://github.com/apache/pinot/pull/16103#issuecomment-2977711176
cc @gortiz
I think the overall direction should be to move all index related configs to `FieldConfig`. Currently the only exception is star-tree index, and this is adding one more exception.
Can you share what functionality is not supported with the new multi-column text index? Maybe I'm missing something, but from my point of view, this is similar to the current single file approach which encapsulate all indexes except for star-tree index and text index, and we are essentially adding one more single file to encapsulate all text indexes.
For now, we should at least add some validation to prevent user from configuring the same column in both places and result in 2 copies of index being generated.
--
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 Multi-column Text index [pinot]
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 merged PR #16103:
URL: https://github.com/apache/pinot/pull/16103
--
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 Multi-column Text index [pinot]
Posted by "gortiz (via GitHub)" <gi...@apache.org>.
gortiz commented on PR #16103:
URL: https://github.com/apache/pinot/pull/16103#issuecomment-2979452929
In general I like the PR, but I would prefer a solution where we don't need to create a new index concept but instead just codify this feature as an internal property of the single-column text index (maybe with a syntax like https://github.com/apache/pinot/pull/16103#issuecomment-2979347157).
I think this perspective has several advantages (easier to explain, keeps using the same syntax... in general, it is simpler), but I don't know if it is technically possible. This is why I said I missed a design document where we could discuss the feature from a different perspective.
With Index SPI, we spend a significant amount of time trying to find a way to specify indexes that can reduce the cognitive complexity associated with them, and this PR is a step backward. That doesn't have to be a blocker (sometimes it is needed), but I would like to be sure there was no other option.
--
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 Multi-column Text index [pinot]
Posted by "gortiz (via GitHub)" <gi...@apache.org>.
gortiz commented on code in PR #16103:
URL: https://github.com/apache/pinot/pull/16103#discussion_r2151645787
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/creator/impl/text/MultiColumnLuceneTextIndexCreator.java:
##########
@@ -0,0 +1,450 @@
+/**
+ * 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.segment.local.segment.creator.impl.text;
+
+import it.unimi.dsi.fastutil.booleans.BooleanList;
+import it.unimi.dsi.fastutil.objects.Object2IntOpenHashMap;
+import java.io.Closeable;
+import java.io.File;
+import java.io.IOException;
+import java.nio.ByteOrder;
+import java.util.List;
+import java.util.Map;
+import javax.annotation.Nullable;
+import org.apache.commons.io.FileUtils;
+import org.apache.lucene.analysis.Analyzer;
+import org.apache.lucene.document.Document;
+import org.apache.lucene.document.Field;
+import org.apache.lucene.document.StoredField;
+import org.apache.lucene.document.TextField;
+import org.apache.lucene.index.DirectoryReader;
+import org.apache.lucene.index.IndexReader;
+import org.apache.lucene.index.IndexWriter;
+import org.apache.lucene.index.IndexWriterConfig;
+import org.apache.lucene.index.LogByteSizeMergePolicy;
+import org.apache.lucene.search.IndexSearcher;
+import org.apache.lucene.store.Directory;
+import org.apache.lucene.store.FSDirectory;
+import org.apache.lucene.store.NRTCachingDirectory;
+import org.apache.pinot.segment.local.realtime.impl.invertedindex.LuceneNRTCachingMergePolicy;
+import org.apache.pinot.segment.local.realtime.impl.invertedindex.RealtimeLuceneTextIndex;
+import org.apache.pinot.segment.local.segment.creator.impl.SegmentColumnarIndexCreator;
+import org.apache.pinot.segment.local.segment.index.readers.text.MultiColumnLuceneTextIndexReader;
+import org.apache.pinot.segment.local.segment.index.readers.text.MultiColumnLuceneTextIndexReader.ColumnConfig;
+import org.apache.pinot.segment.local.segment.index.text.TextIndexConfigBuilder;
+import org.apache.pinot.segment.local.segment.store.TextIndexUtils;
+import org.apache.pinot.segment.spi.V1Constants;
+import org.apache.pinot.segment.spi.index.TextIndexConfig;
+import org.apache.pinot.segment.spi.index.creator.DictionaryBasedInvertedIndexCreator;
+import org.apache.pinot.segment.spi.index.multicolumntext.MultiColumnTextIndexConstants;
+import org.apache.pinot.segment.spi.memory.PinotDataBuffer;
+import org.apache.pinot.segment.spi.store.SegmentDirectoryPaths;
+import org.apache.pinot.spi.config.table.MultiColumnTextIndexConfig;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+
+/**
+ * This class is used to create Lucene-based multi-column text index.
+ * Used for both offline from {@link SegmentColumnarIndexCreator}
+ * and realtime from {@link RealtimeLuceneTextIndex}
+ * While this is an index creator, it consumes values for multiple columns in a single call,
+ * which doesn't fit IndexCreator.
+ */
+public class MultiColumnLuceneTextIndexCreator implements Closeable /*extends AbstractTextIndexCreator*/ {
Review Comment:
Should we remove the `/*extends AbstractTextIndexCreator*/` comment here?
--
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 Multi-column Text index [pinot]
Posted by "bziobrowski (via GitHub)" <gi...@apache.org>.
bziobrowski commented on code in PR #16103:
URL: https://github.com/apache/pinot/pull/16103#discussion_r2166368813
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/TextMatchTransformFunction.java:
##########
@@ -58,6 +58,7 @@ public void init(List<TransformFunction> arguments, Map<String, ColumnContext> c
throw new IllegalArgumentException("Cannot apply TEXT_MATCH on column: " + columnName + " without text index");
}
TextIndexReader indexReader = dataSource.getTextIndex();
+ // TODO: handle multi-column index (if defined)
Review Comment:
It's addressed in https://github.com/apache/pinot/pull/16160.
--
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 Multi-column Text index [pinot]
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on PR #16103:
URL: https://github.com/apache/pinot/pull/16103#issuecomment-2978082197
@xiangfu0 We can consider deprecating the one file per column storage format, but I feel the `FieldConfig` way of configuring it is still preferred
--
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 Multi-column Text index [pinot]
Posted by "bziobrowski (via GitHub)" <gi...@apache.org>.
bziobrowski commented on PR #16103:
URL: https://github.com/apache/pinot/pull/16103#issuecomment-2979781416
Making config pretend to be field-level is possible but it'd come with a number of disadvantages I mentioned above.
Multi column index doesn't really fit existing single-column structures on many levels (interfaces, index creation) and config is just the surface.
The use case the new index is addressing is a table with 100+ string columns text-indexed with same settings.
--
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 Multi-column Text index [pinot]
Posted by "gortiz (via GitHub)" <gi...@apache.org>.
gortiz commented on PR #16103:
URL: https://github.com/apache/pinot/pull/16103#issuecomment-2979347157
I think we could have study the syntax a bit more before merging it. We may have created a design document to discuss different interfaces.
For example, I could imagine a different syntax like:
```json
{
"fieldConfigList": [
{
"name": "hobbies",
"indexes": {
"text": {"enabled": true, "shared": true, "caseSensitive": false}
}
},
{
"name": "skills",
"indexes": {
"text": {"enabled": true, "shared": true, "caseSensitive": false}
}
},
{
"name": "titles",
"indexes": {
"text": {"enabled": true, "shared": true, "caseSensitive": true}
}
}
]
}
```
--
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 Multi-column Text index [pinot]
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 commented on PR #16103:
URL: https://github.com/apache/pinot/pull/16103#issuecomment-2977868898
Based on the perf benchmark result, I would suggest to deprecate single column text index and always go with the multi-col index.
The only down side here is to rebuild the full text index when there are any column modification in the text index
--
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 Multi-column Text index [pinot]
Posted by "gortiz (via GitHub)" <gi...@apache.org>.
gortiz commented on code in PR #16103:
URL: https://github.com/apache/pinot/pull/16103#discussion_r2151607735
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/TextMatchTransformFunction.java:
##########
@@ -58,6 +58,7 @@ public void init(List<TransformFunction> arguments, Map<String, ColumnContext> c
throw new IllegalArgumentException("Cannot apply TEXT_MATCH on column: " + columnName + " without text index");
}
TextIndexReader indexReader = dataSource.getTextIndex();
+ // TODO: handle multi-column index (if defined)
Review Comment:
Do we plan to work on this TODO?
--
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