From a293f864f27ce2bae27656877399e077da1dbc10 Mon Sep 17 00:00:00 2001 From: Matt Jaffee Date: Wed, 14 Oct 2020 15:58:21 -0500 Subject: [PATCH 1/2] add test of single = int query over multiple shards --- executor_test.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/executor_test.go b/executor_test.go index e4267d5a1..df0c6a5de 100644 --- a/executor_test.go +++ b/executor_test.go @@ -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) + 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) { From 937bc271deba0046b08bab7ac9207dc1bc359d4d Mon Sep 17 00:00:00 2001 From: Matt Jaffee Date: Wed, 14 Oct 2020 16:10:20 -0500 Subject: [PATCH 2/2] fix data race on call map in single = logic This moves the code which modifies the PQL call object if a Row query on an int field uses a single = instead of ==. Instead of processing this at the shard level, we'll process it during the initial translation step so that it isn't operated on concurrently. --- executor.go | 37 ++++++++++++++++++------------------- 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/executor.go b/executor.go index d1fc96ac0..00de0b484 100644 --- a/executor.go +++ b/executor.go @@ -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 }