Merge pull request #980 from jaffee/equalsdatarace2.1

Fix data race in executor when processing int with single '='
This commit is contained in:
Matthew Jaffee 2020-10-15 09:12:52 -05:00 committed by GitHub
commit 451ced5e33
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 25 additions and 19 deletions

View file

@ -2503,24 +2503,6 @@ func (e *executor) executeRowShard(ctx context.Context, index string, c *pql.Cal
}
}
timeNotSet := fromTime.IsZero() && toTime.IsZero()
// This is workaround to support pql.ASSIGN ('=') as condition ('==') for int and decimal fields
if c.Name == "Row" && timeNotSet &&
(f.Type() == FieldTypeInt || f.Type() == FieldTypeDecimal) {
// re-write args as conditions for fieldName
for k, v := range c.Args {
if _, ok := v.(*pql.Condition); k == fieldName && !ok {
c.Args[k] = &pql.Condition{
Op: pql.EQ,
Value: v,
}
return e.executeRowBSIGroupShard(ctx, index, c, shard)
}
}
}
rowID, rowOK, rowErr := c.UintArg(fieldName)
if rowErr != nil {
return nil, fmt.Errorf("Row() error with arg for row: %v", rowErr)
@ -2529,6 +2511,7 @@ func (e *executor) executeRowShard(ctx context.Context, index string, c *pql.Cal
}
// Simply return row if times are not set.
timeNotSet := fromTime.IsZero() && toTime.IsZero()
if c.Name == "Row" && timeNotSet {
frag := e.Holder.fragment(index, fieldName, viewStandard, shard)
if frag == nil {
@ -4073,8 +4056,9 @@ func (e *executor) translateCall(ctx context.Context, indexName string, c *pql.C
}
// Translate row key, if field is specified & key exists.
var field *Field
if fieldName != "" {
field := idx.Field(fieldName)
field = idx.Field(fieldName)
if field == nil {
// Instead of returning ErrFieldNotFound here,
// we just return, and don't attempt the translation.
@ -4212,6 +4196,21 @@ func (e *executor) translateCall(ctx context.Context, indexName string, c *pql.C
}
}
// This is workaround to support pql.ASSIGN ('=') as condition ('==') for int and decimal fields
if c.Name == "Row" && field != nil &&
(field.Type() == FieldTypeInt || field.Type() == FieldTypeDecimal) {
// re-write args as conditions for fieldName
for k, v := range c.Args {
if _, ok := v.(*pql.Condition); k == fieldName && !ok {
c.Args[k] = &pql.Condition{
Op: pql.EQ,
Value: v,
}
break
}
}
}
return nil
}

View file

@ -2432,6 +2432,13 @@ func TestExecutor_Execute_Row_BSIGroup(t *testing.T) {
} else if got, exp := result.Results[0].(*pilosa.Row).Columns(), []uint64{50, (5 * ShardWidth) + 100}; !reflect.DeepEqual(exp, got) {
t.Fatalf("Query().Row.Columns=%#v, expected %#v", got, exp)
}
// EQ (single = form) <int>
if result, err := c[0].API.Query(context.Background(), &pilosa.QueryRequest{Index: "i", Query: `Row(foo = 20)`}); err != nil {
t.Fatal(err)
} else if got, exp := result.Results[0].(*pilosa.Row).Columns(), []uint64{50, (5 * ShardWidth) + 100}; !reflect.DeepEqual(exp, got) {
t.Fatalf("Query().Row.Columns=%#v, expected %#v", got, exp)
}
})
t.Run("NEQ", func(t *testing.T) {