You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@paimon.apache.org by "zhuangchong (via GitHub)" <gi...@apache.org> on 2023/11/14 11:40:00 UTC

[PR] [core] Filesystem catalog supports setting case-sensitive option. [incubator-paimon]

zhuangchong opened a new pull request, #2313:
URL: https://github.com/apache/incubator-paimon/pull/2313

   <!-- Please specify the module before the PR name: [core] ... or [flink] ... -->
   
   ### Purpose
   
   <!-- Linking this pull request to the issue -->
    Filesystem catalog supports setting case-sensitive option.
   
   <!-- What is the purpose of the change -->
   
   ### Tests
   
   <!-- List UT and IT cases to verify this change -->
   
   ### API and Format
   
   <!-- Does this change affect API or storage format -->
   
   ### Documentation
   
   <!-- Does this change introduce a new feature -->
   


-- 
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: issues-unsubscribe@paimon.apache.org

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


Re: [PR] [core] Filesystem catalog supports setting case-sensitive option. [incubator-paimon]

Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on code in PR #2313:
URL: https://github.com/apache/incubator-paimon/pull/2313#discussion_r1393922409


##########
paimon-core/src/main/java/org/apache/paimon/catalog/Catalog.java:
##########
@@ -224,6 +228,23 @@ default boolean caseSensitive() {
         return true;
     }
 
+    /** Validate whether table or field names are valid when case-insensitive. */
+    default void validateCaseInsensitive(String type, String... names) {

Review Comment:
   Why here? put this to AbstractCatalog.



##########
paimon-core/src/main/java/org/apache/paimon/catalog/FileSystemCatalog.java:
##########
@@ -160,4 +163,9 @@ public void close() throws Exception {}
     public String warehouse() {
         return warehouse.toString();
     }
+
+    @Override
+    public boolean caseSensitive() {
+        return Options.fromMap(options()).get(CASE_SENSITIVE);

Review Comment:
   Store `Options catalogOptions` in `AbstractCatalog`?



##########
paimon-flink/paimon-flink-cdc/src/main/java/org/apache/paimon/flink/action/cdc/kafka/KafkaSyncDatabaseAction.java:
##########
@@ -183,21 +180,9 @@ public void build() throws Exception {
     }
 
     private void validateCaseInsensitive() {
-        checkArgument(
-                database.equals(database.toLowerCase()),
-                String.format(
-                        "Database name [%s] cannot contain upper case in case-insensitive catalog.",
-                        database));
-        checkArgument(
-                tablePrefix.equals(tablePrefix.toLowerCase()),
-                String.format(
-                        "Table prefix [%s] cannot contain upper case in case-insensitive catalog.",
-                        tablePrefix));
-        checkArgument(
-                tableSuffix.equals(tableSuffix.toLowerCase()),
-                String.format(
-                        "Table suffix [%s] cannot contain upper case in case-insensitive catalog.",
-                        tableSuffix));
+        catalog.validateCaseInsensitive("Database", database);

Review Comment:
   This should be just a util method... Not a public method.



-- 
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: issues-unsubscribe@paimon.apache.org

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


Re: [PR] [core] Filesystem catalog supports setting case-sensitive option. [incubator-paimon]

Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on PR #2313:
URL: https://github.com/apache/incubator-paimon/pull/2313#issuecomment-1812113260

   Thanks @zhuangchong , left comment about public method in `Catalog`.


-- 
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: issues-unsubscribe@paimon.apache.org

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


Re: [PR] [core] Filesystem catalog supports setting case-sensitive option. [incubator-paimon]

Posted by "zhuangchong (via GitHub)" <gi...@apache.org>.
zhuangchong commented on PR #2313:
URL: https://github.com/apache/incubator-paimon/pull/2313#issuecomment-1812367791

   > Thanks @zhuangchong , left comment about public method in `Catalog`.
   
   Thanks @JingsongLi ,  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: issues-unsubscribe@paimon.apache.org

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


Re: [PR] [core] Filesystem catalog supports setting case-sensitive option. [incubator-paimon]

Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi merged PR #2313:
URL: https://github.com/apache/incubator-paimon/pull/2313


-- 
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: issues-unsubscribe@paimon.apache.org

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