somandal commented on PR #10260: URL: https://github.com/apache/pinot/pull/10260#issuecomment-1426595563
> > We have a scenario where the dictionary can be disabled for a forward index disabled column. For such scenarios we don't handle anything on reload today. Are you saying that even for toggling whether the dictionary is enabled or disabled we should just treat reload as a no-op rather than throwing error? What about indexes like the range index that need to be modified when the column has the dictionary enabled or disabled? > > Disable dictionary should be independent of disabling forward index. Currently it is no-op because we want to ensure data can be recovered. With the new change, we should allow no dictionary + no forward index. Note that disable dictionary + enable inverted index is invalid config, and we should reject it during table config validation. When both forward index and dictionary are disabled, no further index modification is allowed. Sure that makes sense. So if I'm understanding correctly I should make the following changes: - When dictionary is disabled for a forward index disabled column we just remove the dictionary, update the segment metadata and remove the related indexes. If range index is present we can throw an error since we shouldn't try to regenerate it under any circumstances. - If someone tries to enable the dictionary on a column without forward index, again we should avoid any regeneration business and just throw an error on reload. - Add validations that inverted index and FST are also disabled if dictionary is disabled on a forward index disabled column (if such a compatibility check doesn't already exist for all columns) Let me know if the above sounds good or not. I'll keep all code paths marked as [current code path] in the list in the main comment. -- 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]
