yashmayya commented on code in PR #15773:
URL: https://github.com/apache/pinot/pull/15773#discussion_r2115465839
##########
pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/logicaltable/BaseLogicalTableIntegrationTest.java:
##########
@@ -558,4 +594,28 @@ public void testQueryTimeOut()
exceptions = response.get("exceptions");
assertTrue(exceptions.isEmpty(), "Query should not throw exception");
}
+
+ @Test(dataProvider = "useBothQueryEngines")
+ public void testLogicalTableWithEmptyOfflineTable(boolean
useMultiStageQueryEngine)
+ throws Exception {
+
+ setUseMultiStageQueryEngine(useMultiStageQueryEngine);
+
+ String logicalTableName = EMPTY_OFFLINE_TABLE_NAME + "_logical";
+ // Query should return empty result
+ JsonNode queryResponse = postQuery("SELECT count(*) FROM " +
logicalTableName);
+ assertEquals(queryResponse.get("numDocsScanned").asInt(), 0);
+ assertEquals(queryResponse.get("numServersQueried").asInt(),
useMultiStageQueryEngine ? 1 : 0);
+ assertTrue(queryResponse.get("exceptions").isEmpty());
+ }
+
+ @Test(dataProvider = "useBothQueryEngines")
+ void testControllerQuerySubmit(boolean useMultiStageQueryEngine)
+ throws Exception {
+ setUseMultiStageQueryEngine(useMultiStageQueryEngine);
+ @Language("sql")
+ String query = "SELECT count(*) FROM " + getLogicalTableName();
+ JsonNode response = postQueryToController(query);
+ assertTrue(response.get("exceptions").isEmpty());
Review Comment:
```suggestion
assertNoError(response)
```
##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotQueryResource.java:
##########
@@ -240,12 +241,13 @@ private List<String> getInstanceIds(String query,
List<String> tableNames, Strin
List<String> instanceIds;
if (!tableNames.isEmpty()) {
List<TableConfig> tableConfigList = getListTableConfigs(tableNames,
database);
- if (tableConfigList == null || tableConfigList.isEmpty()) {
+ List<LogicalTableConfig> logicalTableConfigList =
getListLogicalTableConfigs(tableNames, database);
+ if ((tableConfigList == null || tableConfigList.isEmpty()) &&
logicalTableConfigList.isEmpty()) {
Review Comment:
`tableConfigList` can only be empty right?
##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotQueryResource.java:
##########
@@ -356,10 +372,17 @@ private List<String>
findCommonBrokerInstances(Set<String> brokerTenants) {
}
// return the union of brokerTenants from the tables list.
- private Set<String> getBrokerTenantsUnion(List<TableConfig> tableConfigList)
{
+ private Set<String> getBrokerTenantsUnion(@Nullable List<TableConfig>
tableConfigList,
Review Comment:
Same comment as above, it doesn't seem like `tableConfigList` can be `null`,
only empty (same as `logicalTableConfigList`) so it looks a little strange that
we're handling the two differently.
--
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]
