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

Reply via email to