Merge pull request #28 from molecula/pql-float-values

Pql float values

api.go

@@ -387,7 +387,7 @@ func (api *API) ImportRoaring(ctx context.Context, indexName, fieldName string,
 
 	// only set and time fields are supported
 	if field.Type() != FieldTypeSet && field.Type() != FieldTypeTime {
-		return NewBadRequestError(errors.New("roaring import is only supported for set and time fields"))
+		return NewBadRequestError(errors.Errorf("roaring import is only supported for set and time fields, not '%s' fields.", field.Type()))
 	}
 
 	errCh := make(chan error, len(nodes))

api_test.go

@@ -293,7 +293,7 @@ func TestAPI_ImportValue(t *testing.T) {
 			t.Fatal(err)
 		}
 
-		pql := fmt.Sprintf("Row(%s>60)", field)
+		pql := fmt.Sprintf("Row(%s>6)", field)
 
 		// Query node0.
 		if res, err := m0.API.Query(ctx, &pilosa.QueryRequest{Index: index, Query: pql}); err != nil {
@@ -389,7 +389,7 @@ func TestAPI_ImportValue(t *testing.T) {
 			t.Fatal(err)
 		}
 
-		pql := fmt.Sprintf("Row(%s>60)", field)
+		pql := fmt.Sprintf("Row(%s>600)", field)
 
 		// Query node0.
 		if res, err := m0.API.Query(ctx, &pilosa.QueryRequest{Index: index, Query: pql}); err != nil {

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,9 @@ 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")
+		value, err := getScaledInt(f, cond.Value)
+		if err != nil {
+			return nil, errors.Wrap(err, "getting scaled integer")
 		}
 
 		// Find bsiGroup.
@@ -3746,3 +3746,65 @@ 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 {
+		s, err := getScaledInt(f, v)
+		if err != nil {
+			return nil, errors.Wrap(err, "getting scaled integer")
+		}
+		ret[i] = s
+	}
+
+	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
+}
+
+// getScaledInt gets the scaled integer value for v based on
+// the field type.
+func getScaledInt(f *Field, v interface{}) (int64, error) {
+	var value int64
+	if f.Options().Type == FieldTypeDecimal {
+		scale := f.Options().Scale
+		switch tv := v.(type) {
+		case int64:
+			value = int64(float64(tv) * math.Pow10(int(scale)))
+		case uint64:
+			value = int64(float64(tv) * math.Pow10(int(scale)))
+		case float64:
+			value = int64(tv * math.Pow10(int(scale)))
+		default:
+			return 0, errors.Errorf("unexpected decimal value type %T, val %v", tv, tv)
+		}
+	} else {
+		switch tv := v.(type) {
+		case int64:
+			value = tv
+		case uint64:
+			value = int64(tv)
+		default:
+			return 0, errors.Errorf("unexpected value type %T, val %v", tv, tv)
+		}
+	}
+	return value, nil
+}

fragment.go

@@ -1175,7 +1175,8 @@ func (f *fragment) rangeLTUnsigned(filter *Row, bitDepth uint, predicate uint64,
 
 	// if the predicate is larger than all representable numbers given
 	// our bitDepth... then just return everything.
-	if msb(predicate) >= bitDepth {
+	if msb(predicate) > bitDepth {
+
 		return filter, nil
 	}
 

fragment_internal_test.go

@@ -3243,6 +3243,25 @@ func TestFragmentPositionsForValue(t *testing.T) {
 	}
 }
 
+func TestIntLTRegression(t *testing.T) {
+	f := mustOpenFragment("i", "f", "v", 0, CacheTypeNone)
+	defer f.Clean(t)
+
+	_, err := f.setValue(1, 6, 33)
+	if err != nil {
+		t.Fatalf("setting value: %v", err)
+	}
+
+	row, err := f.rangeOp(pql.LT, 6, 33)
+	if err != nil {
+		t.Fatalf("doing range of: %v", err)
+	}
+
+	if !row.IsEmpty() {
+		t.Errorf("expected nothing, but got: %v", row.Columns())
+	}
+}
+
 func TestImportClearRestart(t *testing.T) {
 	tests := []struct {
 		rows []uint64

http/client_test.go

@@ -1095,7 +1095,8 @@ func TestClient_CreateDecimalField(t *testing.T) {
 		t.Fatalf("importing float values: %v", err)
 	}
 
-	resp, err := c.Query(context.Background(), index, &pilosa.QueryRequest{Index: index, Query: "Row(dfield>21)"})
+	// Integer predicate.
+	resp, err := c.Query(context.Background(), index, &pilosa.QueryRequest{Index: index, Query: "Row(dfield>2)"})
 	if err != nil {
 		t.Fatalf("querying: %v", err)
 	}
@@ -1103,6 +1104,72 @@ func TestClient_CreateDecimalField(t *testing.T) {
 		t.Fatalf("unexpected results: %v", resp.Results[0].(*pilosa.Row).Columns())
 	}
 
+	// Float predicate.
+	resp, err = c.Query(context.Background(), index, &pilosa.QueryRequest{Index: index, Query: "Row(dfield>2.1)"})
+	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())
+	}
+
+	// Integer predicates.
+	resp, err = c.Query(context.Background(), index, &pilosa.QueryRequest{Index: index, Query: "Row(1<dfield<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())
+	}
+
+	// Float predicates.
+	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{