From ad350c2d49944a1ae61c66eea626cf39976df525 Mon Sep 17 00:00:00 2001 From: rachithrr Date: Wed, 7 Dec 2022 21:05:54 +0530 Subject: [PATCH] FB-1739: Add ability to add a description to a table on creation (#2332) * FB-1739: Add ability to add a description to a table on creation - Added CommentOption to handle text after COMMENT option. - added description field in the createtable plan. - The description is stored in the existing index metadata. --- sql3/parser/ast.go | 15 +++++++++++++++ sql3/parser/parser.go | 20 +++++++++++++++++++- sql3/parser/token.go | 4 ++-- sql3/planner/compilecreatetable.go | 13 ++++++++++++- sql3/planner/opcreatetable.go | 8 +++++++- sql3/test/defs/defs_create_table.go | 20 ++++++++++++++++++++ 6 files changed, 75 insertions(+), 5 deletions(-) diff --git a/sql3/parser/ast.go b/sql3/parser/ast.go index 0415b4c3e..84bc019fa 100644 --- a/sql3/parser/ast.go +++ b/sql3/parser/ast.go @@ -93,6 +93,7 @@ func (*UsingConstraint) node() {} func (*Window) node() {} func (*WindowDefinition) node() {} func (*WithClause) node() {} +func (*CommentOption) node() {} type Statement interface { Node @@ -771,6 +772,7 @@ type TableOption interface { func (*KeyPartitionsOption) option() {} func (*ShardWidthOption) option() {} +func (*CommentOption) option() {} type KeyPartitionsOption struct { KeyPartitions Pos // position of KEYPARTITIONS keyword @@ -798,6 +800,19 @@ func (o *ShardWidthOption) String() string { return buf.String() } +type CommentOption struct { + Comment Pos // position of COMMENT keyword + Expr Expr // expression +} + +func (o *CommentOption) String() string { + var buf bytes.Buffer + buf.WriteString("COMMENT (") + buf.WriteString(o.Expr.String()) + buf.WriteString(")") + return buf.String() +} + type Constraint interface { Node constraint() diff --git a/sql3/parser/parser.go b/sql3/parser/parser.go index 7320a3b5e..763d7e6ab 100644 --- a/sql3/parser/parser.go +++ b/sql3/parser/parser.go @@ -445,12 +445,30 @@ func (p *Parser) parseTableOption() (_ TableOption, err error) { switch p.peek() { case KEYPARTITIONS: return p.parseKeyPartitionsOption(optionPos) + case COMMENT: + return p.parseCommentOption(optionPos) default: assert(p.peek() == SHARDWIDTH) return p.parseShardWidthOption(optionPos) } } +func (p *Parser) parseCommentOption(optionPos Pos) (_ *CommentOption, err error) { + assert(p.peek() == COMMENT) + + var opt CommentOption + + opt.Comment, _, _ = p.scan() + + if isLiteralToken(p.peek()) { + opt.Expr = p.mustParseLiteral() + } else { + return &opt, p.errorExpected(p.pos, p.tok, "literal") + } + + return &opt, nil +} + func (p *Parser) parseKeyPartitionsOption(optionPos Pos) (_ *KeyPartitionsOption, err error) { assert(p.peek() == KEYPARTITIONS) @@ -3418,7 +3436,7 @@ func (e Error) Error() string { // isTableOptionStartToken returns true if tok is the initial token of a table option. func isTableOptionStartToken(tok Token) bool { switch tok { - case KEYPARTITIONS, SHARDWIDTH: + case KEYPARTITIONS, SHARDWIDTH, COMMENT: return true default: return false diff --git a/sql3/parser/token.go b/sql3/parser/token.go index d3b11494b..b79809c4d 100644 --- a/sql3/parser/token.go +++ b/sql3/parser/token.go @@ -28,7 +28,6 @@ const ( // Special tokens ILLEGAL Token = iota EOF - COMMENT SPACE literal_beg @@ -103,6 +102,7 @@ const ( COLUMNS COLUMNKW COMMIT + COMMENT CONFLICT CONSTRAINT CREATE @@ -256,7 +256,6 @@ const ( var tokens = [...]string{ ILLEGAL: "ILLEGAL", EOF: "EOF", - COMMENT: "COMMENT", SPACE: "SPACE", IDENT: "IDENT", @@ -326,6 +325,7 @@ var tokens = [...]string{ COLUMNS: "COLUMNS", COLUMNKW: "COLUMNKW", COMMIT: "COMMIT", + COMMENT: "COMMENT", CONFLICT: "CONFLICT", CONSTRAINT: "CONSTRAINT", CREATE: "CREATE", diff --git a/sql3/planner/compilecreatetable.go b/sql3/planner/compilecreatetable.go index 842526b37..ae40b8797 100644 --- a/sql3/planner/compilecreatetable.go +++ b/sql3/planner/compilecreatetable.go @@ -30,6 +30,7 @@ func (p *ExecutionPlanner) compileCreateTableStatement(stmt *parser.CreateTableS // apply table options keyPartitions := 0 + description := "" for _, option := range stmt.Options { switch o := option.(type) { case *parser.KeyPartitionsOption: @@ -39,6 +40,9 @@ func (p *ExecutionPlanner) compileCreateTableStatement(stmt *parser.CreateTableS return nil, err } keyPartitions = int(i) + case *parser.CommentOption: + e := o.Expr.(*parser.StringLit) + description = e.Value } } @@ -63,7 +67,7 @@ func (p *ExecutionPlanner) compileCreateTableStatement(stmt *parser.CreateTableS columns = append(columns, column) } - return NewPlanOpQuery(p, NewPlanOpCreateTable(p, tableName, failIfExists, isKeyed, keyPartitions, columns), p.sql), nil + return NewPlanOpQuery(p, NewPlanOpCreateTable(p, tableName, failIfExists, isKeyed, keyPartitions, description, columns), p.sql), nil } // compiles a column def @@ -301,6 +305,13 @@ func (p *ExecutionPlanner) analyzeCreateTableStatement(stmt *parser.CreateTableS return sql3.NewErrInvalidShardWidthValue(o.Expr.Pos().Line, o.Expr.Pos().Column, i) } + case *parser.CommentOption: + + _, ok := o.Expr.(*parser.StringLit) + if !ok { + return sql3.NewErrStringLiteral(o.Expr.Pos().Line, o.Expr.Pos().Column) + } + default: return sql3.NewErrInternalf("unhandled table option type '%T'", option) } diff --git a/sql3/planner/opcreatetable.go b/sql3/planner/opcreatetable.go index 55ee15c8e..d7625c29f 100644 --- a/sql3/planner/opcreatetable.go +++ b/sql3/planner/opcreatetable.go @@ -18,11 +18,13 @@ type PlanOpCreateTable struct { failIfExists bool isKeyed bool keyPartitions int + description string columns []*createTableField warnings []string } -func NewPlanOpCreateTable(p *ExecutionPlanner, tableName string, failIfExists bool, isKeyed bool, keyPartitions int, columns []*createTableField) *PlanOpCreateTable { +// NewPlanOpCreateTable returns a new PlanOpCreateTable planoperator +func NewPlanOpCreateTable(p *ExecutionPlanner, tableName string, failIfExists bool, isKeyed bool, keyPartitions int, description string, columns []*createTableField) *PlanOpCreateTable { return &PlanOpCreateTable{ planner: p, tableName: tableName, @@ -30,6 +32,7 @@ func NewPlanOpCreateTable(p *ExecutionPlanner, tableName string, failIfExists bo isKeyed: isKeyed, keyPartitions: keyPartitions, columns: columns, + description: description, warnings: make([]string, 0), } } @@ -75,6 +78,7 @@ func (p *PlanOpCreateTable) Iterator(ctx context.Context, row types.Row) (types. isKeyed: p.isKeyed, keyPartitions: p.keyPartitions, columns: p.columns, + description: p.description, }, nil } @@ -88,6 +92,7 @@ type createTableRowIter struct { failIfExists bool isKeyed bool keyPartitions int + description string columns []*createTableField } @@ -99,6 +104,7 @@ func (i *createTableRowIter) Next(ctx context.Context) (types.Row, error) { Keys: i.isKeyed, TrackExistence: true, PartitionN: i.keyPartitions, + Description: i.description, } fields := make([]pilosa.CreateFieldObj, len(i.columns)) diff --git a/sql3/test/defs/defs_create_table.go b/sql3/test/defs/defs_create_table.go index 64c2fbe92..7d9b8f22f 100644 --- a/sql3/test/defs/defs_create_table.go +++ b/sql3/test/defs/defs_create_table.go @@ -44,6 +44,26 @@ var createTable = TableTest{ "create table foo (_id id, i1 int) shardwidth 131072", ), }, + { + name: "commentInt", + SQLs: sqls( + "create table foo (_id id, i1 int) comment 34", + ), + ExpErr: "string literal expected", + }, + { + name: "commentStringNoQuote", + SQLs: sqls( + "create table foo (_id id, i1 int) comment bad", + ), + ExpErr: "expected literal, found bad", + }, + { + name: "commentString", + SQLs: sqls( + "create table bar (_id id, i1 int) comment 'this should work'", + ), + }, }, }