Jackie-Jiang commented on code in PR #12554:
URL: https://github.com/apache/pinot/pull/12554#discussion_r1513901059
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/startree/v2/store/StarTreeIndexMapUtils.java:
##########
@@ -198,9 +199,9 @@ public static List<Map<IndexKey, IndexValue>>
loadFromInputStream(InputStream in
column = StringUtils.join(split, KEY_SEPARATOR, 1,
columnSplitEndIndex);
}
// Convert metric (function-column pair) to stored name for
backward-compatibility
- if (column.contains(AggregationFunctionColumnPair.DELIMITER)) {
- AggregationFunctionColumnPair functionColumnPair =
AggregationFunctionColumnPair.fromColumnName(column);
- column =
AggregationFunctionColumnPair.resolveToStoredType(functionColumnPair).toColumnName();
+ Optional<AggregationFunctionColumnPair> functionColumnPair =
AggregationFunctionColumnPair.accept(column);
Review Comment:
Suggest doing a try-catch around
`AggregationFunctionColumnPair.fromColumnName(column)` and simply ignore the
exception as this pieces of code is purely for backward compatibility and is
not regular code path. We should also add some comments
```suggestion
if (column.contains(AggregationFunctionColumnPair.DELIMITER)) {
try {
AggregationFunctionColumnPair functionColumnPair =
AggregationFunctionColumnPair.fromColumnName(column);
column =
AggregationFunctionColumnPair.resolveToStoredType(functionColumnPair).toColumnName();
} catch (Exception e) {
// Ignoring this exception for columns that are not metric
(function-column pair)
}
}
```
##########
pinot-core/src/test/java/org/apache/pinot/core/startree/v2/BaseStarTreeV2Test.java:
##########
@@ -91,32 +91,35 @@ abstract class BaseStarTreeV2Test<R, A> {
private static final int NUM_SEGMENT_RECORDS = 100_000;
private static final int MAX_LEAF_RECORDS = RANDOM.nextInt(100) + 1;
- private static final String DIMENSION_D1 = "d1";
- private static final String DIMENSION_D2 = "d2";
+ private static final String DIMENSION_D1 = "d1__COLUMN_NAME";
Review Comment:
Let's add some comments on why choosing this strange dimension name
--
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]
