github-actions[bot] commented on code in PR #65588:
URL: https://github.com/apache/doris/pull/65588#discussion_r3578099222


##########
fe/fe-core/src/main/java/org/apache/doris/job/extensions/insert/streaming/DataSourceConfigValidator.java:
##########
@@ -106,14 +120,72 @@ public static void validateSource(Map<String, String> 
input,
                 throw new IllegalArgumentException("Unexpected key: '" + key + 
"'");
             }
 
+            if 
(DataSourceType.OCEANBASE.name().equalsIgnoreCase(dataSourceType)
+                    && OCEANBASE_UNSUPPORTED_KEYS.contains(key)) {
+                throw new IllegalArgumentException(
+                        "Property '" + key + "' is not supported for 
OceanBase");
+            }
+
             if (!isValidValue(key, value, dataSourceType)) {
                 throw new IllegalArgumentException("Invalid value for key '" + 
key + "': " + value);
             }
         }
 
+        validateOceanBaseSource(input, dataSourceType);
         validateSslVerifyCaPair(input);
     }
 
+    private static void validateOceanBaseSource(Map<String, String> input, 
String dataSourceType) {
+        if (!DataSourceType.OCEANBASE.name().equalsIgnoreCase(dataSourceType)) 
{
+            return;
+        }
+        if (input.containsKey(DataSourceConfigKeys.JDBC_URL)
+                && 
!input.get(DataSourceConfigKeys.JDBC_URL).startsWith(MYSQL_JDBC_URL_PREFIX)) {
+            throw new IllegalArgumentException(
+                    "OceanBase jdbc_url must start with '" + 
MYSQL_JDBC_URL_PREFIX + "'");
+        }
+    }

Review Comment:
   This CREATE-time preflight is also needed after ALTER merges source 
properties. `jdbc_url` is immutable, but OceanBase tenant selection is part of 
the username (`root@test` in these tests), and ALTER currently allows 
`user`/`password`/driver changes before `StreamingInsertJob.alterJob` persists 
the merged map. A job can therefore be created against a MySQL-mode tenant, 
paused, altered to credentials for an Oracle-mode tenant on the same endpoint, 
and then resumed; the CDC reader receives `OCEANBASE` plus the merged props and 
never rechecks `ob_compatibility_mode`. Please run this validation on the 
merged source properties in the ALTER path, or make the tenant-selecting 
credentials immutable for OceanBase jobs.



##########
docker/thirdparties/docker-compose/oceanbase/oceanbase.yaml.tpl:
##########
@@ -19,16 +19,18 @@ version: "2.1"
 
 services:
   doris--oceanbase:
-    image: oceanbase/oceanbase-ce:4.2.1-lts
+    image: quay.io/oceanbase/obbinlog-ce:4.2.5-test

Review Comment:
   This image switch drops the old init-script behavior that the rest of this 
compose file still depends on. The service still mounts `./init` to 
`/root/boot/init.d`, and the new healthcheck queries `doris_test.all_types`; 
that database/table is only created by the mounted init SQL in this repo. The 
obbinlog image startup path does not execute that directory, while 
`run-thirdparties-docker.sh` uses `docker compose up -d --wait`, so the 
container never becomes healthy and the existing OceanBase JDBC catalog suite 
has no seeded `all_types` table. Please either run the init SQL explicitly for 
the obbinlog image before the healthcheck depends on it, or move the 
seeded-data setup out of compose and make the healthcheck probe only 
Observer/OBProxy/binlog readiness.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to