vrajat commented on code in PR #15900:
URL: https://github.com/apache/pinot/pull/15900#discussion_r2115344865
##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java:
##########
@@ -422,6 +423,19 @@ public SuccessResponse deleteTable(
List<String> tablesDeleted = new LinkedList<>();
try {
tableName = DatabaseUtils.translateTableName(tableName, headers);
+ // Validate the table is not referenced in any logical table config.
Review Comment:
Still confused by the possiblities. Also the truth table in
`verifyTableType` confused me more.
Will the validation fail if `tableType` is null ? `tableName.equals` will
fail. Is it better to move these tests into the two if clauses ? So in each of
the if clause below:
```
if (tableExists) {
// Check in all logical table configs.
}
##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/TableConfigsRestletResource.java:
##########
@@ -283,6 +285,17 @@ public SuccessResponse deleteConfig(
String tableName, @Context HttpHeaders headers) {
try {
tableName = DatabaseUtils.translateTableName(tableName, headers);
+
+ // Validate the table is not referenced in any logical table config.
Review Comment:
Is the physical table name with `OFFLINE|REALTIME` a valid input ? If yes,
then should the code extract raw table name before checking for equality in
`validateRefTableName` ?
--
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
