From 21a478a7281108243d0894a253fcd637ff0d04f0 Mon Sep 17 00:00:00 2001 From: Seebs Date: Tue, 1 Mar 2022 09:40:37 -0600 Subject: [PATCH] don't look up a field by name to find out its name If a field doesn't exist, looking up that field produces a nil, and querying the name of a nil field fails. Don't do that. Instead, just use the name you're looking it up by. We could in theory return an error here, but we already handle nonexistent fields elsewhere and checking this when we already have checks for it seems unnecessary, I think? Also, we add a test for this. The test is over in server/grpc_test.go because we have infrastructure there for testing the SQL server functionality, and you can't actually write reasonable self-contained tests for the SQL stuff because it has no way to create a working server. --- server/grpc_test.go | 4 ++++ sql/select.go | 3 +-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/server/grpc_test.go b/server/grpc_test.go index e2ecae03f..782063859 100644 --- a/server/grpc_test.go +++ b/server/grpc_test.go @@ -1007,6 +1007,10 @@ func TestQuerySQLWithError(t *testing.T) { sql: "select _id, age, field_not_found from grouper", err: pilosa.ErrFieldNotFound, }, + { + sql: "select age, color, count(*) from grouper group by field_not_found, age, color", + err: pilosa.ErrFieldNotFound, + }, } for i, test := range tests { diff --git a/sql/select.go b/sql/select.go index d7afc2eec..2682a5349 100644 --- a/sql/select.go +++ b/sql/select.go @@ -598,8 +598,7 @@ func (h handlerSelectGroupBy) Apply(stmt *sqlparser.Select, qm QueryMask, indexF rowsQueries := []string{} for _, fieldName := range groupByFieldNames { - field := index.Field(fieldName) - rowsQueries = append(rowsQueries, Rows(field.Name())) + rowsQueries = append(rowsQueries, Rows(fieldName)) } var wherePQL string