From 07b8bd2f5dc738e0293305dfc459c13632d5ea65 Mon Sep 17 00:00:00 2001 From: Mayur Kale Date: Fri, 7 Jun 2024 04:48:27 +0000 Subject: [PATCH] feat(spanner): add OrderBy feature (#10289) * feat(spanner): add OrderBy feature * fix comment --------- Co-authored-by: Sri Harsha CH <57220027+harshachinta@users.noreply.github.com> --- spanner/client_test.go | 23 ++++++++++++++++------- spanner/transaction.go | 12 ++++++++++++ 2 files changed, 28 insertions(+), 7 deletions(-) diff --git a/spanner/client_test.go b/spanner/client_test.go index 4dc859e88e5e..83459cafffbb 100644 --- a/spanner/client_test.go +++ b/spanner/client_test.go @@ -874,6 +874,9 @@ func checkReqsForReadOptions(t *testing.T, server InMemSpannerServer, ro ReadOpt if got, want := sqlReq.DirectedReadOptions, ro.DirectedReadOptions; got.String() != want.String() { t.Fatalf("Directed Read Options mismatch, got %v, want %v", got, want) } + if got, want := sqlReq.OrderBy, ro.OrderBy; got != want { + t.Fatalf("OrderBy mismatch, got %v, want %v", got, want) + } } func checkReqsForTransactionOptions(t *testing.T, server InMemSpannerServer, txo TransactionOptions) { @@ -4436,20 +4439,26 @@ func readOptionsTestCases() []ReadOptionsTestCase { return []ReadOptionsTestCase{ { name: "Client level", - client: &ReadOptions{Index: "testIndex", Limit: 100, Priority: sppb.RequestOptions_PRIORITY_LOW, RequestTag: "testRequestTag"}, - want: &ReadOptions{Index: "testIndex", Limit: 100, Priority: sppb.RequestOptions_PRIORITY_LOW, RequestTag: "testRequestTag"}, + client: &ReadOptions{Index: "testIndex", Limit: 100, Priority: sppb.RequestOptions_PRIORITY_LOW, RequestTag: "testRequestTag", OrderBy: sppb.ReadRequest_ORDER_BY_NO_ORDER}, + want: &ReadOptions{Index: "testIndex", Limit: 100, Priority: sppb.RequestOptions_PRIORITY_LOW, RequestTag: "testRequestTag", OrderBy: sppb.ReadRequest_ORDER_BY_NO_ORDER}, + }, + { + name: "Client level has precendence when ORDER_BY_UNSPECIFIED at read level", + client: &ReadOptions{Index: "testIndex", Limit: 100, Priority: sppb.RequestOptions_PRIORITY_LOW, RequestTag: "testRequestTag", OrderBy: sppb.ReadRequest_ORDER_BY_NO_ORDER}, + read: &ReadOptions{Index: "testIndex", Limit: 100, Priority: sppb.RequestOptions_PRIORITY_LOW, RequestTag: "testRequestTag"}, + want: &ReadOptions{Index: "testIndex", Limit: 100, Priority: sppb.RequestOptions_PRIORITY_LOW, RequestTag: "testRequestTag", OrderBy: sppb.ReadRequest_ORDER_BY_NO_ORDER}, }, { name: "Read level", client: &ReadOptions{}, - read: &ReadOptions{Index: "testIndex", Limit: 100, Priority: sppb.RequestOptions_PRIORITY_LOW, RequestTag: "testRequestTag"}, - want: &ReadOptions{Index: "testIndex", Limit: 100, Priority: sppb.RequestOptions_PRIORITY_LOW, RequestTag: "testRequestTag"}, + read: &ReadOptions{Index: "testIndex", Limit: 100, Priority: sppb.RequestOptions_PRIORITY_LOW, RequestTag: "testRequestTag", OrderBy: sppb.ReadRequest_ORDER_BY_NO_ORDER}, + want: &ReadOptions{Index: "testIndex", Limit: 100, Priority: sppb.RequestOptions_PRIORITY_LOW, RequestTag: "testRequestTag", OrderBy: sppb.ReadRequest_ORDER_BY_NO_ORDER}, }, { name: "Read level has precedence than client level", - client: &ReadOptions{Index: "clientIndex", Limit: 10, Priority: sppb.RequestOptions_PRIORITY_LOW, RequestTag: "clientRequestTag"}, - read: &ReadOptions{Index: "readIndex", Limit: 20, Priority: sppb.RequestOptions_PRIORITY_MEDIUM, RequestTag: "readRequestTag"}, - want: &ReadOptions{Index: "readIndex", Limit: 20, Priority: sppb.RequestOptions_PRIORITY_MEDIUM, RequestTag: "readRequestTag"}, + client: &ReadOptions{Index: "clientIndex", Limit: 10, Priority: sppb.RequestOptions_PRIORITY_LOW, RequestTag: "clientRequestTag", OrderBy: sppb.ReadRequest_ORDER_BY_NO_ORDER}, + read: &ReadOptions{Index: "readIndex", Limit: 20, Priority: sppb.RequestOptions_PRIORITY_MEDIUM, RequestTag: "readRequestTag", OrderBy: sppb.ReadRequest_ORDER_BY_PRIMARY_KEY}, + want: &ReadOptions{Index: "readIndex", Limit: 20, Priority: sppb.RequestOptions_PRIORITY_MEDIUM, RequestTag: "readRequestTag", OrderBy: sppb.ReadRequest_ORDER_BY_PRIMARY_KEY}, }, } } diff --git a/spanner/transaction.go b/spanner/transaction.go index f54c87dba16d..9d52b12f2f3b 100644 --- a/spanner/transaction.go +++ b/spanner/transaction.go @@ -182,6 +182,9 @@ type ReadOptions struct { // ReadOptions option used to set the DirectedReadOptions for all ReadRequests which indicate // which replicas or regions should be used for running read operations. DirectedReadOptions *sppb.DirectedReadOptions + + // An option to control the order in which rows are returned from a read. + OrderBy sppb.ReadRequest_OrderBy } // merge combines two ReadOptions that the input parameter will have higher @@ -194,6 +197,7 @@ func (ro ReadOptions) merge(opts ReadOptions) ReadOptions { RequestTag: ro.RequestTag, DataBoostEnabled: ro.DataBoostEnabled, DirectedReadOptions: ro.DirectedReadOptions, + OrderBy: ro.OrderBy, } if opts.Index != "" { merged.Index = opts.Index @@ -213,6 +217,9 @@ func (ro ReadOptions) merge(opts ReadOptions) ReadOptions { if opts.DirectedReadOptions != nil { merged.DirectedReadOptions = opts.DirectedReadOptions } + if opts.OrderBy != sppb.ReadRequest_ORDER_BY_UNSPECIFIED { + merged.OrderBy = opts.OrderBy + } return merged } @@ -245,6 +252,7 @@ func (t *txReadOnly) ReadWithOptions(ctx context.Context, table string, keys Key requestTag := t.ro.RequestTag dataBoostEnabled := t.ro.DataBoostEnabled directedReadOptions := t.ro.DirectedReadOptions + orderBy := t.ro.OrderBy if opts != nil { index = opts.Index if opts.Limit > 0 { @@ -258,6 +266,9 @@ func (t *txReadOnly) ReadWithOptions(ctx context.Context, table string, keys Key if opts.DirectedReadOptions != nil { directedReadOptions = opts.DirectedReadOptions } + if opts.OrderBy != sppb.ReadRequest_ORDER_BY_UNSPECIFIED { + orderBy = opts.OrderBy + } } var setTransactionID func(transactionID) if _, ok := ts.Selector.(*sppb.TransactionSelector_Begin); ok { @@ -285,6 +296,7 @@ func (t *txReadOnly) ReadWithOptions(ctx context.Context, table string, keys Key RequestOptions: createRequestOptions(prio, requestTag, t.txOpts.TransactionTag), DataBoostEnabled: dataBoostEnabled, DirectedReadOptions: directedReadOptions, + OrderBy: orderBy, }) if err != nil { if _, ok := t.getTransactionSelector().GetSelector().(*sppb.TransactionSelector_Begin); ok {