justinmclean commented on code in PR #6573:
URL: https://github.com/apache/gravitino/pull/6573#discussion_r1976544094


##########
clients/cli/src/main/java/org/apache/gravitino/cli/commands/ListColumns.java:
##########
@@ -48,53 +47,57 @@ public ListColumns(
     this.table = table;
   }
 
-  /** Displays the details of a table's columns. */
-  @Override
-  public void handle() {
-    Column[] columns = null;
+    /** Displays the details of a table's columns. */  
+    @Override
+    public void handle() {
+        try {
+            NameIdentifier name = NameIdentifier.of(schema, table);
+            Column[] columns = tableCatalog().loadTable(name).columns();
 
-    try {
-      NameIdentifier name = NameIdentifier.of(schema, table);
-      columns = tableCatalog().loadTable(name).columns();
-    } catch (NoSuchTableException noSuchTableException) {
-      exitWithError(
-          ErrorMessages.UNKNOWN_TABLE + Joiner.on(".").join(metalake, catalog, 
schema, table));
-      return;
-    } catch (Exception exp) {
-      exitWithError(exp.getMessage());
-      return;
-    }
+            if (columns != null && columns.length > 0) {
+                StringBuilder all = new StringBuilder();
+                boolean hasAutoIncrement = false;
 
-    // Null / empty check before processing columns
-    if (columns == null || columns.length == 0) {
-      exitWithError("No columns found for table: " + table);
-      return;
-    }
+                // Check if any column supports auto-increment
+                for (Column column : columns) {
+                    if (column != null && column.autoIncrement()) {
+                        hasAutoIncrement = true;
+                        break;
+                    }
+                }
 
-    StringBuilder all = new StringBuilder();
-    
all.append("name,datatype,comment,nullable,auto_increment").append(System.lineSeparator());
+                all.append("name,datatype,comment,nullable");
+                if (hasAutoIncrement) {
+                    all.append(",auto_increment");
+                }
+                all.append(System.lineSeparator());
 
-    for (Column column : columns) {
-      if (column == null) continue; // Skip any unexpected null columns
+                for (Column column : columns) {
+                    if (column == null) {
+                        continue;
+                    }
 
-      String name = column.name();
-      String dataType = column.dataType() != null ? 
column.dataType().simpleString() : "UNKNOWN";
-      String comment = column.comment() != null ? column.comment() : "N/A";
-      String nullable = column.nullable() ? "true" : "false";
-      String autoIncrement = column.autoIncrement() ? "true" : "false";
+                    StringBuilder columnDetails = new StringBuilder();
+                    columnDetails.append(column.name()).append(",");
+                    columnDetails.append(column.dataType() != null ? 
column.dataType().simpleString() : "UNKNOWN").append(",");
+                    columnDetails.append(column.comment() != null ? 
column.comment() : "N/A").append(",");
+                    columnDetails.append(column.nullable() ? "true" : "false");
 
-      all.append(name)
-          .append(",")
-          .append(dataType)
-          .append(",")
-          .append(comment)
-          .append(",")
-          .append(nullable)
-          .append(",")
-          .append(autoIncrement)
-          .append(System.lineSeparator());
-    }
+                    if (hasAutoIncrement) {
+                        
columnDetails.append(",").append(column.autoIncrement() ? "true" : "");
+                    }

Review Comment:
   While what you are doing here seems like a good idea, it can break any 
scripts that users have already written. Having a column that can optionally 
show up will also make it harder for scripts to use this output.



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

Reply via email to