This is an automated email from the ASF dual-hosted git repository.
Jackie-Jiang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pinot.git
The following commit(s) were added to refs/heads/master by this push:
new 47a5f8671f6 Return HTTP 400 for DDL table validator rejections (#19544)
47a5f8671f6 is described below
commit 47a5f8671f6626f95ddb329c0bb9b2eec27dce43
Author: Xiang Fu <[email protected]>
AuthorDate: Mon Sep 14 13:28:05 2026 -0700
Return HTTP 400 for DDL table validator rejections (#19544)
---
.../api/resources/PinotDdlRestletResource.java | 3 +-
.../api/PinotDdlRestletResourceTest.java | 39 ++++++++++++++++++++++
2 files changed, 41 insertions(+), 1 deletion(-)
diff --git
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotDdlRestletResource.java
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotDdlRestletResource.java
index 9a7c72caadd..d640f88687c 100644
---
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotDdlRestletResource.java
+++
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotDdlRestletResource.java
@@ -76,6 +76,7 @@ import org.apache.pinot.spi.data.DateTimeFieldSpec;
import org.apache.pinot.spi.data.FieldSpec;
import org.apache.pinot.spi.data.LogicalTableConfig;
import org.apache.pinot.spi.data.Schema;
+import org.apache.pinot.spi.exception.ConfigValidationException;
import org.apache.pinot.spi.exception.DatabaseConflictException;
import org.apache.pinot.spi.utils.CommonConstants;
import org.apache.pinot.spi.utils.JsonUtils;
@@ -603,7 +604,7 @@ public class PinotDdlRestletResource {
_pinotHelixResourceManager, _controllerConf, _pinotTaskManager);
} catch (ControllerApplicationException e) {
throw e;
- } catch (IllegalArgumentException | IllegalStateException e) {
+ } catch (IllegalArgumentException | IllegalStateException |
ConfigValidationException e) {
// The Pinot validators consistently raise these for user-facing config
errors
// (upsert without primary keys, field configs referencing non-existent
columns,
// bad task configs, etc.). Surface as 400 — the caller can fix their
DDL.
diff --git
a/pinot-controller/src/test/java/org/apache/pinot/controller/api/PinotDdlRestletResourceTest.java
b/pinot-controller/src/test/java/org/apache/pinot/controller/api/PinotDdlRestletResourceTest.java
index 8d1b9d6a857..9203bf91d9c 100644
---
a/pinot-controller/src/test/java/org/apache/pinot/controller/api/PinotDdlRestletResourceTest.java
+++
b/pinot-controller/src/test/java/org/apache/pinot/controller/api/PinotDdlRestletResourceTest.java
@@ -25,14 +25,19 @@ import java.util.Map;
import org.apache.commons.lang3.StringUtils;
import org.apache.commons.lang3.tuple.Pair;
import org.apache.pinot.controller.helix.ControllerTest;
+import org.apache.pinot.spi.config.table.TableConfigValidator;
+import org.apache.pinot.spi.config.table.TableConfigValidatorRegistry;
+import org.apache.pinot.spi.exception.ConfigValidationException;
import org.apache.pinot.spi.utils.JsonUtils;
import org.testng.annotations.AfterClass;
import org.testng.annotations.BeforeClass;
+import org.testng.annotations.DataProvider;
import org.testng.annotations.Test;
import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertFalse;
import static org.testng.Assert.assertNotNull;
+import static org.testng.Assert.assertNull;
import static org.testng.Assert.assertTrue;
import static org.testng.Assert.fail;
@@ -103,6 +108,40 @@ public class PinotDdlRestletResourceTest extends
ControllerTest {
"Dry-run table must not be persisted; got " + listResponse);
}
+ @DataProvider(name = "tableValidatorFailures")
+ public Object[][] tableValidatorFailures() {
+ return new Object[][]{{false, true}, {true, true}, {false, false}, {true,
false}};
+ }
+
+ @Test(dataProvider = "tableValidatorFailures")
+ public void tableValidatorFailureDoesNotPersist(boolean dryRun, boolean
configValidationFailure)
+ throws IOException {
+ String tableName = "ddlValidatorFailure" + dryRun +
configValidationFailure;
+ String tableNameWithType = tableName + "_OFFLINE";
+ String message = "Validator rejected column id for " + tableName;
+ TableConfigValidator validator = (tableConfig, schema) -> {
+ if (tableNameWithType.equals(tableConfig.getTableName())) {
+ if (configValidationFailure) {
+ throw new ConfigValidationException(message);
+ }
+ throw new RuntimeException(message);
+ }
+ };
+ TableConfigValidatorRegistry.register(validator);
+ try {
+ String url = DEFAULT_INSTANCE.getControllerBaseApiUrl() +
"/sql/ddl?dryRun=" + dryRun;
+ String sql = "CREATE TABLE " + tableName + " (id INT) TABLE_TYPE =
OFFLINE";
+ Pair<Integer, String> response = postRequestWithStatusCode(url,
+ JsonUtils.objectToString(Map.of("sql", sql)));
+ assertEquals(response.getLeft().intValue(), configValidationFailure ?
400 : 500, response.getRight());
+ assertTrue(response.getRight().contains(message), response.getRight());
+
assertNull(DEFAULT_INSTANCE.getHelixResourceManager().getTableConfig(tableNameWithType));
+
assertNull(DEFAULT_INSTANCE.getHelixResourceManager().getSchema(tableName));
+ } finally {
+ TableConfigValidatorRegistry.unregister(validator);
+ }
+ }
+
@Test
public void createIfNotExistsIsIdempotent()
throws IOException {
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
