diff --git a/Makefile b/Makefile index abd07304d..d622a235c 100644 --- a/Makefile +++ b/Makefile @@ -17,7 +17,7 @@ RELEASE ?= 0 RELEASE_ENABLED = $(subst 0,,$(RELEASE)) BUILD_TAGS += $(if $(RELEASE_ENABLED),release) BUILD_TAGS += shardwidth$(SHARD_WIDTH) -TEST_TAGS = roaringparanoia +TEST_TAGS = roaringparanoia paranoia define LICENSE_HASH_CODE head -13 $1 | sed -e 's/Copyright 20[0-9][0-9]/Copyright 20XX/g' | shasum | cut -f 1 -d " " endef diff --git a/executor.go b/executor.go index 39bfa30f8..6694deeb1 100644 --- a/executor.go +++ b/executor.go @@ -1421,7 +1421,10 @@ func (e *executor) executeDistinctShard(ctx context.Context, qcx *Qcx, index str } bsig := field.bsiGroup(fieldName) if bsig == nil { - result = new(Row) + result = &Row{ + Index: index, + Field: fieldName, + } } else { result = SignedRow{} } @@ -5188,7 +5191,10 @@ func makeEmbeddedDataForShards(allRows []*Row, shards []uint64) []*Row { } segments := row.segments segmentIndex := 0 - newRows[i] = &Row{} + newRows[i] = &Row{ + Index: row.Index, + Field: row.Field, + } for _, shard := range shards { for segmentIndex < len(segments) && segments[segmentIndex].shard < shard { segmentIndex++ diff --git a/executor_test.go b/executor_test.go index 76174d4b6..e065004ef 100644 --- a/executor_test.go +++ b/executor_test.go @@ -6844,7 +6844,7 @@ func TestVariousQueries(t *testing.T) { }, }, { - query: "Distinct(Row(affinity>=0), field=affinity)", + query: "Distinct(Row(affinity>=0),field=affinity)", verifier: 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()) @@ -6855,7 +6855,7 @@ func TestVariousQueries(t *testing.T) { }, }, { - query: "Count(Distinct(Row(affinity>=0), field=affinity))", + query: "Count(Distinct(Row(affinity>=0),field=affinity))", verifier: func(t *testing.T, resp pilosa.QueryResponse) { if resp.Results[0].(uint64) != 3 { t.Errorf("wrong number of values: %+v", resp.Results[0]) @@ -6877,6 +6877,30 @@ func TestVariousQueries(t *testing.T) { // } // }, // }, + { + query: "Distinct(Row(affinity<0),field=likes)", + verifier: 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]) + } + }, + }, + { + query: "Distinct(Row(affinity>0),field=likes)", + verifier: 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]) + } + }, + }, + { + query: "Distinct(Row(likenums=1),field=likes)", + verifier: 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]) + } + }, + }, { query: "Distinct(field=likes)", verifier: func(t *testing.T, resp pilosa.QueryResponse) { @@ -6886,9 +6910,17 @@ func TestVariousQueries(t *testing.T) { }, }, { - query: "Distinct(Row(affinity<0), field=likes)", + query: "Distinct(All(),field=likes)", verifier: func(t *testing.T, resp pilosa.QueryResponse) { - if !reflect.DeepEqual(resp.Results[0].(*pilosa.Row).Keys, []string{"pilosa", "zebra", "icecream"}) { + 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]) + } + }, + }, + { + query: "Distinct(field=likes )", + verifier: 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]) } }, diff --git a/nop_paranoia.go b/nop_paranoia.go new file mode 100644 index 000000000..77cdae411 --- /dev/null +++ b/nop_paranoia.go @@ -0,0 +1,19 @@ +// Copyright 2020 Pilosa Corp. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// +build !paranoia + +package pilosa + +const paranoia = false diff --git a/paranoia.go b/paranoia.go new file mode 100644 index 000000000..fb717db91 --- /dev/null +++ b/paranoia.go @@ -0,0 +1,19 @@ +// Copyright 2020 Pilosa Corp. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// +build paranoia + +package pilosa + +const paranoia = true diff --git a/row.go b/row.go index 5e2256841..98f5c9275 100644 --- a/row.go +++ b/row.go @@ -70,6 +70,8 @@ func (r *Row) Clone() (clone *Row) { clone = &Row{ Keys: keyClone, Attrs: attrClone, + Index: r.Index, + Field: r.Field, } for _, seg := range r.segments { @@ -299,6 +301,11 @@ func (r *Row) Union(others ...*Row) *Row { toProcess := make([]*rowSegment, 0, len(others)+1) var output []rowSegment for _, other := range others { + if paranoia { // nolint:staticcheck + // TODO I think there is a good check we can do here, but + // it's nontrivial to check whether two rows are + // compatible because of foreign indexes and such. + } if len(other.segments) > 0 { segments = append(segments, other.segments) } diff --git a/server.go b/server.go index 89d197fef..b664e4c87 100644 --- a/server.go +++ b/server.go @@ -657,7 +657,6 @@ func (s *Server) monitorResetTranslationSync() { case <-s.closing: return case <-s.resetTranslationSyncCh: - s.logger.Printf("holder translation sync beginning") s.wg.Add(1) go func() { // Obtaining this lock ensures that there is only