albertobastos commented on code in PR #14830:
URL: https://github.com/apache/pinot/pull/14830#discussion_r1926469613


##########
pinot-query-planner/src/main/java/org/apache/pinot/query/catalog/PinotCatalog.java:
##########
@@ -80,8 +82,20 @@ public Table getTable(String name) {
    */
   @Override
   public Set<String> getTableNames() {
-    return _tableCache.getTableNameMap().keySet().stream().filter(n -> 
DatabaseUtils.isPartOfDatabase(n, _databaseName))
-        .collect(Collectors.toSet());
+    if (StringUtils.isBlank(_databaseName)) {
+      return _tableCache.getTableNameMap().keySet().stream()
+          .filter(n -> DatabaseUtils.isPartOfDatabase(n, _databaseName))
+          .collect(Collectors.toSet());
+    } else {
+      Set<String> result = new HashSet<>();
+      for (String n: _tableCache.getTableNameMap().keySet()) {

Review Comment:
   Done (and also unified both default and non-default scenarios after last 
modifications):
   
   
https://github.com/apache/pinot/pull/14830/commits/115908e1953cd4690e909307a441006c5ac0bd7f



##########
pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/OfflineClusterIntegrationTest.java:
##########
@@ -2994,25 +2994,29 @@ public void testCaseSensitivityV2()
     int daysSinceEpoch = 16138;
     int hoursSinceEpoch = 16138 * 24;
     int secondsSinceEpoch = 16138 * 24 * 60 * 60;
-    List<String> baseQueries = Arrays.asList("SELECT * FROM mytable",
-        "SELECT DaysSinceEpoch, timeConvert(DaysSinceEpoch,'DAYS','SECONDS') 
FROM mytable",
+    List<String> baseQueries = Arrays.asList("SELECT * FROM mytable limit 
10000",
+        "SELECT DaysSinceEpoch, timeConvert(DaysSinceEpoch,'DAYS','SECONDS') 
FROM mytable limit 10000",
         "SELECT DaysSinceEpoch, timeConvert(DaysSinceEpoch,'DAYS','SECONDS') 
FROM mytable order by DaysSinceEpoch "
             + "limit 10000",
         "SELECT DaysSinceEpoch, timeConvert(DaysSinceEpoch,'DAYS','SECONDS') 
FROM mytable order by timeConvert"
             + "(DaysSinceEpoch,'DAYS','SECONDS') DESC limit 10000",
-        "SELECT count(*) FROM mytable WHERE DaysSinceEpoch = " + 
daysSinceEpoch,
-        "SELECT count(*) FROM mytable WHERE 
timeConvert(DaysSinceEpoch,'DAYS','HOURS') = " + hoursSinceEpoch,
-        "SELECT count(*) FROM mytable WHERE 
timeConvert(DaysSinceEpoch,'DAYS','SECONDS') = " + secondsSinceEpoch,
-        "SELECT MAX(timeConvert(DaysSinceEpoch,'DAYS','SECONDS')) FROM 
mytable",
+        "SELECT count(*) FROM mytable WHERE DaysSinceEpoch = " + 
daysSinceEpoch + " limit 10000",
+        "SELECT count(*) FROM mytable WHERE 
timeConvert(DaysSinceEpoch,'DAYS','HOURS') = " + hoursSinceEpoch
+            + " limit 10000",
+        "SELECT count(*) FROM mytable WHERE 
timeConvert(DaysSinceEpoch,'DAYS','SECONDS') = " + secondsSinceEpoch
+            + " limit 10000",
+        "SELECT MAX(timeConvert(DaysSinceEpoch,'DAYS','SECONDS')) FROM mytable 
limit 10000",
         "SELECT COUNT(*) FROM mytable GROUP BY 
dateTimeConvert(DaysSinceEpoch,'1:DAYS:EPOCH','1:HOURS:EPOCH',"
-            + "'1:HOURS')");
+            + "'1:HOURS') limit 10000");
     List<String> queries = new ArrayList<>();
     baseQueries.forEach(q -> queries.add(q.replace("mytable", 
"MYTABLE").replace("DaysSinceEpoch", "DAYSSinceEpOch")));
     baseQueries.forEach(
-        q -> queries.add(q.replace("mytable", 
"MYDB.MYTABLE").replace("DaysSinceEpoch", "DAYSSinceEpOch")));
+        q -> queries.add(q.replace("mytable", 
"DEFAULT.MYTABLE").replace("DaysSinceEpoch", "DAYSSinceEpOch")));

Review Comment:
   We can, we should, and we have the tools to do it :)
   
   
https://github.com/apache/pinot/pull/14830/commits/cf4a72f5979137438bac62513f0097b51372248a
   
   I believe I didn't miss any test that can be unified.



##########
pinot-query-planner/src/main/java/org/apache/pinot/query/catalog/PinotCatalog.java:
##########
@@ -80,8 +82,20 @@ public Table getTable(String name) {
    */
   @Override
   public Set<String> getTableNames() {
-    return _tableCache.getTableNameMap().keySet().stream().filter(n -> 
DatabaseUtils.isPartOfDatabase(n, _databaseName))
-        .collect(Collectors.toSet());
+    if (StringUtils.isBlank(_databaseName)) {
+      return _tableCache.getTableNameMap().keySet().stream()
+          .filter(n -> DatabaseUtils.isPartOfDatabase(n, _databaseName))

Review Comment:
   That's a reasonable doubt, and we can be easily prepared against both 
scenarios.
   
   
https://github.com/apache/pinot/pull/14830/commits/115908e1953cd4690e909307a441006c5ac0bd7f



-- 
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

Reply via email to