iceiceiceCN commented on code in PR #22831:
URL: https://github.com/apache/flink/pull/22831#discussion_r1241576587


##########
flink-connectors/flink-connector-files/src/main/java/org/apache/flink/connector/file/table/PartitionCommitPolicyFactory.java:
##########
@@ -63,11 +75,23 @@ public List<PartitionCommitPolicy> createPolicyChain(
                                             successFileName, fsSupplier.get());
                                 case PartitionCommitPolicy.CUSTOM:
                                     try {
-                                        return (PartitionCommitPolicy)
-                                                
cl.loadClass(customClass).newInstance();
-                                    } catch (ClassNotFoundException
-                                            | IllegalAccessException
-                                            | InstantiationException e) {
+                                        if (parameters != null && 
!parameters.isEmpty()) {

Review Comment:
   If users input
   `'sink.partition-commit.policy.class.parameters'='',`
   parameters != null but parameters will empty,
   so yes, it maybe get the constructor with empty arguments.



##########
flink-connectors/flink-connector-files/src/main/java/org/apache/flink/connector/file/table/PartitionCommitPolicyFactory.java:
##########
@@ -63,11 +75,23 @@ public List<PartitionCommitPolicy> createPolicyChain(
                                             successFileName, fsSupplier.get());
                                 case PartitionCommitPolicy.CUSTOM:
                                     try {
-                                        return (PartitionCommitPolicy)
-                                                
cl.loadClass(customClass).newInstance();
-                                    } catch (ClassNotFoundException
-                                            | IllegalAccessException
-                                            | InstantiationException e) {
+                                        if (parameters != null && 
!parameters.isEmpty()) {

Review Comment:
   > I'm wondering if change it to
   > 
   > ```
   > parameters != null
   > ```
   > 
   > Can it also get the constructor with empty arguments? If so, we should 
consider to change to `parameters != null` to make code more clear.
   
   If users input
   `'sink.partition-commit.policy.class.parameters'='',`
   parameters != null but parameters will empty,
   so yes, it maybe get the constructor with empty arguments.



-- 
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-unsubscr...@flink.apache.org

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

Reply via email to