Skip to content

figuring out if we have an issue with unary datemath #229

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
epsniff wants to merge 1 commit into
base: master
Choose a base branch
from
Open

figuring out if we have an issue with unary datemath #229

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 28 additions & 14 deletions vm/datemath.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
package vm

import (
"fmt"
"strings"
"time"

u "github.com/araddon/gou"
"github.com/lytics/datemath"

"github.com/araddon/qlbridge/expr"
"github.com/araddon/qlbridge/lex"
"github.com/araddon/qlbridge/value"
"github.com/lytics/datemath"
)

// DateConverter can help inspect a boolean expression to determine if there is
Expand All @@ -35,7 +35,7 @@ func NewDateConverter(ctx expr.EvalIncludeContext, n expr.Node) (*DateConverter,
at: time.Now(),
ctx: ctx,
}
dc.findDateMath(n)
dc.findDateMath(false, n, 0)
if dc.err == nil && len(dc.TimeStrings) > 0 {
dc.HasDateMath = true
}
Expand All @@ -53,8 +53,19 @@ func (d *DateConverter) addBoundary(bt time.Time) {
d.bt = bt
}
}
func (d *DateConverter) addValue(lhv value.Value, op lex.TokenType, val string) {
func (d *DateConverter) addValue(negated bool, lhv value.Value, op lex.TokenType, val string) {

if negated {
switch op {
case lex.TokenEqual, lex.TokenEqualEqual, lex.TokenNE:
// none of these are supported operators for finding boundary
return
case lex.TokenGT, lex.TokenGE:
op = lex.TokenLT
case lex.TokenLT, lex.TokenLE:
op = lex.TokenGT
}
}
ct, ok := value.ValueToTime(lhv)
if !ok {
u.Debugf("Could not convert %T: %v to time.Time", lhv, lhv)
Expand Down Expand Up @@ -115,8 +126,11 @@ func (d *DateConverter) Boundary() time.Time {
}

// Determine if this expression node uses datemath (ie, "now-4h")
func (d *DateConverter) findDateMath(node expr.Node) {

func (d *DateConverter) findDateMath(negated bool, node expr.Node, depth int) {
for i := 0; i < depth; i++ {
fmt.Printf(" ")
}
fmt.Printf("node %T %s negated:%v\n", node, node, negated)
switch n := node.(type) {
case *expr.BinaryNode:

Expand Down Expand Up @@ -160,39 +174,39 @@ func (d *DateConverter) findDateMath(node expr.Node) {
}
}

d.addValue(lhv, op, val)
d.addValue(negated, lhv, op, val)
continue
}
default:
d.findDateMath(arg)
d.findDateMath(negated, arg, depth+1)
}
}

case *expr.BooleanNode:
for _, arg := range n.Args {
d.findDateMath(arg)
d.findDateMath(false, arg, depth+1)
}
case *expr.UnaryNode:
d.findDateMath(n.Arg)
d.findDateMath(true, n.Arg, depth+1)
case *expr.TriNode:
for _, arg := range n.Args {
d.findDateMath(arg)
d.findDateMath(false, arg, depth+1)
}
case *expr.FuncNode:
for _, arg := range n.Args {
d.findDateMath(arg)
d.findDateMath(false, arg, depth+1)
}
case *expr.ArrayNode:
for _, arg := range n.Args {
d.findDateMath(arg)
d.findDateMath(false, arg, depth+1)
}
case *expr.IncludeNode:
if err := resolveInclude(d.ctx, n, 0); err != nil {
d.err = err
return
}
if n.ExprNode != nil {
d.findDateMath(n.ExprNode)
d.findDateMath(negated, n.ExprNode, depth+1)
}
case *expr.NumberNode, *expr.ValueNode, *expr.IdentityNode, *expr.StringNode:
// Scalar/Literal values cannot be datemath, must be binary-expression
Expand Down
122 changes: 76 additions & 46 deletions vm/datemath_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
package vm_test

import (
"fmt"
"testing"
"time"

u "github.com/araddon/gou"
"github.com/stretchr/testify/assert"

"github.com/araddon/qlbridge/datasource"
"github.com/araddon/qlbridge/expr"
"github.com/araddon/qlbridge/rel"
"github.com/araddon/qlbridge/vm"
"github.com/stretchr/testify/assert"
)

var _ = u.EMPTY
Expand Down Expand Up @@ -179,6 +179,7 @@ func TestDateMath(t *testing.T) {
"subscription_expires": t1.Add(time.Hour * 24 * 6),
"lastevent": map[string]time.Time{"signedup": t1},
"first.event": map[string]time.Time{"has.period": t1},
"last_event_2": t1.Add(time.Hour * 25),
}, true),
}

Expand All @@ -187,67 +188,96 @@ func TestDateMath(t *testing.T) {
includeStatements := `
FILTER signedup < "now-2d" ALIAS signedup_onedayago;
FILTER subscription_expires < "now+1w" ALIAS subscription_expires_oneweek;
FILTER last_event_2 < "now-1d" ALIAS older_than_one_day;
FILTER last_event_2 < "now-3d" ALIAS older_than_three_day;
`
evalCtx := newIncluderCtx(nc, includeStatements)

tests := []dateTestCase{
// {
// filter: `FILTER last_event < "now-1d"`,
// ts: []string{"now-1d"},
// tm: t1.Add(time.Hour * 72),
// },
{
filter: `FILTER last_event < "now-1d"`,
filter: `FILTER NOT last_event < "now-1d"`,
ts: []string{"now-1d"},
tm: t1.Add(time.Hour * 72),
},
{
filter: `FILTER AND (EXISTS event, last_event < "now-1d", INCLUDE signedup_onedayago)`,
ts: []string{"now-1d", "now-2d"},
tm: t1.Add(time.Hour * 72),
},
// {
// filter: `FILTER AND (EXISTS event, last_event < "now-1d", INCLUDE signedup_onedayago)`,
// ts: []string{"now-1d", "now-2d"},
// tm: t1.Add(time.Hour * 72),
// },
// {
// filter: `FILTER AND (last_event_2 < "now-1d", NOT (last_event_2 < "now-3d"))`,
// ts: []string{"now+1d", "now+1h"},
// },
// {
// filter: `FILTER AND (INCLUDE older_than_one_day, INCLUDE older_than_three_day)`,
// ts: []string{"now+1d", "now+1h"},
// },
}
// test-todo
// x include w resolution
// - variety of +/-
// - between
// - urnary
// - false now, will be true in 24 hours, then exit in 48
// - not cases
for _, tc := range tests {
fs := rel.MustParseFilter(tc.filter)

// Converter to find/calculate date operations
dc, err := vm.NewDateConverter(evalCtx, fs.Filter)
assert.Equal(t, nil, err)
assert.True(t, dc.HasDateMath)
testRun := func(t *testing.T, tc dateTestCase) func(t *testing.T) {
return func(t *testing.T) {

// Ensure we inline/include all of the expressions
node, err := expr.InlineIncludes(evalCtx, fs.Filter)
assert.Equal(t, nil, err)
fs := rel.MustParseFilter(tc.filter)

// Converter to find/calculate date operations
dc, err = vm.NewDateConverter(evalCtx, node)
assert.Equal(t, nil, err)
assert.True(t, dc.HasDateMath)
// Converter to find/calculate date operations
dc, err := vm.NewDateConverter(evalCtx, fs.Filter)
assert.Equal(t, nil, err)
assert.True(t, dc.HasDateMath)

// initially we should not match
matched, evalOk := vm.Matches(evalCtx, fs)
assert.True(t, evalOk)
assert.Equal(t, false, matched)
// Ensure we inline/include all of the expressions
node, err := expr.InlineIncludes(evalCtx, fs.Filter)
assert.Equal(t, nil, err)

// Ensure the expected time-strings are found
assert.Equal(t, tc.ts, dc.TimeStrings)
// Converter to find/calculate date operations
dc, err = vm.NewDateConverter(evalCtx, node)
assert.Equal(t, nil, err)
assert.True(t, dc.HasDateMath)

/*
// TODO: I was trying to calculate the date in the future that
// this filter statement would no longer be true. BUT, need to change
// tests to change the input event timestamp instead of this approach
// initially we should not match
matched, evalOk := vm.Matches(evalCtx, fs)
assert.True(t, evalOk)
assert.Equal(t, false, matched)

// Time at which this will match
futureContext := newIncluderCtx(
datasource.NewNestedContextReader(readers, tc.tm),
includeStatements)
// Ensure the expected time-strings are found
assert.Equal(t, tc.ts, dc.TimeStrings)

matched, evalOk = vm.Matches(futureContext, fs)
assert.True(t, evalOk)
assert.Equal(t, true, matched, tc.filter)
*/
fmt.Printf("filter :%v\n", tc.filter)
fmt.Printf("t1: :%v\n", t1)
fmt.Printf("boundary :%v\n", dc.Boundary())
fmt.Printf("diff :%v\n", dc.Boundary().Sub(t1))
fmt.Printf("timeStrings :%v\n", dc.TimeStrings)

/*
// TODO: I was trying to calculate the date in the future that
// this filter statement would no longer be true. BUT, need to change
// tests to change the input event timestamp instead of this approach

// Time at which this will match
futureContext := newIncluderCtx(
datasource.NewNestedContextReader(readers, tc.tm),
includeStatements)

matched, evalOk = vm.Matches(futureContext, fs)
assert.True(t, evalOk)
assert.Equal(t, true, matched, tc.filter)
*/
}
}

// test-todo
// x include w resolution
// - variety of +/-
// - between
// - urnary
// - false now, will be true in 24 hours, then exit in 48
// - not cases
for _, tc := range tests {
t.Run(tc.filter, testRun(t, tc))
}

fs := rel.MustParseFilter(`FILTER AND (INCLUDE not_valid_lookup)`)
Expand Down