Remove the feature flag allow.table.name.with.database (#12402)

pinot-broker/src/test/java/org/apache/pinot/broker/requesthandler/BaseBrokerRequestHandlerTest.java

@@ -48,7 +48,6 @@
 import org.apache.pinot.spi.metrics.PinotMetricUtils;
 import org.apache.pinot.spi.trace.RequestContext;
 import org.apache.pinot.spi.trace.Tracing;
-import org.apache.pinot.spi.utils.CommonConstants;
 import org.apache.pinot.spi.utils.JsonUtils;
 import org.apache.pinot.sql.parsers.CalciteSqlParser;
 import org.apache.pinot.util.TestUtils;
@@ -144,46 +143,6 @@ public void testGetActualColumnNameCaseInSensitive() {
     Assert.assertEquals(wrongColumnName3, "mytable");
   }
 
-  @Test
-  public void testGetActualTableNameBanningDots() {
-    // not allowing dots
-    PinotConfiguration configuration = new PinotConfiguration();
-    configuration.setProperty(CommonConstants.Helix.ALLOW_TABLE_NAME_WITH_DATABASE, false);
-
-    TableCache tableCache = Mockito.mock(TableCache.class);
-    when(tableCache.isIgnoreCase()).thenReturn(true);
-    when(tableCache.getActualTableName("mytable")).thenReturn("mytable");
-    Assert.assertEquals(BaseBrokerRequestHandler.getActualTableName("mytable", tableCache), "mytable");
-    when(tableCache.getActualTableName("db.mytable")).thenReturn(null);
-    Assert.assertEquals(BaseBrokerRequestHandler.getActualTableName("db.mytable", tableCache), "mytable");
-
-    when(tableCache.isIgnoreCase()).thenReturn(false);
-    Assert.assertEquals(BaseBrokerRequestHandler.getActualTableName("db.mytable", tableCache), "mytable");
-  }
-
-  @Test
-  public void testGetActualTableNameAllowingDots() {
-
-    TableCache tableCache = Mockito.mock(TableCache.class);
-    when(tableCache.isIgnoreCase()).thenReturn(true);
-    // the tableCache should have only "db.mytable" in it since this is the only table
-    when(tableCache.getActualTableName("mytable")).thenReturn(null);
-    when(tableCache.getActualTableName("db.mytable")).thenReturn("db.mytable");
-    when(tableCache.getActualTableName("other.mytable")).thenReturn(null);
-    when(tableCache.getActualTableName("test_table")).thenReturn(null);
-
-    Assert.assertEquals(BaseBrokerRequestHandler.getActualTableName("test_table", tableCache), "test_table");
-    Assert.assertEquals(BaseBrokerRequestHandler.getActualTableName("mytable", tableCache), "mytable");
-
-    Assert.assertEquals(BaseBrokerRequestHandler.getActualTableName("db.mytable", tableCache), "db.mytable");
-    Assert.assertEquals(BaseBrokerRequestHandler.getActualTableName("other.mytable", tableCache), "other.mytable");
-
-    when(tableCache.isIgnoreCase()).thenReturn(false);
-    Assert.assertEquals(BaseBrokerRequestHandler.getActualTableName("db.mytable", tableCache), "db.mytable");
-    Assert.assertEquals(BaseBrokerRequestHandler.getActualTableName("db.namespace.mytable", tableCache),
-        "db.namespace.mytable");
-  }
-
   @Test
   public void testCancelQuery()
       throws Exception {

pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java

@@ -215,12 +215,7 @@ public ConfigSuccessResponse addTable(String tableConfigStr,
 
       // TableConfigUtils.validate(...) is used across table create/update.
       TableConfigUtils.validate(tableConfig, schema, typesToSkip, _controllerConf.isDisableIngestionGroovy());
-      // TableConfigUtils.validateTableName(...) checks table name rules.
-      // So it won't affect already created tables.
-      boolean allowTableNameWithDatabase =
-          _controllerConf.getProperty(CommonConstants.Helix.ALLOW_TABLE_NAME_WITH_DATABASE,
-              CommonConstants.Helix.DEFAULT_ALLOW_TABLE_NAME_WITH_DATABASE);
-      TableConfigUtils.validateTableName(tableConfig, allowTableNameWithDatabase);
+      TableConfigUtils.validateTableName(tableConfig);
     } catch (Exception e) {
       throw new ControllerApplicationException(LOGGER, e.getMessage(), Response.Status.BAD_REQUEST, e);
     }

pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/TableConfigsRestletResource.java

@@ -70,7 +70,6 @@
 import org.apache.pinot.spi.config.TableConfigs;
 import org.apache.pinot.spi.config.table.TableConfig;
 import org.apache.pinot.spi.data.Schema;
-import org.apache.pinot.spi.utils.CommonConstants;
 import org.apache.pinot.spi.utils.JsonUtils;
 import org.apache.pinot.spi.utils.builder.TableNameBuilder;
 import org.glassfish.grizzly.http.server.Request;
@@ -463,21 +462,18 @@ private TableConfigs validateConfig(TableConfigs tableConfigs, @Nullable String
       Preconditions.checkState(rawTableName.equals(schema.getSchemaName()),
           "'tableName': %s must be equal to 'schemaName' from 'schema': %s", rawTableName, schema.getSchemaName());
       SchemaUtils.validate(schema);
-      boolean allowTableNameWithDatabase = _controllerConf.getProperty(
-          CommonConstants.Helix.ALLOW_TABLE_NAME_WITH_DATABASE,
-          CommonConstants.Helix.DEFAULT_ALLOW_TABLE_NAME_WITH_DATABASE);
       if (offlineTableConfig != null) {
         String offlineRawTableName = TableNameBuilder.extractRawTableName(offlineTableConfig.getTableName());
         Preconditions.checkState(offlineRawTableName.equals(rawTableName),
             "Name in 'offline' table config: %s must be equal to 'tableName': %s", offlineRawTableName, rawTableName);
-        TableConfigUtils.validateTableName(offlineTableConfig, allowTableNameWithDatabase);
+        TableConfigUtils.validateTableName(offlineTableConfig);
         TableConfigUtils.validate(offlineTableConfig, schema, typesToSkip, _controllerConf.isDisableIngestionGroovy());
       }
       if (realtimeTableConfig != null) {
         String realtimeRawTableName = TableNameBuilder.extractRawTableName(realtimeTableConfig.getTableName());
         Preconditions.checkState(realtimeRawTableName.equals(rawTableName),
             "Name in 'realtime' table config: %s must be equal to 'tableName': %s", realtimeRawTableName, rawTableName);
-        TableConfigUtils.validateTableName(realtimeTableConfig, allowTableNameWithDatabase);
+        TableConfigUtils.validateTableName(realtimeTableConfig);
         TableConfigUtils.validate(realtimeTableConfig, schema, typesToSkip, _controllerConf.isDisableIngestionGroovy());
       }
       if (offlineTableConfig != null && realtimeTableConfig != null) {

pinot-segment-local/src/main/java/org/apache/pinot/segment/local/utils/TableConfigUtils.java

@@ -195,21 +195,16 @@ private static Set<ValidationType> parseTypesToSkipString(@Nullable String types
   /**
    * Validates the table name with the following rules:
    * <ul>
-   *   <li>If there is a flag allowing database name in it, table name can have one dot in it.
-   *   <li>Otherwise, there is no dot allowed in table name.</li>
+   *   <li>Table name can have at most one dot in it.
+   *   <li>Table name does not have whitespace.</li>
    * </ul>
    */
-  public static void validateTableName(TableConfig tableConfig, boolean allowTableNameWithDatabase) {
+  public static void validateTableName(TableConfig tableConfig) {
     String tableName = tableConfig.getTableName();
     int dotCount = StringUtils.countMatches(tableName, '.');
-    // For transitioning into full [database_name].[table_name] support, we allow the table name
-    // with one dot at max, so the admin may create mydb.mytable with a feature knob.
-    if (allowTableNameWithDatabase && dotCount > 1) {
+    if (dotCount > 1) {
       throw new IllegalStateException("Table name: '" + tableName + "' containing more than one '.' is not allowed");
     }
-    if (!allowTableNameWithDatabase && dotCount > 0) {
-      throw new IllegalStateException("Table name: '" + tableName + "' containing '.' is not allowed");
-    }
     if (StringUtils.containsWhitespace(tableName)) {
       throw new IllegalStateException("Table name: '" + tableName + "' containing space is not allowed");
     }

pinot-segment-local/src/test/java/org/apache/pinot/segment/local/utils/TableConfigUtilsTest.java

@@ -961,33 +961,23 @@ public void validateTierConfigs() {
 
   @Test
   public void testTableName() {
-    String[] malformedTableName = {"test.table", "test table"};
+    String[] malformedTableName = {"test.test.table", "test table"};
     for (int i = 0; i < 2; i++) {
       String tableName = malformedTableName[i];
       TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE).setTableName(tableName).build();
       try {
-        TableConfigUtils.validateTableName(tableConfig, CommonConstants.Helix.DEFAULT_ALLOW_TABLE_NAME_WITH_DATABASE);
+        TableConfigUtils.validateTableName(tableConfig);
         Assert.fail("Should fail for malformed table name : " + tableName);
       } catch (IllegalStateException e) {
         // expected
       }
     }
 
-    String allowedWithConfig = "test.table";
-    TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE).setTableName(allowedWithConfig).build();
-    try {
-      TableConfigUtils.validateTableName(tableConfig, true);
-    } catch (IllegalStateException e) {
-      Assert.fail("Should allow table name with dot if configuration is turned on");
-    }
-
-    String rejected = "test.another.table";
-    tableConfig = new TableConfigBuilder(TableType.OFFLINE).setTableName(rejected).build();
-    try {
-      TableConfigUtils.validateTableName(tableConfig, true);
-      Assert.fail("Should fail for malformed table name : " + rejected);
-    } catch (IllegalStateException e) {
-      // expected
+    String[] allowedTableName = {"test.table", "testTable"};
+    for (int i = 0; i < 2; i++) {
+      String tableName = allowedTableName[i];
+      TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE).setTableName(tableName).build();
+      TableConfigUtils.validateTableName(tableConfig);
     }
   }
 

pinot-spi/src/main/java/org/apache/pinot/spi/utils/CommonConstants.java

@@ -91,8 +91,6 @@ public static class Helix {
 
     public static final String ENABLE_CASE_INSENSITIVE_KEY = "enable.case.insensitive";
     public static final boolean DEFAULT_ENABLE_CASE_INSENSITIVE = true;
-    public static final String ALLOW_TABLE_NAME_WITH_DATABASE = "allow.table.name.with.database";
-    public static final boolean DEFAULT_ALLOW_TABLE_NAME_WITH_DATABASE = false;
 
     public static final String DEFAULT_HYPERLOGLOG_LOG2M_KEY = "default.hyperloglog.log2m";
     public static final int DEFAULT_HYPERLOGLOG_LOG2M = 8;