LYT-515 | Lex unquoted negative numeric literals as signed literals in FilterQL

lex/dialect_filterql_test.go

@@ -2,6 +2,7 @@ package lex
 
 import (
 	"testing"
+	"time"
 
 	u "github.com/araddon/gou"
 	"github.com/stretchr/testify/assert"
@@ -239,3 +240,194 @@ func TestFilterQLIntersects(t *testing.T) {
 			tv(TokenRightParenthesis, ")"),
 		})
 }
+
+// An unquoted negative numeric literal in a value position must lex as a
+// single signed TokenInteger/TokenFloat, not a TokenMinus followed by a
+// positive number.
+func TestFilterQLNegativeLiteral(t *testing.T) {
+	verifyFilterQLTokens(t, `FILTER visitct = -1`,
+		[]Token{
+			tv(TokenFilter, "FILTER"),
+			tv(TokenIdentity, "visitct"),
+			tv(TokenEqual, "="),
+			tv(TokenInteger, "-1"),
+		})
+
+	verifyFilterQLTokens(t, `FILTER visitct = -1.5`,
+		[]Token{
+			tv(TokenFilter, "FILTER"),
+			tv(TokenIdentity, "visitct"),
+			tv(TokenEqual, "="),
+			tv(TokenFloat, "-1.5"),
+		})
+
+	verifyFilterQLTokens(t, `FILTER visitct IN (-1)`,
+		[]Token{
+			tv(TokenFilter, "FILTER"),
+			tv(TokenIdentity, "visitct"),
+			tv(TokenIN, "IN"),
+			tv(TokenLeftParenthesis, "("),
+			tv(TokenInteger, "-1"),
+			tv(TokenRightParenthesis, ")"),
+		})
+
+	verifyFilterQLTokens(t, `FILTER visitct IN (-1, 3)`,
+		[]Token{
+			tv(TokenFilter, "FILTER"),
+			tv(TokenIdentity, "visitct"),
+			tv(TokenIN, "IN"),
+			tv(TokenLeftParenthesis, "("),
+			tv(TokenInteger, "-1"),
+			tv(TokenComma, ","),
+			tv(TokenInteger, "3"),
+			tv(TokenRightParenthesis, ")"),
+		})
+
+	verifyFilterQLTokens(t, `FILTER city IN (-1)`,
+		[]Token{
+			tv(TokenFilter, "FILTER"),
+			tv(TokenIdentity, "city"),
+			tv(TokenIN, "IN"),
+			tv(TokenLeftParenthesis, "("),
+			tv(TokenInteger, "-1"),
+			tv(TokenRightParenthesis, ")"),
+		})
+}
+
+// A negative literal must not consume the clause continuation: everything
+// after it (infix AND/OR, the rest of a list) still has to lex.
+func TestFilterQLNegativeLiteralInfix(t *testing.T) {
+	verifyFilterQLTokens(t, `FILTER visitct = -1 AND city = "sf"`,
+		[]Token{
+			tv(TokenFilter, "FILTER"),
+			tv(TokenIdentity, "visitct"),
+			tv(TokenEqual, "="),
+			tv(TokenInteger, "-1"),
+			tv(TokenLogicAnd, "AND"),
+			tv(TokenIdentity, "city"),
+			tv(TokenEqual, "="),
+			tv(TokenValue, "sf"),
+		})
+
+	verifyFilterQLTokens(t, `FILTER visitct = -1 OR city = "sf"`,
+		[]Token{
+			tv(TokenFilter, "FILTER"),
+			tv(TokenIdentity, "visitct"),
+			tv(TokenEqual, "="),
+			tv(TokenInteger, "-1"),
+			tv(TokenLogicOr, "OR"),
+			tv(TokenIdentity, "city"),
+			tv(TokenEqual, "="),
+			tv(TokenValue, "sf"),
+		})
+
+	verifyFilterQLTokens(t, `FILTER visitct > -1 AND visitct < 5`,
+		[]Token{
+			tv(TokenFilter, "FILTER"),
+			tv(TokenIdentity, "visitct"),
+			tv(TokenGT, ">"),
+			tv(TokenInteger, "-1"),
+			tv(TokenLogicAnd, "AND"),
+			tv(TokenIdentity, "visitct"),
+			tv(TokenLT, "<"),
+			tv(TokenInteger, "5"),
+		})
+
+	verifyFilterQLTokens(t, `FILTER visitct BETWEEN 5 AND -1`,
+		[]Token{
+			tv(TokenFilter, "FILTER"),
+			tv(TokenIdentity, "visitct"),
+			tv(TokenBetween, "BETWEEN"),
+			tv(TokenInteger, "5"),
+			tv(TokenLogicAnd, "AND"),
+			tv(TokenInteger, "-1"),
+		})
+
+	// Sign directly after BETWEEN: the only shape where the previous token is
+	// TokenBetween itself.
+	verifyFilterQLTokens(t, `FILTER visitct BETWEEN -5 AND -1`,
+		[]Token{
+			tv(TokenFilter, "FILTER"),
+			tv(TokenIdentity, "visitct"),
+			tv(TokenBetween, "BETWEEN"),
+			tv(TokenInteger, "-5"),
+			tv(TokenLogicAnd, "AND"),
+			tv(TokenInteger, "-1"),
+		})
+}
+
+// A negative anywhere but first in a list: the `,` continuation must survive.
+func TestFilterQLNegativeLiteralNotFirstInList(t *testing.T) {
+	verifyFilterQLTokens(t, `FILTER visitct IN (1, -3)`,
+		[]Token{
+			tv(TokenFilter, "FILTER"),
+			tv(TokenIdentity, "visitct"),
+			tv(TokenIN, "IN"),
+			tv(TokenLeftParenthesis, "("),
+			tv(TokenInteger, "1"),
+			tv(TokenComma, ","),
+			tv(TokenInteger, "-3"),
+			tv(TokenRightParenthesis, ")"),
+		})
+
+	verifyFilterQLTokens(t, `FILTER visitct IN ("a", -1)`,
+		[]Token{
+			tv(TokenFilter, "FILTER"),
+			tv(TokenIdentity, "visitct"),
+			tv(TokenIN, "IN"),
+			tv(TokenLeftParenthesis, "("),
+			tv(TokenValue, "a"),
+			tv(TokenComma, ","),
+			tv(TokenInteger, "-1"),
+			tv(TokenRightParenthesis, ")"),
+		})
+}
+
+// A sign the number scanner would reject must fall back to TokenMinus rather
+// than commit to a literal LexNumber then hard-errors on.
+func TestFilterQLSignedLiteralScannerDisagreement(t *testing.T) {
+	verifyFilterQLTokens(t, `FILTER visitct = -.5`,
+		[]Token{
+			tv(TokenFilter, "FILTER"),
+			tv(TokenIdentity, "visitct"),
+			tv(TokenEqual, "="),
+			tv(TokenMinus, "-"),
+			tv(TokenIdentity, ".5"),
+		})
+
+	verifyFilterQLTokens(t, `FILTER visitct = -0x1A`,
+		[]Token{
+			tv(TokenFilter, "FILTER"),
+			tv(TokenIdentity, "visitct"),
+			tv(TokenEqual, "="),
+			tv(TokenMinus, "-"),
+			tv(TokenInteger, "0x1A"),
+		})
+}
+
+// A trailing operator after a negative literal must terminate the scan.  An
+// unbalanced state stack live-locks here instead, so the whole lex runs on a
+// goroutine and the test fails on timeout rather than hanging the suite.
+func TestFilterQLNegativeLiteralTrailingOperatorTerminates(t *testing.T) {
+	for _, ql := range []string{`FILTER visitct = -1-`, `FILTER visitct = -1/`} {
+		done := make(chan bool, 1)
+		go func() {
+			l := NewFilterQLLexer(ql)
+			for i := 0; i < 100; i++ {
+				tok := l.NextToken()
+				if tok.T == TokenEOF || tok.T == TokenError {
+					done <- true
+					return
+				}
+			}
+			done <- false
+		}()
+
+		select {
+		case ok := <-done:
+			assert.True(t, ok, "%s must reach EOF or Error within 100 tokens", ql)
+		case <-time.After(10 * time.Second):
+			t.Fatalf("%s did not terminate: lexer state stack is unbalanced", ql)
+		}
+	}
+}

lex/lexer.go

@@ -1291,6 +1291,14 @@ func LexListOfArgs(l *Lexer) StateFn {
 		l.backup()
 		return LexExpression
 	case '!', '=', '>', '<', '-', '+', '%', '&', '/', '|':
+		if r == '-' && valueExpectedTokens[l.lastToken.T] && l.numericAfterSign() {
+			// A negative literal is a single list value, not a binary operator
+			// between two args: push this list back on so the following `,`/`)`
+			// is lexed here rather than by the enclosing LexParenRight.
+			l.backup()
+			l.Push("LexListOfArgs", LexListOfArgs)
+			return LexNumber
+		}
 		l.backup()
 		return LexExpression
 	case ';':
@@ -2244,6 +2252,47 @@ func LexLogical(l *Lexer) StateFn {
 	return LexExpression(l)
 }
 
+// valueExpectedTokens are the previously-emitted tokens after which an
+// unquoted `-` begins a signed numeric literal rather than the binary-minus
+// operator: comparators, arithmetic operators, open-paren, comma, logic,
+// IN/BETWEEN, and the start of input (TokenNil).
+var valueExpectedTokens = map[TokenType]bool{
+	TokenNil:             true,
+	TokenEqual:           true,
+	TokenEqualEqual:      true,
+	TokenNE:              true,
+	TokenGE:              true,
+	TokenLE:              true,
+	TokenGT:              true,
+	TokenLT:              true,
+	TokenMinus:           true,
+	TokenPlus:            true,
+	TokenMultiply:        true,
+	TokenDivide:          true,
+	TokenModulus:         true,
+	TokenLeftParenthesis: true,
+	TokenComma:           true,
+	TokenLogicAnd:        true,
+	TokenLogicOr:         true,
+	TokenAnd:             true,
+	TokenOr:              true,
+	TokenIN:              true,
+	TokenBetween:         true,
+}
+
+// numericAfterSign reports whether the runes after an already-consumed sign
+// begin a literal scanNumericOrDuration will accept. LexNumber runs with
+// SUPPORT_DURATION, so signed durations (`-1d`, `-30d`) are included.
+func (l *Lexer) numericAfterSign() bool {
+	next := l.PeekX(2)
+	if len(next) == 0 || !isDigit(rune(next[0])) {
+		return false
+	}
+	// The scanner refuses a sign before hex, and a committed gate has no
+	// fallback: LexNumber would hard-error instead of emitting TokenMinus.
+	return !(len(next) == 2 && next[0] == '0' && (next[1] == 'x' || next[1] == 'X'))
+}
+
 // <expr>   Handle single logical expression which may be nested and  has
 //
 //	user defined function names that are NOT validated by lexer
@@ -2313,13 +2362,21 @@ func LexExpression(l *Lexer) StateFn {
 		foundLogical := false
 		foundOperator := false
 		switch r {
-		case '-': // comment?  or minus?
+		case '-': // negative numeric literal, comment, or minus?
 			p := l.Peek()
-			if p == '-' {
+			switch {
+			case p == '-':
 				l.backup()
 				l.Push("LexExpression", LexExpression)
 				return LexInlineComment
-			} else {
+			case valueExpectedTokens[l.lastToken.T] && l.numericAfterSign():
+				// LexNumber ends with `return nil`, which pops a frame; push the
+				// clause continuation so it unwinds into this clause rather than
+				// consuming the enclosing statement's.
+				l.backup()
+				l.Push("LexExpression", l.clauseState())
+				return LexNumber
+			default:
 				l.Emit(TokenMinus)
 				return l.clauseState()
 			}

lex/lexer_test.go

@@ -270,6 +270,83 @@ func TestLexDuration(t *testing.T) {
 	}
 }
 
+// Binary minus (subtraction) must be unaffected by the signed-numeric-literal
+// fix: LexExpression is shared with the SQL dialect, and `a - b` / `5 - 3`
+// have an identity/number as the previous token, not a value-expected one.
+func TestLexBinaryMinusUnchanged(t *testing.T) {
+	verifyTokens(t, `SELECT a - b FROM x`,
+		[]Token{
+			tv(TokenSelect, "SELECT"),
+			tv(TokenIdentity, "a"),
+			tv(TokenMinus, "-"),
+			tv(TokenIdentity, "b"),
+			tv(TokenFrom, "FROM"),
+			tv(TokenIdentity, "x"),
+		})
+
+	verifyTokens(t, `SELECT 5 - 3 FROM x`,
+		[]Token{
+			tv(TokenSelect, "SELECT"),
+			tv(TokenInteger, "5"),
+			tv(TokenMinus, "-"),
+			tv(TokenInteger, "3"),
+			tv(TokenFrom, "FROM"),
+			tv(TokenIdentity, "x"),
+		})
+
+	// The column-list cases above never reach the changed branch; a WHERE
+	// clause does, so these are what actually guard it.
+	verifyTokens(t, `SELECT a FROM t WHERE (x - 1) > 5`,
+		[]Token{
+			tv(TokenSelect, "SELECT"),
+			tv(TokenIdentity, "a"),
+			tv(TokenFrom, "FROM"),
+			tv(TokenIdentity, "t"),
+			tv(TokenWhere, "WHERE"),
+			tv(TokenLeftParenthesis, "("),
+			tv(TokenIdentity, "x"),
+			tv(TokenMinus, "-"),
+			tv(TokenInteger, "1"),
+			tv(TokenRightParenthesis, ")"),
+			tv(TokenGT, ">"),
+			tv(TokenInteger, "5"),
+		})
+
+	verifyTokens(t, `SELECT a FROM t WHERE x > 5 - 3`,
+		[]Token{
+			tv(TokenSelect, "SELECT"),
+			tv(TokenIdentity, "a"),
+			tv(TokenFrom, "FROM"),
+			tv(TokenIdentity, "t"),
+			tv(TokenWhere, "WHERE"),
+			tv(TokenIdentity, "x"),
+			tv(TokenGT, ">"),
+			tv(TokenInteger, "5"),
+			tv(TokenMinus, "-"),
+			tv(TokenInteger, "3"),
+		})
+}
+
+// A signed literal in a SQL WHERE clause must lex as one token and leave the
+// infix continuation intact, same as FilterQL.
+func TestLexSignedLiteralInWhere(t *testing.T) {
+	verifyTokens(t, `SELECT a FROM t WHERE age > -1 AND name = "bob"`,
+		[]Token{
+			tv(TokenSelect, "SELECT"),
+			tv(TokenIdentity, "a"),
+			tv(TokenFrom, "FROM"),
+			tv(TokenIdentity, "t"),
+			tv(TokenWhere, "WHERE"),
+			tv(TokenIdentity, "age"),
+			tv(TokenGT, ">"),
+			tv(TokenInteger, "-1"),
+			tv(TokenLogicAnd, "AND"),
+			tv(TokenIdentity, "name"),
+			tv(TokenEqual, "="),
+			tv(TokenValue, "bob"),
+		})
+}
+
 func verifyTokens(t *testing.T, sql string, tokens []Token) {
 	l := NewSqlLexer(sql)
 	u.Debugf("sql: %v", sql)