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

[Abyss-lord]

What changes were proposed in this pull request?

Support table format output for Schema and Table command.

Why are the changes needed?

Fix: #6493

Does this PR introduce any user-facing change?

No.

How was this patch tested?

local test.

gcli schema list -m demo_metalake --name Hive_catalog --output table -i
+---------------------------+
|          Schema           |
+---------------------------+
| convert_example           |
| default                   |
| hive_best_performance     |
| test_hive_convert_example |
| test_multiple_data_source |
+---------------------------+

gcli schema details -m demo_metalake --name Hive_catalog.default --output table -i
+---------+-----------------------+
| Schema  |        Comment        |
+---------+-----------------------+
| default | Default Hive database |
+---------+-----------------------+

gcli table list -m demo_metalake --name Hive_catalog.default -i --output table
+---------------------------------------+
|                 Table                 |
+---------------------------------------+
| test_create_txt_tbl_without_delimited |
| test_jinjia_tbl1                      |
| test_jinjia_tbl2                      |
| test                                  |
| test1                                 |
| example_table                         |
| example_table2                        |
+---------------------------------------+

gcli table details -m demo_metalake --name Hive_catalog.default.test1  -i --output table

+----------+---------+---------------+----------+---------+
|   Name   |  Type   | AutoIncrement | Nullable | Comment |
+----------+---------+---------------+----------+---------+
| id       | integer | false         | true     | N/A     |
| name     | string  | false         | true     | N/A     |
| standard | long    | false         | true     | N/A     |
| dt       | string  | false         | true     | N/A     |
+----------+---------+---------------+----------+---------+

[Abyss-lord]

@justinmclean @tengqm could you please review this PR when you have time? I’d really appreciate your feedback.

[tengqm]

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?

[Abyss-lord]

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

[tengqm]

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()

[Abyss-lord]

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.

[tengqm]

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.

[Abyss-lord]

@justinmclean I’ve finished updating the code. Please take a look at the PR again when you have time.

[justinmclean]

I'm a little concerned that we are calling loadTable/loadSchema when we probably don't need to. If the only reason is for an exact object type, why not create one?

[justinmclean]

Or come up with another way i.e. pass an array of strings with the first value being the title and the rest the values in the first column. It seems we are trying to force this code to fit when there are other probably better ways of doing it?

[Abyss-lord]

Or come up with another way i.e. pass an array of strings with the first value being the title and the rest the values in the first column. It seems we are trying to force this code to fit when there are other probably better ways of doing it?

Hi @justinmclean There is no need for load, and since the list only needs name, it can be built directly.

[justinmclean]

That looks a lot better thanks.

[justinmclean]

Thanks, all looks good

[tengqm]

Sad thing is that we are merging this prematurely.