From c8c88ab0ee1afa0ccf97044b5de3b32cffb80938 Mon Sep 17 00:00:00 2001 From: Seebs Date: Mon, 3 Apr 2023 15:44:44 -0500 Subject: [PATCH] don't panic on failed table creation The attempt to set the TrackExistence option for fields happened before checking whether the field was created successfully or not. Credit to Rachith for spotting this. Bug was introduced with the TrackExistence stuff, but we apparently never had a test case for invalid min/max values. --- sql3/planner/opaltertable.go | 4 ++-- sql3/planner/opcreatetable.go | 4 ++-- sql3/test/defs/defs_create_table.go | 7 +++++++ 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/sql3/planner/opaltertable.go b/sql3/planner/opaltertable.go index 6da8db0e5..2b6495843 100644 --- a/sql3/planner/opaltertable.go +++ b/sql3/planner/opaltertable.go @@ -94,11 +94,11 @@ func (i *alterTableRowIter) Next(ctx context.Context) (types.Row, error) { fos := i.columnDef.fos fld, err := pilosa.FieldFromFieldOptions(fname, fos...) - // all newly created fields unconditionally have TrackExistence turned on. - fld.Options.TrackExistence = true if err != nil { return nil, err } + // all newly created fields unconditionally have TrackExistence turned on. + fld.Options.TrackExistence = true if err := i.planner.schemaAPI.CreateField(ctx, tname, fld); err != nil { return nil, err diff --git a/sql3/planner/opcreatetable.go b/sql3/planner/opcreatetable.go index 25de51bbf..5b143c9b6 100644 --- a/sql3/planner/opcreatetable.go +++ b/sql3/planner/opcreatetable.go @@ -112,11 +112,11 @@ func (i *createTableRowIter) Next(ctx context.Context) (types.Row, error) { for _, f := range i.columns { fld, err := pilosa.FieldFromFieldOptions(dax.FieldName(f.name), f.fos...) - // We unconditionally turn on TrackExistence for all newly-created fields. - fld.Options.TrackExistence = true if err != nil { return nil, errors.Wrapf(err, "creating field from field options: %s", f.name) } + // We unconditionally turn on TrackExistence for all newly-created fields. + fld.Options.TrackExistence = true fields = append(fields, fld) } diff --git a/sql3/test/defs/defs_create_table.go b/sql3/test/defs/defs_create_table.go index 724dd03f2..b80ffe81f 100644 --- a/sql3/test/defs/defs_create_table.go +++ b/sql3/test/defs/defs_create_table.go @@ -31,6 +31,13 @@ var createTable = TableTest{ ), ExpErr: "expected literal, found bad", }, + { + name: "minAboveMax", + SQLs: sqls( + "create table bar (_id id, i1 int min 20 max 19)", + ), + ExpErr: "int field min cannot be greater than max", + }, { name: "commentString", SQLs: sqls(