From 28c2ccf0747079de55d57d3ee5b677c3b20e0515 Mon Sep 17 00:00:00 2001 From: Seebs Date: Wed, 30 Mar 2022 14:04:55 -0500 Subject: [PATCH] allow ingest API delete to work at all, add test for it The ingest API tried to do a Qcx operation that needs a write Tx after requesting a write Tx. This doesn't work. The ingest API is the only caller of clearExistenceColumns, so it's easy to just make it work for a given shard using a provided Tx. This isn't especially clean, but a lot of this is due for an overhaul anyway because the Qcx/Tx stuff is sort of broken. Also, add any test case at all for this, since we didn't have one, and also fix the fact that the test case failed because the test harness didn't allow empty result sets. --- api.go | 23 +++++++++++------------ ingest_test.go | 3 +++ ingest_testdata/sample.tc | 10 ++++++++++ 3 files changed, 24 insertions(+), 12 deletions(-) diff --git a/api.go b/api.go index 34155ee32..938b16ed2 100644 --- a/api.go +++ b/api.go @@ -1919,7 +1919,7 @@ func (api *API) applyOperations(ctx context.Context, qcx *Qcx, index *Index, sha // We delete from the existence field unconditionally and other fields // if we know they exist. if op.OpType == ingest.OpDelete { - err = clearExistenceColumns(qcx, index, op.ClearRecordIDs, shard) + err = clearExistenceColumns(tx, index, op.ClearRecordIDs, shard) if err != nil { return fmt.Errorf("clearing existence columns: %w", err) } @@ -1997,21 +1997,20 @@ func importExistenceColumns(qcx *Qcx, index *Index, columnIDs []uint64, shard ui return ef.Import(qcx, existenceRowIDs, columnCopy, nil, shard, &options) } -func clearExistenceColumns(qcx *Qcx, index *Index, columnIDs []uint64, shard uint64) error { +func clearExistenceColumns(tx Tx, index *Index, columnIDs []uint64, shard uint64) error { ef := index.existenceField() if ef == nil { return nil } - - existenceRowIDs := make([]uint64, len(columnIDs)) - // If we don't gratuitously hand-duplicate things in field.Import, - // the fact that fragment.bulkImport rewrites its row and column - // lists can burn us if we don't make a copy before doing the - // existence field write. - columnCopy := make([]uint64, len(columnIDs)) - copy(columnCopy, columnIDs) - options := ImportOptions{Clear: true} - return ef.Import(qcx, existenceRowIDs, columnCopy, nil, shard, &options) + v := ef.view("standard") + if v == nil { + return nil + } + f := v.Fragment(shard) + if f == nil { + return nil + } + return f.ClearRecords(tx, columnIDs) } // ShardDistribution returns an object representing the distribution of shards diff --git a/ingest_test.go b/ingest_test.go index 712b834e6..7339f75e9 100644 --- a/ingest_test.go +++ b/ingest_test.go @@ -146,6 +146,9 @@ func parseExpectedResults(data []byte) (ints []uint64, keys []string, err error) return nil, nil, errors.New("expecting [] results") } words := bytes.Split(data[1:len(data)-1], []byte{','}) + if len(words) == 1 && len(words[0]) == 0 { + return nil, nil, nil + } for _, word := range words { word = bytes.TrimSpace(word) if len(word) == 0 { diff --git a/ingest_testdata/sample.tc b/ingest_testdata/sample.tc index 6ddff22a4..764965025 100644 --- a/ingest_testdata/sample.tc +++ b/ingest_testdata/sample.tc @@ -87,3 +87,13 @@ ingest-error: } } ] +ingest: +[ + { + "action": "delete", + "record_ids": [ 1 ] + } +] +queries: +Row(setkey="a") +[]