SHOW CREATE TABLE issues (fb-1810) (#2365)

* fixed ddl issues with cache type/size; removed shardwidth option; improved error message

(cherry picked from commit c88d60c9ab)
This commit is contained in:
pokeeffe-molecula 2022-12-14 16:27:55 -06:00 committed by Joe Friedrich
parent f6c0cf1112
commit 4ef70c19da
9 changed files with 77 additions and 109 deletions

View file

@ -75,10 +75,10 @@ const (
ErrTableIDColumnConstraints errors.Code = "ErrTableIDColumnConstraints"
ErrTableIDColumnAlter errors.Code = "ErrTableIDColumnAlter"
ErrTableNotFound errors.Code = "ErrTableNotFound"
ErrTableExists errors.Code = "ErrTableExists"
ErrColumnNotFound errors.Code = "ErrColumnNotFound"
ErrTableColumnNotFound errors.Code = "ErrTableColumnNotFound"
ErrInvalidKeyPartitionsValue errors.Code = "ErrInvalidKeyPartitionsValue"
ErrInvalidShardWidthValue errors.Code = "ErrInvalidShardWidthValue"
ErrBadColumnConstraint errors.Code = "ErrBadColumnConstraint"
ErrConflictingColumnConstraint errors.Code = "ErrConflictingColumnConstraint"
@ -504,6 +504,13 @@ func NewErrTableNotFound(line, col int, tableName string) error {
)
}
func NewErrTableExists(line, col int, tableName string) error {
return errors.New(
ErrTableExists,
fmt.Sprintf("[%d:%d] table '%s' already exists", line, col, tableName),
)
}
func NewErrColumnNotFound(line, col int, columnName string) error {
return errors.New(
ErrColumnNotFound,
@ -525,13 +532,6 @@ func NewErrInvalidKeyPartitionsValue(line, col int, keypartitions int64) error {
)
}
func NewErrInvalidShardWidthValue(line, col int, shardwidth int64) error {
return errors.New(
ErrInvalidShardWidthValue,
fmt.Sprintf("[%d:%d] invalid value '%d' for shardwidth (should be a number that is a power of 2 and greater or equal to 2^16)", line, col, shardwidth),
)
}
func NewErrBadColumnConstraint(line, col int, constraint, columnType string) error {
return errors.New(
ErrBadColumnConstraint,

View file

@ -78,7 +78,6 @@ func (*ResultColumn) node() {}
func (*RollbackStatement) node() {}
func (*SavepointStatement) node() {}
func (*SelectStatement) node() {}
func (*ShardWidthOption) node() {}
func (*StringLit) node() {}
func (*TableValuedFunction) node() {}
func (*TimeUnitConstraint) node() {}
@ -771,7 +770,6 @@ type TableOption interface {
}
func (*KeyPartitionsOption) option() {}
func (*ShardWidthOption) option() {}
func (*CommentOption) option() {}
type KeyPartitionsOption struct {
@ -787,19 +785,6 @@ func (o *KeyPartitionsOption) String() string {
return buf.String()
}
type ShardWidthOption struct {
ShardWidth Pos // position of SHARDWIDTH keyword
Expr Expr // expression
}
func (o *ShardWidthOption) String() string {
var buf bytes.Buffer
buf.WriteString("SHARDWIDTH (")
buf.WriteString(o.Expr.String())
buf.WriteString(")")
return buf.String()
}
type CommentOption struct {
Comment Pos // position of COMMENT keyword
Expr Expr // expression

View file

@ -441,15 +441,13 @@ func (p *Parser) parseTableOption() (_ TableOption, err error) {
var optionPos Pos
// Parse column constraints.
// Parse table options.
switch p.peek() {
case KEYPARTITIONS:
return p.parseKeyPartitionsOption(optionPos)
case COMMENT:
return p.parseCommentOption(optionPos)
default:
assert(p.peek() == SHARDWIDTH)
return p.parseShardWidthOption(optionPos)
assert(p.peek() == COMMENT)
return p.parseCommentOption(optionPos)
}
}
@ -484,21 +482,6 @@ func (p *Parser) parseKeyPartitionsOption(optionPos Pos) (_ *KeyPartitionsOption
return &opt, nil
}
func (p *Parser) parseShardWidthOption(optionPos Pos) (_ *ShardWidthOption, err error) {
assert(p.peek() == SHARDWIDTH)
var opt ShardWidthOption
opt.ShardWidth, _, _ = 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) parseColumnDefinitions() (_ []*ColumnDefinition, err error) {
var columns []*ColumnDefinition
for {
@ -3436,7 +3419,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, COMMENT:
case KEYPARTITIONS, COMMENT:
return true
default:
return false

View file

@ -214,7 +214,6 @@ const (
SELECT
SELECT_COLUMN
SET
SHARDWIDTH
SIZE
SHOW
SPAN
@ -438,7 +437,6 @@ var tokens = [...]string{
SELECT_COLUMN: "SELECT_COLUMN",
SET: "SET",
SIZE: "SIZE",
SHARDWIDTH: "SHARDWIDTH",
SHOW: "SHOW",
SPAN: "SPAN",
TABLE: "TABLE",

View file

@ -67,7 +67,11 @@ func (p *ExecutionPlanner) compileCreateTableStatement(stmt *parser.CreateTableS
columns = append(columns, column)
}
return NewPlanOpQuery(p, NewPlanOpCreateTable(p, tableName, failIfExists, isKeyed, keyPartitions, description, columns), p.sql), nil
cop := NewPlanOpCreateTable(p, tableName, failIfExists, isKeyed, keyPartitions, description, columns)
if keyPartitions > 0 {
cop.AddWarning("The value of KEYPARTITIONS is currently ignored")
}
return NewPlanOpQuery(p, cop, p.sql), nil
}
// compiles a column def
@ -289,22 +293,6 @@ func (p *ExecutionPlanner) analyzeCreateTableStatement(stmt *parser.CreateTableS
return sql3.NewErrInvalidKeyPartitionsValue(o.Expr.Pos().Line, o.Expr.Pos().Column, i)
}
case *parser.ShardWidthOption:
//check the type of the expression
literal, ok := o.Expr.(*parser.IntegerLit)
if !ok {
return sql3.NewErrIntegerLiteral(o.Expr.Pos().Line, o.Expr.Pos().Column)
}
//shardwidth needs to be a power of 2 and > 2^16
i, err := strconv.ParseInt(literal.Value, 10, 64)
if err != nil {
return err
}
isPwrOf2 := (i & (i - 1)) == 0
if (i == 0) || !isPwrOf2 || i < (1<<16) {
return sql3.NewErrInvalidShardWidthValue(o.Expr.Pos().Line, o.Expr.Pos().Column, i)
}
case *parser.CommentOption:
_, ok := o.Expr.(*parser.StringLit)

View file

@ -135,7 +135,7 @@ func (i *createTableRowIter) Next(ctx context.Context) (types.Row, error) {
if err := i.planner.schemaAPI.CreateTable(ctx, tbl); err != nil {
if _, ok := errors.Cause(err).(pilosa.ConflictError); ok {
if i.failIfExists {
return nil, err
return nil, sql3.NewErrTableExists(0, 0, i.tableName)
}
} else {
return nil, err

View file

@ -68,11 +68,6 @@ var systemTables = map[string]*systemTable{
ColumnName: "node_count",
Type: parser.NewDataTypeInt(),
},
&types.PlannerColumn{
RelationName: fbClusterInfo,
ColumnName: "shard_width",
Type: parser.NewDataTypeInt(),
},
&types.PlannerColumn{
RelationName: fbClusterInfo,
ColumnName: "replica_count",
@ -312,7 +307,6 @@ func (i *fbClusterInfoRowIter) Next(ctx context.Context) (types.Row, error) {
i.planner.systemAPI.Version(),
i.planner.systemAPI.ClusterState(),
i.planner.systemAPI.ClusterNodeCount(),
i.planner.systemAPI.ShardWidth(),
i.planner.systemAPI.ClusterReplicaCount(),
}
i.rowIndex += 1
@ -436,7 +430,11 @@ func (i *fbTableDDLRowIter) Next(ctx context.Context) (types.Row, error) {
fmt.Fprintf(&buf, " cachetype %s", col.Options.CacheType)
}
if col.Options.CacheSize != pilosa.DefaultCacheSize && col.Options.CacheSize > 0 {
fmt.Fprintf(&buf, " cachesize %d", col.Options.CacheSize)
// if we still have the default, we need to print that out if we have a non-default size
if col.Options.CacheType == pilosa.DefaultCacheType && len(col.Options.CacheType) > 0 {
fmt.Fprintf(&buf, " cachetype %s", col.Options.CacheType)
}
fmt.Fprintf(&buf, " size %d", col.Options.CacheSize)
}
case *parser.DataTypeIDSet, *parser.DataTypeStringSet:
@ -444,7 +442,11 @@ func (i *fbTableDDLRowIter) Next(ctx context.Context) (types.Row, error) {
fmt.Fprintf(&buf, " cachetype %s", col.Options.CacheType)
}
if col.Options.CacheSize != pilosa.DefaultCacheSize && col.Options.CacheSize > 0 {
fmt.Fprintf(&buf, " cachesize %d", col.Options.CacheSize)
// if we still have the default, we need to print that out if we have a non-default size
if col.Options.CacheType == pilosa.DefaultCacheType && len(col.Options.CacheType) > 0 {
fmt.Fprintf(&buf, " cachetype %s", col.Options.CacheType)
}
fmt.Fprintf(&buf, " size %d", col.Options.CacheSize)
}
case *parser.DataTypeIDSetQuantum, *parser.DataTypeStringSetQuantum:
@ -452,7 +454,11 @@ func (i *fbTableDDLRowIter) Next(ctx context.Context) (types.Row, error) {
fmt.Fprintf(&buf, " cachetype %s", col.Options.CacheType)
}
if col.Options.CacheSize != pilosa.DefaultCacheSize && col.Options.CacheSize > 0 {
fmt.Fprintf(&buf, " cachesize %d", col.Options.CacheSize)
// if we still have the default, we need to print that out if we have a non-default size
if col.Options.CacheType == pilosa.DefaultCacheType && len(col.Options.CacheType) > 0 {
fmt.Fprintf(&buf, " cachetype %s", col.Options.CacheType)
}
fmt.Fprintf(&buf, " size %d", col.Options.CacheSize)
}
if !col.Options.TimeQuantum.IsEmpty() {
fmt.Fprintf(&buf, " timequantum '%s'", col.Options.TimeQuantum)

View file

@ -68,7 +68,7 @@ func TestPlanner_Show(t *testing.T) {
}
t.Run("SystemTablesInfo", func(t *testing.T) {
results, columns, err := sql_test.MustQueryRows(t, c.GetNode(0).Server, `select name, platform, platform_version, db_version, state, node_count, shard_width, replica_count from fb_cluster_info`)
results, columns, err := sql_test.MustQueryRows(t, c.GetNode(0).Server, `select name, platform, platform_version, db_version, state, node_count, replica_count from fb_cluster_info`)
if err != nil {
t.Fatal(err)
}
@ -83,7 +83,6 @@ func TestPlanner_Show(t *testing.T) {
wireQueryFieldString("db_version"),
wireQueryFieldString("state"),
wireQueryFieldInt("node_count"),
wireQueryFieldInt("shard_width"),
wireQueryFieldInt("replica_count"),
}, columns); diff != "" {
t.Fatal(diff)
@ -204,6 +203,42 @@ func TestPlanner_Show(t *testing.T) {
}
})
t.Run("ShowCreateTableCacheTypes", func(t *testing.T) {
_, _, err := sql_test.MustQueryRows(t, c.GetNode(0).Server, `create table iris1 (
_id id,
speciesid id cachetype ranked size 1000
species string cachetype ranked size 1000
speciesids idset cachetype ranked size 1000
speciess stringset cachetype ranked size 1000
speciesidsq idset timequantum 'YMD'
speciessq stringset timequantum 'YMD'
) keypartitions 12
`)
if err != nil {
t.Fatal(err)
}
results, columns, err := sql_test.MustQueryRows(t, c.GetNode(0).Server, `SHOW CREATE TABLE iris1`)
if err != nil {
t.Fatal(err)
}
if len(results) != 1 {
t.Fatal(fmt.Errorf("unexpected result set length: %d", len(results)))
}
if diff := cmp.Diff([][]interface{}{
{string("create table iris1 (_id id, speciesid id cachetype ranked size 1000, species string cachetype ranked size 1000, speciesids idset cachetype ranked size 1000, speciess stringset cachetype ranked size 1000, speciesidsq idset timequantum 'YMD', speciessq stringset timequantum 'YMD');")},
}, results); diff != "" {
t.Fatal(diff)
}
if diff := cmp.Diff([]*pilosa.WireQueryField{
wireQueryFieldString("ddl"),
}, columns); diff != "" {
t.Fatal(diff)
}
})
t.Run("ShowColumns", func(t *testing.T) {
results, columns, err := sql_test.MustQueryRows(t, c.GetNode(0).Server, fmt.Sprintf(`SHOW COLUMNS FROM %i`, c))
if err != nil {
@ -313,7 +348,7 @@ func TestPlanner_CoverCreateTable(t *testing.T) {
// Build the create table statement based on the fields slice above.
sql := "create table " + tableName + "_" + fld.name + " (_id id, "
sql += fld.name + " " + fld.typ + " " + fld.constraints
sql += `) keypartitions 12 shardwidth 1024`
sql += `) keypartitions 12`
// Run the create table statement.
_, _, err := sql_test.MustQueryRows(t, server, sql)
@ -478,7 +513,7 @@ func TestPlanner_CoverCreateTable(t *testing.T) {
}
}
sql += strings.Join(fieldDefs, ", ")
sql += `) keypartitions 12 shardwidth 65536`
sql += `) keypartitions 12`
// Run the create table statement.
results, columns, err := sql_test.MustQueryRows(t, server, sql)
@ -553,7 +588,7 @@ func TestPlanner_CreateTable(t *testing.T) {
stringcol string,
stringsetcol stringset,
idcol id,
idsetcol idset) keypartitions 12 shardwidth 65536`)
idsetcol idset) keypartitions 12`)
if err != nil {
t.Fatal(err)
}
@ -576,11 +611,11 @@ func TestPlanner_CreateTable(t *testing.T) {
stringcol string,
stringsetcol stringset,
idcol id,
idsetcol idset) keypartitions 12 shardwidth 65536`)
idsetcol idset) keypartitions 12`)
if err == nil {
t.Fatal("expected error")
} else {
if err.Error() != "creating index: index already exists" {
if err.Error() != "[0:0] table 'allcoltypes' already exists" {
t.Fatal(err)
}
}
@ -606,7 +641,7 @@ func TestPlanner_CreateTable(t *testing.T) {
idcol id cachetype ranked size 1000,
idsetcol idset cachetype lru,
idsetcolsz idset cachetype lru size 1000,
idsetcolq idset timequantum 'YMD' ttl '24h') keypartitions 12 shardwidth 65536`)
idsetcolq idset timequantum 'YMD' ttl '24h') keypartitions 12`)
if err != nil {
t.Fatal(err)
}
@ -1740,7 +1775,7 @@ func TestPlanner_BulkInsert(t *testing.T) {
petallength decimal(2),
petalwidth decimal(2),
species string cachetype ranked size 1000
) keypartitions 12 shardwidth 65536;`)
) keypartitions 12;`)
if err != nil {
t.Fatal(err)
}

View file

@ -17,33 +17,6 @@ var createTable = TableTest{
),
ExpErr: "invalid value '10001' for key partitions (should be a number between 1-10000)",
},
{
name: "shardWidthSetTo0",
SQLs: sqls(
"create table foo (_id id, i1 int) shardwidth 0",
),
ExpErr: "invalid value '0' for shardwidth (should be a number that is a power of 2 and greater or equal to 2^16)",
},
{
name: "shardWidthSetTo11",
SQLs: sqls(
"create table foo (_id id, i1 int) shardwidth 11",
),
ExpErr: "invalid value '11' for shardwidth (should be a number that is a power of 2 and greater or equal to 2^16)",
},
{
name: "shardWidthSetTo11",
SQLs: sqls(
"create table foo (_id id, i1 int) shardwidth 32",
),
ExpErr: "invalid value '32' for shardwidth (should be a number that is a power of 2 and greater or equal to 2^16)",
},
{
name: "shardWidthSetTo131072",
SQLs: sqls(
"create table foo (_id id, i1 int) shardwidth 131072",
),
},
{
name: "commentInt",
SQLs: sqls(