gortiz commented on code in PR #10184:
URL: https://github.com/apache/pinot/pull/10184#discussion_r1148867232
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/indexsegment/immutable/ImmutableSegmentLoader.java:
##########
@@ -166,6 +165,8 @@ public static ImmutableSegment load(SegmentDirectory
segmentDirectory, IndexLoad
segmentMetadata.removeColumn(column);
}
}
+ } else {
+ indexLoadingConfig.addKnownColumns(columnMetadataMap.keySet());
Review Comment:
This is being used in `IndexType`s that also implement
`ConfigurableFromIndexLoadingConfig`. From the javadoc of this interface:
```java
/**
* Returns a {@link ColumnConfigDeserializer} that can be used to
deserialize the index config.
*
* This deserializer is used with higher priority whenever the index
configuration needs to be read from an
* {@link IndexLoadingConfig}.
*
* Sometimes {@link IndexLoadingConfig} is not completely configured and
* {@link IndexLoadingConfig#getAllKnownColumns()} does not return all
columns in the table.
* Therefore the returned deserializer can be partial.
*/
```
At the end of the day the problem we try to resolve here is that `index-spi`
assumes that the source of truth is the `TableConfig` but we have tons of tests
that instead of changing the `TableConfig`, they directly modify
`IndexLoadingConfig`. I call it a _false mock_, given that we only use that in
tests. Therefore we need to add all this code here in order to support our own
tests without much changes.
@Jackie-Jiang and I want to remove `IndexLoadingConfig` mutability (or even
better, the class!), but it is not a sensible job and there are more
implications than we initially expected, so we decided to have this
`ConfigurableFromIndexLoadingConfig` interface as a poor man _temporal_
solution.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
