Skip to content

Commit

Permalink
fix(length): removes unnecessary TableObject guards (#4500)
Browse files Browse the repository at this point in the history
Now that the type system can distinguish between streams (aka tables)
and vanilla arrays we no longer need the TableObject interface and its
associated type guards.

Fixes #4343
  • Loading branch information
nathanielc authored Feb 18, 2022
1 parent c57baa9 commit 4996f84
Show file tree
Hide file tree
Showing 19 changed files with 7 additions and 137 deletions.
5 changes: 0 additions & 5 deletions compile.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,11 +92,6 @@ type TableObject struct {
Parents []*TableObject
}

// TableObject satisfies the values.TableObject interface.
// This is a hacky workaround to avoid an import cycles.
// XXX: remove when array/stream are different types <https://github.com/influxdata/flux/issues/4343>
func (t *TableObject) TableObject() {}

func (t *TableObject) Operation(ider IDer) *Operation {
if iderOpSpec, ok := t.Spec.(IDerOpSpec); ok {
iderOpSpec.IDer(ider)
Expand Down
4 changes: 0 additions & 4 deletions compiler/runtime.go
Original file line number Diff line number Diff line change
Expand Up @@ -610,10 +610,6 @@ func (e *arrayIndexEvaluator) Eval(ctx context.Context, scope Scope) (values.Val
if typ := i.Type().Nature(); typ != semantic.Int {
return nil, errors.Newf(codes.Invalid, "cannot index into an array with value of type %s; expected an int", typ)
}
// XXX: remove when array/stream are different types <https://github.com/influxdata/flux/issues/4343>
if _, ok := a.(values.TableObject); ok {
return nil, errors.Newf(codes.Invalid, "cannot index into a table stream; expected an array")
}
ix := int(i.Int())
l := a.Array().Len()
if ix < 0 || ix >= l {
Expand Down
28 changes: 0 additions & 28 deletions interpreter/interpreter.go
Original file line number Diff line number Diff line change
Expand Up @@ -236,10 +236,6 @@ func (irtp *Interpreter) evaluateNowOption(ctx context.Context, name string, ini
}

func convert(rules values.Array) ([]string, error) {
// XXX: remove when array/stream are different types <https://github.com/influxdata/flux/issues/4343>
if _, ok := rules.(values.TableObject); ok {
return nil, errors.New(codes.Invalid, "got table stream; expected an array")
}
noRules := rules.Len()
rs := make([]string, noRules)
rules.Range(func(i int, v values.Value) {
Expand Down Expand Up @@ -394,10 +390,6 @@ func (itrp *Interpreter) doExpression(ctx context.Context, expr semantic.Express
return nil, err
}
ix := int(idx.Int())
// XXX: remove when array/stream are different types <https://github.com/influxdata/flux/issues/4343>
if _, ok := arr.(values.TableObject); ok {
return nil, errors.New(codes.Invalid, "cannot index into table stream; expected an array")
}
l := arr.Array().Len()
if ix < 0 || ix >= l {
return nil, errors.Newf(codes.Invalid, "cannot access element %v of array of length %v", ix, l)
Expand Down Expand Up @@ -1271,10 +1263,6 @@ func resolveValue(v values.Value) (semantic.Node, bool, error) {
return nil, false, nil
case semantic.Array:
arr := v.Array()
// XXX: remove when array/stream are different types <https://github.com/influxdata/flux/issues/4343>
if _, ok := arr.(values.TableObject); ok {
return nil, false, errors.New(codes.Invalid, "got a table stream; expected an array")
}
node := new(semantic.ArrayExpression)
node.Elements = make([]semantic.Expression, arr.Len())
var (
Expand Down Expand Up @@ -1380,10 +1368,6 @@ func ToStringArray(a values.Array) ([]string, error) {
if t.Nature() != semantic.String {
return nil, errors.Newf(codes.Invalid, "cannot convert array of %v to an array of strings", t)
}
// XXX: remove when array/stream are different types <https://github.com/influxdata/flux/issues/4343>
if _, ok := a.(values.TableObject); ok {
return nil, errors.New(codes.Invalid, "got a table stream; expected an array")
}
strs := make([]string, a.Len())
a.Range(func(i int, v values.Value) {
strs[i] = v.Str()
Expand All @@ -1398,10 +1382,6 @@ func ToFloatArray(a values.Array) ([]float64, error) {
if t.Nature() != semantic.Float {
return nil, errors.Newf(codes.Invalid, "cannot convert array of %v to an array of floats", t)
}
// XXX: remove when array/stream are different types <https://github.com/influxdata/flux/issues/4343>
if _, ok := a.(values.TableObject); ok {
return nil, errors.New(codes.Invalid, "got a table stream; expected an array")
}
vs := make([]float64, a.Len())
a.Range(func(i int, v values.Value) {
vs[i] = v.Float()
Expand Down Expand Up @@ -1577,10 +1557,6 @@ func (a *arguments) GetArrayAllowEmpty(name string, t semantic.Nature) (values.A
return nil, ok, err
}
arr := v.Array()
// XXX: remove when array/stream are different types <https://github.com/influxdata/flux/issues/4343>
if _, ok := arr.(values.TableObject); ok {
return nil, false, errors.New(codes.Invalid, "got a table stream; expected an array")
}
if arr.Len() > 0 {
et, err := arr.Type().ElemType()
if err != nil {
Expand Down Expand Up @@ -1616,10 +1592,6 @@ func (a *arguments) GetRequiredArrayAllowEmpty(name string, t semantic.Nature) (
return nil, err
}
arr := v.Array()
// XXX: remove when array/stream are different types <https://github.com/influxdata/flux/issues/4343>
if _, ok := arr.(values.TableObject); ok {
return nil, errors.New(codes.Invalid, "got a table stream; expected an array")
}
if arr.Array().Len() > 0 {
et, err := arr.Type().ElemType()
if err != nil {
Expand Down
4 changes: 0 additions & 4 deletions lang/compiler.go
Original file line number Diff line number Diff line change
Expand Up @@ -596,10 +596,6 @@ func getOptionValues(pkg values.Object, optionName string) ([]string, error) {
}

rules := value.Array()
// XXX: remove when array/stream are different types <https://github.com/influxdata/flux/issues/4343>
if _, ok := rules.(values.TableObject); ok {
return nil, errors.New(codes.Invalid, "got a table stream; expected an array")
}
noRules := rules.Len()
rs := make([]string, noRules)
rules.Range(func(i int, v values.Value) {
Expand Down
8 changes: 0 additions & 8 deletions stdlib/array/from.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,6 @@ func createFromOpSpec(args flux.Arguments, a *flux.Administration) (flux.Operati
}
spec.Rows = rows.Array()
}
// XXX: remove when array/stream are different types <https://github.com/influxdata/flux/issues/4343>
if _, ok := spec.Rows.(values.TableObject); ok {
return nil, errors.Newf(codes.Invalid, "rows cannot be a table stream; expected an array")
}
if spec.Rows.Len() == 0 {
return nil, errors.New(codes.Invalid, "rows must be non-empty")
}
Expand Down Expand Up @@ -147,10 +143,6 @@ func buildTable(rows values.Array, mem *memory.Allocator) (flux.Table, error) {

key := execute.NewGroupKey(nil, nil)
builder := table.NewArrowBuilder(key, mem)
// XXX: remove when array/stream are different types <https://github.com/influxdata/flux/issues/4343>
if _, ok := rows.(values.TableObject); ok {
return nil, errors.Newf(codes.Invalid, "rows cannot be a table stream; expected an array")
}
for _, col := range cols {
i, err := builder.AddCol(col)
if err != nil {
Expand Down
4 changes: 0 additions & 4 deletions stdlib/contrib/sranka/opsgenie/responders_to_json.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,6 @@ func RespondersToJSON(args interpreter.Arguments) (values.Value, error) {
if err != nil {
return nil, err
}
// XXX: remove when array/stream are different types <https://github.com/influxdata/flux/issues/4343>
if _, ok := v.(values.TableObject); ok {
return nil, errors.New("got a table stream; expected an array")
}
responders := make([]interface{}, v.Len())
for i := 0; i < v.Len(); i++ {
item := v.Get(i).Str()
Expand Down
4 changes: 0 additions & 4 deletions stdlib/experimental/geo/geo.go
Original file line number Diff line number Diff line change
Expand Up @@ -342,10 +342,6 @@ func parseGeometryArgument(name string, arg values.Object, units *units) (geom i
points, polygonOk := arg.Get("points")
if polygonOk && arg.Len() == 1 {
array := points.Array()
// XXX: remove when array/stream are different types <https://github.com/influxdata/flux/issues/4343>
if _, ok := array.(values.TableObject); ok {
return nil, errors.New(codes.Invalid, "points cannot be a table stream; expected an array")
}
if array.Len() < 3 {
err = errors.Newf(codes.Invalid, "polygon must have at least 3 points")
}
Expand Down
10 changes: 1 addition & 9 deletions stdlib/experimental/mqtt/to.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import (
"github.com/influxdata/flux/runtime"
"github.com/influxdata/flux/semantic"
"github.com/influxdata/flux/values"
"github.com/influxdata/line-protocol"
protocol "github.com/influxdata/line-protocol"
)

const (
Expand Down Expand Up @@ -92,10 +92,6 @@ func (o *ToMQTTOpSpec) ReadArgs(args flux.Arguments) error {
}
o.TagColumns = o.TagColumns[:0]
if ok {
// XXX: remove when array/stream are different types <https://github.com/influxdata/flux/issues/4343>
if _, ok := tagColumns.(values.TableObject); ok {
return errors.New(codes.Invalid, "tagColumns cannot be a table stream; expected an array")
}
for i := 0; i < tagColumns.Len(); i++ {
o.TagColumns = append(o.TagColumns, tagColumns.Get(i).Str())
}
Expand All @@ -107,10 +103,6 @@ func (o *ToMQTTOpSpec) ReadArgs(args flux.Arguments) error {
return err
}
o.ValueColumns = o.ValueColumns[:0]
// XXX: remove when array/stream are different types <https://github.com/influxdata/flux/issues/4343>
if _, ok := valueColumns.(values.TableObject); ok {
return errors.New(codes.Invalid, "valueColumns cannot be a table stream; expected an array")
}
if !ok || valueColumns.Len() == 0 {
o.ValueColumns = append(o.ValueColumns, execute.DefaultValueColLabel)
} else {
Expand Down
4 changes: 0 additions & 4 deletions stdlib/influxdata/influxdb/to.go
Original file line number Diff line number Diff line change
Expand Up @@ -504,10 +504,6 @@ func (o *ToOpSpec) ReadArgs(args flux.Arguments) error {
}

if tags, ok, _ := args.GetArray("tagColumns", semantic.String); ok {
// XXX: remove when array/stream are different types <https://github.com/influxdata/flux/issues/4343>
if _, ok := tags.(values.TableObject); ok {
return errors.New(codes.Invalid, "tagColumns cannot be a table stream; expected an array")
}
o.TagColumns = make([]string, tags.Len())
tags.Sort(func(i, j values.Value) bool {
return i.Str() < j.Str()
Expand Down
12 changes: 0 additions & 12 deletions stdlib/kafka/to.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,10 +70,6 @@ func (o *ToKafkaOpSpec) ReadArgs(args flux.Arguments) error {
if err != nil {
return err
}
// XXX: remove when array/stream are different types <https://github.com/influxdata/flux/issues/4343>
if _, ok := brokers.(values.TableObject); ok {
return errors.New(codes.Invalid, "brokers cannot be a table stream; expected an array")
}
l := brokers.Len()

o.Brokers = make([]string, l)
Expand Down Expand Up @@ -123,10 +119,6 @@ func (o *ToKafkaOpSpec) ReadArgs(args flux.Arguments) error {
}
o.TagColumns = o.TagColumns[:0]
if ok {
// XXX: remove when array/stream are different types <https://github.com/influxdata/flux/issues/4343>
if _, ok := tagColumns.(values.TableObject); ok {
return errors.New(codes.Invalid, "tagColumns cannot be a table stream; expected an array")
}
for i := 0; i < tagColumns.Len(); i++ {
o.TagColumns = append(o.TagColumns, tagColumns.Get(i).Str())
}
Expand All @@ -136,10 +128,6 @@ func (o *ToKafkaOpSpec) ReadArgs(args flux.Arguments) error {
if err != nil {
return err
}
// XXX: remove when array/stream are different types <https://github.com/influxdata/flux/issues/4343>
if _, ok := valueColumns.(values.TableObject); ok {
return errors.New(codes.Invalid, "valueColumns cannot be a table stream; expected an array")
}
o.ValueColumns = o.ValueColumns[:0]
if !ok || valueColumns.Len() == 0 {
o.ValueColumns = append(o.ValueColumns, execute.DefaultValueColLabel)
Expand Down
4 changes: 0 additions & 4 deletions stdlib/pagerduty/pagerduty.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,6 @@ func createDedupKeyOpSpec(args flux.Arguments, a *flux.Administration) (flux.Ope
},
)
}
// XXX: remove when array/stream are different types <https://github.com/influxdata/flux/issues/4343>
if _, ok := exclude.(values.TableObject); ok {
return nil, errors.New(codes.Invalid, "exclude cannot be a table stream; expected an array")
}
spec := &DedupKeyOpSpec{
Exclude: make([]string, exclude.Len()),
}
Expand Down
4 changes: 0 additions & 4 deletions stdlib/strings/strings.go
Original file line number Diff line number Diff line change
Expand Up @@ -447,10 +447,6 @@ func init() {
return nil, fmt.Errorf("missing argument %q", "arr")
}
arr := val.Array()
// XXX: remove when array/stream are different types <https://github.com/influxdata/flux/issues/4343>
if _, ok := arr.(values.TableObject); ok {
return nil, fmt.Errorf("%q cannot be a table stream; expected an array", "arr")
}
if arr.Len() >= 0 {
et, _ := arr.Type().ElemType()
if et.Nature() != semantic.String {
Expand Down
4 changes: 0 additions & 4 deletions stdlib/universe/contains.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,6 @@ func MakeContainsFunc() values.Function {

set := setarg.Array()
found := false
// XXX: remove when array/stream are different types <https://github.com/influxdata/flux/issues/4343>
if _, ok := set.(values.TableObject); ok {
return nil, errors.New(codes.Invalid, "set cannot be a table stream; expected an array")
}

if set.Len() > 0 {
for i := 0; i < set.Len(); i++ {
Expand Down
3 changes: 0 additions & 3 deletions stdlib/universe/join.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,6 @@ func createJoinOpSpec(args flux.Arguments, a *flux.Administration) (flux.Operati
// On specifies the columns to join on, and is required.
if array, err := args.GetRequiredArray("on", semantic.String); err != nil {
return nil, err
} else if _, ok := array.(values.TableObject); ok {
// XXX: remove when array/stream are different types <https://github.com/influxdata/flux/issues/4343>
return nil, errors.New(codes.Invalid, "on cannot be a table stream; expected an array")
} else if array.Len() == 0 {
return nil, errors.New(codes.Invalid, "at least one column in 'on' column list is required")
} else {
Expand Down
17 changes: 3 additions & 14 deletions stdlib/universe/length.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,20 +27,9 @@ func MakeLengthFunc() values.Function {
return nil, errors.Newf(codes.Invalid, "arr must be an array, got %s", got)
}

maybeArr := v.Array()
switch maybeArr.(type) {
// The type system currently conflates TableObject (aka "table stream")
// with fully-realized arrays of records requiring it to implement
// the Array interface. Since TableObject will currently panic
// if the methods provided by this interface are invoked, short-circuit
// by returning an error.
// XXX: remove when array/stream are different types <https://github.com/influxdata/flux/issues/4343>
case values.TableObject:
return nil, errors.New(codes.Invalid, "arr must be an array, got table stream")
default:
l := maybeArr.Len()
return values.NewInt(int64(l)), nil
}
arr := v.Array()
l := arr.Len()
return values.NewInt(int64(l)), nil
}, false,
)
}
Expand Down
7 changes: 0 additions & 7 deletions values/array.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,13 +128,6 @@ func (a *array) Equal(rhs Value) bool {
return false
}
r := rhs.Array()
// XXX: remove when array/stream are different types <https://github.com/influxdata/flux/issues/4343>
if _, ok := r.(TableObject); ok {
// When RHS is a table stream instead of array, mark it false.
// This short-circuits the invalid `Len()` call below that would
// otherwise panic.
return false
}
if a.Len() != r.Len() {
return false
}
Expand Down
6 changes: 3 additions & 3 deletions values/display.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,6 @@ func display(w *bufio.Writer, v Value, indent int) (err error) {
if v.IsNull() {
_, err = w.WriteString("<null>")
return
} else if _, ok := v.(TableObject); ok {
_, err = w.WriteString("<table stream>")
return
}
switch v.Type().Nature() {
default:
Expand All @@ -43,6 +40,9 @@ func display(w *bufio.Writer, v Value, indent int) (err error) {
case semantic.Invalid:
_, err = w.WriteString("<invalid>")
return
case semantic.Stream:
_, err = w.WriteString("<stream>")
return
case semantic.String:
_, err = w.WriteString(v.Str())
return
Expand Down
5 changes: 0 additions & 5 deletions values/display_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"time"

"github.com/google/go-cmp/cmp"
"github.com/influxdata/flux"
"github.com/influxdata/flux/semantic"
"github.com/influxdata/flux/values"
)
Expand All @@ -18,10 +17,6 @@ func TestDisplay(t *testing.T) {
value values.Value
display string
}{
{
value: &flux.TableObject{},
display: "<table stream>",
},
{
value: values.NewNull(semantic.BasicInt),
display: "<null>",
Expand Down
11 changes: 0 additions & 11 deletions values/values.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,6 @@ type Typer interface {
Type() semantic.MonoType
}

// TableObject serves as sort of a "marker trait" to allow us to check if a
// value is a flux.TableObject without having to import the flux package which
// in many cases will cause a cyclical import.
// XXX: remove when array/stream are different types <https://github.com/influxdata/flux/issues/4343>
type TableObject interface {
TableObject()
}

type Value interface {
Typer
IsNull() bool
Expand Down Expand Up @@ -200,9 +192,6 @@ func Unwrap(v Value) interface{} {
return v.Regexp()
case semantic.Array:
arr := v.Array()
if _, ok := arr.(TableObject); ok {
panic(errors.New(codes.Invalid, "cannot unwrap a table stream"))
}
a := make([]interface{}, arr.Len())
arr.Range(func(i int, v Value) {
val := Unwrap(v)
Expand Down

0 comments on commit 4996f84

Please sign in to comment.