vmware-archive / quickstep

Quickstep Project
Apache License 2.0
27 stars 13 forks source link

Quickstep catalog print #118

Closed rogersjeffreyl closed 8 years ago

rogersjeffreyl commented 8 years ago

This PR adds the execution support for the following commands to the CLI

\d This command lists all the tables in the database

quickstep> \d
       List of relations
 Name | Type |
+------+------+
 test    | table 
 test1  | table 

\d This command is the equivalent of describe table. The columns and their types are shown. Additionally index information is also shown

quickstep> \d test
 Table "test"
Column     |Type       
+-----------+-----------+
a          |Int        
Indexes:
"test_index" CSB_TREE (a)

\dt Lists all the tables in the database

quickstep> \dt
       List of relations
 Name | Type |
+------+------+
 test    | table 
 test1  | table 

\dt< table_name> Lists the table name from the list of tables in the database

quickstep> \dt test
       List of relations

 Name | Type |
+------+------+
 test    | table 
cramja commented 8 years ago

@rogersjeffreyl I added some initial comments but did not look at the workhorse code in the Command.cpp file. I'd say looks like a good start, especially since the line printing methods can be tricky to write correctly. I will be around this afternoon from 2-5 if you want to talk in person then.

zuyu commented 8 years ago

@rogersjeffreyl Before diving into the code, I have the following questions.

Do we have \help, \h, or similar to show the help messages for all the commands you added?

Are \d and \dt two alias to the same commends?

Also, is it possible to print the relations better? For example in the demonstrated result below, the print length of all the relation names should be the same, so is the type. And the bar should align with +.

quickstep> \d
       List of relations
  Name   | Type   |
+--------+--------+
 test    | table 
 test1   | table 

Thanks!

zuyu commented 8 years ago

@rogersjeffreyl Please update the copyright information for all the files you touched.

rogersjeffreyl commented 8 years ago

@zuyu

\d and \dt are not exactly the same commands. They behave the same way when they take no arguments. Following is the output from postgres: postgres=# \d List of relations Schema | Name | Type | Owner --------+-------+-------+------- public | test | table | rl public | test1 | table | rl

postgres=# \dt List of relations Schema | Name | Type | Owner --------+-------+-------+------- public | test | table | rl public | test1 | table | rl

However , when the table name is applied as argument the behavior changes:\

postgres=# \dt test List of relations Schema | Name | Type | Owner --------+------+-------+------- public | test | table | rl

postgres=# \d test Table "public.test" Column | Type | Modifiers --------+---------+----------- a | integer | Indexes: "test_index" btree (a)

rogersjeffreyl commented 8 years ago

@zuyu We donot have support for \h or \help yet. Regarding the output, when i paste the output into github I think it gets formatted. When I ran them on the cli the output is cleaner

cramja commented 8 years ago

@rogersjeffreyl and I talked about whether to make separate command classes versus static methods in a single command class. We came to the conclusion that at this point, using static methods is fine because there is no persistent state.

Also, @zuyu , \help hasn't been created yet. The original idea was to have \help describe a SQL command usage, (\help select would show the user the possible syntax). However, this not implemented because of the large number of grammars that need to be described.

A simple \help could display a list of available \ commands similar to sqlite's interface. A seperate command could be used for SQL interpretation, something like \usage SQL.

--my 2p

rogersjeffreyl commented 8 years ago

@pateljm have fixed the alignment issue in the output

cramja commented 8 years ago

@rogersjeffreyl Are we going to add unit tests in this PR?

rogersjeffreyl commented 8 years ago

@cramja unit tests will be a separate pr

zuyu commented 8 years ago

@rogersjeffreyl We typically do not merge new code without any unit test coverages. Please add the unit test in the same PR.

cramja commented 8 years ago

Sorry @rogersjeffreyl I accidently closed the PR when I meant to cancel the comment I was writing.

rogersjeffreyl commented 8 years ago

@cramja @zuyu CI failed earlier due to the ref count issue. Have merged with master and started CI again.

rogersjeffreyl commented 8 years ago

@cramja @zuyu The CI has passed now.

zuyu commented 8 years ago

@rogersjeffreyl I've done one pass. After addressing all the comments, we should merge this PR. Thanks!

zuyu commented 8 years ago

@rogersjeffreyl LGTM. Nice job!