Index spi: all types #10193
Index spi: all types #10193
Conversation
Codecov Report
@@ Coverage Diff @@
## master #10193 +/- ##
============================================
- Coverage 70.27% 68.23% -2.04%
+ Complexity 6113 6104 -9
============================================
Files 2070 2088 +18
Lines 112024 112149 +125
Branches 17051 17054 +3
============================================
- Hits 78725 76530 -2195
- Misses 27751 30149 +2398
+ Partials 5548 5470 -78
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 182 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
…o index-spi-index-service
…nto index-spi-index-type-only
…o index-spi-index-service
|
|
||
| @Override | ||
| public IndexReaderFactory<IndexReader> getReaderFactory() { | ||
| throw new UnsupportedOperationException(); |
There was a problem hiding this comment.
Some functions are unsupported. some are implemented.
I assume the unsupported functions will be added in next PR. Is that the case?
...t-local/src/main/java/org/apache/pinot/segment/local/segment/index/range/RangeIndexType.java
Outdated
Show resolved
Hide resolved
| /** | ||
| * The default range index version used when not specified in the TableConfig. | ||
| * | ||
| * This value should be equal to the one used in {@link org.apache.pinot.spi.config.table.IndexingConfig} |
There was a problem hiding this comment.
is this comment for this INSTANCE constant? I feel it's not quite related, but I may have missed some details.
There was a problem hiding this comment.
Good catch and a funny story. I created this PR by using diff from changes in index-spi branch and then removing things I didn't want to show in this PR. Here I removed a static constant but didn't removed the Javadoc.
The problem with my approach is that now if I delete the javadoc here, once I merge this with #10184 the comment will be removed without conflict and I may forget about it. Therefore what I'm going to do is to bring back the static constant. It is not going to be used in this PR, but it is going to ensure we don't accidentally remove the javadoc.
...ent-local/src/main/java/org/apache/pinot/segment/local/segment/index/text/TextIndexType.java
Show resolved
Hide resolved
pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/StandardIndexes.java
Show resolved
Hide resolved
| * | ||
| * This value should be equal to the one used in {@link org.apache.pinot.spi.config.table.IndexingConfig} | ||
| */ | ||
| public static final int DEFAULT_RANGE_INDEX_VERSION = 2; |
There was a problem hiding this comment.
Not sure where this would be used.. and how we change the index version
There was a problem hiding this comment.
It will be used in the next PR. We have been talking about it here.
TL;DR: The const is not used here, but I added the javadoc by mistake on this PR. We can either add here the const (no problematic), let the javadoc written without the attribute (which seems rare) or remove here the javadoc. The later may seem to be the best option, but if I do that, the javadoc will be removed from the full PR once I merge the changes. If I don't remember to add it later, we will lose the javadoc. Given that this process is error prone, I decided to just bring the const to this PR, which causes no problem at the cost of having a const we don't actually use here.
and how we change the index version
This is the range index version we use right now.
6 participants
This is a partial draft PR related to #10183. The whole PR is in #10184. It isn't intended to be merged as it is and it doesn't require to compile or pass the checks. It has been created in order to make it easier to understand the PR with all the changes.
This PR depends on #10192. Readers only interested in the difference between this and the previous PR can focus on gortiz#2
This PR shows all IndexTypes partially implemented without actually modifying the code.