Github user arjansh commented on a diff in the pull request:

    https://github.com/apache/metamodel/pull/185#discussion_r211947345
  
    --- Diff: 
jdbc/src/main/java/org/apache/metamodel/jdbc/JdbcCreateTableBuilder.java ---
    @@ -115,21 +115,23 @@ private String createSqlStatement(Table table) {
                     sb.append(" NOT NULL");
                 }
             }
    -        boolean primaryKeyExists = false;
    -        for (int i = 0; i < columns.size(); i++) {
    -            if (columns.get(i).isPrimaryKey()) {
    -                if (!primaryKeyExists) {
    -                    sb.append(", PRIMARY KEY(");
    -                    sb.append(columns.get(i).getName());
    -                    primaryKeyExists = true;
    -                } else {
    -                    sb.append(",");
    -                    sb.append(columns.get(i).getName());
    +        if (queryRewriter.isPrimaryKeySupported()) {
    +            boolean primaryKeyExists = false;
    --- End diff --
    
    Not completely related to this pull request, but something that we can 
address in this context anyway, because the added if statement here only adds 
to the cognitive complexity of this method. Lines 119 through 134 are a carbon 
copy of lines 130 through 145 from the super class 
`AbstractTableCreationBuilder`. So maybe it's an option to extract that code 
into a separate method on the `AbstractTableCreationBuilder`, which can be 
invoked here too, this both removes the copy/pasted code and reduces the 
cognitive complexity of this method.


---

Reply via email to