Akanksha-kedia commented on PR #18920:
URL: https://github.com/apache/pinot/pull/18920#issuecomment-5236127415
Thanks for the feedback @J-HowHuang — this is a fair point and we've
addressed it in the latest push.
The three private helpers that were specific to `JsonIndexHandler`
(`loadMetadataProperties`, `readStoredJsonIndexConfig`,
`setStoredJsonIndexConfig`) have been lifted into `BaseIndexHandler` as
protected generic utilities:
```java
// BaseIndexHandler (now available to all handlers)
protected PropertiesConfiguration loadMetadataProperties()
protected static <T> T readStoredIndexConfig(
String columnName, String configKey, Class<T> configClass,
@Nullable PropertiesConfiguration properties)
protected static <T> void setStoredIndexConfig(
String columnName, String configKey, T config,
PropertiesConfiguration properties)
```
Any handler that wants config-change detection can now call these directly —
no boilerplate to duplicate. For example, a future `BloomFilterHandler` would
just do:
```java
BloomFilterConfig stored = readStoredIndexConfig(col, "bloomFilterConfig",
BloomFilterConfig.class, props);
```
The `metadata.properties` approach was chosen as the uniform standard since
it's already the existing Pinot convention for segment-level metadata (e.g.
`NullValueVectorHandler`), doesn't require embedding config in the index file
format (which would complicate the `BloomFilter` / `column.psf` approach), and
doesn't add extra files per index type (unlike the vector index approach). The
other two PRs (#19046, #18898) can migrate to this pattern independently.
--
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]
