From c45b07e3de2284d1622d1c389f7ce401cf89a0f9 Mon Sep 17 00:00:00 2001 From: AndrewSisley Date: Wed, 20 Nov 2024 04:48:44 -0500 Subject: [PATCH] feat: Add support for cid-only time travel queries (#3256) ## Relevant issue(s) Resolves #3214 ## Description Adds support for cid-only time travel queries. Also removes some dead code. --- internal/db/fetcher/versioned.go | 21 +------ internal/planner/select.go | 13 +++-- .../request/graphql/schema/descriptions.go | 2 +- .../integration/query/simple/with_cid_test.go | 58 +++++++++++++------ 4 files changed, 54 insertions(+), 40 deletions(-) diff --git a/internal/db/fetcher/versioned.go b/internal/db/fetcher/versioned.go index 24f3ab8467..199ca38d21 100644 --- a/internal/db/fetcher/versioned.go +++ b/internal/db/fetcher/versioned.go @@ -153,33 +153,18 @@ func (vf *VersionedFetcher) Init( // Start serializes the correct state according to the Key and CID. func (vf *VersionedFetcher) Start(ctx context.Context, spans ...core.Span) error { - if vf.col == nil { - return client.NewErrUninitializeProperty("VersionedFetcher", "CollectionDescription") - } - - if len(spans) != 1 { - return ErrSingleSpanOnly - } - // VersionedFetcher only ever recieves a headstore key //nolint:forcetypeassert prefix := spans[0].Start.(keys.HeadstoreDocKey) - dk := prefix.DocID - cid := prefix.Cid - if dk == "" { - return client.NewErrUninitializeProperty("Spans", "DocID") - } else if !cid.Defined() { - return client.NewErrUninitializeProperty("Spans", "CID") - } vf.ctx = ctx vf.dsKey = keys.DataStoreKey{ CollectionRootID: vf.col.Description().RootID, - DocID: dk, + DocID: prefix.DocID, } - if err := vf.seekTo(cid); err != nil { - return NewErrFailedToSeek(cid, err) + if err := vf.seekTo(prefix.Cid); err != nil { + return NewErrFailedToSeek(prefix.Cid, err) } return vf.DocumentFetcher.Start(ctx) diff --git a/internal/planner/select.go b/internal/planner/select.go index f1b3d05867..56245666cf 100644 --- a/internal/planner/select.go +++ b/internal/planner/select.go @@ -255,19 +255,24 @@ func (n *selectNode) initSource() ([]aggregateNode, error) { origScan.filter = n.filter n.filter = nil - // If we have both a DocID and a CID, then we need to run - // a TimeTravel (History-Traversing Versioned) query, which means - // we need to propagate the values to the underlying VersionedFetcher + // If we have a CID, then we need to run a TimeTravel (History-Traversing Versioned) + // query, which means we need to propagate the values to the underlying VersionedFetcher if n.selectReq.Cid.HasValue() { c, err := cid.Decode(n.selectReq.Cid.Value()) if err != nil { return nil, err } + + var docID string + if len(n.selectReq.DocIDs.Value()) > 0 { + docID = n.selectReq.DocIDs.Value()[0] + } + origScan.Spans( []core.Span{ core.NewSpan( keys.HeadstoreDocKey{ - DocID: n.selectReq.DocIDs.Value()[0], + DocID: docID, Cid: c, }, keys.HeadstoreDocKey{}, diff --git a/internal/request/graphql/schema/descriptions.go b/internal/request/graphql/schema/descriptions.go index b667410c2c..07a6873d61 100644 --- a/internal/request/graphql/schema/descriptions.go +++ b/internal/request/graphql/schema/descriptions.go @@ -73,7 +73,7 @@ An optional set of docIDs for this field. Only documents with a docID be ignored. ` cidArgDescription string = ` -An optional value that specifies the commit ID of the document to return. +An optional value that specifies the commit ID of a document to return. This CID does not need to be the most recent for a document, if it corresponds to an older version of a document the document will be returned at the state it was in at the time of that commit. If a matching commit is diff --git a/tests/integration/query/simple/with_cid_test.go b/tests/integration/query/simple/with_cid_test.go index e4c07987e0..4bf6d5e224 100644 --- a/tests/integration/query/simple/with_cid_test.go +++ b/tests/integration/query/simple/with_cid_test.go @@ -13,10 +13,6 @@ package simple import ( "testing" - "github.com/sourcenetwork/immutable" - "github.com/stretchr/testify/require" - - "github.com/sourcenetwork/defradb/tests/change_detector" testUtils "github.com/sourcenetwork/defradb/tests/integration" ) @@ -44,20 +40,45 @@ func TestQuerySimpleWithInvalidCid(t *testing.T) { executeTestCase(t, test) } -// This test documents a bug: -// https://github.com/sourcenetwork/defradb/issues/3214 func TestQuerySimpleWithCid(t *testing.T) { - if change_detector.Enabled { - t.Skipf("Change detector does not support requiring panics") + test := testUtils.TestCase{ + Actions: []any{ + testUtils.SchemaUpdate{ + Schema: ` + type Users { + name: String + } + `, + }, + testUtils.CreateDoc{ + Doc: `{ + "name": "John" + }`, + }, + testUtils.Request{ + Request: `query { + Users ( + cid: "bafyreib7afkd5hepl45wdtwwpai433bhnbd3ps5m2rv3masctda7b6mmxe" + ) { + name + } + }`, + Results: map[string]any{ + "Users": []map[string]any{ + { + "name": "John", + }, + }, + }, + }, + }, } + testUtils.ExecuteTestCase(t, test) +} + +func TestQuerySimpleWithCid_MultipleDocs(t *testing.T) { test := testUtils.TestCase{ - SupportedClientTypes: immutable.Some( - []testUtils.ClientType{ - // The CLI/Http clients don't panic in this context - testUtils.GoClientType, - }, - ), Actions: []any{ testUtils.SchemaUpdate{ Schema: ` @@ -71,6 +92,11 @@ func TestQuerySimpleWithCid(t *testing.T) { "name": "John" }`, }, + testUtils.CreateDoc{ + Doc: `{ + "name": "Fred" + }`, + }, testUtils.Request{ Request: `query { Users ( @@ -90,7 +116,5 @@ func TestQuerySimpleWithCid(t *testing.T) { }, } - require.Panics(t, func() { - testUtils.ExecuteTestCase(t, test) - }) + testUtils.ExecuteTestCase(t, test) }