Jackie-Jiang commented on code in PR #14462:
URL: https://github.com/apache/pinot/pull/14462#discussion_r1844402887
##########
pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/BaseClusterIntegrationTestSet.java:
##########
@@ -788,43 +760,66 @@ public void testReload(boolean includeOfflineTable)
JsonNode resultTable = queryResponse.get("resultTable");
assertEquals(resultTable.get("dataSchema").get("columnNames").size(),
schema.size());
assertEquals(resultTable.get("rows").size(), 10);
+ assertEquals(queryResponse.get("totalDocs").asLong(), numTotalDocs);
+
// Test aggregation query to include querying all segemnts (including
realtime)
String aggregationQuery = "SELECT SUMMV(NewIntMVDimension) FROM " +
rawTableName;
queryResponse = postQuery(aggregationQuery);
assertEquals(queryResponse.get("exceptions").size(), 0);
- // Test filter on all new added columns
- String countStarQuery = "SELECT COUNT(*) FROM " + rawTableName
- + " WHERE NewIntSVDimension < 0 AND NewLongSVDimension < 0 AND
NewFloatSVDimension < 0 AND "
- + "NewDoubleSVDimension < 0 AND NewStringSVDimension = 'null' AND
NewIntMVDimension < 0 AND "
- + "NewLongMVDimension < 0 AND NewFloatMVDimension < 0 AND
NewDoubleMVDimension < 0 AND "
- + "NewStringMVDimension = 'null' AND NewIntMetric = 0 AND
NewLongMetric = 0 AND NewFloatMetric = 0 "
- + "AND NewDoubleMetric = 0 AND NewBytesMetric = ''";
- queryResponse = postQuery(countStarQuery);
- assertEquals(queryResponse.get("exceptions").size(), 0);
-
assertEquals(queryResponse.get("resultTable").get("rows").get(0).get(0).asLong(),
countStarResult);
+ //tests that reload is not needed on the table after reloading all
segments from controller api
Review Comment:
(minor) Add a space and capitalize
```suggestion
// Tests that reload is not needed on the table after reloading all
segments from controller api
```
##########
pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/BaseClusterIntegrationTestSet.java:
##########
@@ -714,58 +714,29 @@ public void testReload(boolean includeOfflineTable)
assertEquals(queryResponse.get("resultTable").get("dataSchema").get("columnNames").size(),
schema.size());
long numTotalDocs = queryResponse.get("totalDocs").asLong();
- schema.addField(constructNewDimension(FieldSpec.DataType.INT, true));
- schema.addField(constructNewDimension(FieldSpec.DataType.LONG, true));
- schema.addField(constructNewDimension(FieldSpec.DataType.FLOAT, true));
- schema.addField(constructNewDimension(FieldSpec.DataType.DOUBLE, true));
- schema.addField(constructNewDimension(FieldSpec.DataType.STRING, true));
- schema.addField(constructNewDimension(FieldSpec.DataType.INT, false));
- schema.addField(constructNewDimension(FieldSpec.DataType.LONG, false));
- schema.addField(constructNewDimension(FieldSpec.DataType.FLOAT, false));
- schema.addField(constructNewDimension(FieldSpec.DataType.DOUBLE, false));
- schema.addField(constructNewDimension(FieldSpec.DataType.STRING, false));
- schema.addField(constructNewMetric(FieldSpec.DataType.INT));
- schema.addField(constructNewMetric(FieldSpec.DataType.LONG));
- schema.addField(constructNewMetric(FieldSpec.DataType.FLOAT));
- schema.addField(constructNewMetric(FieldSpec.DataType.DOUBLE));
- schema.addField(constructNewMetric(FieldSpec.DataType.BYTES));
-
- // Upload the schema with extra columns
- addSchema(schema);
+ addNewSchemaFields(schema);
String tableNameWithTypeOffline =
TableNameBuilder.forType(TableType.OFFLINE).tableNameWithType(rawTableName);
String tableNameWithTypeRealtime =
TableNameBuilder.forType(TableType.REALTIME).tableNameWithType(rawTableName);
- // Reload the table
+ String reloadJob;
+ //tests that reload is needed on the table from controller api
segments/{tableNameWithType}/needReload
Review Comment:
(minor) Add a space and capitalize, same for other places
```suggestion
// Tests that reload is needed on the table from controller api
segments/{tableNameWithType}/needReload
```
##########
pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/BaseClusterIntegrationTestSet.java:
##########
@@ -788,43 +760,66 @@ public void testReload(boolean includeOfflineTable)
JsonNode resultTable = queryResponse.get("resultTable");
assertEquals(resultTable.get("dataSchema").get("columnNames").size(),
schema.size());
assertEquals(resultTable.get("rows").size(), 10);
+ assertEquals(queryResponse.get("totalDocs").asLong(), numTotalDocs);
+
// Test aggregation query to include querying all segemnts (including
realtime)
String aggregationQuery = "SELECT SUMMV(NewIntMVDimension) FROM " +
rawTableName;
queryResponse = postQuery(aggregationQuery);
assertEquals(queryResponse.get("exceptions").size(), 0);
- // Test filter on all new added columns
- String countStarQuery = "SELECT COUNT(*) FROM " + rawTableName
- + " WHERE NewIntSVDimension < 0 AND NewLongSVDimension < 0 AND
NewFloatSVDimension < 0 AND "
- + "NewDoubleSVDimension < 0 AND NewStringSVDimension = 'null' AND
NewIntMVDimension < 0 AND "
- + "NewLongMVDimension < 0 AND NewFloatMVDimension < 0 AND
NewDoubleMVDimension < 0 AND "
- + "NewStringMVDimension = 'null' AND NewIntMetric = 0 AND
NewLongMetric = 0 AND NewFloatMetric = 0 "
- + "AND NewDoubleMetric = 0 AND NewBytesMetric = ''";
- queryResponse = postQuery(countStarQuery);
- assertEquals(queryResponse.get("exceptions").size(), 0);
-
assertEquals(queryResponse.get("resultTable").get("rows").get(0).get(0).asLong(),
countStarResult);
+ //tests that reload is not needed on the table after reloading all
segments from controller api
+ // segments/{tableNameWithType}/needReload
if (includeOfflineTable) {
- String needAfterReloadResponseWithNoVerbose =
checkIfReloadIsNeeded(tableNameWithTypeOffline, false);
- String needAfterReloadResponseWithVerbose =
checkIfReloadIsNeeded(tableNameWithTypeOffline, true);
- JsonNode jsonNeedReloadResponseAfterWithNoVerbose =
- JsonUtils.stringToJsonNode(needAfterReloadResponseWithNoVerbose);
- JsonNode jsonNeedReloadResponseAfterWithVerbose =
JsonUtils.stringToJsonNode(needAfterReloadResponseWithVerbose);
- //test to check if reload on offline table is needed i.e false after
reload is finished
-
assertFalse(jsonNeedReloadResponseAfterWithNoVerbose.get("needReload").asBoolean());
-
assertFalse(jsonNeedReloadResponseAfterWithVerbose.get("needReload").asBoolean());
-
assertFalse(jsonNeedReloadResponseRealTimeWithVerbose.get("serverToSegmentsCheckReloadList").isEmpty());
+ testTableNeedReloadFalse(tableNameWithTypeOffline);
}
- String needAfterReloadResponseRealtimeWithNoVerbose =
checkIfReloadIsNeeded(tableNameWithTypeRealtime, false);
- String needAfterReloadResponseRealTimeWithVerbose =
checkIfReloadIsNeeded(tableNameWithTypeRealtime, true);
- JsonNode jsonNeedReloadResponseRealtimeAfterWithNoVerbose =
-
JsonUtils.stringToJsonNode(needAfterReloadResponseRealtimeWithNoVerbose);
- JsonNode jsonNeedReloadResponseRealtimeAfterWithVerbose =
- JsonUtils.stringToJsonNode(needAfterReloadResponseRealTimeWithVerbose);
-
- //test to check if reload on real time table is needed i.e false after
reload is finished
-
assertFalse(jsonNeedReloadResponseRealtimeAfterWithNoVerbose.get("needReload").asBoolean());
-
assertFalse(jsonNeedReloadResponseRealtimeAfterWithVerbose.get("needReload").asBoolean());
-
assertFalse(jsonNeedReloadResponseRealtimeAfterWithVerbose.get("serverToSegmentsCheckReloadList").isEmpty());
+ testTableNeedReloadFalse(tableNameWithTypeRealtime);
+ }
+
+ private void addNewSchemaFields(Schema schema)
+ throws IOException {
+ schema.addField(constructNewDimension(FieldSpec.DataType.INT, true));
+ schema.addField(constructNewDimension(FieldSpec.DataType.LONG, true));
+ schema.addField(constructNewDimension(FieldSpec.DataType.FLOAT, true));
+ schema.addField(constructNewDimension(FieldSpec.DataType.DOUBLE, true));
+ schema.addField(constructNewDimension(FieldSpec.DataType.STRING, true));
+ schema.addField(constructNewDimension(FieldSpec.DataType.INT, false));
+ schema.addField(constructNewDimension(FieldSpec.DataType.LONG, false));
+ schema.addField(constructNewDimension(FieldSpec.DataType.FLOAT, false));
+ schema.addField(constructNewDimension(FieldSpec.DataType.DOUBLE, false));
+ schema.addField(constructNewDimension(FieldSpec.DataType.STRING, false));
+ schema.addField(constructNewMetric(FieldSpec.DataType.INT));
+ schema.addField(constructNewMetric(FieldSpec.DataType.LONG));
+ schema.addField(constructNewMetric(FieldSpec.DataType.FLOAT));
+ schema.addField(constructNewMetric(FieldSpec.DataType.DOUBLE));
+ schema.addField(constructNewMetric(FieldSpec.DataType.BYTES));
+ // Upload the schema with extra columns
+ addSchema(schema);
+ }
+
+ private void testTableNeedReloadFalse(String tableNameWithTypeOffline)
+ throws IOException {
+ String needAfterReloadResponseWithNoVerbose =
checkIfReloadIsNeeded(tableNameWithTypeOffline, false);
+ String needAfterReloadResponseWithVerbose =
checkIfReloadIsNeeded(tableNameWithTypeOffline, true);
+ JsonNode jsonNeedReloadResponseAfterWithNoVerbose =
+ JsonUtils.stringToJsonNode(needAfterReloadResponseWithNoVerbose);
+ JsonNode jsonNeedReloadResponseAfterWithVerbose =
JsonUtils.stringToJsonNode(needAfterReloadResponseWithVerbose);
+ //test to check if reload on offline table is needed i.e false after
reload is finished
+
assertFalse(jsonNeedReloadResponseAfterWithNoVerbose.get("needReload").asBoolean());
+
assertFalse(jsonNeedReloadResponseAfterWithVerbose.get("needReload").asBoolean());
+
assertFalse(jsonNeedReloadResponseAfterWithVerbose.get("serverToSegmentsCheckReloadList").isEmpty());
+ }
+
+ private JsonNode testTableNeedReloadTrue(String tableNameWithTypeOffline)
+ throws IOException {
+ String needBeforeReloadResponseWithNoVerbose =
checkIfReloadIsNeeded(tableNameWithTypeOffline, false);
+ String needBeforeReloadResponseWithVerbose =
checkIfReloadIsNeeded(tableNameWithTypeOffline, true);
+ JsonNode jsonNeedReloadResponseWithNoVerbose =
JsonUtils.stringToJsonNode(needBeforeReloadResponseWithNoVerbose);
+ JsonNode jsonNeedReloadResponseWithVerbose =
JsonUtils.stringToJsonNode(needBeforeReloadResponseWithVerbose);
+ //test to check if reload is needed i.e true
+
assertTrue(jsonNeedReloadResponseWithNoVerbose.get("needReload").asBoolean());
+
assertTrue(jsonNeedReloadResponseWithVerbose.get("needReload").asBoolean());
+
assertFalse(jsonNeedReloadResponseWithVerbose.get("serverToSegmentsCheckReloadList").isEmpty());
+ return jsonNeedReloadResponseWithNoVerbose;
}
Review Comment:
These 2 can be merged by adding a boolean argument `expectedNeedReload`
##########
pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/BaseClusterIntegrationTestSet.java:
##########
@@ -714,58 +714,29 @@ public void testReload(boolean includeOfflineTable)
assertEquals(queryResponse.get("resultTable").get("dataSchema").get("columnNames").size(),
schema.size());
long numTotalDocs = queryResponse.get("totalDocs").asLong();
- schema.addField(constructNewDimension(FieldSpec.DataType.INT, true));
- schema.addField(constructNewDimension(FieldSpec.DataType.LONG, true));
- schema.addField(constructNewDimension(FieldSpec.DataType.FLOAT, true));
- schema.addField(constructNewDimension(FieldSpec.DataType.DOUBLE, true));
- schema.addField(constructNewDimension(FieldSpec.DataType.STRING, true));
- schema.addField(constructNewDimension(FieldSpec.DataType.INT, false));
- schema.addField(constructNewDimension(FieldSpec.DataType.LONG, false));
- schema.addField(constructNewDimension(FieldSpec.DataType.FLOAT, false));
- schema.addField(constructNewDimension(FieldSpec.DataType.DOUBLE, false));
- schema.addField(constructNewDimension(FieldSpec.DataType.STRING, false));
- schema.addField(constructNewMetric(FieldSpec.DataType.INT));
- schema.addField(constructNewMetric(FieldSpec.DataType.LONG));
- schema.addField(constructNewMetric(FieldSpec.DataType.FLOAT));
- schema.addField(constructNewMetric(FieldSpec.DataType.DOUBLE));
- schema.addField(constructNewMetric(FieldSpec.DataType.BYTES));
-
- // Upload the schema with extra columns
- addSchema(schema);
+ addNewSchemaFields(schema);
String tableNameWithTypeOffline =
TableNameBuilder.forType(TableType.OFFLINE).tableNameWithType(rawTableName);
String tableNameWithTypeRealtime =
TableNameBuilder.forType(TableType.REALTIME).tableNameWithType(rawTableName);
Review Comment:
Not introduced in this PR, but we usually call them `offlineTableName` and
`realtimeTableName` for concise
--
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
