siddharthteotia commented on a change in pull request #7031: URL: https://github.com/apache/incubator-pinot/pull/7031#discussion_r648767241
########## File path: pinot-controller/src/main/java/org/apache/pinot/controller/recommender/io/InputManager.java ########## @@ -236,39 +236,28 @@ private void reorderDimsAndBuildMap() _intToColNameMap = new String[_dimNames.size() + _metricNames.size() + _dateTimeNames.size()]; _colNameToIntMap = new HashMap<>(); - _dimNamesInvertedSortedIndexApplicable = new HashSet<>(_dimNames); - _dimNamesInvertedSortedIndexApplicable.remove(sortedColumn); - _dimNamesInvertedSortedIndexApplicable.removeAll(invertedIndexColumns); - _dimNamesInvertedSortedIndexApplicable.removeAll(noDictionaryColumns); + // Inverted index and sorted index will be recommended on all types of columns : dimensions, metrics and date time + _columnNamesInvertedSortedIndexApplicable = new HashSet<>(_dimNames); + _columnNamesInvertedSortedIndexApplicable.addAll(_metricNames); + _columnNamesInvertedSortedIndexApplicable.addAll(_dateTimeNames); - HashSet<String> dimNamesInveredSortedIndexNotApplicable = new HashSet<>(_dimNames); - dimNamesInveredSortedIndexNotApplicable.removeAll(_dimNamesInvertedSortedIndexApplicable); - - LOGGER.debug("_dimNamesInveredSortedIndexApplicable {}", _dimNamesInvertedSortedIndexApplicable); AtomicInteger counter = new AtomicInteger(0); - _dimNamesInvertedSortedIndexApplicable.forEach(name -> { + _columnNamesInvertedSortedIndexApplicable.forEach(name -> { _intToColNameMap[counter.get()] = name; _colNameToIntMap.put(name, counter.getAndIncrement()); }); - dimNamesInveredSortedIndexNotApplicable.forEach(name -> { - _intToColNameMap[counter.get()] = name; - _colNameToIntMap.put(name, counter.getAndIncrement()); - }); + _columnNamesInvertedSortedIndexApplicable.remove(sortedColumn); + _columnNamesInvertedSortedIndexApplicable.removeAll(invertedIndexColumns); + _columnNamesInvertedSortedIndexApplicable.removeAll(noDictionaryColumns); + + LOGGER.debug("_columnNamesInvertedSortedIndexApplicable {}", _columnNamesInvertedSortedIndexApplicable); LOGGER.debug("_dimNames{}", _dimNames); LOGGER.debug("_metricNames{}", _metricNames); LOGGER.debug("_dateTimeNames{}", _dateTimeNames); - _metricNames.forEach(name -> { - _intToColNameMap[counter.get()] = name; - _colNameToIntMap.put(name, counter.getAndIncrement()); - }); - _dateTimeNames.forEach(name -> { - _intToColNameMap[counter.get()] = name; - _colNameToIntMap.put(name, counter.getAndIncrement()); - }); - LOGGER.info("*Num dims we can apply index on: {}", getNumDimsInvertedSortedApplicable()); + LOGGER.info("*Num dims we can apply index on: {}", getNumColumnsInvertedSortedApplicable()); Review comment: Should "Num cols we ...." On that note, we should do a PR to fix logging. Too much of info level logs here. We should look at what can be info and what can be moved to debug. -- 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. 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