You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by GitBox <gi...@apache.org> on 2023/01/07 02:34:12 UTC

[GitHub] [doris] morningman opened a new pull request, #15692: [fix](multi-catalog) fix image writing bug when create catalog with resource

morningman opened a new pull request, #15692:
URL: https://github.com/apache/doris/pull/15692

   # Proposed changes
   
   Issue Number: close #xxx
   
   ## Problem summary
   
   Describe your changes.
   
   ## Checklist(Required)
   
   1. Does it affect the original behavior: 
       - [ ] Yes
       - [ ] No
       - [ ] I don't know
   2. Has unit tests been added:
       - [ ] Yes
       - [ ] No
       - [ ] No Need
   3. Has document been added or modified:
       - [ ] Yes
       - [ ] No
       - [ ] No Need
   4. Does it need to update dependencies:
       - [ ] Yes
       - [ ] No
   5. Are there any changes that cannot be rolled back:
       - [ ] Yes (If Yes, please explain WHY)
       - [ ] No
   
   ## Further comments
   
   If this is a relatively large or complex change, kick off the discussion at [dev@doris.apache.org](mailto:dev@doris.apache.org) by explaining why you chose the solution you did and what alternatives you considered, etc...
   
   


-- 
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


[GitHub] [doris] yiguolei merged pull request #15692: [fix](multi-catalog) fix image loading failture when create catalog with resource

Posted by GitBox <gi...@apache.org>.
yiguolei merged PR #15692:
URL: https://github.com/apache/doris/pull/15692


-- 
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


[GitHub] [doris] BePPPower commented on a diff in pull request #15692: [fix](multi-catalog) fix image loading failture when create catalog with resource

Posted by GitBox <gi...@apache.org>.
BePPPower commented on code in PR #15692:
URL: https://github.com/apache/doris/pull/15692#discussion_r1063959340


##########
fe/fe-core/src/main/java/org/apache/doris/catalog/JdbcResource.java:
##########
@@ -189,29 +190,61 @@ private void computeObjectChecksum() throws DdlException {
                 }
                 digest.update(buf, 0, bytesRead);
             } while (true);
-            String checkSum = Hex.encodeHexString(digest.digest());
-            configs.put(CHECK_SUM, checkSum);
+            return Hex.encodeHexString(digest.digest());
         } catch (IOException e) {
-            throw new DdlException("compute driver checksum from url: " + getProperty(DRIVER_URL)
+            throw new DdlException("compute driver checksum from url: " + driverPath
                     + " meet an IOException: " + e.getMessage());
         } catch (NoSuchAlgorithmException e) {
-            throw new DdlException("compute driver checksum from url: " + getProperty(DRIVER_URL)
+            throw new DdlException("compute driver checksum from url: " + driverPath
                     + " could not find algorithm: " + e.getMessage());
         }
     }
 
-    private String getRealDriverPath() {
-        String path = getProperty(DRIVER_URL);
+    private static String getRealDriverPath(String driverUrl) {
         try {
-            URI uri = new URI(path);
+            URI uri = new URI(driverUrl);
             String schema = uri.getScheme();
-            if (schema == null && !path.startsWith("/")) {
-                return "file://" + Config.jdbc_drivers_dir + "/" + path;
+            if (schema == null && !driverUrl.startsWith("/")) {
+                return "file://" + Config.jdbc_drivers_dir + "/" + driverUrl;
             }
-            return path;
+            return driverUrl;
         } catch (URISyntaxException e) {
-            LOG.warn("invalid jdbc driver url: " + path);
-            return path;
+            LOG.warn("invalid jdbc driver url: " + driverUrl);
+            return driverUrl;
+        }
+    }
+
+
+    // 1. `yearIsDateType` is a parameter of JDBC, and the default is true.
+    // We force the use of `yearIsDateType=false`
+    // 2. `tinyInt1isBit` is a parameter of JDBC, and the default is true.
+    // We force the use of `tinyInt1isBit=false`, so that for mysql type tinyint,
+    // it will convert to Doris tinyint, not bit.
+    public static String handleJdbcUrl(String jdbcUrl) {
+        // delete all space in jdbcUrl
+        String newJdbcUrl = jdbcUrl.replaceAll(" ", "");
+        newJdbcUrl = checkJdbcUrlParam(newJdbcUrl, "yearIsDateType", "true", "false");
+        newJdbcUrl = checkJdbcUrlParam(newJdbcUrl, "tinyInt1isBit", "true", "false");
+        return newJdbcUrl;
+    }

Review Comment:
   I think here should add a judgement: 
   `if (jdbcUrl.startsWith("jdbc::mysql") || jdbcUrl.startsWith("jdbc:mariadb")){...}`
   Because `yearIsDateType` and `tinyInt1isBit` are used for `jdbc::mysql`



-- 
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


[GitHub] [doris] morningman commented on a diff in pull request #15692: [fix](multi-catalog) fix image loading failture when create catalog with resource

Posted by GitBox <gi...@apache.org>.
morningman commented on code in PR #15692:
URL: https://github.com/apache/doris/pull/15692#discussion_r1063997420


##########
fe/fe-core/src/main/java/org/apache/doris/catalog/JdbcResource.java:
##########
@@ -189,29 +190,61 @@ private void computeObjectChecksum() throws DdlException {
                 }
                 digest.update(buf, 0, bytesRead);
             } while (true);
-            String checkSum = Hex.encodeHexString(digest.digest());
-            configs.put(CHECK_SUM, checkSum);
+            return Hex.encodeHexString(digest.digest());
         } catch (IOException e) {
-            throw new DdlException("compute driver checksum from url: " + getProperty(DRIVER_URL)
+            throw new DdlException("compute driver checksum from url: " + driverPath
                     + " meet an IOException: " + e.getMessage());
         } catch (NoSuchAlgorithmException e) {
-            throw new DdlException("compute driver checksum from url: " + getProperty(DRIVER_URL)
+            throw new DdlException("compute driver checksum from url: " + driverPath
                     + " could not find algorithm: " + e.getMessage());
         }
     }
 
-    private String getRealDriverPath() {
-        String path = getProperty(DRIVER_URL);
+    private static String getRealDriverPath(String driverUrl) {
         try {
-            URI uri = new URI(path);
+            URI uri = new URI(driverUrl);
             String schema = uri.getScheme();
-            if (schema == null && !path.startsWith("/")) {
-                return "file://" + Config.jdbc_drivers_dir + "/" + path;
+            if (schema == null && !driverUrl.startsWith("/")) {
+                return "file://" + Config.jdbc_drivers_dir + "/" + driverUrl;
             }
-            return path;
+            return driverUrl;
         } catch (URISyntaxException e) {
-            LOG.warn("invalid jdbc driver url: " + path);
-            return path;
+            LOG.warn("invalid jdbc driver url: " + driverUrl);
+            return driverUrl;
+        }
+    }
+
+
+    // 1. `yearIsDateType` is a parameter of JDBC, and the default is true.
+    // We force the use of `yearIsDateType=false`
+    // 2. `tinyInt1isBit` is a parameter of JDBC, and the default is true.
+    // We force the use of `tinyInt1isBit=false`, so that for mysql type tinyint,
+    // it will convert to Doris tinyint, not bit.
+    public static String handleJdbcUrl(String jdbcUrl) {
+        // delete all space in jdbcUrl
+        String newJdbcUrl = jdbcUrl.replaceAll(" ", "");
+        newJdbcUrl = checkJdbcUrlParam(newJdbcUrl, "yearIsDateType", "true", "false");
+        newJdbcUrl = checkJdbcUrlParam(newJdbcUrl, "tinyInt1isBit", "true", "false");
+        return newJdbcUrl;
+    }

Review Comment:
   Done



-- 
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


[GitHub] [doris] hello-stephen commented on pull request #15692: [fix](multi-catalog) fix image writing bug when create catalog with resource

Posted by GitBox <gi...@apache.org>.
hello-stephen commented on PR #15692:
URL: https://github.com/apache/doris/pull/15692#issuecomment-1374359501

   TeamCity pipeline, clickbench performance test result:
    the sum of best hot time: 34.75 seconds
    load time: 469 seconds
    storage size: 17122358507 Bytes
    https://doris-community-test-1308700295.cos.ap-hongkong.myqcloud.com/tmp/20230107025608_clickbench_pr_75202.html


-- 
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


[GitHub] [doris] AshinGau commented on pull request #15692: [fix](multi-catalog) fix image loading failture when create catalog with resource

Posted by GitBox <gi...@apache.org>.
AshinGau commented on PR #15692:
URL: https://github.com/apache/doris/pull/15692#issuecomment-1374994631

   LGTM


-- 
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