mirror of
https://github.com/featurebasedb/featurebase.git
synced 2026-09-12 23:51:03 +00:00
fix potential nil dereference in SignedRow.ToRows
This used to be possible to hit, but I think now that Distinct on a set field returns a *Row rather than a SignedRow it isn't an issue. (I wasn't able to trigger it in the tests). Adding the fix anyway as it seems safer than not. The rest of the changes are test infrastructure to make it easy to call GRPC queries and verify the results as CSV.
This commit is contained in:
Matt Jaffee
parent
1372bafe02
commit
446950979a
6 changed files with 220 additions and 45 deletions
58
executor.go
58
executor.go
|
|
@ -6690,38 +6690,42 @@ func (s SignedRow) ToTable() (*pb.TableResponse, error) {
|
|||
func (s SignedRow) ToRows(callback func(*pb.RowResponse) error) error {
|
||||
|
||||
ci := []*pb.ColumnInfo{{Name: s.Field(), Datatype: "int64"}}
|
||||
negs := s.Neg.Columns()
|
||||
for i := len(negs) - 1; i >= 0; i-- {
|
||||
val, err := toNegInt64(negs[i])
|
||||
if err != nil {
|
||||
return errors.Wrap(err, "converting uint64 to int64 (negative)")
|
||||
}
|
||||
if s.Neg != nil {
|
||||
negs := s.Neg.Columns()
|
||||
for i := len(negs) - 1; i >= 0; i-- {
|
||||
val, err := toNegInt64(negs[i])
|
||||
if err != nil {
|
||||
return errors.Wrap(err, "converting uint64 to int64 (negative)")
|
||||
}
|
||||
|
||||
if err := callback(&pb.RowResponse{
|
||||
Headers: ci,
|
||||
Columns: []*pb.ColumnResponse{
|
||||
&pb.ColumnResponse{ColumnVal: &pb.ColumnResponse_Int64Val{Int64Val: val}},
|
||||
},
|
||||
}); err != nil {
|
||||
return errors.Wrap(err, "calling callback")
|
||||
if err := callback(&pb.RowResponse{
|
||||
Headers: ci,
|
||||
Columns: []*pb.ColumnResponse{
|
||||
&pb.ColumnResponse{ColumnVal: &pb.ColumnResponse_Int64Val{Int64Val: val}},
|
||||
},
|
||||
}); err != nil {
|
||||
return errors.Wrap(err, "calling callback")
|
||||
}
|
||||
ci = nil
|
||||
}
|
||||
ci = nil
|
||||
}
|
||||
for _, id := range s.Pos.Columns() {
|
||||
val, err := toInt64(id)
|
||||
if err != nil {
|
||||
return errors.Wrap(err, "converting uint64 to int64 (positive)")
|
||||
}
|
||||
if s.Pos != nil {
|
||||
for _, id := range s.Pos.Columns() {
|
||||
val, err := toInt64(id)
|
||||
if err != nil {
|
||||
return errors.Wrap(err, "converting uint64 to int64 (positive)")
|
||||
}
|
||||
|
||||
if err := callback(&pb.RowResponse{
|
||||
Headers: ci,
|
||||
Columns: []*pb.ColumnResponse{
|
||||
&pb.ColumnResponse{ColumnVal: &pb.ColumnResponse_Int64Val{Int64Val: val}},
|
||||
},
|
||||
}); err != nil {
|
||||
return errors.Wrap(err, "calling callback")
|
||||
if err := callback(&pb.RowResponse{
|
||||
Headers: ci,
|
||||
Columns: []*pb.ColumnResponse{
|
||||
&pb.ColumnResponse{ColumnVal: &pb.ColumnResponse_Int64Val{Int64Val: val}},
|
||||
},
|
||||
}); err != nil {
|
||||
return errors.Wrap(err, "calling callback")
|
||||
}
|
||||
ci = nil
|
||||
}
|
||||
ci = nil
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
|
|
|||
120
executor_test.go
120
executor_test.go
|
|
@ -6797,44 +6797,69 @@ func TestVariousQueries(t *testing.T) {
|
|||
})
|
||||
|
||||
tests := []struct {
|
||||
query string
|
||||
verifier func(t *testing.T, resp pilosa.QueryResponse)
|
||||
query string
|
||||
qrVerifier func(t *testing.T, resp pilosa.QueryResponse)
|
||||
csvVerifier func(t *testing.T, resp string)
|
||||
}{
|
||||
{
|
||||
query: "Count(All())",
|
||||
verifier: func(t *testing.T, resp pilosa.QueryResponse) {
|
||||
qrVerifier: func(t *testing.T, resp pilosa.QueryResponse) {
|
||||
if resp.Results[0].(uint64) != 6 {
|
||||
t.Errorf("expected 6, got %+v", resp.Results[0])
|
||||
}
|
||||
},
|
||||
csvVerifier: func(t *testing.T, resp string) {
|
||||
exp := "6\n"
|
||||
if resp != exp {
|
||||
t.Errorf("expected '%s', got '%s'", exp, resp)
|
||||
}
|
||||
},
|
||||
},
|
||||
{
|
||||
query: "Count(Distinct(field=likenums))",
|
||||
verifier: func(t *testing.T, resp pilosa.QueryResponse) {
|
||||
qrVerifier: func(t *testing.T, resp pilosa.QueryResponse) {
|
||||
if resp.Results[0].(uint64) != 7 {
|
||||
t.Errorf("wrong count: %+v", resp.Results[0])
|
||||
}
|
||||
},
|
||||
csvVerifier: func(t *testing.T, resp string) {
|
||||
exp := "7\n"
|
||||
if resp != exp {
|
||||
t.Errorf("expected '%s', got '%s'", exp, resp)
|
||||
}
|
||||
},
|
||||
},
|
||||
{
|
||||
query: "Distinct(field=likenums)",
|
||||
verifier: func(t *testing.T, resp pilosa.QueryResponse) {
|
||||
qrVerifier: func(t *testing.T, resp pilosa.QueryResponse) {
|
||||
if !reflect.DeepEqual(resp.Results[0].(*pilosa.Row).Columns(), []uint64{1, 2, 3, 4, 5, 6, 7}) {
|
||||
t.Errorf("wrong values: %+v %+v", resp.Results[0].(*pilosa.Row).Columns(), resp.Results[0].(*pilosa.Row))
|
||||
}
|
||||
},
|
||||
csvVerifier: func(t *testing.T, resp string) {
|
||||
exp := "1\n2\n3\n4\n5\n6\n7\n"
|
||||
if resp != exp {
|
||||
t.Errorf("expected '%s', got '%s'", exp, resp)
|
||||
}
|
||||
},
|
||||
},
|
||||
{
|
||||
query: "Count(Distinct(field=likes))",
|
||||
verifier: func(t *testing.T, resp pilosa.QueryResponse) {
|
||||
qrVerifier: func(t *testing.T, resp pilosa.QueryResponse) {
|
||||
if resp.Results[0].(uint64) != 7 {
|
||||
t.Errorf("wrong count: %+v", resp.Results[0])
|
||||
}
|
||||
},
|
||||
csvVerifier: func(t *testing.T, resp string) {
|
||||
exp := "7\n"
|
||||
if resp != exp {
|
||||
t.Errorf("expected '%s', got '%s'", exp, resp)
|
||||
}
|
||||
},
|
||||
},
|
||||
{
|
||||
query: "Distinct(field=affinity)",
|
||||
verifier: func(t *testing.T, resp pilosa.QueryResponse) {
|
||||
qrVerifier: func(t *testing.T, resp pilosa.QueryResponse) {
|
||||
if !reflect.DeepEqual(resp.Results[0].(pilosa.SignedRow).Pos.Columns(), []uint64{0, 5, 10}) {
|
||||
t.Errorf("wrong positive records: %+v", resp.Results[0].(pilosa.SignedRow).Pos.Columns())
|
||||
}
|
||||
|
|
@ -6842,10 +6867,16 @@ func TestVariousQueries(t *testing.T) {
|
|||
t.Errorf("wrong negative records: %+v", resp.Results[0].(pilosa.SignedRow).Neg.Columns())
|
||||
}
|
||||
},
|
||||
csvVerifier: func(t *testing.T, resp string) {
|
||||
exp := "-10\n-5\n0\n5\n10\n"
|
||||
if resp != exp {
|
||||
t.Errorf("expected '%s', got '%s'", exp, resp)
|
||||
}
|
||||
},
|
||||
},
|
||||
{
|
||||
query: "Distinct(Row(affinity>=0),field=affinity)",
|
||||
verifier: func(t *testing.T, resp pilosa.QueryResponse) {
|
||||
qrVerifier: func(t *testing.T, resp pilosa.QueryResponse) {
|
||||
if !reflect.DeepEqual(resp.Results[0].(pilosa.SignedRow).Pos.Columns(), []uint64{0, 5, 10}) {
|
||||
t.Errorf("wrong positive records: %+v", resp.Results[0].(pilosa.SignedRow).Pos.Columns())
|
||||
}
|
||||
|
|
@ -6853,14 +6884,26 @@ func TestVariousQueries(t *testing.T) {
|
|||
t.Errorf("wrong negative records: %+v", resp.Results[0].(pilosa.SignedRow).Neg.Columns())
|
||||
}
|
||||
},
|
||||
csvVerifier: func(t *testing.T, resp string) {
|
||||
exp := "0\n5\n10\n"
|
||||
if resp != exp {
|
||||
t.Errorf("expected '%s', got '%s'", exp, resp)
|
||||
}
|
||||
},
|
||||
},
|
||||
{
|
||||
query: "Count(Distinct(Row(affinity>=0),field=affinity))",
|
||||
verifier: func(t *testing.T, resp pilosa.QueryResponse) {
|
||||
qrVerifier: func(t *testing.T, resp pilosa.QueryResponse) {
|
||||
if resp.Results[0].(uint64) != 3 {
|
||||
t.Errorf("wrong number of values: %+v", resp.Results[0])
|
||||
}
|
||||
},
|
||||
csvVerifier: func(t *testing.T, resp string) {
|
||||
exp := "3\n"
|
||||
if resp != exp {
|
||||
t.Errorf("expected '%s', got '%s'", exp, resp)
|
||||
}
|
||||
},
|
||||
},
|
||||
|
||||
// Handling this case properly will require changing the way
|
||||
|
|
@ -6879,58 +6922,103 @@ func TestVariousQueries(t *testing.T) {
|
|||
// },
|
||||
{
|
||||
query: "Distinct(Row(affinity<0),field=likes)",
|
||||
verifier: func(t *testing.T, resp pilosa.QueryResponse) {
|
||||
qrVerifier: func(t *testing.T, resp pilosa.QueryResponse) {
|
||||
if !reflect.DeepEqual(resp.Results[0].(*pilosa.Row).Keys, []string{"pilosa", "zebra", "icecream"}) {
|
||||
t.Errorf("wrong values: %+v", resp.Results[0])
|
||||
}
|
||||
},
|
||||
csvVerifier: func(t *testing.T, resp string) {
|
||||
exp := "pilosa\nzebra\nicecream\n"
|
||||
if resp != exp {
|
||||
t.Errorf("expected '%s', got '%s'", exp, resp)
|
||||
}
|
||||
},
|
||||
},
|
||||
{
|
||||
query: "Distinct(Row(affinity>0),field=likes)",
|
||||
verifier: func(t *testing.T, resp pilosa.QueryResponse) {
|
||||
qrVerifier: func(t *testing.T, resp pilosa.QueryResponse) {
|
||||
if !reflect.DeepEqual(resp.Results[0].(*pilosa.Row).Keys, []string{"molecula", "pangolin", "icecream"}) {
|
||||
t.Errorf("wrong values: %+v", resp.Results[0])
|
||||
}
|
||||
},
|
||||
csvVerifier: func(t *testing.T, resp string) {
|
||||
exp := "molecula\npangolin\nicecream\n"
|
||||
if resp != exp {
|
||||
t.Errorf("expected '%s', got '%s'", exp, resp)
|
||||
}
|
||||
},
|
||||
},
|
||||
{
|
||||
query: "Distinct(Row(likenums=1),field=likes)",
|
||||
verifier: func(t *testing.T, resp pilosa.QueryResponse) {
|
||||
qrVerifier: func(t *testing.T, resp pilosa.QueryResponse) {
|
||||
if !reflect.DeepEqual(resp.Results[0].(*pilosa.Row).Keys, []string{"molecula", "icecream"}) {
|
||||
t.Errorf("wrong values: %+v", resp.Results[0])
|
||||
}
|
||||
},
|
||||
csvVerifier: func(t *testing.T, resp string) {
|
||||
exp := "molecula\nicecream\n"
|
||||
if resp != exp {
|
||||
t.Errorf("expected '%s', got '%s'", exp, resp)
|
||||
}
|
||||
},
|
||||
},
|
||||
{
|
||||
query: "Distinct(field=likes)",
|
||||
verifier: func(t *testing.T, resp pilosa.QueryResponse) {
|
||||
qrVerifier: func(t *testing.T, resp pilosa.QueryResponse) {
|
||||
if !reflect.DeepEqual(resp.Results[0].(*pilosa.Row).Keys, []string{"molecula", "pilosa", "pangolin", "zebra", "toucan", "dog", "icecream"}) {
|
||||
t.Errorf("wrong values: %+v", resp.Results[0])
|
||||
}
|
||||
},
|
||||
csvVerifier: func(t *testing.T, resp string) {
|
||||
exp := "molecula\npilosa\npangolin\nzebra\ntoucan\ndog\nicecream\n"
|
||||
if resp != exp {
|
||||
t.Errorf("expected '%s', got '%s'", exp, resp)
|
||||
}
|
||||
},
|
||||
},
|
||||
{
|
||||
query: "Distinct(All(),field=likes)",
|
||||
verifier: func(t *testing.T, resp pilosa.QueryResponse) {
|
||||
qrVerifier: func(t *testing.T, resp pilosa.QueryResponse) {
|
||||
if !reflect.DeepEqual(resp.Results[0].(*pilosa.Row).Keys, []string{"molecula", "pilosa", "pangolin", "zebra", "toucan", "dog", "icecream"}) {
|
||||
t.Errorf("wrong values: %+v", resp.Results[0])
|
||||
}
|
||||
},
|
||||
csvVerifier: func(t *testing.T, resp string) {
|
||||
exp := "molecula\npilosa\npangolin\nzebra\ntoucan\ndog\nicecream\n"
|
||||
if resp != exp {
|
||||
t.Errorf("expected '%s', got '%s'", exp, resp)
|
||||
}
|
||||
},
|
||||
},
|
||||
{
|
||||
query: "Distinct(field=likes )",
|
||||
verifier: func(t *testing.T, resp pilosa.QueryResponse) {
|
||||
qrVerifier: func(t *testing.T, resp pilosa.QueryResponse) {
|
||||
if !reflect.DeepEqual(resp.Results[0].(*pilosa.Row).Keys, []string{"molecula", "pilosa", "pangolin", "zebra", "toucan", "dog", "icecream"}) {
|
||||
t.Errorf("wrong values: %+v", resp.Results[0])
|
||||
}
|
||||
},
|
||||
csvVerifier: func(t *testing.T, resp string) {
|
||||
exp := "molecula\npilosa\npangolin\nzebra\ntoucan\ndog\nicecream\n"
|
||||
if resp != exp {
|
||||
t.Errorf("expected '%s', got '%s'", exp, resp)
|
||||
}
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
for i, tst := range tests {
|
||||
t.Run(fmt.Sprintf("%d-%s", i, tst.query), func(t *testing.T) {
|
||||
resp := c.Query(t, "users", tst.query)
|
||||
tst.verifier(t, resp)
|
||||
tr := c.QueryGRPC(t, "users", tst.query)
|
||||
if tst.qrVerifier != nil {
|
||||
tst.qrVerifier(t, resp)
|
||||
}
|
||||
csvString := tr.ToCSVString()
|
||||
// verify everything after header
|
||||
tst.csvVerifier(t, csvString[strings.Index(csvString, "\n")+1:])
|
||||
|
||||
// TODO: add HTTP and Postgres and ability to convert
|
||||
// those results to CSV to run through CSV verifier
|
||||
})
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -15,6 +15,8 @@
|
|||
package proto
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"encoding/csv"
|
||||
"fmt"
|
||||
"io"
|
||||
"strings"
|
||||
|
|
@ -325,3 +327,51 @@ func (c ConstRowser) ToRows(fn func(*RowResponse) error) error {
|
|||
|
||||
return nil
|
||||
}
|
||||
|
||||
func (m *TableResponse) ToCSV(w io.Writer) error {
|
||||
writer := csv.NewWriter(w)
|
||||
record := make([]string, len(m.Headers))
|
||||
for i, h := range m.Headers {
|
||||
record[i] = h.Name
|
||||
}
|
||||
err := writer.Write(record)
|
||||
if err != nil {
|
||||
return errors.Wrap(err, "writing header")
|
||||
}
|
||||
for i, row := range m.Rows {
|
||||
record = record[:0]
|
||||
for colIndex, col := range row.Columns {
|
||||
switch m.Headers[colIndex].Datatype {
|
||||
case "[]string":
|
||||
record = append(record, fmt.Sprintf("%v", col.GetStringArrayVal()))
|
||||
case "[]uint64":
|
||||
record = append(record, fmt.Sprintf("%v", col.GetUint64ArrayVal()))
|
||||
case "string":
|
||||
record = append(record, fmt.Sprintf("%v", col.GetStringVal()))
|
||||
case "uint64":
|
||||
record = append(record, fmt.Sprintf("%v", col.GetUint64Val()))
|
||||
case "decimal":
|
||||
record = append(record, fmt.Sprintf("%v", col.GetDecimalVal().String()))
|
||||
case "bool":
|
||||
record = append(record, fmt.Sprintf("%v", col.GetBoolVal()))
|
||||
case "int64":
|
||||
record = append(record, fmt.Sprintf("%v", col.GetInt64Val()))
|
||||
}
|
||||
}
|
||||
err := writer.Write(record)
|
||||
if err != nil {
|
||||
return errors.Wrapf(err, "writing row %d", i)
|
||||
}
|
||||
}
|
||||
writer.Flush()
|
||||
return nil
|
||||
}
|
||||
|
||||
func (m *TableResponse) ToCSVString() string {
|
||||
buf := &bytes.Buffer{}
|
||||
err := m.ToCSV(buf)
|
||||
if err != nil {
|
||||
panic(fmt.Sprintf("shouldn't get an error writing to bytes.Buffer, got: %v", err))
|
||||
}
|
||||
return buf.String()
|
||||
}
|
||||
|
|
|
|||
|
|
@ -499,6 +499,10 @@ func (s *Server) InternalClient() InternalClient {
|
|||
return s.defaultClient
|
||||
}
|
||||
|
||||
func (s *Server) GRPCURI() URI {
|
||||
return s.grpcURI
|
||||
}
|
||||
|
||||
// UpAndDown brings the server up minimally and shuts it down
|
||||
// again; basically, it exists for testing holder open and close.
|
||||
func (s *Server) UpAndDown() error {
|
||||
|
|
|
|||
|
|
@ -26,6 +26,8 @@ import (
|
|||
"time"
|
||||
|
||||
"github.com/pilosa/pilosa/v2"
|
||||
"github.com/pilosa/pilosa/v2/api/client"
|
||||
"github.com/pilosa/pilosa/v2/proto"
|
||||
"github.com/pilosa/pilosa/v2/server"
|
||||
"github.com/pkg/errors"
|
||||
)
|
||||
|
|
@ -53,6 +55,33 @@ func (c *Cluster) Query(t testing.TB, index, query string) pilosa.QueryResponse
|
|||
return c.Nodes[0].QueryAPI(t, &pilosa.QueryRequest{Index: index, Query: query})
|
||||
}
|
||||
|
||||
func (c *Cluster) QueryHTTP(t testing.TB, index, query string) (string, error) {
|
||||
t.Helper()
|
||||
if len(c.Nodes) == 0 {
|
||||
t.Fatal("must have at least one node in cluster to QueryHTTP")
|
||||
}
|
||||
return c.Nodes[0].Query(t, index, "", query)
|
||||
}
|
||||
|
||||
func (c *Cluster) QueryGRPC(t testing.TB, index, query string) *proto.TableResponse {
|
||||
t.Helper()
|
||||
if len(c.Nodes) == 0 {
|
||||
t.Fatal("must have at least one node in cluster to QueryGRPC")
|
||||
}
|
||||
|
||||
grpcClient, err := client.NewGRPCClient([]string{fmt.Sprintf("%s:%d", c.Nodes[0].Server.GRPCURI().Host, c.Nodes[0].Server.GRPCURI().Port)}, nil)
|
||||
if err != nil {
|
||||
t.Fatalf("getting GRPC client: %v", err)
|
||||
}
|
||||
|
||||
tableResp, err := grpcClient.QueryUnary(context.Background(), index, query)
|
||||
if err != nil {
|
||||
t.Fatalf("querying unary: %v", err)
|
||||
}
|
||||
|
||||
return tableResp
|
||||
}
|
||||
|
||||
func (c *Cluster) GetNode(n int) *Command {
|
||||
return c.Nodes[n]
|
||||
}
|
||||
|
|
|
|||
|
|
@ -196,7 +196,7 @@ func (m *Command) Client() *http.InternalClient {
|
|||
}
|
||||
|
||||
// Query executes a query against the program through the HTTP API.
|
||||
func (m *Command) Query(t *testing.T, index, rawQuery, query string) (string, error) {
|
||||
func (m *Command) Query(t testing.TB, index, rawQuery, query string) (string, error) {
|
||||
resp := Do(t, "POST", fmt.Sprintf("%s/index/%s/query?%s", m.URL(), index, rawQuery), query)
|
||||
if resp.StatusCode != gohttp.StatusOK {
|
||||
return "", fmt.Errorf("invalid status: %d, body=%s", resp.StatusCode, resp.Body)
|
||||
|
|
@ -285,7 +285,7 @@ func (m *Command) RecalculateCaches(t *testing.T) error {
|
|||
}
|
||||
|
||||
// Do executes http.Do() with an http.NewRequest().
|
||||
func Do(t *testing.T, method, urlStr string, body string) *httpResponse {
|
||||
func Do(t testing.TB, method, urlStr string, body string) *httpResponse {
|
||||
t.Helper()
|
||||
req, err := gohttp.NewRequest(
|
||||
method,
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue