allow floats in PQL queries for decimal fields

had to workaround some cruft in the parser that was trying to only
support a BETWEEN query as LTE, LTE. Now we have operations for all
combinations of LT and LTE.

unrelated - changed the port a test was binding to as it conflicted
with a port I was using locally.

cmd/server_test.go

@@ -42,7 +42,7 @@ func TestServerConfig(t *testing.T) {
 	tests := []commandTest{
 		// TEST 0
 		{
-			args: []string{"server", "--data-dir", actualDataDir, "--cluster.hosts", "localhost:42454,localhost:10110", "--bind", "localhost:42454", "--bind-grpc", "localhost:20111", "--translation.map-size", "100000"},
+			args: []string{"server", "--data-dir", actualDataDir, "--cluster.hosts", "localhost:42454,localhost:10110", "--bind", "localhost:42454", "--bind-grpc", "localhost:30112", "--translation.map-size", "100000"},
 			env: map[string]string{
 				"PILOSA_DATA_DIR":                "/tmp/myEnvDatadir",
 				"PILOSA_CLUSTER_LONG_QUERY_TIME": "1m30s",

executor.go

@@ -18,6 +18,7 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"math"
 	"sort"
 	"sync"
 	"time"
@@ -1929,8 +1930,9 @@ func (e *executor) executeRowBSIGroupShard(ctx context.Context, index string, c
 
 		return frag.notNull()
 
-	} else if cond.Op == pql.BETWEEN {
-		predicates, err := cond.IntSliceValue()
+	} else if cond.Op == pql.BETWEEN || cond.Op == pql.BTWN_LT_LT ||
+		cond.Op == pql.BTWN_LTE_LT || cond.Op == pql.BTWN_LT_LTE {
+		predicates, err := getCondIntSlice(f, cond)
 		if err != nil {
 			return nil, errors.Wrap(err, "getting condition value")
 		}
@@ -1970,11 +1972,17 @@ func (e *executor) executeRowBSIGroupShard(ctx context.Context, index string, c
 		return frag.rangeBetween(bsig.BitDepth, baseValueMin, baseValueMax)
 
 	} else {
-
-		// Only support integers for now.
 		value, ok := cond.Value.(int64)
 		if !ok {
-			return nil, errors.New("Row(): conditions only support integer values")
+			if floatVal, ok := cond.Value.(float64); ok {
+				if f.Options().Type != FieldTypeDecimal {
+					return nil, errors.Errorf("Float value '%f' given in query to non-decimal field", floatVal)
+				}
+				scale := f.Options().Scale
+				value = int64(floatVal * math.Pow10(int(scale)))
+			} else {
+				return nil, errors.New("Row(): conditions only support integer values (or floats for decimal fields)")
+			}
 		}
 
 		// Find bsiGroup.
@@ -3746,3 +3754,47 @@ func (gbi *groupByIterator) Next() (ret GroupCount, done bool) {
 
 	return ret, false
 }
+
+// getCondIntSlice looks at the field, the cond op type (which is
+// expected to be one of the BETWEEN ops types), and the values in the
+// conditional and returns a slice of int64 which is scaled for
+// decimal fields and has the values modulated such that the BETWEEN
+// op can be treated as being of the form a<=x<=b.
+func getCondIntSlice(f *Field, cond *pql.Condition) ([]int64, error) {
+	val, ok := cond.Value.([]interface{})
+	if !ok {
+		return nil, errors.Errorf("expected conditional to have []interface{} Value, but got %v of %[1]T", cond.Value)
+	}
+
+	ret := make([]int64, len(val))
+	for i, v := range val {
+		switch tv := v.(type) {
+		case int64:
+			ret[i] = tv
+		case uint64:
+			ret[i] = int64(tv)
+		case float64:
+			if f.Options().Type != FieldTypeDecimal {
+				return nil, errors.Errorf("got a float value '%f' in a query to an integer field", tv)
+			}
+			scale := f.Options().Scale
+			iv := int64(tv * math.Pow10(int(scale)))
+			ret[i] = iv
+		default:
+			return nil, errors.Errorf("unexpected value type %T, val %v", tv, tv)
+		}
+	}
+
+	switch cond.Op {
+	case pql.BTWN_LT_LTE: // a < x <= b
+		ret[0]++
+	case pql.BTWN_LTE_LT: // a < x <= b
+		ret[1]--
+	case pql.BTWN_LT_LT: // a < x < b
+		ret[0]++
+		ret[1]--
+	}
+
+	return ret, nil
+
+}

http/client_test.go

@@ -1103,6 +1103,53 @@ func TestClient_CreateDecimalField(t *testing.T) {
 		t.Fatalf("unexpected results: %v", resp.Results[0].(*pilosa.Row).Columns())
 	}
 
+	resp, err = c.Query(context.Background(), index, &pilosa.QueryRequest{Index: index, Query: "Row(1.1<dfield<3.3)"})
+	if err != nil {
+		t.Fatalf("querying: %v", err)
+	}
+	if !reflect.DeepEqual(resp.Results[0].(*pilosa.Row).Columns(), []uint64{2}) {
+		t.Fatalf("unexpected results: %v", resp.Results[0].(*pilosa.Row).Columns())
+	}
+
+	resp, err = c.Query(context.Background(), index, &pilosa.QueryRequest{Index: index, Query: "Row(1.1<=dfield<3.3)"})
+	if err != nil {
+		t.Fatalf("querying: %v", err)
+	}
+	if !reflect.DeepEqual(resp.Results[0].(*pilosa.Row).Columns(), []uint64{1, 2}) {
+		t.Fatalf("unexpected results: %v", resp.Results[0].(*pilosa.Row).Columns())
+	}
+
+	resp, err = c.Query(context.Background(), index, &pilosa.QueryRequest{Index: index, Query: "Row(1.1<dfield<=3.3)"})
+	if err != nil {
+		t.Fatalf("querying: %v", err)
+	}
+	if !reflect.DeepEqual(resp.Results[0].(*pilosa.Row).Columns(), []uint64{2, 3}) {
+		t.Fatalf("unexpected results: %v", resp.Results[0].(*pilosa.Row).Columns())
+	}
+
+	resp, err = c.Query(context.Background(), index, &pilosa.QueryRequest{Index: index, Query: "Row(dfield<3.3)"})
+	if err != nil {
+		t.Fatalf("querying: %v", err)
+	}
+	if !reflect.DeepEqual(resp.Results[0].(*pilosa.Row).Columns(), []uint64{1, 2}) {
+		t.Fatalf("unexpected results: %v", resp.Results[0].(*pilosa.Row).Columns())
+	}
+
+	resp, err = c.Query(context.Background(), index, &pilosa.QueryRequest{Index: index, Query: "Row(dfield>2.2)"})
+	if err != nil {
+		t.Fatalf("querying: %v", err)
+	}
+	if !reflect.DeepEqual(resp.Results[0].(*pilosa.Row).Columns(), []uint64{3}) {
+		t.Fatalf("unexpected results: %v", resp.Results[0].(*pilosa.Row).Columns())
+	}
+
+	resp, err = c.Query(context.Background(), index, &pilosa.QueryRequest{Index: index, Query: "Row(dfield>=2.2)"})
+	if err != nil {
+		t.Fatalf("querying: %v", err)
+	}
+	if !reflect.DeepEqual(resp.Results[0].(*pilosa.Row).Columns(), []uint64{2, 3}) {
+		t.Fatalf("unexpected results: %v", resp.Results[0].(*pilosa.Row).Columns())
+	}
 }
 
 // Client represents a test wrapper for pilosa.Client.

pql/ast.go

@@ -87,19 +87,26 @@ func (q *Query) endConditional() {
 	if len(q.conditional) != 5 {
 		panic(fmt.Sprintf("conditional of wrong length: %#v", q.conditional))
 	}
-	low, _ := strconv.ParseInt(q.conditional[0], 10, 64)
+	low := parseNum(q.conditional[0])
 	field := q.conditional[2]
-	high, _ := strconv.ParseInt(q.conditional[4], 10, 64)
+	high := parseNum(q.conditional[4])
 
-	if q.conditional[1] == "<" {
-		low++
-	}
-	if q.conditional[3] == "<" {
-		high--
+	var op Token
+	switch q.conditional[1] + q.conditional[3] {
+	case "<<":
+		op = BTWN_LT_LT
+	case "<=<":
+		op = BTWN_LTE_LT
+	case "<<=":
+		op = BTWN_LT_LTE
+	case "<=<=":
+		op = BETWEEN
+	default:
+		panic(fmt.Sprintf("impossible conditional ops: '%s' and '%s'", q.conditional[1], q.conditional[3]))
 	}
 
 	elem := q.lastCallStackElem()
-	elem.call.Args[field] = &Condition{Op: BETWEEN, Value: []interface{}{low, high}}
+	elem.call.Args[field] = &Condition{Op: op, Value: []interface{}{low, high}}
 
 	q.conditional = nil
 }
@@ -155,16 +162,7 @@ func (q *Query) addNumVal(val string) {
 	if elem == nil || elem.lastField == "" {
 		panic(fmt.Sprintf("addIntVal called with '%s' when lastField is empty", val))
 	}
-	var ival interface{}
-	var err error
-	if strings.Contains(val, ".") {
-		ival, err = strconv.ParseFloat(val, 64)
-	} else {
-		ival, err = strconv.ParseInt(val, 10, 64)
-	}
-	if err != nil {
-		panic(fmt.Sprintf("%s: %s", intOutOfRangeError, err))
-	}
+	ival := parseNum(val)
 	if elem.inList {
 		if elem.lastCond != ILLEGAL {
 			list := elem.call.Args[elem.lastField].(*Condition).Value.([]interface{})
@@ -750,6 +748,12 @@ func (cond *Condition) String() string {
 // IntSliceValue reads cond.Value as a slice of uint64.
 // If the value is a slice of uint64 it will convert
 // it to []int64. Otherwise, if it is not a []int64 it will return an error.
+//
+// TODO(2.0) this is now only referenced in a test and should probably
+// be removed. The functionality was replaced by getCondIntSlice in
+// pilosa/executor.go which needed to check for floating point values
+// and also have access to the Pilosa field to see if floating point
+// values were valid and how they needed to be scaled.
 func (cond *Condition) IntSliceValue() ([]int64, error) {
 	val := cond.Value
 
@@ -818,3 +822,17 @@ func joinUint64Slice(a []uint64) string {
 	}
 	return "[" + strings.Join(other, ",") + "]"
 }
+
+func parseNum(val string) interface{} {
+	var ival interface{}
+	var err error
+	if strings.Contains(val, ".") {
+		ival, err = strconv.ParseFloat(val, 64)
+	} else {
+		ival, err = strconv.ParseInt(val, 10, 64)
+	}
+	if err != nil {
+		panic(fmt.Sprintf("%s: %s", intOutOfRangeError, err))
+	}
+	return ival
+}

pql/pql.peg

@@ -32,7 +32,7 @@ COND <- ( '><' { p.addBTWN() }
         )
 
 conditional <- {p.startConditional()} condint condLT condfield condLT condint {p.endConditional()}
-condint <- <'-'? [1-9] [0-9]* / '0'> sp {p.condAdd(buffer[begin:end])}
+condint <- < '-'? [0-9]* '.' [0-9]+ / '0' / '-'? [1-9] [0-9]* > sp {p.condAdd(buffer[begin:end])}
 condLT <- <('<=' / '<')> sp {p.condAdd(buffer[begin:end])}
 condfield <- <fieldExpr> sp {p.condAdd(buffer[begin:end])}
 

pql/pqlpeg_test.go

@@ -533,8 +533,8 @@ func TestPQLDeepEquality(t *testing.T) {
 				Name: "Row",
 				Args: map[string]interface{}{
 					"a": &Condition{
-						Op:    BETWEEN,
-						Value: []interface{}{int64(4), int64(8)},
+						Op:    BTWN_LTE_LT,
+						Value: []interface{}{int64(4), int64(9)},
 					},
 				},
 			}},
@@ -545,8 +545,8 @@ func TestPQLDeepEquality(t *testing.T) {
 				Name: "Row",
 				Args: map[string]interface{}{
 					"a": &Condition{
-						Op:    BETWEEN,
-						Value: []interface{}{int64(5), int64(8)},
+						Op:    BTWN_LT_LT,
+						Value: []interface{}{int64(4), int64(9)},
 					},
 				},
 			}},
@@ -569,8 +569,8 @@ func TestPQLDeepEquality(t *testing.T) {
 				Name: "Row",
 				Args: map[string]interface{}{
 					"a": &Condition{
-						Op:    BETWEEN,
-						Value: []interface{}{int64(5), int64(9)},
+						Op:    BTWN_LT_LTE,
+						Value: []interface{}{int64(4), int64(9)},
 					},
 				},
 			}},
@@ -672,8 +672,8 @@ func TestPQLDeepEquality(t *testing.T) {
 						Name: "Row",
 						Args: map[string]interface{}{
 							"a": &Condition{
-								Op:    BETWEEN,
-								Value: []interface{}{int64(5), int64(8)},
+								Op:    BTWN_LT_LT,
+								Value: []interface{}{int64(4), int64(9)},
 							},
 						},
 					},

pql/token.go

@@ -21,14 +21,24 @@ const (
 	// Special tokens
 	ILLEGAL Token = iota
 
-	ASSIGN  // =
-	EQ      // ==
-	NEQ     // !=
-	LT      // <
-	LTE     // <=
-	GT      // >
-	GTE     // >=
-	BETWEEN // ><
+	ASSIGN // =
+	EQ     // ==
+	NEQ    // !=
+	LT     // <
+	LTE    // <=
+	GT     // >
+	GTE    // >=
+
+	BETWEEN // ><  (this is like a <= x <= b)
+
+	// not used in lexing/parsing, but so that the parser can signal
+	// to the executor how to treat the arguments. We used to just add
+	// 1 to the arguments if they were LT so the executor could assume
+	// it was always <=, <=, but then we needed to support
+	// floats/decimals and couldn't do that any more.
+	BTWN_LT_LTE // a < x <= b
+	BTWN_LTE_LT // a <= x < b
+	BTWN_LT_LT  // a < x < b
 )
 
 var tokens = [...]string{