remove frame argument from Frame.SetFieldValue(). Rename it to Frame.SetValue()

This commit is contained in:
Travis Turner 2018-06-01 22:06:45 -05:00
parent 6b9a0e871f
commit 90a4d957bd
No known key found for this signature in database
GPG key ID: 7F08008DFD9314C9
6 changed files with 53 additions and 66 deletions

View file

@ -1109,17 +1109,6 @@ func (e *Executor) executeSetBitView(ctx context.Context, index string, c *pql.C
// executeSetFieldValue executes a SetFieldValue() call.
func (e *Executor) executeSetFieldValue(ctx context.Context, index string, c *pql.Call, opt *ExecOptions) error {
frameName, ok := c.Args["frame"].(string)
if !ok {
return errors.New("SetFieldValue() frame required")
}
// Retrieve frame.
frame := e.Holder.Frame(index, frameName)
if frame == nil {
return ErrFrameNotFound
}
// Parse labels.
columnID, ok, err := c.UintArg(columnLabel)
if err != nil {
@ -1130,23 +1119,28 @@ func (e *Executor) executeSetFieldValue(ctx context.Context, index string, c *pq
// Copy args and remove reserved fields.
args := pql.CopyArgs(c.Args)
delete(args, "frame")
// While frame could technically work as a ColumnAttr argument, we are treating it as a reserved word primarily to avoid confusion.
// Also, if we ever need to make ColumnAttrs frame-specific, then having this reserved word prevents backward incompatibility.
delete(args, columnLabel)
// Set values.
for name, value := range args {
// Retrieve frame.
frame := e.Holder.Frame(index, name)
if frame == nil {
return ErrFrameNotFound
}
switch value := value.(type) {
case int64:
if _, err := frame.SetFieldValue(columnID, name, value); err != nil {
if _, err := frame.SetValue(columnID, value); err != nil {
return err
}
default:
return ErrInvalidFieldValueType
}
frame.Stats.Count("SetFieldValue", 1, 1.0)
}
frame.Stats.Count("SetFieldValue", 1, 1.0)
// Do not forward call if this is already being forwarded.
if opt.Remote {

View file

@ -283,9 +283,9 @@ func TestExecutor_Execute_SetFieldValue(t *testing.T) {
// Set field values.
e := test.NewExecutor(hldr.Holder, test.NewCluster(1))
if _, err := e.Execute(context.Background(), "i", test.MustParse(`SetFieldValue(col=10, frame=f, f=25)`), nil, nil); err != nil {
if _, err := e.Execute(context.Background(), "i", test.MustParse(`SetFieldValue(col=10, f=25)`), nil, nil); err != nil {
t.Fatal(err)
} else if _, err := e.Execute(context.Background(), "i", test.MustParse(`SetFieldValue(col=100, frame=f, f=10)`), nil, nil); err != nil {
} else if _, err := e.Execute(context.Background(), "i", test.MustParse(`SetFieldValue(col=100, f=10)`), nil, nil); err != nil {
t.Fatal(err)
}
@ -319,30 +319,23 @@ func TestExecutor_Execute_SetFieldValue(t *testing.T) {
t.Fatal(err)
}
t.Run("ErrFrameRequired", func(t *testing.T) {
e := test.NewExecutor(hldr.Holder, test.NewCluster(1))
if _, err := e.Execute(context.Background(), "i", test.MustParse(`SetFieldValue(col=10, f=100)`), nil, nil); err == nil || err.Error() != `SetFieldValue() frame required` {
t.Fatalf("unexpected error: %s", err)
}
})
t.Run("ErrColumnFieldRequired", func(t *testing.T) {
e := test.NewExecutor(hldr.Holder, test.NewCluster(1))
if _, err := e.Execute(context.Background(), "i", test.MustParse(`SetFieldValue(invalid_column_name=10, frame=f, f=100)`), nil, nil); err == nil || err.Error() != `SetFieldValue() column field 'col' required` {
if _, err := e.Execute(context.Background(), "i", test.MustParse(`SetFieldValue(invalid_column_name=10, f=100)`), nil, nil); err == nil || err.Error() != `SetFieldValue() column field 'col' required` {
t.Fatalf("unexpected error: %s", err)
}
})
t.Run("ErrColumnFieldValue", func(t *testing.T) {
e := test.NewExecutor(hldr.Holder, test.NewCluster(1))
if _, err := e.Execute(context.Background(), "i", test.MustParse(`SetFieldValue(invalid_column_name="bad_column", frame=f, f=100)`), nil, nil); err == nil || err.Error() != `SetFieldValue() column field 'col' required` {
if _, err := e.Execute(context.Background(), "i", test.MustParse(`SetFieldValue(invalid_column_name="bad_column", f=100)`), nil, nil); err == nil || err.Error() != `SetFieldValue() column field 'col' required` {
t.Fatalf("unexpected error: %s", err)
}
})
t.Run("ErrInvalidFieldValueType", func(t *testing.T) {
e := test.NewExecutor(hldr.Holder, test.NewCluster(1))
if _, err := e.Execute(context.Background(), "i", test.MustParse(`SetFieldValue(col=10, frame=f, f="hello")`), nil, nil); err == nil || err.Error() != `invalid field value type` {
if _, err := e.Execute(context.Background(), "i", test.MustParse(`SetFieldValue(col=10, f="hello")`), nil, nil); err == nil || err.Error() != `invalid field value type` {
t.Fatalf("unexpected error: %s", err)
}
})
@ -598,14 +591,14 @@ func TestExecutor_Execute_MinMax(t *testing.T) {
SetBit(frame=x, row=1, col=1)
SetBit(frame=x, row=2, col=`+strconv.Itoa(SliceWidth+2)+`)
SetFieldValue(frame=f, f=20, col=0)
SetFieldValue(frame=f, f=-5, col=1)
SetFieldValue(frame=f, f=-5, col=2)
SetFieldValue(frame=f, f=10, col=3)
SetFieldValue(frame=f, f=30, col=`+strconv.Itoa(SliceWidth)+`)
SetFieldValue(frame=f, f=40, col=`+strconv.Itoa(SliceWidth+2)+`)
SetFieldValue(frame=f, f=50, col=`+strconv.Itoa((5*SliceWidth)+100)+`)
SetFieldValue(frame=f, f=60, col=`+strconv.Itoa(SliceWidth+1)+`)
SetFieldValue(f=20, col=0)
SetFieldValue(f=-5, col=1)
SetFieldValue(f=-5, col=2)
SetFieldValue(f=10, col=3)
SetFieldValue(f=30, col=`+strconv.Itoa(SliceWidth)+`)
SetFieldValue(f=40, col=`+strconv.Itoa(SliceWidth+2)+`)
SetFieldValue(f=50, col=`+strconv.Itoa((5*SliceWidth)+100)+`)
SetFieldValue(f=60, col=`+strconv.Itoa(SliceWidth+1)+`)
`), nil, nil); err != nil {
t.Fatal(err)
}
@ -706,13 +699,13 @@ func TestExecutor_Execute_Sum(t *testing.T) {
SetBit(frame=x, row=0, col=0)
SetBit(frame=x, row=0, col=`+strconv.Itoa(SliceWidth+1)+`)
SetFieldValue(frame=foo, foo=20, col=0)
SetFieldValue(frame=bar, bar=2000, col=0)
SetFieldValue(frame=foo, foo=30, col=`+strconv.Itoa(SliceWidth)+`)
SetFieldValue(frame=foo, foo=40, col=`+strconv.Itoa(SliceWidth+2)+`)
SetFieldValue(frame=foo, foo=50, col=`+strconv.Itoa((5*SliceWidth)+100)+`)
SetFieldValue(frame=foo, foo=60, col=`+strconv.Itoa(SliceWidth+1)+`)
SetFieldValue(frame=other, other=1000, col=0)
SetFieldValue(foo=20, col=0)
SetFieldValue(bar=2000, col=0)
SetFieldValue(foo=30, col=`+strconv.Itoa(SliceWidth)+`)
SetFieldValue(foo=40, col=`+strconv.Itoa(SliceWidth+2)+`)
SetFieldValue(foo=50, col=`+strconv.Itoa((5*SliceWidth)+100)+`)
SetFieldValue(foo=60, col=`+strconv.Itoa(SliceWidth+1)+`)
SetFieldValue(other=1000, col=0)
`), nil, nil); err != nil {
t.Fatal(err)
}
@ -827,15 +820,15 @@ func TestExecutor_Execute_FieldRange(t *testing.T) {
SetBit(frame=f, row=0, col=0)
SetBit(frame=f, row=0, col=`+strconv.Itoa(SliceWidth+1)+`)
SetFieldValue(frame=foo, foo=20, col=50)
SetFieldValue(frame=bar, bar=2000, col=50)
SetFieldValue(frame=foo, foo=30, col=`+strconv.Itoa(SliceWidth)+`)
SetFieldValue(frame=foo, foo=10, col=`+strconv.Itoa(SliceWidth+2)+`)
SetFieldValue(frame=foo, foo=20, col=`+strconv.Itoa((5*SliceWidth)+100)+`)
SetFieldValue(frame=foo, foo=60, col=`+strconv.Itoa(SliceWidth+1)+`)
SetFieldValue(frame=other, other=1000, col=0)
SetFieldValue(frame=edge, edge=100, col=0)
SetFieldValue(frame=edge, edge=-100, col=1)
SetFieldValue(foo=20, col=50)
SetFieldValue(bar=2000, col=50)
SetFieldValue(foo=30, col=`+strconv.Itoa(SliceWidth)+`)
SetFieldValue(foo=10, col=`+strconv.Itoa(SliceWidth+2)+`)
SetFieldValue(foo=20, col=`+strconv.Itoa((5*SliceWidth)+100)+`)
SetFieldValue(foo=60, col=`+strconv.Itoa(SliceWidth+1)+`)
SetFieldValue(other=1000, col=0)
SetFieldValue(edge=100, col=0)
SetFieldValue(edge=-100, col=1)
`), nil, nil); err != nil {
t.Fatal(err)
}

View file

@ -726,10 +726,10 @@ func (f *Frame) FieldValue(columnID uint64, name string) (value int64, exists bo
return int64(v) + field.Min, true, nil
}
// SetFieldValue sets a field value for a column.
func (f *Frame) SetFieldValue(columnID uint64, name string, value int64) (changed bool, err error) {
// SetValue sets a field value for a column.
func (f *Frame) SetValue(columnID uint64, value int64) (changed bool, err error) {
// Fetch field and validate value.
field := f.Field(name)
field := f.Field(f.name)
if field == nil {
return false, ErrFieldNotFound
} else if value < field.Min {
@ -739,7 +739,7 @@ func (f *Frame) SetFieldValue(columnID uint64, name string, value int64) (change
}
// Fetch target view.
view, err := f.CreateViewIfNotExists(ViewFieldPrefix + name)
view, err := f.CreateViewIfNotExists(ViewFieldPrefix + f.name)
if err != nil {
return false, errors.Wrap(err, "creating view")
}

View file

@ -87,7 +87,7 @@ func TestFrame_SetFieldValue(t *testing.T) {
}
// Set value on field.
if changed, err := f.SetFieldValue(100, "f", 21); err != nil {
if changed, err := f.SetValue(100, 21); err != nil {
t.Fatal(err)
} else if !changed {
t.Fatal("expected change")
@ -103,7 +103,7 @@ func TestFrame_SetFieldValue(t *testing.T) {
}
// Setting value should return no change.
if changed, err := f.SetFieldValue(100, "f", 21); err != nil {
if changed, err := f.SetValue(100, 21); err != nil {
t.Fatal(err)
} else if changed {
t.Fatal("expected no change")
@ -124,14 +124,14 @@ func TestFrame_SetFieldValue(t *testing.T) {
}
// Set value.
if changed, err := f.SetFieldValue(100, "f", 21); err != nil {
if changed, err := f.SetValue(100, 21); err != nil {
t.Fatal(err)
} else if !changed {
t.Fatal("expected change")
}
// Set different value.
if changed, err := f.SetFieldValue(100, "f", 23); err != nil {
if changed, err := f.SetValue(100, 23); err != nil {
t.Fatal(err)
} else if !changed {
t.Fatal("expected change")
@ -152,16 +152,14 @@ func TestFrame_SetFieldValue(t *testing.T) {
defer idx.Close()
f, err := idx.CreateFrame("f", pilosa.FrameOptions{
Type: pilosa.FrameTypeInt,
Min: 0,
Max: 30,
Type: pilosa.FrameTypeSet,
})
if err != nil {
t.Fatal(err)
}
// Set value.
if _, err := f.SetFieldValue(100, "no_such_field", 21); err != pilosa.ErrFieldNotFound {
if _, err := f.SetValue(100, 21); err != pilosa.ErrFieldNotFound {
t.Fatalf("unexpected error: %s", err)
}
})
@ -180,7 +178,7 @@ func TestFrame_SetFieldValue(t *testing.T) {
}
// Set value.
if _, err := f.SetFieldValue(100, "f", 15); err != pilosa.ErrFieldValueTooLow {
if _, err := f.SetValue(100, 15); err != pilosa.ErrFieldValueTooLow {
t.Fatalf("unexpected error: %s", err)
}
})
@ -199,7 +197,7 @@ func TestFrame_SetFieldValue(t *testing.T) {
}
// Set value.
if _, err := f.SetFieldValue(100, "f", 31); err != pilosa.ErrFieldValueTooHigh {
if _, err := f.SetValue(100, 31); err != pilosa.ErrFieldValueTooHigh {
t.Fatalf("unexpected error: %s", err)
}
})

View file

@ -150,6 +150,7 @@ func (t *TestCluster) SetBit(index, frame, view string, rowID, colID uint64, x *
return nil
}
// TODO: remove `name` from this function signature
func (t *TestCluster) SetFieldValue(index, frame string, columnID uint64, name string, value int64) error {
// Determine which node should receive the SetFieldValue.
c0 := t.Clusters[0] // use the first node's cluster to determine slice location.
@ -165,7 +166,7 @@ func (t *TestCluster) SetFieldValue(index, frame string, columnID uint64, name s
if f == nil {
return fmt.Errorf("index/frame does not exist: %s/%s", index, frame)
}
_, err := f.SetFieldValue(columnID, name, value)
_, err := f.SetValue(columnID, value)
if err != nil {
return err
}

View file

@ -141,6 +141,7 @@ func (t *ClusterCluster) SetBit(index, frame, view string, rowID, colID uint64,
return nil
}
// TODO: remove `name` from this function signature
func (t *ClusterCluster) SetFieldValue(index, frame string, columnID uint64, name string, value int64) error {
// Determine which node should receive the SetFieldValue.
c0 := t.Clusters[0] // use the first node's cluster to determine slice location.
@ -156,7 +157,7 @@ func (t *ClusterCluster) SetFieldValue(index, frame string, columnID uint64, nam
if f == nil {
return fmt.Errorf("index/frame does not exist: %s/%s", index, frame)
}
_, err := f.SetFieldValue(columnID, name, value)
_, err := f.SetValue(columnID, value)
if err != nil {
return err
}