Skip to content

Commit

Permalink
feat(stdlib): switch testing.diff to use experimental.diff as its base (
Browse files Browse the repository at this point in the history
#5000)

This causes the `_diff` function to use the diff implementation in
`experimental` as the base diff implementation when the feature flag is
set.

This will cause diff to use a longest common subsequence algorithm to
compute the diff.

This is behind a feature flag in case it has some unknown complication.
  • Loading branch information
jsternberg authored Jul 19, 2022
1 parent 0795f80 commit eb3b3c5
Show file tree
Hide file tree
Showing 5 changed files with 32 additions and 3 deletions.
1 change: 1 addition & 0 deletions execute/executetest/dependencies.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ var testFlags = map[string]interface{}{
"narrowTransformationLimit": true,
"optimizeStateTracking": true,
"optimizeSetTransformation": true,
"experimentalTestingDiff": true,
}

type TestFlagger map[string]interface{}
Expand Down
14 changes: 14 additions & 0 deletions internal/feature/flags.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions internal/feature/flags.yml
Original file line number Diff line number Diff line change
Expand Up @@ -93,3 +93,9 @@
key: vectorizedConst
default: false
contact: Owen Nelson

- name: Experimental Testing Diff
description: Switches testing.diff to use experimental.diff
key: experimentalTestingDiff
default: false
contact: Jonathan Sternberg
4 changes: 2 additions & 2 deletions stdlib/experimental/diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ func createDiffTransformation(id execute.DatasetID, mode execute.AccumulationMod
}

wantID, gotID := a.Parents()[0], a.Parents()[1]
return newDiffTransformation(id, pspec, wantID, gotID, a.Allocator())
return NewDiffTransformation(id, pspec, wantID, gotID, a.Allocator())
}

type diffTransformation struct {
Expand All @@ -107,7 +107,7 @@ type diffTransformation struct {
wantID, gotID execute.DatasetID
}

func newDiffTransformation(id execute.DatasetID, spec *DiffProcedureSpec, wantID, gotID execute.DatasetID, mem memory.Allocator) (execute.Transformation, execute.Dataset, error) {
func NewDiffTransformation(id execute.DatasetID, spec *DiffProcedureSpec, wantID, gotID execute.DatasetID, mem memory.Allocator) (execute.Transformation, execute.Dataset, error) {
tr := &diffTransformation{
d: execute.NewTransportDataset(id, mem),
mem: mem,
Expand Down
10 changes: 9 additions & 1 deletion stdlib/testing/diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,11 @@ import (
"github.com/influxdata/flux/codes"
"github.com/influxdata/flux/execute"
"github.com/influxdata/flux/internal/errors"
"github.com/influxdata/flux/internal/feature"
"github.com/influxdata/flux/memory"
"github.com/influxdata/flux/plan"
"github.com/influxdata/flux/runtime"
"github.com/influxdata/flux/stdlib/experimental"
)

const DiffKind = "diff"
Expand Down Expand Up @@ -296,14 +298,20 @@ func createDiffTransformation(id execute.DatasetID, mode execute.AccumulationMod
return nil, nil, errors.New(codes.Internal, "diff should have exactly 2 parents")
}

wantID, gotID := a.Parents()[0], a.Parents()[1]
if feature.ExperimentalTestingDiff().Enabled(a.Context()) {
spec := &experimental.DiffProcedureSpec{}
return experimental.NewDiffTransformation(id, spec, wantID, gotID, a.Allocator())
}

cache := execute.NewTableBuilderCache(a.Allocator())
dataset := execute.NewDataset(id, mode, cache)
pspec, ok := spec.(*DiffProcedureSpec)
if !ok {
return nil, nil, errors.Newf(codes.Internal, "invalid spec type %T", pspec)
}

transform := NewDiffTransformation(dataset, cache, pspec, a.Parents()[0], a.Parents()[1], a.Allocator())
transform := NewDiffTransformation(dataset, cache, pspec, wantID, gotID, a.Allocator())

return transform, dataset, nil
}
Expand Down

0 comments on commit eb3b3c5

Please sign in to comment.