fix bugs where row index and field weren't always being propagated

I used a "paranoia" check to find these, but then realized the check
had a ton of false positives and doing it properly wasn't going to be
straightforward. I'm leaving the paranoia stuff in unless there are
objections, because I've wanted it before and not had it.

I also removed a log line that is very verbose and I don't think helps
anyone.
This commit is contained in:
Matt Jaffee 2020-12-23 18:53:17 -06:00
parent 427e9cb538
commit 1372bafe02
No known key found for this signature in database
GPG key ID: 08A3DFFF987B11BF
7 changed files with 90 additions and 8 deletions

View file

@ -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

View file

@ -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++

View file

@ -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])
}
},

19
nop_paranoia.go Normal file
View file

@ -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

19
paranoia.go Normal file
View file

@ -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

7
row.go
View file

@ -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)
}

View file

@ -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