gortiz commented on code in PR #10687:
URL: https://github.com/apache/pinot/pull/10687#discussion_r1183415386
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/json/JsonIndexType.java:
##########
@@ -164,4 +167,16 @@ protected void handleIndexSpecificCleanup(TableConfig
tableConfig) {
tableConfig.getIndexingConfig().setJsonIndexColumns(null);
tableConfig.getIndexingConfig().setJsonIndexConfigs(null);
}
+
+ @Nullable
+ @Override
+ public MutableIndex createMutableIndex(MutableIndexContext context,
JsonIndexConfig config) {
+ if (config.isDisabled()) {
+ return null;
+ }
+ if (!context.getFieldSpec().isSingleValueField()) {
Review Comment:
That is an interesting question.
What previous code does when if founds a multi-valued column indexed with an
index it doesnt support is:
- Create the mutable index.
- Never call `add` on it.
AFAIU this means that at query time:
1. Either we check check if the column is multivalued and in that case do
not apply the index.
2. Or we apply the index and we return false results because the
MutableIndex will be empty.
Instead of creating an empty mutable index, the new code do not create the
index at all. That is compatible with option number 1, which I'm assuming is
the one we follow.
> Shouldn't this be caught by validation itself?
A derivative question is: These indexes that do not support mutable
multi-valued versions... do support immutable multi-valued versions? For
example, range index supports multi-valued columns in offline tables but do not
support them in realtime. Therefore it makes sense to define a range index on a
multi-valued column.
I'm assuming that what the older code does in that case is to create a
MutableRangeIndex, never add elements to it and when the segment is committed,
the segment is actually indexed by the RangeIndexCreator. Therefore we cannot
fail when a MutableRangeIndex is trying to be created on a multi-valued column.
In case an index is not supported neither in realltime and in offline tables
on multi-valued columns, we should fail in the `getConfig` function, which is
the one that should do the actual validation.
--
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: commits-unsubscr...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
