From e1d246a19d973b26bfc807dab9b7786fd1c4140f Mon Sep 17 00:00:00 2001 From: Kelvin Wang Date: Thu, 17 Oct 2019 11:59:24 -0400 Subject: [PATCH] fix(predicate): remove unsupported operator parsing --- http/delete_test.go | 31 +++++---- predicate/logical.go | 6 -- predicate/parser.go | 76 ++++------------------ predicate/parser_test.go | 123 ++++++++++++------------------------ predicate/predicate_test.go | 68 -------------------- 5 files changed, 67 insertions(+), 237 deletions(-) diff --git a/http/delete_test.go b/http/delete_test.go index e6092c2835..fde4229e5c 100644 --- a/http/delete_test.go +++ b/http/delete_test.go @@ -3,7 +3,6 @@ package http import ( "bytes" "context" - "fmt" "io/ioutil" "net/http" "net/http/httptest" @@ -63,10 +62,10 @@ func TestDelete(t *testing.T) { wants: wants{ statusCode: http.StatusBadRequest, contentType: "application/json; charset=utf-8", - body: fmt.Sprintf(`{ + body: `{ "code": "invalid", "message": "invalid request; error parsing request json: invalid RFC3339Nano for field start, please format your time with RFC3339Nano format, example: 2009-01-02T23:00:00Z" - }`), + }`, }, }, { @@ -80,10 +79,10 @@ func TestDelete(t *testing.T) { wants: wants{ statusCode: http.StatusBadRequest, contentType: "application/json; charset=utf-8", - body: fmt.Sprintf(`{ + body: `{ "code": "invalid", "message": "invalid request; error parsing request json: invalid RFC3339Nano for field stop, please format your time with RFC3339Nano format, example: 2009-01-01T23:00:00Z" - }`), + }`, }, }, { @@ -106,10 +105,10 @@ func TestDelete(t *testing.T) { wants: wants{ statusCode: http.StatusBadRequest, contentType: "application/json; charset=utf-8", - body: fmt.Sprintf(`{ + body: `{ "code": "invalid", "message": "Please provide either orgID or org" - }`), + }`, }, }, { @@ -141,10 +140,10 @@ func TestDelete(t *testing.T) { wants: wants{ statusCode: http.StatusBadRequest, contentType: "application/json; charset=utf-8", - body: fmt.Sprintf(`{ + body: `{ "code": "invalid", "message": "Please provide either bucketID or bucket" - }`), + }`, }, }, { @@ -177,10 +176,10 @@ func TestDelete(t *testing.T) { wants: wants{ statusCode: http.StatusForbidden, contentType: "application/json; charset=utf-8", - body: fmt.Sprintf(`{ + body: `{ "code": "forbidden", "message": "insufficient permissions to delete" - }`), + }`, }, }, { @@ -227,7 +226,7 @@ func TestDelete(t *testing.T) { }, wants: wants{ statusCode: http.StatusNoContent, - body: fmt.Sprintf(``), + body: ``, }, }, { @@ -278,10 +277,10 @@ func TestDelete(t *testing.T) { }, wants: wants{ statusCode: http.StatusBadRequest, - body: fmt.Sprintf(`{ + body: `{ "code": "invalid", - "message": "invalid request; error parsing request json: Err in Child 1, err: the logical operator OR is not supported for delete predicate yet" - }`), + "message": "invalid request; error parsing request json: the logical operator OR is not supported yet at position 25" + }`, }, }, { @@ -332,7 +331,7 @@ func TestDelete(t *testing.T) { }, wants: wants{ statusCode: http.StatusNoContent, - body: fmt.Sprintf(``), + body: ``, }, }, } diff --git a/predicate/logical.go b/predicate/logical.go index 6620166f18..1ddda57209 100644 --- a/predicate/logical.go +++ b/predicate/logical.go @@ -13,7 +13,6 @@ type LogicalOperator int // LogicalOperators var ( LogicalAnd LogicalOperator = 1 - LogicalOr LogicalOperator = 2 ) // Value returns the node logical type. @@ -21,11 +20,6 @@ func (op LogicalOperator) Value() (datatypes.Node_Logical, error) { switch op { case LogicalAnd: return datatypes.LogicalAnd, nil - case LogicalOr: - return 0, &influxdb.Error{ - Code: influxdb.EInvalid, - Msg: "the logical operator OR is not supported for delete predicate yet", - } default: return 0, &influxdb.Error{ Code: influxdb.EInvalid, diff --git a/predicate/parser.go b/predicate/parser.go index 4eaefba911..c51513d697 100644 --- a/predicate/parser.go +++ b/predicate/parser.go @@ -8,6 +8,7 @@ import ( "github.com/influxdata/influxql" ) +// a fixed buffer ring type buffer [3]struct { tok influxql.Token // last read token pos influxql.Pos // last read pos @@ -75,7 +76,6 @@ func (p *parser) parseLogicalNode() (Node, error) { n := &LogicalNode{ Children: make([]Node, 0), } - var currentOp LogicalOperator for { tok, pos, _ := p.scanIgnoreWhitespace() switch tok { @@ -93,40 +93,12 @@ func (p *parser) parseLogicalNode() (Node, error) { } n.Children = append(n.Children, tr) case influxql.AND: - if currentOp == 0 || currentOp == LogicalAnd { - currentOp = LogicalAnd - n.Operator = LogicalAnd - } else { - lastChild := n.Children[len(n.Children)-1] - var n1 Node - var err error - if tokNext := p.peekTok(); tokNext == influxql.LPAREN { - n1, err = p.parseLogicalNode() - } else { - n1, err = p.parseTagRuleNode() - } - if err != nil { - return *n, err - } - n.Children = append(n.Children[:len(n.Children)-1], LogicalNode{ - Children: []Node{lastChild, n1}, - Operator: LogicalAnd, - }) - } + n.Operator = LogicalAnd case influxql.OR: - if currentOp == 0 || currentOp == LogicalOr { - n.Operator = LogicalOr - } else { - n1, err := p.parseLogicalNode() - if err != nil { - return *n, err - } - n = &LogicalNode{ - Children: []Node{*n, n1}, - Operator: LogicalOr, - } + return *n, &influxdb.Error{ + Code: influxdb.EInvalid, + Msg: fmt.Sprintf("the logical operator OR is not supported yet at position %d", pos.Char), } - currentOp = LogicalOr case influxql.LPAREN: p.openParen++ currParen := p.openParen @@ -186,18 +158,18 @@ func (p *parser) parseTagRuleNode() (TagRuleNode, error) { n.Operator = influxdb.Equal goto scanRegularTagValue case influxql.NEQ: - n.Operator = influxdb.NotEqual - goto scanRegularTagValue + fallthrough case influxql.EQREGEX: - n.Operator = influxdb.RegexEqual - goto scanRegexTagValue + fallthrough case influxql.NEQREGEX: - n.Operator = influxdb.NotRegexEqual - goto scanRegexTagValue + return *n, &influxdb.Error{ + Code: influxdb.EInvalid, + Msg: fmt.Sprintf("operator: %q at position: %d is not supported yet", tok.String(), pos.Char), + } default: return *n, &influxdb.Error{ Code: influxdb.EInvalid, - Msg: fmt.Sprintf("invalid operator %q at position %d", tok.String(), pos), + Msg: fmt.Sprintf("invalid operator %q at position: %d", tok.String(), pos.Char), } } // scan the value @@ -223,28 +195,4 @@ scanRegularTagValue: Msg: fmt.Sprintf("bad tag value: %q, at position %d", lit, pos.Char), } } -scanRegexTagValue: - tok, pos, lit = p.sc.ScanRegex() - switch tok { - case influxql.BADREGEX: - fallthrough - case influxql.BADESCAPE: - return *n, &influxdb.Error{ - Code: influxdb.EInvalid, - Msg: fmt.Sprintf("bad regex at position: %d", pos.Char), - } - default: - n.Value = "/" + lit + "/" - } - return *n, nil -} - -// peekRune returns the next rune that would be read by the scanner. -func (p *parser) peekTok() influxql.Token { - tok, _, _ := p.scanIgnoreWhitespace() - if tok != influxql.EOF { - p.unscan() - } - - return tok } diff --git a/predicate/parser_test.go b/predicate/parser_test.go index a6573fb69b..62611462eb 100644 --- a/predicate/parser_test.go +++ b/predicate/parser_test.go @@ -18,7 +18,7 @@ func TestParseNode(t *testing.T) { err error }{ { - str: ` abc="opq" AND gender="male" AND temp=1123`, + str: ` abc="opq" AND gender="male" AND temp=1123`, node: LogicalNode{Operator: LogicalAnd, Children: []Node{ TagRuleNode{Tag: influxdb.Tag{Key: "abc", Value: "opq"}}, TagRuleNode{Tag: influxdb.Tag{Key: "gender", Value: "male"}}, @@ -27,93 +27,37 @@ func TestParseNode(t *testing.T) { }, { str: ` abc="opq" Or gender="male" OR temp=1123`, - node: LogicalNode{Operator: LogicalOr, Children: []Node{ - TagRuleNode{Tag: influxdb.Tag{Key: "abc", Value: "opq"}}, - TagRuleNode{Tag: influxdb.Tag{Key: "gender", Value: "male"}}, - TagRuleNode{Tag: influxdb.Tag{Key: "temp", Value: "1123"}}, - }}, + err: &influxdb.Error{ + Code: influxdb.EInvalid, + Msg: "the logical operator OR is not supported yet at position 11", + }, }, { - str: ` abc="opq" AND gender="male" OR temp=1123`, - node: LogicalNode{Operator: LogicalOr, Children: []Node{ - LogicalNode{Operator: LogicalAnd, Children: []Node{ - TagRuleNode{Tag: influxdb.Tag{Key: "abc", Value: "opq"}}, - TagRuleNode{Tag: influxdb.Tag{Key: "gender", Value: "male"}}, - }}, - TagRuleNode{Tag: influxdb.Tag{Key: "temp", Value: "1123"}}, - }}, - }, - { - str: ` abc="opq" OR gender="male" AND (temp=1123 or name="tom")`, - node: LogicalNode{Operator: LogicalOr, Children: []Node{ - TagRuleNode{Tag: influxdb.Tag{Key: "abc", Value: "opq"}}, - LogicalNode{Operator: LogicalAnd, Children: []Node{ - TagRuleNode{Tag: influxdb.Tag{Key: "gender", Value: "male"}}, - LogicalNode{Operator: LogicalOr, Children: []Node{ - TagRuleNode{Tag: influxdb.Tag{Key: "temp", Value: "1123"}}, - TagRuleNode{Tag: influxdb.Tag{Key: "name", Value: "tom"}}, - }}, - }}, - }}, - }, - { - str: ` abc="opq" Or gender="male" AND temp=1123 AND name2="jerry" OR name="tom"`, - node: LogicalNode{Operator: LogicalOr, Children: []Node{ - TagRuleNode{Tag: influxdb.Tag{Key: "abc", Value: "opq"}}, - LogicalNode{Operator: LogicalAnd, Children: []Node{ - LogicalNode{Operator: LogicalAnd, Children: []Node{ - TagRuleNode{Tag: influxdb.Tag{Key: "gender", Value: "male"}}, - TagRuleNode{Tag: influxdb.Tag{Key: "temp", Value: "1123"}}, - }}, - TagRuleNode{Tag: influxdb.Tag{Key: "name2", Value: "jerry"}}, - }}, - TagRuleNode{Tag: influxdb.Tag{Key: "name", Value: "tom"}}, - }}, - }, - { - str: ` abc="opq" AND gender="male" OR temp=1123 AND name="tom" OR name2="jerry"`, - node: LogicalNode{Operator: LogicalOr, Children: []Node{ - LogicalNode{Operator: LogicalAnd, Children: []Node{ - TagRuleNode{Tag: influxdb.Tag{Key: "abc", Value: "opq"}}, - TagRuleNode{Tag: influxdb.Tag{Key: "gender", Value: "male"}}, - }}, - LogicalNode{Operator: LogicalOr, Children: []Node{ - LogicalNode{Operator: LogicalAnd, Children: []Node{ - TagRuleNode{Tag: influxdb.Tag{Key: "temp", Value: "1123"}}, - TagRuleNode{Tag: influxdb.Tag{Key: "name", Value: "tom"}}, - }}, - TagRuleNode{Tag: influxdb.Tag{Key: "name2", Value: "jerry"}}, - }}, - }}, - }, - { - str: ` (t1="v1" or t2!="v2") and (t3=v3 and (t4!=v4 or t5=v5 and t6=v6))`, + str: ` (t1="v1" and t2="v2") and (t3=v3 and (t4=v4 and t5=v5 and t6=v6))`, node: LogicalNode{Operator: LogicalAnd, Children: []Node{ - LogicalNode{Operator: LogicalOr, Children: []Node{ + LogicalNode{Operator: LogicalAnd, Children: []Node{ TagRuleNode{Tag: influxdb.Tag{Key: "t1", Value: "v1"}}, - TagRuleNode{Tag: influxdb.Tag{Key: "t2", Value: "v2"}, Operator: influxdb.NotEqual}, + TagRuleNode{Tag: influxdb.Tag{Key: "t2", Value: "v2"}}, }}, LogicalNode{Operator: LogicalAnd, Children: []Node{ TagRuleNode{Tag: influxdb.Tag{Key: "t3", Value: "v3"}}, - LogicalNode{Operator: LogicalOr, Children: []Node{ - TagRuleNode{Tag: influxdb.Tag{Key: "t4", Value: "v4"}, Operator: influxdb.NotEqual}, - LogicalNode{Operator: LogicalAnd, Children: []Node{ - TagRuleNode{Tag: influxdb.Tag{Key: "t5", Value: "v5"}}, - TagRuleNode{Tag: influxdb.Tag{Key: "t6", Value: "v6"}}, - }}, + LogicalNode{Operator: LogicalAnd, Children: []Node{ + TagRuleNode{Tag: influxdb.Tag{Key: "t4", Value: "v4"}}, + TagRuleNode{Tag: influxdb.Tag{Key: "t5", Value: "v5"}}, + TagRuleNode{Tag: influxdb.Tag{Key: "t6", Value: "v6"}}, }}, }}, }}, }, { - str: ` (t1="v1" or t2!="v2") and (`, + str: ` (t1="v1" and t2="v2") and (`, err: &influxdb.Error{ Code: influxdb.EInvalid, Msg: fmt.Sprintf("extra ( seen"), }, }, { - str: ` (t1="v1" or t2!="v2"))`, + str: ` (t1="v1" and t2="v2"))`, err: &influxdb.Error{ Code: influxdb.EInvalid, Msg: fmt.Sprintf("extra ) seen"), @@ -138,34 +82,47 @@ func TestParseTagRule(t *testing.T) { err error }{ { - str: ` abc="opq"`, + str: ` abc = "opq"`, node: TagRuleNode{Tag: influxdb.Tag{Key: "abc", Value: "opq"}}, }, { - str: ` abc != "opq"`, - node: TagRuleNode{Tag: influxdb.Tag{Key: "abc", Value: "opq"}, Operator: influxdb.NotEqual}, + str: ` abc != "opq"`, + err: &influxdb.Error{ + Code: influxdb.EInvalid, + Msg: `operator: "!=" at position: 5 is not supported yet`, + }, }, { - str: `abc!=123`, - node: TagRuleNode{Tag: influxdb.Tag{Key: "abc", Value: "123"}, Operator: influxdb.NotEqual}, + str: `abc=123`, + node: TagRuleNode{Tag: influxdb.Tag{Key: "abc", Value: "123"}, Operator: influxdb.Equal}, }, { - str: `abc!=true`, - node: TagRuleNode{Tag: influxdb.Tag{Key: "abc", Value: "true"}, Operator: influxdb.NotEqual}, + str: `abc=true`, + node: TagRuleNode{Tag: influxdb.Tag{Key: "abc", Value: "true"}, Operator: influxdb.Equal}, }, { str: `abc=false`, node: TagRuleNode{Tag: influxdb.Tag{Key: "abc", Value: "false"}, Operator: influxdb.Equal}, }, { - str: `abc!~/^payments\./`, - node: TagRuleNode{Tag: influxdb.Tag{Key: "abc", Value: `/^payments\./`}, Operator: influxdb.NotRegexEqual}, - }, - { - str: `abc=~/^bad\\\\/`, + str: `abc!~/^payments\./`, err: &influxdb.Error{ Code: influxdb.EInvalid, - Msg: "bad regex at position: 4", + Msg: `operator: "!~" at position: 3 is not supported yet`, + }, + }, + { + str: `abc=~/^payments\./`, + err: &influxdb.Error{ + Code: influxdb.EInvalid, + Msg: `operator: "=~" at position: 3 is not supported yet`, + }, + }, + { + str: `abc>1000`, + err: &influxdb.Error{ + Code: influxdb.EInvalid, + Msg: `invalid operator ">" at position: 3`, }, }, } diff --git a/predicate/predicate_test.go b/predicate/predicate_test.go index a9dc56bba1..980e6f3c29 100644 --- a/predicate/predicate_test.go +++ b/predicate/predicate_test.go @@ -45,74 +45,6 @@ func TestDataTypeConversion(t *testing.T) { }, }, }, - { - name: "unsupported node", - err: &influxdb.Error{ - Code: influxdb.EInvalid, - Msg: "the logical operator OR is not supported for delete predicate yet", - }, - node: &LogicalNode{ - Operator: LogicalOr, - Children: []Node{ - &TagRuleNode{ - Operator: influxdb.Equal, - Tag: influxdb.Tag{ - Key: "k1", - Value: "v1", - }, - }, - &TagRuleNode{ - Operator: influxdb.Equal, - Tag: influxdb.Tag{ - Key: "k2", - Value: "v2", - }, - }, - }, - }, - }, - { - name: "unsupported operator", - err: &influxdb.Error{ - Code: influxdb.EInvalid, - Msg: "Operator notequal is not supported for delete predicate yet", - }, - node: &TagRuleNode{ - Operator: influxdb.NotEqual, - Tag: influxdb.Tag{ - Key: "k2", - Value: "v2", - }, - }, - }, - { - name: "unsupported operator", - err: &influxdb.Error{ - Code: influxdb.EInvalid, - Msg: "Operator equalregex is not supported for delete predicate yet", - }, - node: &TagRuleNode{ - Operator: influxdb.RegexEqual, - Tag: influxdb.Tag{ - Key: "k2", - Value: "v2", - }, - }, - }, - { - name: "unsupported operator", - err: &influxdb.Error{ - Code: influxdb.EInvalid, - Msg: "Operator notequalregex is not supported for delete predicate yet", - }, - node: &TagRuleNode{ - Operator: influxdb.NotRegexEqual, - Tag: influxdb.Tag{ - Key: "k2", - Value: "v2", - }, - }, - }, { name: "logical", node: &LogicalNode{