Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[#6493] feat(CLI): Support table format output for Schema and Table command #6495

Merged
merged 7 commits into from
Feb 24, 2025
Merged

[#6493] feat(CLI): Support table format output for Schema and Table command #6495

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
9666434
[#6493] feat(CLI): Support table format output for Schema and Table c…
Abyss-lord Feb 22, 2025
2b67d23
[#6493] feat(CLI): Support table format output for Schema and Table c…
Abyss-lord Feb 22, 2025
bccc0c1
[#6493] feat(CLI): Support table format output for Schema and Table c…
Abyss-lord Feb 22, 2025
f266f5a
[#6493] feat(CLI): Support table format output for Schema and Table c…
Abyss-lord Feb 23, 2025
2f12d06
[#6493] feat(CLI): Support table format output for Schema and Table c…
Abyss-lord Feb 23, 2025
6b45438
[#6493] feat(CLI): Support table format output for Schema and Table c…
Abyss-lord Feb 24, 2025
9f5f7c6
[#6493] feat(CLI): Support table format output for Schema and Table c…
Abyss-lord Feb 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions clients/cli/src/main/java/org/apache/gravitino/cli/commands/ListSchema.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@

package org.apache.gravitino.cli.commands;

import com.google.common.base.Joiner;
import org.apache.gravitino.cli.CommandContext;
import org.apache.gravitino.cli.ErrorMessages;
import org.apache.gravitino.client.GravitinoClient;
Expand Down Expand Up @@ -60,8 +59,11 @@ public void handle() {
exitWithError(exp.getMessage());
}

String all = schemas.length == 0 ? "No schemas exist." : Joiner.on(",").join(schemas);
if (schemas.length == 0) {
printInformation("No schemas exist.");
return;
}

printInformation(all);
printResults(schemas);
}
}
11 changes: 5 additions & 6 deletions clients/cli/src/main/java/org/apache/gravitino/cli/commands/ListTables.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@

package org.apache.gravitino.cli.commands;

import com.google.common.base.Joiner;
import java.util.ArrayList;
import java.util.List;
import org.apache.gravitino.NameIdentifier;
Expand Down Expand Up @@ -60,11 +59,11 @@ public void handle() {
tableNames.add(tables[i].name());
}

String all =
tableNames.isEmpty()
? "No tables exist."
: Joiner.on(System.lineSeparator()).join(tableNames);
if (tableNames.isEmpty()) {
printInformation("No tables exist.");
return;
}

printResults(all);
printResults(tableNames.toArray(new String[0]));
}
}
2 changes: 1 addition & 1 deletion clients/cli/src/main/java/org/apache/gravitino/cli/commands/SchemaDetails.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ public void handle() {
}

if (result != null) {
printInformation(result.name() + "," + result.comment());
printResults(result);
}
}
}
4 changes: 3 additions & 1 deletion clients/cli/src/main/java/org/apache/gravitino/cli/commands/TableDetails.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ public void handle() {
exitWithError(exp.getMessage());
}

printInformation(gTable.name() + "," + gTable.comment());
if (gTable != null) {
printResults(gTable);
}
}
}
87 changes: 85 additions & 2 deletions clients/cli/src/main/java/org/apache/gravitino/cli/outputs/TableFormat.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,9 @@
import java.util.Objects;
import org.apache.gravitino.Catalog;
import org.apache.gravitino.Metalake;
import org.apache.gravitino.Schema;
import org.apache.gravitino.cli.CommandContext;
import org.apache.gravitino.rel.Table;

/**
* Abstract base class for formatting entity information into ASCII-art tables. Provides
Expand Down Expand Up @@ -75,6 +77,12 @@ public static void output(Object entity, CommandContext context) {
new CatalogTableFormat(context).output((Catalog) entity);
} else if (entity instanceof Catalog[]) {
new CatalogListTableFormat(context).output((Catalog[]) entity);
} else if (entity instanceof Schema) {
new SchemaTableFormat(context).output((Schema) entity);
} else if (entity instanceof Table) {
new TableDetailsTableFormat(context).output((Table) entity);
} else if (entity instanceof String[]) {
new ListTableFormat(context).output((String[]) entity);
} else {
throw new IllegalArgumentException("Unsupported object type");
}
Expand Down Expand Up @@ -487,7 +495,7 @@ public MetalakeListTableFormat(CommandContext context) {
@Override
public String getOutput(Metalake[] metalakes) {

Column columnName = new Column(context, "metalake");
Column columnName = new Column(context, "Name");
Arrays.stream(metalakes).forEach(metalake -> columnName.addCell(metalake.name()));

return getTableFormat(columnName);
Expand Down Expand Up @@ -534,10 +542,85 @@ public CatalogListTableFormat(CommandContext context) {
/** {@inheritDoc} */
@Override
public String getOutput(Catalog[] catalogs) {
Column columnName = new Column(context, "catalog");
Column columnName = new Column(context, "Name");
Arrays.stream(catalogs).forEach(metalake -> columnName.addCell(metalake.name()));

return getTableFormat(columnName);
}
}

/**
* Formats a single {@link Schema} instance into a two-column table display. Displays catalog
* details including name and comment information.
*/
static final class SchemaTableFormat extends TableFormat<Schema> {
public SchemaTableFormat(CommandContext context) {
super(context);
}

/** {@inheritDoc} */
@Override
public String getOutput(Schema schema) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The actual logic that gets overridden is the process of building a list of Columns.
So is it possible to refactor this function into a protected function getColumns, and then we invoke the public getOutput function from the parent class?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fetching the output and fetching the columns represent two different levels of activity, but they're actually doing the same thing in this case.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you can invoke the function from the parent class, which is more generic, then you don't have to create a specialized class to invoke the getOutput method.

I mean this:

   class Animal {
      public describe() {
         legs = getLegs()
         wings = getWings()
         return "legs:" + legs + 'wings:' + wings
      }
      protected getLegs() {
         return 0
       }
      protected getWings() {
      }
    }

    class Dog extends Animal {
       protected getLegs() {
          return 4
       }
    }

    class Duck extends Animal {
      protected getWings() {
        return 2
      }
    }

When invoking this method in future, I'll do:

   // I don't need to care what kind of an animal it is.
   animal.describe()

Copy link
Contributor Author

@Abyss-lord Abyss-lord Feb 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you can invoke the function from the parent class, which is more generic, then you don't have to create a specialized class to invoke the getOutput method.

I mean this:

   class Animal {
      public describe() {
         legs = getLegs()
         wings = getWings()
         return "legs:" + legs + 'wings:' + wings
      }
      protected getLegs() {
         return 0
       }
      protected getWings() {
      }
    }

    class Dog extends Animal {
       protected getLegs() {
          return 4
       }
    }

    class Duck extends Animal {
      protected getWings() {
        return 2
      }
    }

When invoking this method in future, I'll do:

   // I don't need to care what kind of an animal it is.
   animal.describe()

@tengqm In this case, the template pattern is used, BaseOutputFormat#output is responsible for output, it fix the logic of output method, and the getOutput method is implemented by subclasses.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The merit is that you don't need to instantiate the subclasses, you will save a lot of complicated if...else if...else if... calls when invoking this method.
The subclasses provide the materials (columns), the superclass does the main logic.

Column columnName = new Column(context, "schema");
Column columnComment = new Column(context, "comment");

columnName.addCell(schema.name());
columnComment.addCell(schema.comment());

return getTableFormat(columnName, columnComment);
}
}

/**
* Formats a single {@link Table} instance into a two-column table display. Displays table details
* including name and comment information.
*/
static final class TableDetailsTableFormat extends TableFormat<Table> {
public TableDetailsTableFormat(CommandContext context) {
super(context);
}

/** {@inheritDoc} */
@Override
public String getOutput(Table table) {
Column columnName = new Column(context, "name");
Column columnType = new Column(context, "type");
Column columnAutoIncrement = new Column(context, "AutoIncrement");
Column columnNullable = new Column(context, "nullable");
Column columnComment = new Column(context, "comment");

org.apache.gravitino.rel.Column[] columns = table.columns();
for (org.apache.gravitino.rel.Column column : columns) {
columnName.addCell(column.name());
columnType.addCell(column.dataType().simpleString());
columnAutoIncrement.addCell(column.autoIncrement());
columnNullable.addCell(column.nullable());
columnComment.addCell(
column.comment() == null || column.comment().isEmpty() ? "N/A" : column.comment());
}

return getTableFormat(
columnName, columnType, columnAutoIncrement, columnNullable, columnComment);
}
}

static final class ListTableFormat extends TableFormat<String[]> {
/**
* Creates a new {@link TableFormat} with the specified properties.
*
* @param context the command context.
*/
public ListTableFormat(CommandContext context) {
super(context);
}

/** {@inheritDoc} */
@Override
public String getOutput(String[] entities) {
Column column = new Column(context, "name");
Arrays.stream(entities).forEach(column::addCell);

return getTableFormat(column);
}
}
}
4 changes: 2 additions & 2 deletions clients/cli/src/test/java/org/apache/gravitino/cli/integration/test/TableFormatOutputIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ public void testMetalakeListCommand() {
String output = new String(outputStream.toByteArray(), StandardCharsets.UTF_8).trim();
assertEquals(
"+-------------+\n"
+ "| Metalake |\n"
+ "| Name |\n"
+ "+-------------+\n"
+ "| my_metalake |\n"
+ "+-------------+",
Expand Down Expand Up @@ -170,7 +170,7 @@ public void testCatalogListCommand() {
String output = new String(outputStream.toByteArray(), StandardCharsets.UTF_8).trim();
assertEquals(
"+-----------+\n"
+ "| Catalog |\n"
+ "| Name |\n"
+ "+-----------+\n"
+ "| postgres |\n"
+ "| postgres2 |\n"
Expand Down
129 changes: 115 additions & 14 deletions clients/cli/src/test/java/org/apache/gravitino/cli/output/TestTableFormat.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,13 @@
import java.nio.charset.StandardCharsets;
import org.apache.gravitino.Catalog;
import org.apache.gravitino.Metalake;
import org.apache.gravitino.Schema;
import org.apache.gravitino.cli.CommandContext;
import org.apache.gravitino.cli.outputs.Column;
import org.apache.gravitino.cli.outputs.TableFormat;
import org.apache.gravitino.rel.Table;
import org.apache.gravitino.rel.types.Type;
import org.apache.gravitino.rel.types.Types;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeEach;
Expand Down Expand Up @@ -251,7 +255,7 @@ void testListMetalakeWithTableFormat() {
String output = new String(outContent.toByteArray(), StandardCharsets.UTF_8).trim();
Assertions.assertEquals(
"+-----------+\n"
+ "| Metalake |\n"
+ "| Name |\n"
+ "+-----------+\n"
+ "| metalake1 |\n"
+ "| metalake2 |\n"
Expand All @@ -278,23 +282,82 @@ void testCatalogDetailsWithTableFormat() {
@Test
void testListCatalogWithTableFormat() {
CommandContext mockContext = getMockContext();
Catalog mockCatalog1 =
getMockCatalog("catalog1", Catalog.Type.FILESET, "provider1", "This is a catalog");
Catalog mockCatalog2 =
getMockCatalog("catalog2", Catalog.Type.RELATIONAL, "provider2", "This is another catalog");

TableFormat.output(new Catalog[] {mockCatalog1, mockCatalog2}, mockContext);
TableFormat.output(new String[] {"catalog1", "catalog2"}, mockContext);
String output = new String(outContent.toByteArray(), StandardCharsets.UTF_8).trim();
Assertions.assertEquals(
"+----------+\n"
+ "| Catalog |\n"
+ "| Name |\n"
+ "+----------+\n"
+ "| catalog1 |\n"
+ "| catalog2 |\n"
+ "+----------+",
output);
}

@Test
void testSchemaDetailsWithTableFormat() {
CommandContext mockContext = getMockContext();
Schema mockSchema = getMockSchema();

TableFormat.output(mockSchema, mockContext);
String output = new String(outContent.toByteArray(), StandardCharsets.UTF_8).trim();
Assertions.assertEquals(
"+-------------+-----------------------+\n"
+ "| Schema | Comment |\n"
+ "+-------------+-----------------------+\n"
+ "| demo_schema | This is a demo schema |\n"
+ "+-------------+-----------------------+",
output);
}

@Test
void testListSchemaWithTableFormat() {
CommandContext mockContext = getMockContext();
TableFormat.output(new String[] {"schema1", "schema2"}, mockContext);
String output = new String(outContent.toByteArray(), StandardCharsets.UTF_8).trim();
Assertions.assertEquals(
"+---------+\n"
+ "| Name |\n"
+ "+---------+\n"
+ "| schema1 |\n"
+ "| schema2 |\n"
+ "+---------+",
output);
}

@Test
void testTableDetailsWithTableFormat() {
CommandContext mockContext = getMockContext();
Table mockTable = getMockTable();

TableFormat.output(mockTable, mockContext);
String output = new String(outContent.toByteArray(), StandardCharsets.UTF_8).trim();
Assertions.assertEquals(
"+------+---------+---------------+----------+-------------------------+\n"
+ "| Name | Type | AutoIncrement | Nullable | Comment |\n"
+ "+------+---------+---------------+----------+-------------------------+\n"
+ "| id | integer | true | false | This is a int column |\n"
+ "| name | string | false | true | This is a string column |\n"
+ "+------+---------+---------------+----------+-------------------------+",
output);
}

@Test
void testListTableWithTableFormat() {
CommandContext mockContext = getMockContext();

TableFormat.output(new String[] {"table1", "table2"}, mockContext);
String output = new String(outContent.toByteArray(), StandardCharsets.UTF_8).trim();
Assertions.assertEquals(
"+--------+\n"
+ "| Name |\n"
+ "+--------+\n"
+ "| table1 |\n"
+ "| table2 |\n"
+ "+--------+",
output);
}

@Test
void testOutputWithUnsupportType() {
CommandContext mockContext = getMockContext();
Expand All @@ -307,12 +370,6 @@ void testOutputWithUnsupportType() {
});
}

private void addRepeatedCells(Column column, int count) {
for (int i = 0; i < count; i++) {
column.addCell(column.getHeader() + "-" + (i + 1));
}
}

private CommandContext getMockContext() {
CommandContext mockContext = mock(CommandContext.class);

Expand Down Expand Up @@ -345,4 +402,48 @@ private Catalog getMockCatalog(String name, Catalog.Type type, String provider,

return mockCatalog;
}

private Schema getMockSchema() {
return getMockSchema("demo_schema", "This is a demo schema");
}

private Schema getMockSchema(String name, String comment) {
Schema mockSchema = mock(Schema.class);
when(mockSchema.name()).thenReturn(name);
when(mockSchema.comment()).thenReturn(comment);

return mockSchema;
}

private Table getMockTable() {
return getMockTable("demo_table", "This is a demo table");
}

private Table getMockTable(String name, String comment) {
Table mockTable = mock(Table.class);
org.apache.gravitino.rel.Column mockColumnInt =
getMockColumn("id", Types.IntegerType.get(), "This is a int column", false, true);
org.apache.gravitino.rel.Column mockColumnString =
getMockColumn("name", Types.StringType.get(), "This is a string column", true, false);

when(mockTable.name()).thenReturn(name);
when(mockTable.comment()).thenReturn(comment);
when(mockTable.columns())
.thenReturn(new org.apache.gravitino.rel.Column[] {mockColumnInt, mockColumnString});

return mockTable;
}

private org.apache.gravitino.rel.Column getMockColumn(
String name, Type dataType, String comment, boolean nullable, boolean autoIncrement) {

org.apache.gravitino.rel.Column mockColumn = mock(org.apache.gravitino.rel.Column.class);
when(mockColumn.name()).thenReturn(name);
when(mockColumn.dataType()).thenReturn(dataType);
when(mockColumn.comment()).thenReturn(comment);
when(mockColumn.nullable()).thenReturn(nullable);
when(mockColumn.autoIncrement()).thenReturn(autoIncrement);

return mockColumn;
}
}