Fixes #94: Remove redundant recursion that leads to stack overflow

couchbaselabs · Feb 26, 2019 · fd9673b · fd9673b
1 parent 302ae79
commit fd9673b
Show file tree

Hide file tree

Showing 2 changed files with 27 additions and 19 deletions.
diff --git a/filterExprParser.go b/filterExprParser.go
@@ -13,7 +13,7 @@ import (
 
 // FilterExpression = ( AndCondition { "OR" AndCondition } ) { "AND" FilterExpression }
 // AndCondition = { OpenParens } Condition { "AND" Condition } { CloseParen }
-// Condition = ( [ "NOT" ] Condition ) | Operand | FilterExpression
+// Condition = ( [ "NOT" ] Condition ) | Operand
 // Operand = BooleanExpr | ( LHS ( CheckOp | ( CompareOp RHS) ) )
 // BooleanExpr = Boolean | BooleanFuncExpr
 // LHS = ConstFuncExpr | Boolean | Field | Value
@@ -214,16 +214,13 @@ func (f *FEAndCondition) OutputExpression() (Expression, error) {
 }
 
 type FECondition struct {
- Not *FECondition `"NOT" @@`
- Operand *FEOperand `| @@`
- SubExpr *FilterExpression `| @@`
+ Not *FECondition `"NOT" @@`
+ Operand *FEOperand `| @@`
 }
 
 func (f *FECondition) GetTotalOpenParens() (count int) {
  if f.Not != nil {
  count += f.Not.GetTotalOpenParens()
- } else if f.SubExpr != nil {
- count += f.SubExpr.GetTotalOpenParens()
  }
  // Operand has no open or close parens
  return
@@ -232,8 +229,6 @@ func (f *FECondition) GetTotalOpenParens() (count int) {
 func (f *FECondition) GetTotalCloseParens() (count int) {
  if f.Not != nil {
  count += f.Not.GetTotalCloseParens()
- } else if f.SubExpr != nil {
- count += f.SubExpr.GetTotalCloseParens()
  }
  // Operand has no open or close parens
  return
@@ -246,8 +241,6 @@ func (fec *FECondition) String() string {
  outputStr = append(outputStr, fmt.Sprintf("%v %v", OperatorNot, fec.Not.String()))
  } else if fec.Operand != nil {
  outputStr = append(outputStr, fec.Operand.String())
- } else if fec.SubExpr != nil {
- outputStr = append(outputStr, fec.SubExpr.String())
  } else {
  outputStr = append(outputStr, "?? (FECondition)")
  }
@@ -261,8 +254,6 @@ func (f *FECondition) OutputExpression() (Expression, error) {
  return NotExpr{subNot}, err
  } else if f.Operand != nil {
  return f.Operand.OutputExpression()
- } else if f.SubExpr != nil {
- return f.SubExpr.OutputExpression()
  } else {
  return nil, fmt.Errorf("Invalid FECondition %v", f.String())
  }

diff --git a/filterExprParser_test.go b/filterExprParser_test.go
@@ -93,13 +93,14 @@ func TestFilterExpressionParser(t *testing.T) {
  fe = &FilterExpression{}
  err = parser.ParseString("TRUE AND (TRUE OR FALSE) AND FALSE", fe)
  assert.Nil(err)
- assert.Equal(1, len(fe.AndConditions))
- assert.Equal(2, len(fe.AndConditions[0].OrConditions))
- assert.Nil(fe.AndConditions[0].OrConditions[0].SubExpr) // TRUE (AND...)
- assert.NotNil(fe.AndConditions[0].OrConditions[1].SubExpr)
- assert.Equal(2, len(fe.AndConditions[0].OrConditions[1].SubExpr.AndConditions)) // (TRUE OR FALSE) AND FALSE
- assert.Equal(1, len(fe.AndConditions[0].OrConditions[1].SubExpr.AndConditions[0].OrConditions)) // (TRUE OR FALSE)
- assert.Equal(1, len(fe.AndConditions[0].OrConditions[1].SubExpr.AndConditions[1].OrConditions)) // FALSE
+ assert.Equal(1, len(fe.AndConditions)) // TRUE (AND...)
+ assert.Equal(1, len(fe.SubFilterExpr)) // (TRUE OR FALSE) AND FALSE
+ assert.Equal(2, len(fe.SubFilterExpr[0].AndConditions)) // (TRUE OR FALSE) (AND...)
+ assert.Equal(1, len(fe.SubFilterExpr[0].AndConditions[0].OrConditions))
+ assert.Equal(1, len(fe.SubFilterExpr[0].AndConditions[1].OrConditions))
+ assert.Equal(1, len(fe.SubFilterExpr[0].SubFilterExpr)) // FALSE
+ assert.Equal(1, len(fe.SubFilterExpr[0].SubFilterExpr[0].AndConditions))
+ assert.Equal(0, len(fe.SubFilterExpr[0].SubFilterExpr[0].SubFilterExpr))
  expr, err = fe.OutputExpression()
  assert.Nil(err)
 
@@ -581,4 +582,20 @@ func TestFilterExpressionParser(t *testing.T) {
  fe = &FilterExpression{}
  err = parser.ParseString("achievement * 2 +1", fe)
  assert.NotNil(err)
+
+ // Typos
+ _, _, err = NewFilterExpressionParser("REGEX_CONTAINS(KEY, \"something\")")
+ assert.NotNil(err)
+ _, _, err = NewFilterExpressionParser("REGEXP_CONTAINS(METAS().id, \"something\")")
+ assert.NotNil(err)
+ _, _, err = NewFilterExpressionParser("REGEXP_CONTAINS(METAS().ID(), \"something\")")
+ assert.NotNil(err)
+
+ // Unfinished
+ _, _, err = NewFilterExpressionParser("REGEX_CONTAINS(KEY, \"something\") AND OR")
+ assert.NotNil(err)
+ _, _, err = NewFilterExpressionParser("REGEXP_CONTAINS(METAS().ID(), \"something)")
+ assert.NotNil(err)
+ _, _, err = NewFilterExpressionParser("`field is unfinished = \"unfinished_value")
+ assert.NotNil(err)
 }