Copilot commented on code in PR #17992:
URL: https://github.com/apache/pinot/pull/17992#discussion_r2997977777
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/store/SingleFileIndexDirectory.java:
##########
@@ -233,7 +233,14 @@ private void loadMap()
for (String key : CommonsConfigurationUtils.getKeys(mapConfig)) {
String[] parsedKeys = ColumnIndexUtils.parseIndexMapKeys(key,
_segmentDirectory.getPath());
- IndexKey indexKey = IndexKey.fromIndexName(parsedKeys[0], parsedKeys[1]);
+ IndexKey indexKey;
+ try {
+ indexKey = IndexKey.fromIndexName(parsedKeys[0], parsedKeys[1]);
+ } catch (IllegalArgumentException e) {
+ LOGGER.warn("Skipping unknown index type: {} for column: {} in
segment: {}", parsedKeys[1], parsedKeys[0],
+ _segmentDirectory);
+ continue;
Review Comment:
This warning will be emitted once per map entry. Since each index typically
has at least START_OFFSET and SIZE entries, an unknown index type will likely
log the same warning multiple times (log spam). Consider deduplicating (e.g.,
keep a Set of (column,indexType) already-warned) or lowering to DEBUG after the
first occurrence.
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/store/SingleFileIndexDirectory.java:
##########
@@ -233,7 +233,14 @@ private void loadMap()
for (String key : CommonsConfigurationUtils.getKeys(mapConfig)) {
String[] parsedKeys = ColumnIndexUtils.parseIndexMapKeys(key,
_segmentDirectory.getPath());
- IndexKey indexKey = IndexKey.fromIndexName(parsedKeys[0], parsedKeys[1]);
+ IndexKey indexKey;
+ try {
+ indexKey = IndexKey.fromIndexName(parsedKeys[0], parsedKeys[1]);
+ } catch (IllegalArgumentException e) {
+ LOGGER.warn("Skipping unknown index type: {} for column: {} in
segment: {}", parsedKeys[1], parsedKeys[0],
+ _segmentDirectory);
+ continue;
+ }
Review Comment:
This change alters load behavior to skip unknown index types instead of
failing. Please add a unit test that writes an index map entry with an
unrecognized index id and asserts that SingleFileIndexDirectory still loads
(and that known indices remain accessible).
--
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]
