Merge pull request #999 from seebs/pqlCleanup

Fixes some minor PQL issues; adds support for case-insensitive PQL calls
This commit is contained in:
seebs 2020-10-20 16:02:42 -05:00 committed by GitHub
commit 4cea813ae8
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 2674 additions and 2457 deletions

View file

@ -4287,7 +4287,7 @@ func (e *executor) executeSetRowAttrs(ctx context.Context, qcx *Qcx, index strin
}
// Copy args and remove reserved fields.
attrs := pql.CopyArgs(c.Args)
attrs := pql.CopyArgsDecimalToFloat(c.Args)
delete(attrs, "_field")
delete(attrs, "_"+rowLabel)
@ -4354,7 +4354,7 @@ func (e *executor) executeBulkSetRowAttrs(ctx context.Context, qcx *Qcx, index s
}
// Copy args and remove reserved fields.
attrs := pql.CopyArgs(c.Args)
attrs := pql.CopyArgsDecimalToFloat(c.Args)
delete(attrs, "_field")
delete(attrs, "_"+rowLabel)
@ -4438,7 +4438,7 @@ func (e *executor) executeSetColumnAttrs(ctx context.Context, qcx *Qcx, index st
}
// Copy args and remove reserved fields.
attrs := pql.CopyArgs(c.Args)
attrs := pql.CopyArgsDecimalToFloat(c.Args)
delete(attrs, "_"+columnLabel)
delete(attrs, "field")

View file

@ -33,6 +33,10 @@ type Query struct {
}
func (q *Query) startCall(name string) {
// Coerce every name into a canonical form if we know of one.
if canon, ok := canonicalCaps[strings.ToLower(name)]; ok {
name = canon
}
newCall := &Call{Name: name}
q.callStack = append(q.callStack, &callStackElem{call: newCall})
@ -60,7 +64,7 @@ func (q *Query) lastCallStackElem() *callStackElem {
func (q *Query) addPosNum(key, value string) {
q.addField(key)
q.addNumVal(value, false)
q.addNumVal(value)
}
func (q *Query) addPosStr(key, value string) {
@ -85,9 +89,9 @@ func (q *Query) endConditional() {
if len(q.conditional) != 5 {
panic(fmt.Sprintf("conditional of wrong length: %#v", q.conditional))
}
low := parseNum(q.conditional[0], false)
low := parseNum(q.conditional[0])
field := q.conditional[2]
high := parseNum(q.conditional[4], false)
high := parseNum(q.conditional[4])
var op Token
switch q.conditional[1] + q.conditional[3] {
@ -162,12 +166,12 @@ func (q *Query) addVal(val interface{}) {
elem.lastCond = ILLEGAL
}
func (q *Query) addNumVal(val string, asFloat bool) {
func (q *Query) addNumVal(val string) {
elem := q.lastCallStackElem()
if elem == nil || elem.lastField == "" {
panic(fmt.Sprintf("addIntVal called with '%s' when lastField is empty", val))
}
ival := parseNum(val, asFloat)
ival := parseNum(val)
if elem.inList {
if elem.lastCond != ILLEGAL {
list := elem.call.Args[elem.lastField].(*Condition).Value.([]interface{})
@ -478,6 +482,21 @@ var callInfoByFunc = map[string]callInfo{
},
}
// We want to allow case-insensitive names, but we want to continue using
// friendly easy-to-read names like "SetRowAttrs", not "setrowattrs". So,
// we make a map; put in a ToLower() string, get back the canonical
// capitalization. This might not have seemed like the best strategy if we
// didn't already have so much code relying on the exact strings.
var canonicalCaps = makeCanonicalMap(callInfoByFunc)
func makeCanonicalMap(from map[string]callInfo) map[string]string {
m := make(map[string]string, len(from))
for k := range from {
m[strings.ToLower(k)] = k
}
return m
}
// CheckCallInfo tries to validate that arguments are correct and valid for the
// given call. It does not guarantee checking all possible errors; for instance,
// if an argument is a field name, CheckCallInfo can't validate that the field
@ -991,6 +1010,20 @@ func CopyArgs(m map[string]interface{}) map[string]interface{} {
return other
}
// CopyArgsDecimalToFloat makes a copy of m, but in the process,
// replaces any Decimal values with Float64 values.
func CopyArgsDecimalToFloat(m map[string]interface{}) map[string]interface{} {
other := make(map[string]interface{}, len(m))
for k, v := range m {
if dec, ok := v.(Decimal); ok {
other[k] = dec.Float64()
} else {
other[k] = v
}
}
return other
}
func joinInterfaceSlice(a []interface{}) string {
other := make([]string, len(a))
for i := range a {
@ -1012,15 +1045,11 @@ func joinUint64Slice(a []uint64) string {
return "[" + strings.Join(other, ",") + "]"
}
func parseNum(val string, asFloat bool) interface{} {
func parseNum(val string) interface{} {
var ival interface{}
var err error
if strings.Contains(val, ".") {
if asFloat {
ival, err = strconv.ParseFloat(val, 64)
} else {
ival, err = ParseDecimal(val)
}
ival, err = ParseDecimal(val)
} else {
ival, err = strconv.ParseInt(val, 10, 64)
}

View file

@ -309,7 +309,6 @@ func ParseDecimal(s string) (Decimal, error) {
var err error
// General steps:
// - Trim leading whitespace/zeros
// - Get the sign value
// - Trim leading zeros
// - Push characters into a buffer
@ -328,26 +327,23 @@ func ParseDecimal(s string) (Decimal, error) {
switch state {
case stateSign:
switch s[i] {
case ' ':
continue
case '-':
sign = true
fallthrough
case '+':
state = stateLeadingZeros
default:
state = stateLeadingZeros
i--
// Resume loop and look at next character
continue
}
state = stateLeadingZeros
fallthrough
case stateLeadingZeros:
switch s[i] {
case '0':
if s[i] == '0' {
foundLeadingZero = true
continue
default:
state = stateMantissa
i--
}
state = stateMantissa
fallthrough
case stateMantissa:
switch s[i] {
case '.':
@ -371,23 +367,14 @@ func ParseDecimal(s string) (Decimal, error) {
return Decimal{}, errors.New("decimal string is empty")
}
// Trim trailing zeros/spaces of mantissa
// for any portion that would have been to the
// right of the decimal.
// Trim trailing zeros from mantissa. If we ended up with no
// characters at all in string, thus, pos == 0, the loop doesn't
// happen and we pick [:0], which is correct, probably.
trimZeroCnt := 0
trimSpaceCnt := 0
for i := len(mantissa) - 1; i >= 0; i-- {
switch mantissa[i] {
case uint8(0), uint8(32): // nil/space
trimSpaceCnt++
continue
case uint8(48): // zero
trimZeroCnt++
continue
}
break
for i := pos - 1; i >= 0 && mantissa[i] == '0'; i-- {
trimZeroCnt++
}
mantissa = mantissa[:len(mantissa)-trimSpaceCnt-trimZeroCnt]
mantissa = mantissa[:pos-trimZeroCnt]
// Based on where (or if) the decimal was found,
// calculate scale.

View file

@ -34,10 +34,9 @@ func TestDecimal(t *testing.T) {
{"0", pql.Decimal{0, 0}, ""},
{"-0", pql.Decimal{0, 0}, ""},
{"0.0", pql.Decimal{0, 0}, ""},
{"0.", pql.Decimal{0, 0}, ""},
{"-0.00", pql.Decimal{0, 0}, ""},
{"123.4567", pql.Decimal{1234567, 4}, ""},
{" 123.4567", pql.Decimal{1234567, 4}, ""},
{" 123.4567 ", pql.Decimal{1234567, 4}, ""},
{"123.456700", pql.Decimal{1234567, 4}, ""},
{"00123.4567", pql.Decimal{1234567, 4}, ""},
{"+123.4567", pql.Decimal{1234567, 4}, ""},
@ -56,8 +55,7 @@ func TestDecimal(t *testing.T) {
{".123", pql.Decimal{123, 3}, ""},
{"0.123", pql.Decimal{123, 3}, ""},
{"0.001230", pql.Decimal{123, 5}, ""},
{" 0.001230 ", pql.Decimal{123, 5}, ""},
{"-0.001230 ", pql.Decimal{-123, 5}, ""},
{"-0.001230", pql.Decimal{-123, 5}, ""},
// int64 edges.
{".000009223372036854775807", pql.Decimal{9223372036854775807, 24}, ""},
@ -83,6 +81,10 @@ func TestDecimal(t *testing.T) {
{"abc", pql.Decimal{}, "invalid syntax"},
{"0.12.3", pql.Decimal{}, "invalid decimal string"},
{"--12300", pql.Decimal{}, "invalid syntax"},
{" 123.4567 ", pql.Decimal{}, "invalid syntax"},
{" 123.4567", pql.Decimal{}, "invalid syntax"},
{"123.4567 ", pql.Decimal{}, "invalid syntax"},
{"0.a", pql.Decimal{}, "invalid syntax"},
// These are no longer error cases since we introduced precision adjustment.
//{"922337203685477580.9", pql.Decimal{}, "value out of range"},
@ -94,12 +96,12 @@ func TestDecimal(t *testing.T) {
dec, err := pql.ParseDecimal(test.s)
if test.expErr != "" {
if err == nil || !strings.Contains(err.Error(), test.expErr) {
t.Fatalf("test %d expected error to contain: %s, but got: %v", i, test.expErr, err)
t.Fatalf("test %d parsing string `%s`: expected error to contain: %s, but got: %v", i, test.s, test.expErr, err)
}
} else if err != nil {
t.Fatalf("test %d parsing string `%s`: %s", i, test.s, err)
} else if dec != test.exp {
t.Fatalf("test %d expected: %v, but got: %v", i, test.exp, dec)
t.Fatalf("test %d parsing string `%s`: expected: %v, but got: %v", i, test.s, test.exp, dec)
}
}
})
@ -141,8 +143,6 @@ func TestDecimal(t *testing.T) {
exp string
}{
{"123.4567", "123.4567"},
{" 123.4567", "123.4567"},
{" 123.4567 ", "123.4567"},
{"123.456700", "123.4567"},
{"00123.4567", "123.4567"},
{"+123.4567", "123.4567"},
@ -161,8 +161,8 @@ func TestDecimal(t *testing.T) {
{"0.123", "0.123"},
{"0.001230", "0.00123"},
{" 0.001230 ", "0.00123"},
{"-0.001230 ", "-0.00123"},
{"+0.001230", "0.00123"},
{"-0.001230", "-0.00123"},
}
for i, test := range tests {
dec, err := pql.ParseDecimal(test.s)

View file

@ -61,7 +61,10 @@ func (p *parser) Parse() (*Query, error) {
p.PQL = PQL{
Buffer: string(buf),
}
p.Init()
err = p.Init()
if err != nil {
return nil, errors.Wrap(err, "creating parser")
}
err = p.PQL.Parse()
if err != nil {
return nil, errors.Wrap(err, "parsing")

View file

@ -193,6 +193,37 @@ func TestParser_Parse(t *testing.T) {
t.Fatalf("unexpected call: %#v", q.Calls[0])
}
})
t.Run("MixedCase", func(t *testing.T) {
q, err := pql.ParseString(`roW(x=3)`)
if err != nil {
t.Fatal(err)
} else if !reflect.DeepEqual(q.Calls[0],
&pql.Call{
Name: "Row",
Args: map[string]interface{}{
"x": int64(3),
},
},
) {
t.Fatalf("unexpected call: %#v", q.Calls[0])
}
})
t.Run("VariousSpaces", func(t *testing.T) {
q, err := pql.ParseString(`TopN( x )`)
if err != nil {
t.Fatal(err)
} else if !reflect.DeepEqual(q.Calls[0],
&pql.Call{
Name: "TopN",
Args: map[string]interface{}{"_field": "x"},
},
) {
t.Fatalf("unexpected call: %#v", q.Calls[0])
}
})
}
func TestUnquote(t *testing.T) {

View file

@ -4,101 +4,75 @@ type PQL Peg {
Query
}
# All input queries consist of a sequence of calls, at the top level.
Calls <- sp (Call sp)* !.
Call <- 'Set' {p.startCall("Set")} open col comma dargs (comma timestamp)? close {p.endCall()}
/ 'SetRowAttrs' {p.startCall("SetRowAttrs")} open posfield comma row comma fargs close {p.endCall()}
/ 'SetColumnAttrs' {p.startCall("SetColumnAttrs")} open col comma fargs close {p.endCall()}
/ 'Clear' {p.startCall("Clear")} open col comma dargs close {p.endCall()}
/ 'ClearRow' {p.startCall("ClearRow")} open darg close {p.endCall()}
/ 'Store' {p.startCall("Store")} open Call comma darg close {p.endCall()}
/ 'TopN' {p.startCall("TopN")} open posfield (comma allargs)? close {p.endCall()}
/ 'Rows' {p.startCall("Rows")} open posfield (comma allargs)? close {p.endCall()}
/ 'Range' {p.startCall("Range")} open field sp '=' sp fvalue comma 'from='? {p.addField("from")} timestampfmt {p.addVal(text)} comma 'to='? sp {p.addField("to")} timestampfmt {p.addVal(text)} close {p.endCall()}
/ < IDENT > { p.startCall(text ) } open allargs comma? close { p.endCall() }
allargs <- Call (comma Call)* (comma dargs)? / dargs / sp
fargs <- farg (comma fargs)? sp
farg <- ( field sp '=' sp fvalue
/ field sp COND sp fvalue
Call <- "Set" {p.startCall("Set")} open col comma args (comma timestamp)? close {p.endCall()}
/ "SetRowAttrs" {p.startCall("SetRowAttrs")} open posfield comma row comma args close {p.endCall()}
/ "SetColumnAttrs" {p.startCall("SetColumnAttrs")} open col comma args close {p.endCall()}
/ "Clear" {p.startCall("Clear")} open col comma args close {p.endCall()}
/ "ClearRow" {p.startCall("ClearRow")} open arg close {p.endCall()}
/ "Store" {p.startCall("Store")} open Call comma arg close {p.endCall()}
/ "TopN" {p.startCall("TopN")} open posfield (comma allargs)? close {p.endCall()}
/ "Rows" {p.startCall("Rows")} open posfield (comma allargs)? close {p.endCall()}
/ "Range" {p.startCall("Range")} open field eq value comma 'from='? {p.addField("from")} timestampfmt {p.addVal(text)} comma 'to='? sp {p.addField("to")} timestampfmt {p.addVal(text)} close {p.endCall()}
/ < IDENT > { p.startCall(text) } open allargs comma? close { p.endCall() }
allargs <- Call (comma Call)* (comma args)? / args / sp
args <- arg (comma args)? sp
arg <- field eq value
/ field sp COND sp value
/ conditional
)
dargs <- darg (comma dargs)? sp
darg <- ( field sp '=' sp dvalue
/ field sp COND sp dvalue
/ conditional
)
COND <- ( '><' { p.addBTWN() }
COND <- '><' { p.addBTWN() }
/ '<=' { p.addLTE() }
/ '>=' { p.addGTE() }
/ '==' { p.addEQ() }
/ '!=' { p.addNEQ() }
/ '<' { p.addLT() }
/ '>' { p.addGT() }
)
conditional <- {p.startConditional()} condint condLT condfield condLT condint {p.endConditional()}
condint <- < '-'? [0-9]* '.' [0-9]+ / '0' / '-'? [1-9] [0-9]* > sp {p.condAdd(text)}
condint <- < decimal > sp {p.condAdd(text)}
condLT <- <('<=' / '<')> sp {p.condAdd(text)}
condfield <- <fieldExpr> sp {p.condAdd(text)}
dvalue <- ( ditem
/ lbrack { p.startList() } dlist rbrack { p.endList() }
)
fvalue <- ( fitem
/ lbrack { p.startList() } flist rbrack { p.endList() }
)
dlist <- ditem (comma dlist)?
flist <- fitem (comma flist)?
ditem <- ( itema
/ decimal
/ itemb
)
fitem <- ( itema
/ float
/ itemb
)
itema <- ( 'null' &(comma / sp close) { p.addVal(nil) }
/ 'true' &(comma / sp close) { p.addVal(true) }
/ 'false' &(comma / sp close) { p.addVal(false) }
value <- item
/ lbrack { p.startList() } items rbrack { p.endList() }
items <- item (comma items)?
item <- 'null' &(comma / close) { p.addVal(nil) }
/ 'true' &(comma / close) { p.addVal(true) }
/ 'false' &(comma / close) { p.addVal(false) }
/ timestampfmt { p.addVal(text) }
)
itemb <- ( < IDENT > { p.startCall(text) } open allargs comma? close { p.addVal(p.endCall()) }
/ < decimal > { p.addNumVal(text) }
/ < IDENT > { p.startCall(text) } open allargs comma? close { p.addVal(p.endCall()) }
/ < ([[A-Z]] / [0-9] / '-' / '_' / ':')+ > { p.addVal(text) }
/ < '"' doublequotedstring '"' > { p.addVal(text) }
/ < '\'' singlequotedstring '\'' > { p.addVal(text) }
)
float <- ( < '-'? [0-9]+ ('.'[0-9]*)? > { p.addNumVal(text, true) }
/ < '-'? '.'[0-9]+ > { p.addNumVal(text, true) }
)
decimal <- ( < '-'? [0-9]+ ('.'[0-9]*)? > { p.addNumVal(text, false) }
/ < '-'? '.'[0-9]+ > { p.addNumVal(text, false) }
)
doublequotedstring <- ( '\\"' / '\\\\' / '\\n' / '\\t' / [^"\\] )*
singlequotedstring <- ( '\\\'' / '\\\\' / '\\n' / '\\t' / [^'\\] )*
fieldExpr <- ( [[A-Z]] / '_' ) ( [[A-Z]] / [0-9] / '_' / '-' )*
field <- <fieldExpr / reserved> { p.addField(text) }
reserved <- ('_row' / '_col' / '_start' / '_end' / '_timestamp' / '_field')
reserved <- '_row' / '_col' / '_start' / '_end' / '_timestamp' / '_field'
posfield <- <fieldExpr> { p.addPosStr("_field", text) }
uint <- [1-9] [0-9]* / '0'
col <- ( <uint> {p.addPosNum("_col", text)}
col <- < digits > {p.addPosNum("_col", text)}
/ < '\'' singlequotedstring '\'' > {p.addPosStr("_col", text)}
/ < '"' doublequotedstring '"' > {p.addPosStr("_col", text)}
)
row <- ( <uint> {p.addPosNum("_row", text)}
row <- < digits > {p.addPosNum("_row", text)}
/ < '\'' singlequotedstring '\'' > {p.addPosStr("_row", text)}
/ < '"' doublequotedstring '"' > {p.addPosStr("_row", text)}
)
open <- '(' sp
close <- ')' sp
sp <- ( ' ' / '\t' / '\n' )*
close <- sp ')' sp
sp <- [ \t\n]*
eq <- sp '=' sp
comma <- sp ',' sp
lbrack <- '[' sp
rbrack <- sp ']' sp
IDENT <- [[A-Z]] ([[A-Z]] / [0-9])*
digits <- [0-9]+
signedDigits <- '-'? digits
decimal <- signedDigits ('.' digits?)?
/ '-'? '.' digits
timestampbasicfmt <- [0-9][0-9][0-9][0-9]'-'[01][0-9]'-'[0-3][0-9]'T'[0-9][0-9]':'[0-9][0-9]
timestampfmt <- '"' <timestampbasicfmt> '"' / '\'' <timestampbasicfmt> '\'' / <timestampbasicfmt>

File diff suppressed because it is too large Load diff

View file

@ -19,20 +19,28 @@ import (
"reflect"
"strconv"
"testing"
"github.com/pkg/errors"
)
func TestPEG(t *testing.T) {
p := PQL{Buffer: `
SetBit(Union(Zitmap(row==4), Intersect(Qitmap(blah>4), Ritmap(field="http://zoo9.com=\\'hello' and \"hello\"")), Hitmap(row=ag-bee)), a="4z", b=5) Count(Union(Witmap(row=5.73, frame=.10), Row(zztop><[2, 9]))) TopN(blah, fields=["hello", "goodbye", "zero"])`[1:]}
p.Init()
err := p.Parse()
err := p.Init()
if err != nil {
t.Fatal(errors.Wrap(err, "creating parser"))
}
err = p.Parse()
if err != nil {
t.Fatalf("parse error: %v", err)
}
p.Execute()
p = PQL{Buffer: `SetRowAttrs(attr="http://zoo9.com=\\'hello' "and \"hello\"")`}
p.Init()
err = p.Init()
if err != nil {
t.Fatal(errors.Wrap(err, "creating parser"))
}
err = p.Parse()
if err == nil {
t.Fatalf("should have been an error because of the interior unescaped double quote")