tidy up show tables behavior (#2374)

* tidy up show tables behavior

* made cli integration test whole again

* Update fbsql \d meta-command to show system tables (#2376)

---------

Co-authored-by: Travis Turner <travis@molecula.com>
This commit is contained in:
Pat Okeeffe 2023-04-07 12:58:09 -05:00 committed by GitHub
parent 3b142af2c7
commit 7f75193cf2
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
10 changed files with 199 additions and 66 deletions

View file

@ -364,7 +364,7 @@ Input/Output
\warn [-n] [STRING] write string to standard error (-n for no newline)
Informational
\d list tables
\d list tables, including system tables
\d NAME describe table
\dt list tables
\dv list views
@ -504,8 +504,17 @@ func (m *metaDescribe) execute(cmd *Command) (responseAction, error) {
switch len(m.args) {
case 0:
// Describe with no args should list all relations (tables, views,
// etc.). For now, we're just going to list the tables.
return newMetaListTables().execute(cmd)
// etc.). For now, we're just going to list the tables, including system
// tables.
qry := []queryPart{
newPartRaw("SHOW TABLES WITH SYSTEM"),
}
if err := cmd.executeAndWriteQuery(qry); err != nil {
return actionNone, errors.Wrap(err, "executing query")
}
return actionReset, nil
case 1:
// Describe with a single arg will assume the arg is a table name, so it

40
cli/testdata/table vendored
View file

@ -1,5 +1,5 @@
// Show tables for database using SHOW TABLES.
SEND:SHOW TABLES;
// Show tables for database using SHOW TABLES WITH SYSTEM.
SEND:SHOW TABLES WITH SYSTEM;
EXPECT:+-------------------------+-------------------------+-------+------------+----------------------+----------------------+-------+------------+-------------+
EXPECT:| _id | name | owner | updated_by | created_at | updated_at | keys | space_used | description |
EXPECT:+-------------------------+-------------------------+-------+------------+----------------------+----------------------+-------+------------+-------------+
@ -11,8 +11,8 @@ EXPECTCOMP:WithFormat:| fb_____________________ | fb_____________________ |
EXPECT:+-------------------------+-------------------------+-------+------------+----------------------+----------------------+-------+------------+-------------+
EXPECT:
// Show tables for database using \dt.
SEND:\dt
// Show tables for database using \d.
SEND:\d
EXPECT:+-------------------------+-------------------------+-------+------------+----------------------+----------------------+-------+------------+-------------+
EXPECT:| _id | name | owner | updated_by | created_at | updated_at | keys | space_used | description |
EXPECT:+-------------------------+-------------------------+-------+------------+----------------------+----------------------+-------+------------+-------------+
@ -24,6 +24,23 @@ EXPECTCOMP:WithFormat:| fb_____________________ | fb_____________________ |
EXPECT:+-------------------------+-------------------------+-------+------------+----------------------+----------------------+-------+------------+-------------+
EXPECT:
// Show tables for database using SHOW TABLES.
SEND:SHOW TABLES;
EXPECT:+-----+------+-------+------------+------------+------------+------+------------+-------------+
EXPECT:| _id | name | owner | updated_by | created_at | updated_at | keys | space_used | description |
EXPECT:+-----+------+-------+------------+------------+------------+------+------------+-------------+
EXPECT:+-----+------+-------+------------+------------+------------+------+------------+-------------+
EXPECT:
// Show tables for database using \dt.
SEND:\dt
EXPECT:+-----+------+-------+------------+------------+------------+------+------------+-------------+
EXPECT:| _id | name | owner | updated_by | created_at | updated_at | keys | space_used | description |
EXPECT:+-----+------+-------+------------+------------+------------+------+------------+-------------+
EXPECT:+-----+------+-------+------------+------------+------------+------+------------+-------------+
EXPECT:
// Create a table. That can be used for general testing.
SEND:CREATE TABLE users (_id id, name string, age int);
EXPECT:
@ -33,16 +50,11 @@ EXPECT:
// Show tables for database to get the newly created table.
SEND:\dt
EXPECT:+-------------------------+-------------------------+-------+------------+----------------------+----------------------+-------+------------+-------------+
EXPECT:| _id | name | owner | updated_by | created_at | updated_at | keys | space_used | description |
EXPECT:+-------------------------+-------------------------+-------+------------+----------------------+----------------------+-------+------------+-------------+
EXPECTCOMP:WithFormat:| users | users | | | {timestamp} | {timestamp} | false | 0 | |
EXPECTCOMP:WithFormat:| fb_____________________ | fb_____________________ | | | {timestamp} | {timestamp} | false | 0 | |
EXPECTCOMP:WithFormat:| fb_____________________ | fb_____________________ | | | {timestamp} | {timestamp} | false | 0 | |
EXPECTCOMP:WithFormat:| fb_____________________ | fb_____________________ | | | {timestamp} | {timestamp} | false | 0 | |
EXPECTCOMP:WithFormat:| fb_____________________ | fb_____________________ | | | {timestamp} | {timestamp} | false | 0 | |
EXPECTCOMP:WithFormat:| fb_____________________ | fb_____________________ | | | {timestamp} | {timestamp} | false | 0 | |
EXPECT:+-------------------------+-------------------------+-------+------------+----------------------+----------------------+-------+------------+-------------+
EXPECT:+-------+-------+-------+------------+----------------------+----------------------+-------+------------+-------------+
EXPECT:| _id | name | owner | updated_by | created_at | updated_at | keys | space_used | description |
EXPECT:+-------+-------+-------+------------+----------------------+----------------------+-------+------------+-------------+
EXPECTCOMP:WithFormat:| users | users | | | {timestamp} | {timestamp} | false | 0 | |
EXPECT:+-------+-------+-------+------------+----------------------+----------------------+-------+------------+-------------+
EXPECT:
// We don't select from users until AFTER we check SHOW TABLES above because

View file

@ -152,6 +152,9 @@ const (
// query hints
ErrUnknownQueryHint errors.Code = "ErrInvalidQueryHint"
ErrInvalidQueryHintParameterCount errors.Code = "ErrInvalidQueryHintParameterCount"
// show options
ErrUnknownShowOption errors.Code = "ErrUnknownShowOption"
)
func NewErrDuplicateColumn(line int, col int, column string) error {
@ -931,3 +934,12 @@ func NewErrInvalidQueryHintParameterCount(line, col int, hintName string, desire
fmt.Sprintf("[%d:%d] query hint '%s' expected %d parameter(s) (%s), got %d parameters", line, col, hintName, desiredCount, desiredList, actualCount),
)
}
// show options
func NewErrUnknownShowOption(line, col int, optionName string) error {
return errors.New(
ErrUnknownShowOption,
fmt.Sprintf("[%d:%d] unknown show option '%s'", line, col, optionName),
)
}

View file

@ -540,11 +540,19 @@ func (s *ShowDatabasesStatement) Clone() *ShowDatabasesStatement {
type ShowTablesStatement struct {
Show Pos // position of SHOW
Tables Pos // position of TABLES
With Pos
System *Ident
}
// String returns the string representation of the statement.
func (s *ShowTablesStatement) String() string {
return "SHOW TABLES"
var buf bytes.Buffer
buf.WriteString("SHOW TABLES")
if s.With.IsValid() {
buf.WriteString(" WITH")
fmt.Fprintf(&buf, " %s", s.System.String())
}
return buf.String()
}
func (s *ShowTablesStatement) Clone() *ShowTablesStatement {

View file

@ -176,12 +176,18 @@ func (p *Parser) parseShowDatabasesStatement(showPos Pos) (*ShowDatabasesStateme
}
}
func (p *Parser) parseShowTablesStatement(showPos Pos) (*ShowTablesStatement, error) {
func (p *Parser) parseShowTablesStatement(showPos Pos) (_ *ShowTablesStatement, err error) {
switch p.peek() {
case TABLES:
var stmt ShowTablesStatement
stmt.Show = showPos
stmt.Tables, _, _ = p.scan()
if p.peek() == WITH {
stmt.With, _, _ = p.scan()
if stmt.System, err = p.parseIdent("show tables option"); err != nil {
return &stmt, err
}
}
return &stmt, nil
default:
return nil, p.errorExpected(p.pos, p.tok, "TABLES")

View file

@ -658,8 +658,18 @@ func TestParser_ParseStatement(t *testing.T) {
Show: pos(0),
Tables: pos(5),
})
AssertParseStatement(t, `SHOW TABLES WITH SYSTEM`, &parser.ShowTablesStatement{
Show: pos(0),
Tables: pos(5),
With: pos(12),
System: &parser.Ident{
Name: "SYSTEM",
NamePos: pos(17),
},
})
AssertParseStatementError(t, `SHOW`, `1:4: expected DATABASES, TABLES, COLUMNS or CREATE, found 'EOF'`)
AssertParseStatementError(t, `SHOW BLAH`, `1:6: expected DATABASES, TABLES, COLUMNS or CREATE, found BLAH`)
AssertParseStatementError(t, `SHOW TABLES WITH`, `1:16: expected show tables option, found 'EOF'`)
})
t.Run("ShowColumns", func(t *testing.T) {

View file

@ -73,7 +73,19 @@ func (p *ExecutionPlanner) compileShowDatabasesStatement(ctx context.Context, st
return NewPlanOpQuery(p, NewPlanOpProjection(columns, NewPlanOpFeatureBaseDatabases(p, dbs)), p.sql), nil
}
func (p *ExecutionPlanner) compileShowTablesStatement(ctx context.Context, stmt parser.Statement) (types.PlanOperator, error) {
func (p *ExecutionPlanner) compileShowTablesStatement(ctx context.Context, stmt *parser.ShowTablesStatement) (types.PlanOperator, error) {
showSystem := false
if stmt.With.IsValid() {
opt := parser.IdentName(stmt.System)
if !strings.EqualFold("system", opt) {
return nil, sql3.NewErrUnknownShowOption(stmt.System.NamePos.Line, stmt.System.NamePos.Column, opt)
}
showSystem = true
}
tbls, err := p.schemaAPI.Tables(ctx)
if err != nil {
return nil, errors.Wrap(err, "getting tables")
@ -135,7 +147,7 @@ func (p *ExecutionPlanner) compileShowTablesStatement(ctx context.Context, stmt
dataType: parser.NewDataTypeString(),
}}
return NewPlanOpQuery(p, NewPlanOpProjection(columns, NewPlanOpFeatureBaseTables(p, pilosa.TablesToIndexInfos(tbls))), p.sql), nil
return NewPlanOpQuery(p, NewPlanOpProjection(columns, NewPlanOpFeatureBaseTables(p, pilosa.TablesToIndexInfos(tbls), showSystem)), p.sql), nil
}
func (p *ExecutionPlanner) compileShowColumnsStatement(ctx context.Context, stmt *parser.ShowColumnsStatement) (_ types.PlanOperator, err error) {

View file

@ -4,6 +4,7 @@ package planner
import (
"context"
"sort"
"time"
pilosa "github.com/featurebasedb/featurebase/v3"
@ -81,6 +82,11 @@ func (s *systemTableDefinitionsWrapper) Tables(ctx context.Context) ([]*dax.Tabl
tbls = append(tbls, pilosa.IndexInfoToTable(ii))
}
// order the result - by ID asc right now
sort.Slice(tbls, func(i, j int) bool {
return tbls[i].ID < tbls[j].ID
})
return tbls, nil
}

View file

@ -17,16 +17,18 @@ import (
// PlanOpFeatureBaseTables wraps a []*IndexInfo that is returned from
// schemaAPI.Schema().
type PlanOpFeatureBaseTables struct {
planner *ExecutionPlanner
indexInfo []*pilosa.IndexInfo
warnings []string
planner *ExecutionPlanner
indexInfo []*pilosa.IndexInfo
withSystem bool
warnings []string
}
func NewPlanOpFeatureBaseTables(planner *ExecutionPlanner, indexInfo []*pilosa.IndexInfo) *PlanOpFeatureBaseTables {
func NewPlanOpFeatureBaseTables(planner *ExecutionPlanner, indexInfo []*pilosa.IndexInfo, withSystem bool) *PlanOpFeatureBaseTables {
return &PlanOpFeatureBaseTables{
planner: planner,
indexInfo: indexInfo,
warnings: make([]string, 0),
planner: planner,
indexInfo: indexInfo,
withSystem: withSystem,
warnings: make([]string, 0),
}
}
@ -105,65 +107,85 @@ func (p *PlanOpFeatureBaseTables) Children() []types.PlanOperator {
func (p *PlanOpFeatureBaseTables) Iterator(ctx context.Context, row types.Row) (types.RowIterator, error) {
return &showTablesRowIter{
planner: p.planner,
indexInfo: p.indexInfo,
planner: p.planner,
indexInfo: p.indexInfo,
withSystem: p.withSystem,
}, nil
}
func (p *PlanOpFeatureBaseTables) WithChildren(children ...types.PlanOperator) (types.PlanOperator, error) {
return NewPlanOpFeatureBaseTables(p.planner, p.indexInfo), nil
return NewPlanOpFeatureBaseTables(p.planner, p.indexInfo, p.withSystem), nil
}
type showTablesRowIter struct {
planner *ExecutionPlanner
indexInfo []*pilosa.IndexInfo
rowIndex int
planner *ExecutionPlanner
indexInfo []*pilosa.IndexInfo
withSystem bool
result types.Rows
}
var _ types.RowIterator = (*showTablesRowIter)(nil)
func (i *showTablesRowIter) Next(ctx context.Context) (types.Row, error) {
if i.rowIndex < len(i.indexInfo) {
if i.result == nil {
i.result = make(types.Rows, 0)
indexName := i.indexInfo[i.rowIndex].Name
for _, idx := range i.indexInfo {
var err error
var spaceUsed pilosa.DiskUsage
switch strings.ToLower(indexName) {
case fbDatabaseInfo, fbDatabaseNodes, fbPerformanceCounters, fbExecRequests, fbTableDDL:
spaceUsed = pilosa.DiskUsage{
Usage: 0,
indexName := idx.Name
// if we don't want system tables filter them out (currently by name prefix)
// TODO(pok) - we need an is_system attribute so we can filter on that instead
if !i.withSystem && strings.HasPrefix(indexName, "fb_") {
continue
}
default:
u := i.planner.systemAPI.DataDir()
// TODO(tlt): GetDiskUsage needs to be behind an interface because
// this doesn't work in serverless. For now I'm just going to skip
// it based on the emtpy DataDir, but let's do this the right way.
if u != "" {
u = fmt.Sprintf("%s/indexes/%s", u, indexName)
var err error
var spaceUsed pilosa.DiskUsage
switch strings.ToLower(indexName) {
case fbDatabaseInfo, fbDatabaseNodes, fbPerformanceCounters, fbExecRequests, fbTableDDL:
spaceUsed = pilosa.DiskUsage{
Usage: 0,
}
default:
u := i.planner.systemAPI.DataDir()
spaceUsed, err = pilosa.GetDiskUsage(u)
if err != nil {
return nil, err
// TODO(tlt): GetDiskUsage needs to be behind an interface because
// this doesn't work in serverless. For now I'm just going to skip
// it based on the emtpy DataDir, but let's do this the right way.
if u != "" {
u = fmt.Sprintf("%s/indexes/%s", u, indexName)
spaceUsed, err = pilosa.GetDiskUsage(u)
if err != nil {
return nil, err
}
}
}
}
createdAt := time.Unix(0, i.indexInfo[i.rowIndex].CreatedAt)
updatedAt := time.Unix(0, i.indexInfo[i.rowIndex].UpdatedAt)
row := []interface{}{
indexName,
indexName,
i.indexInfo[i.rowIndex].Owner,
i.indexInfo[i.rowIndex].LastUpdateUser,
createdAt.Format(time.RFC3339),
updatedAt.Format(time.RFC3339),
i.indexInfo[i.rowIndex].Options.Keys,
spaceUsed.Usage,
i.indexInfo[i.rowIndex].Options.Description,
createdAt := time.Unix(0, idx.CreatedAt)
updatedAt := time.Unix(0, idx.UpdatedAt)
row := []interface{}{
indexName,
indexName,
idx.Owner,
idx.LastUpdateUser,
createdAt.Format(time.RFC3339),
updatedAt.Format(time.RFC3339),
idx.Options.Keys,
spaceUsed.Usage,
idx.Options.Description,
}
i.result = append(i.result, row)
}
i.rowIndex += 1
}
if len(i.result) > 0 {
row := i.result[0]
// Move to next result element.
i.result = i.result[1:]
return row, nil
}
return nil, types.ErrNoMoreRows

View file

@ -266,6 +266,42 @@ func TestPlanner_Show(t *testing.T) {
}
})
t.Run("ShowTablesWithBadOption", func(t *testing.T) {
_, _, _, err := sql_test.MustQueryRows(t, nil, c.GetNode(0).Server, `SHOW TABLES WITH SCOTCH`)
if err != nil {
if err.Error() != "[1:18] unknown show option 'SCOTCH'" {
t.Fatal(err)
}
}
})
t.Run("ShowTablesWithSystem", func(t *testing.T) {
results, columns, _, err := sql_test.MustQueryRows(t, nil, c.GetNode(0).Server, `SHOW TABLES WITH SYSTEM`)
if err != nil {
t.Fatal(err)
}
// we keep adding system tables on the fly so as
// long as we get more than 0 tables, we're good
if len(results) == 0 {
t.Fatal(fmt.Errorf("unexpected result set length"))
}
if diff := cmp.Diff([]*pilosa.WireQueryField{
wireQueryFieldString("_id"),
wireQueryFieldString("name"),
wireQueryFieldString("owner"),
wireQueryFieldString("updated_by"),
wireQueryFieldTimestamp("created_at"),
wireQueryFieldTimestamp("updated_at"),
wireQueryFieldBool("keys"),
wireQueryFieldInt("space_used"),
wireQueryFieldString("description"),
}, columns); diff != "" {
t.Fatal(diff)
}
})
t.Run("ShowCreateTable", func(t *testing.T) {
results, columns, _, err := sql_test.MustQueryRows(t, nil, c.GetNode(0).Server, fmt.Sprintf(`SHOW CREATE TABLE %i`, c))
if err != nil {