[GitHub] [doris] morningman commented on a diff in pull request #20566: [Refactor](External) Move Common ODBC Methods to JDBC Class and Add Default config to Disable ODBC Creation

["morningman (via GitHub)" <gi...@apache.org>]

morningman commented on code in PR #20566:
URL: https://github.com/apache/doris/pull/20566#discussion_r1225090144


##########
fe/fe-core/src/main/java/org/apache/doris/analysis/CreateResourceStmt.java:
##########
@@ -93,6 +94,9 @@ public void analyze(Analyzer analyzer) throws UserException {
         if (resourceType == ResourceType.SPARK && !isExternal) {
             throw new AnalysisException("Spark is external resource");
         }
+        if (resourceType == ResourceType.ODBC_CATALOG && !Config.enable_odbc_table) {
+            throw new AnalysisException("Do not support odbc external table, please set enable_odbc_table as true");

Review Comment:
   ```suggestion
               throw new AnalysisException("ODBC table is deprecated, use JDBC instead. Or you can set `enable_odbc_table=true` in fe.conf to enable ODBC again.");
   ```



##########
fe/fe-common/src/main/java/org/apache/doris/common/Config.java:
##########
@@ -1766,6 +1766,12 @@ public class Config extends ConfigBase {
     @ConfField(mutable = false, expType = ExperimentalType.EXPERIMENTAL)
     public static boolean enable_fqdn_mode = false;
 
+    /**
+     * enable use odbc table
+     */
+    @ConfField(mutable = true, masterOnly = true)
+    public static boolean enable_odbc_table = false;

Review Comment:
   use `description` field in `ConfField` annotation to add document of this config.



##########
fe/fe-core/src/main/java/org/apache/doris/analysis/CreateTableStmt.java:
##########
@@ -626,6 +626,9 @@ private void analyzeEngineName() throws AnalysisException {
         if (engineName.equals("mysql") || engineName.equals("odbc") || engineName.equals("broker")
                 || engineName.equals("elasticsearch") || engineName.equals("hive")
                 || engineName.equals("iceberg") || engineName.equals("hudi") || engineName.equals("jdbc")) {
+            if (engineName.equals("odbc") && !Config.enable_odbc_table) {
+                throw new AnalysisException("Do not support odbc external table, please set enable_odbc_table as true");

Review Comment:
   ditto



-- 
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-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org