From f8ed81e3ad38574fe1e17688c204709d17f70aa8 Mon Sep 17 00:00:00 2001 From: Georgy Moiseev Date: Thu, 5 Oct 2023 16:16:54 +0300 Subject: [PATCH 1/6] doc: make crud options description more specific Since `upsert_object` and `upsert_object_many` not support `skip_nullability_check_on_flatten` option [1], `UpsertObjectOpts` is `SimpleOperationOpts` and not `SimpleOperationObjectOpts`, same for many operation. But it is possible to get confused while studying options descriptions. 1. https://github.com/tarantool/crud/commit/7504da391c20ac1d2dfbbb7de6d3c32d73d75f60 --- crud/options.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/crud/options.go b/crud/options.go index c073b7222..f9cde7be8 100644 --- a/crud/options.go +++ b/crud/options.go @@ -138,6 +138,7 @@ func (opts BaseOpts) EncodeMsgpack(enc *msgpack.Encoder) error { } // SimpleOperationOpts describes options for simple CRUD operations. +// It also covers `upsert_object` options. type SimpleOperationOpts struct { // Timeout is a `vshard.call` timeout and vshard // master discovery timeout (in seconds). @@ -168,7 +169,7 @@ func (opts SimpleOperationOpts) EncodeMsgpack(enc *msgpack.Encoder) error { } // SimpleOperationObjectOpts describes options for simple CRUD -// operations with objects. +// operations with objects. It doesn't cover `upsert_object` options. type SimpleOperationObjectOpts struct { // Timeout is a `vshard.call` timeout and vshard // master discovery timeout (in seconds). @@ -203,6 +204,7 @@ func (opts SimpleOperationObjectOpts) EncodeMsgpack(enc *msgpack.Encoder) error } // OperationManyOpts describes options for CRUD operations with many tuples. +// It also covers `upsert_object_many` options. type OperationManyOpts struct { // Timeout is a `vshard.call` timeout and vshard // master discovery timeout (in seconds). @@ -239,7 +241,7 @@ func (opts OperationManyOpts) EncodeMsgpack(enc *msgpack.Encoder) error { } // OperationObjectManyOpts describes options for CRUD operations -// with many objects. +// with many objects. It doesn't cover `upsert_object_many` options. type OperationObjectManyOpts struct { // Timeout is a `vshard.call` timeout and vshard // master discovery timeout (in seconds). From 5fc84837af3233c2226be8135401470737cfe261 Mon Sep 17 00:00:00 2001 From: Georgy Moiseev Date: Thu, 5 Oct 2023 18:26:27 +0300 Subject: [PATCH 2/6] crud: fix Get options Before this patch, `vshard_router`, `fields`, `bucket_id`, `mode`, `prefer_replica`, `balance` were either ignored or had a wrong name. This patch fixes the issue. --- CHANGELOG.md | 2 ++ crud/get.go | 9 +++++---- crud/tarantool_test.go | 21 +++++++++++++++++++++ 3 files changed, 28 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 211ea25a2..020a37c70 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -55,6 +55,8 @@ Versioning](http://semver.org/spec/v2.0.0.html) except to the first release. - Incorrect decoding of an MP_DECIMAL when the `scale` value is negative (#314) - Incorrect options (`after`, `batch_size` and `force_map_call`) setup for crud.SelectRequest (#320) +- Incorrect options (`vshard_router`, `fields`, `bucket_id`, `mode`, + `prefer_replica`, `balance`) setup for crud.GetRequest (#335) ## [1.12.0] - 2023-06-07 diff --git a/crud/get.go b/crud/get.go index e1855f35c..a957219dc 100644 --- a/crud/get.go +++ b/crud/get.go @@ -42,10 +42,11 @@ func (opts GetOpts) EncodeMsgpack(enc *msgpack.Encoder) error { exists := [optsCnt]bool{} values[0], exists[0] = opts.Timeout.Get() values[1], exists[1] = opts.VshardRouter.Get() - values[1], exists[1] = opts.BucketId.Get() - values[2], exists[2] = opts.Mode.Get() - values[3], exists[3] = opts.PreferReplica.Get() - values[4], exists[4] = opts.Balance.Get() + values[2], exists[2] = opts.Fields.Get() + values[3], exists[3] = opts.BucketId.Get() + values[4], exists[4] = opts.Mode.Get() + values[5], exists[5] = opts.PreferReplica.Get() + values[6], exists[6] = opts.Balance.Get() return encodeOptions(enc, names[:], values[:], exists[:]) } diff --git a/crud/tarantool_test.go b/crud/tarantool_test.go index 5cf29f66a..aa32aae95 100644 --- a/crud/tarantool_test.go +++ b/crud/tarantool_test.go @@ -808,6 +808,27 @@ func TestStorageInfoResult(t *testing.T) { } } +func TestGetAdditionalOpts(t *testing.T) { + conn := connect(t) + defer conn.Close() + + req := crud.MakeGetRequest(spaceName).Key(key).Opts(crud.GetOpts{ + Timeout: crud.MakeOptUint(1), + Fields: crud.MakeOptTuple([]interface{}{"name"}), + Mode: crud.MakeOptString("read"), + PreferReplica: crud.MakeOptBool(true), + Balance: crud.MakeOptBool(true), + }) + resp := crud.Result{} + + testCrudRequestPrepareData(t, conn) + + err := conn.Do(req).GetTyped(&resp) + if err != nil { + t.Fatalf("Failed to Do CRUD request: %s", err) + } +} + // runTestMain is a body of TestMain function // (see https://pkg.go.dev/testing#hdr-Main). // Using defer + os.Exit is not works so TestMain body From c33f35102d143a8ac7a3991f227b8db513062a07 Mon Sep 17 00:00:00 2001 From: Georgy Moiseev Date: Fri, 6 Oct 2023 11:42:05 +0300 Subject: [PATCH 3/6] crud: support fetch_latest_metadata option `fetch_latest_metadata` option was introduced in crud 1.2.0 [1]. 1. https://github.com/tarantool/crud/commit/2675925d17240d7065a718be2e4ad685a4a21913 --- CHANGELOG.md | 1 + Makefile | 2 +- crud/get.go | 10 ++- crud/options.go | 54 +++++++++--- crud/select.go | 10 ++- crud/tarantool_test.go | 183 +++++++++++++++++++++++++++++++++++++++++ 6 files changed, 245 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 020a37c70..1fc71aac7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ Versioning](http://semver.org/spec/v2.0.0.html) except to the first release. - Meaningful description for read/write socket errors (#129) - Support password and password file to decrypt private SSL key file (#319) - Support `operation_data` in `crud.Error` (#330) +- Support `fetch_latest_metadata` option for crud requests with metadata (#335) ### Changed diff --git a/Makefile b/Makefile index 4f676a530..c435764a8 100644 --- a/Makefile +++ b/Makefile @@ -27,7 +27,7 @@ clean: .PHONY: deps deps: clean ( cd ./queue/testdata; $(TTCTL) rocks install queue 1.3.0 ) - ( cd ./crud/testdata; $(TTCTL) rocks install crud 1.1.1 ) + ( cd ./crud/testdata; $(TTCTL) rocks install crud 1.3.0 ) .PHONY: datetime-timezones datetime-timezones: diff --git a/crud/get.go b/crud/get.go index a957219dc..fcfef5426 100644 --- a/crud/get.go +++ b/crud/get.go @@ -29,15 +29,20 @@ type GetOpts struct { // Balance is a parameter to use replica according to vshard // load balancing policy. Balance OptBool + // FetchLatestMetadata guarantees the up-to-date metadata (space format) + // in first return value, otherwise it may not take into account + // the latest migration of the data format. Performance overhead is up to 15%. + // Disabled by default. + FetchLatestMetadata OptBool } // EncodeMsgpack provides custom msgpack encoder. func (opts GetOpts) EncodeMsgpack(enc *msgpack.Encoder) error { - const optsCnt = 7 + const optsCnt = 8 names := [optsCnt]string{timeoutOptName, vshardRouterOptName, fieldsOptName, bucketIdOptName, modeOptName, - preferReplicaOptName, balanceOptName} + preferReplicaOptName, balanceOptName, fetchLatestMetadataOptName} values := [optsCnt]interface{}{} exists := [optsCnt]bool{} values[0], exists[0] = opts.Timeout.Get() @@ -47,6 +52,7 @@ func (opts GetOpts) EncodeMsgpack(enc *msgpack.Encoder) error { values[4], exists[4] = opts.Mode.Get() values[5], exists[5] = opts.PreferReplica.Get() values[6], exists[6] = opts.Balance.Get() + values[7], exists[7] = opts.FetchLatestMetadata.Get() return encodeOptions(enc, names[:], values[:], exists[:]) } diff --git a/crud/options.go b/crud/options.go index f9cde7be8..8a6997380 100644 --- a/crud/options.go +++ b/crud/options.go @@ -21,6 +21,7 @@ const ( firstOptName = "first" afterOptName = "after" batchSizeOptName = "batch_size" + fetchLatestMetadataOptName = "fetch_latest_metadata" ) // OptUint is an optional uint. @@ -150,20 +151,26 @@ type SimpleOperationOpts struct { Fields OptTuple // BucketId is a bucket ID. BucketId OptUint + // FetchLatestMetadata guarantees the up-to-date metadata (space format) + // in first return value, otherwise it may not take into account + // the latest migration of the data format. Performance overhead is up to 15%. + // Disabled by default. + FetchLatestMetadata OptBool } // EncodeMsgpack provides custom msgpack encoder. func (opts SimpleOperationOpts) EncodeMsgpack(enc *msgpack.Encoder) error { - const optsCnt = 4 + const optsCnt = 5 names := [optsCnt]string{timeoutOptName, vshardRouterOptName, - fieldsOptName, bucketIdOptName} + fieldsOptName, bucketIdOptName, fetchLatestMetadataOptName} values := [optsCnt]interface{}{} exists := [optsCnt]bool{} values[0], exists[0] = opts.Timeout.Get() values[1], exists[1] = opts.VshardRouter.Get() values[2], exists[2] = opts.Fields.Get() values[3], exists[3] = opts.BucketId.Get() + values[4], exists[4] = opts.FetchLatestMetadata.Get() return encodeOptions(enc, names[:], values[:], exists[:]) } @@ -184,14 +191,20 @@ type SimpleOperationObjectOpts struct { // SkipNullabilityCheckOnFlatten is a parameter to allow // setting null values to non-nullable fields. SkipNullabilityCheckOnFlatten OptBool + // FetchLatestMetadata guarantees the up-to-date metadata (space format) + // in first return value, otherwise it may not take into account + // the latest migration of the data format. Performance overhead is up to 15%. + // Disabled by default. + FetchLatestMetadata OptBool } // EncodeMsgpack provides custom msgpack encoder. func (opts SimpleOperationObjectOpts) EncodeMsgpack(enc *msgpack.Encoder) error { - const optsCnt = 5 + const optsCnt = 6 names := [optsCnt]string{timeoutOptName, vshardRouterOptName, - fieldsOptName, bucketIdOptName, skipNullabilityCheckOnFlattenOptName} + fieldsOptName, bucketIdOptName, skipNullabilityCheckOnFlattenOptName, + fetchLatestMetadataOptName} values := [optsCnt]interface{}{} exists := [optsCnt]bool{} values[0], exists[0] = opts.Timeout.Get() @@ -199,6 +212,7 @@ func (opts SimpleOperationObjectOpts) EncodeMsgpack(enc *msgpack.Encoder) error values[2], exists[2] = opts.Fields.Get() values[3], exists[3] = opts.BucketId.Get() values[4], exists[4] = opts.SkipNullabilityCheckOnFlatten.Get() + values[5], exists[5] = opts.FetchLatestMetadata.Get() return encodeOptions(enc, names[:], values[:], exists[:]) } @@ -221,14 +235,20 @@ type OperationManyOpts struct { // RollbackOnError is a parameter because of what any failed operation // will lead to rollback on a storage, where the operation is failed. RollbackOnError OptBool + // FetchLatestMetadata guarantees the up-to-date metadata (space format) + // in first return value, otherwise it may not take into account + // the latest migration of the data format. Performance overhead is up to 15%. + // Disabled by default. + FetchLatestMetadata OptBool } // EncodeMsgpack provides custom msgpack encoder. func (opts OperationManyOpts) EncodeMsgpack(enc *msgpack.Encoder) error { - const optsCnt = 5 + const optsCnt = 6 names := [optsCnt]string{timeoutOptName, vshardRouterOptName, - fieldsOptName, stopOnErrorOptName, rollbackOnErrorOptName} + fieldsOptName, stopOnErrorOptName, rollbackOnErrorOptName, + fetchLatestMetadataOptName} values := [optsCnt]interface{}{} exists := [optsCnt]bool{} values[0], exists[0] = opts.Timeout.Get() @@ -236,6 +256,7 @@ func (opts OperationManyOpts) EncodeMsgpack(enc *msgpack.Encoder) error { values[2], exists[2] = opts.Fields.Get() values[3], exists[3] = opts.StopOnError.Get() values[4], exists[4] = opts.RollbackOnError.Get() + values[5], exists[5] = opts.FetchLatestMetadata.Get() return encodeOptions(enc, names[:], values[:], exists[:]) } @@ -261,15 +282,20 @@ type OperationObjectManyOpts struct { // SkipNullabilityCheckOnFlatten is a parameter to allow // setting null values to non-nullable fields. SkipNullabilityCheckOnFlatten OptBool + // FetchLatestMetadata guarantees the up-to-date metadata (space format) + // in first return value, otherwise it may not take into account + // the latest migration of the data format. Performance overhead is up to 15%. + // Disabled by default. + FetchLatestMetadata OptBool } // EncodeMsgpack provides custom msgpack encoder. func (opts OperationObjectManyOpts) EncodeMsgpack(enc *msgpack.Encoder) error { - const optsCnt = 6 + const optsCnt = 7 names := [optsCnt]string{timeoutOptName, vshardRouterOptName, fieldsOptName, stopOnErrorOptName, rollbackOnErrorOptName, - skipNullabilityCheckOnFlattenOptName} + skipNullabilityCheckOnFlattenOptName, fetchLatestMetadataOptName} values := [optsCnt]interface{}{} exists := [optsCnt]bool{} values[0], exists[0] = opts.Timeout.Get() @@ -278,6 +304,7 @@ func (opts OperationObjectManyOpts) EncodeMsgpack(enc *msgpack.Encoder) error { values[3], exists[3] = opts.StopOnError.Get() values[4], exists[4] = opts.RollbackOnError.Get() values[5], exists[5] = opts.SkipNullabilityCheckOnFlatten.Get() + values[6], exists[6] = opts.FetchLatestMetadata.Get() return encodeOptions(enc, names[:], values[:], exists[:]) } @@ -292,18 +319,25 @@ type BorderOpts struct { VshardRouter OptString // Fields is field names for getting only a subset of fields. Fields OptTuple + // FetchLatestMetadata guarantees the up-to-date metadata (space format) + // in first return value, otherwise it may not take into account + // the latest migration of the data format. Performance overhead is up to 15%. + // Disabled by default. + FetchLatestMetadata OptBool } // EncodeMsgpack provides custom msgpack encoder. func (opts BorderOpts) EncodeMsgpack(enc *msgpack.Encoder) error { - const optsCnt = 3 + const optsCnt = 4 - names := [optsCnt]string{timeoutOptName, vshardRouterOptName, fieldsOptName} + names := [optsCnt]string{timeoutOptName, vshardRouterOptName, fieldsOptName, + fetchLatestMetadataOptName} values := [optsCnt]interface{}{} exists := [optsCnt]bool{} values[0], exists[0] = opts.Timeout.Get() values[1], exists[1] = opts.VshardRouter.Get() values[2], exists[2] = opts.Fields.Get() + values[3], exists[3] = opts.FetchLatestMetadata.Get() return encodeOptions(enc, names[:], values[:], exists[:]) } diff --git a/crud/select.go b/crud/select.go index 073c9492b..0d0b5708a 100644 --- a/crud/select.go +++ b/crud/select.go @@ -41,17 +41,22 @@ type SelectOpts struct { // Fullscan describes if a critical log entry will be skipped on // potentially long select. Fullscan OptBool + // FetchLatestMetadata guarantees the up-to-date metadata (space format) + // in first return value, otherwise it may not take into account + // the latest migration of the data format. Performance overhead is up to 15%. + // Disabled by default. + FetchLatestMetadata OptBool } // EncodeMsgpack provides custom msgpack encoder. func (opts SelectOpts) EncodeMsgpack(enc *msgpack.Encoder) error { - const optsCnt = 12 + const optsCnt = 13 names := [optsCnt]string{timeoutOptName, vshardRouterOptName, fieldsOptName, bucketIdOptName, modeOptName, preferReplicaOptName, balanceOptName, firstOptName, afterOptName, batchSizeOptName, - forceMapCallOptName, fullscanOptName} + forceMapCallOptName, fullscanOptName, fetchLatestMetadataOptName} values := [optsCnt]interface{}{} exists := [optsCnt]bool{} values[0], exists[0] = opts.Timeout.Get() @@ -66,6 +71,7 @@ func (opts SelectOpts) EncodeMsgpack(enc *msgpack.Encoder) error { values[9], exists[9] = opts.BatchSize.Get() values[10], exists[10] = opts.ForceMapCall.Get() values[11], exists[11] = opts.Fullscan.Get() + values[12], exists[12] = opts.FetchLatestMetadata.Get() return encodeOptions(enc, names[:], values[:], exists[:]) } diff --git a/crud/tarantool_test.go b/crud/tarantool_test.go index aa32aae95..457cb2aa0 100644 --- a/crud/tarantool_test.go +++ b/crud/tarantool_test.go @@ -829,6 +829,189 @@ func TestGetAdditionalOpts(t *testing.T) { } } +var testMetadataCases = []struct { + name string + req tarantool.Request +}{ + { + "Insert", + crud.MakeInsertRequest(spaceName). + Tuple(tuple). + Opts(crud.InsertOpts{ + FetchLatestMetadata: crud.MakeOptBool(true), + }), + }, + { + "InsertObject", + crud.MakeInsertObjectRequest(spaceName). + Object(object). + Opts(crud.InsertObjectOpts{ + FetchLatestMetadata: crud.MakeOptBool(true), + }), + }, + { + "InsertMany", + crud.MakeInsertManyRequest(spaceName). + Tuples(tuples). + Opts(crud.InsertManyOpts{ + FetchLatestMetadata: crud.MakeOptBool(true), + }), + }, + { + "InsertObjectMany", + crud.MakeInsertObjectManyRequest(spaceName). + Objects(objects). + Opts(crud.InsertObjectManyOpts{ + FetchLatestMetadata: crud.MakeOptBool(true), + }), + }, + { + "Replace", + crud.MakeReplaceRequest(spaceName). + Tuple(tuple). + Opts(crud.ReplaceOpts{ + FetchLatestMetadata: crud.MakeOptBool(true), + }), + }, + { + "ReplaceObject", + crud.MakeReplaceObjectRequest(spaceName). + Object(object). + Opts(crud.ReplaceObjectOpts{ + FetchLatestMetadata: crud.MakeOptBool(true), + }), + }, + { + "ReplaceMany", + crud.MakeReplaceManyRequest(spaceName). + Tuples(tuples). + Opts(crud.ReplaceManyOpts{ + FetchLatestMetadata: crud.MakeOptBool(true), + }), + }, + { + "ReplaceObjectMany", + crud.MakeReplaceObjectManyRequest(spaceName). + Objects(objects). + Opts(crud.ReplaceObjectManyOpts{ + FetchLatestMetadata: crud.MakeOptBool(true), + }), + }, + { + "Upsert", + crud.MakeUpsertRequest(spaceName). + Tuple(tuple). + Operations(operations). + Opts(crud.UpsertOpts{ + FetchLatestMetadata: crud.MakeOptBool(true), + }), + }, + { + "UpsertObject", + crud.MakeUpsertObjectRequest(spaceName). + Object(object). + Operations(operations). + Opts(crud.UpsertObjectOpts{ + FetchLatestMetadata: crud.MakeOptBool(true), + }), + }, + { + "UpsertMany", + crud.MakeUpsertManyRequest(spaceName). + TuplesOperationsData(tuplesOperationsData). + Opts(crud.UpsertManyOpts{ + FetchLatestMetadata: crud.MakeOptBool(true), + }), + }, + { + "UpsertObjectMany", + crud.MakeUpsertObjectManyRequest(spaceName). + ObjectsOperationsData(objectsOperationData). + Opts(crud.UpsertObjectManyOpts{ + FetchLatestMetadata: crud.MakeOptBool(true), + }), + }, + { + "Select", + crud.MakeSelectRequest(spaceName). + Conditions(conditions). + Opts(crud.SelectOpts{ + FetchLatestMetadata: crud.MakeOptBool(true), + }), + }, + { + "Get", + crud.MakeGetRequest(spaceName). + Key(key). + Opts(crud.GetOpts{ + FetchLatestMetadata: crud.MakeOptBool(true), + }), + }, + { + "Update", + crud.MakeUpdateRequest(spaceName). + Key(key). + Operations(operations). + Opts(crud.UpdateOpts{ + FetchLatestMetadata: crud.MakeOptBool(true), + }), + }, + { + "Delete", + crud.MakeDeleteRequest(spaceName). + Key(key). + Opts(crud.DeleteOpts{ + FetchLatestMetadata: crud.MakeOptBool(true), + }), + }, + { + "Min", + crud.MakeMinRequest(spaceName). + Opts(crud.MinOpts{ + FetchLatestMetadata: crud.MakeOptBool(true), + }), + }, + { + "Max", + crud.MakeMaxRequest(spaceName). + Opts(crud.MaxOpts{ + FetchLatestMetadata: crud.MakeOptBool(true), + }), + }, +} + +func TestFetchLatestMetadataOption(t *testing.T) { + conn := connect(t) + defer conn.Close() + + for _, testCase := range testMetadataCases { + t.Run(testCase.name, func(t *testing.T) { + for i := 1010; i < 1020; i++ { + req := tarantool.NewDeleteRequest(spaceName). + Key([]interface{}{uint(i)}) + conn.Do(req).Get() + } + + resp := crud.Result{} + + err := conn.Do(testCase.req).GetTyped(&resp) + if err != nil { + t.Fatalf("Failed to Do CRUD request: %s", err) + } + + if len(resp.Metadata) == 0 { + t.Fatalf("Failed to get relevant metadata") + } + + for i := 1010; i < 1020; i++ { + req := tarantool.NewDeleteRequest(spaceName). + Key([]interface{}{uint(i)}) + conn.Do(req).Get() + } + }) + } +} + // runTestMain is a body of TestMain function // (see https://pkg.go.dev/testing#hdr-Main). // Using defer + os.Exit is not works so TestMain body From be8ba74f33278bcb500e67f0b03e8e6dd08229e9 Mon Sep 17 00:00:00 2001 From: Georgy Moiseev Date: Fri, 6 Oct 2023 13:03:03 +0300 Subject: [PATCH 4/6] crud: more careful response process In go-tarantool, crud API result decoding assumes that there are always two response values, except for truncate. Such approach is based on actual experience: success result is always `return res, nil`, not `return res`. But this is not a feature guaranteed by crud module API, just a current implementation quirk [1] (since in Lua function result process there is no any difference). See also similar patch in tarantool/python [2]. 1. https://github.com/tarantool/crud/blob/53457477974fed42351cbd87f566d11e9f7e39bb/crud/common/schema.lua#L88 2. https://github.com/tarantool/tarantool-python/commit/a4b734aa46facc89fd967de1e74f9768341e56fc --- crud/result.go | 96 ++++++++++++++++++++++++------------------ crud/tarantool_test.go | 26 ++++++------ 2 files changed, 67 insertions(+), 55 deletions(-) diff --git a/crud/result.go b/crud/result.go index e65b3e55e..cc875a5de 100644 --- a/crud/result.go +++ b/crud/result.go @@ -75,8 +75,8 @@ func (r *Result) DecodeMsgpack(d *msgpack.Decoder) error { return err } - if arrLen < 2 { - return fmt.Errorf("array len doesn't match: %d", arrLen) + if arrLen == 0 { + return fmt.Errorf("unexpected empty response array") } l, err := d.DecodeMapLen() @@ -130,27 +130,32 @@ func (r *Result) DecodeMsgpack(d *msgpack.Decoder) error { } } - code, err := d.PeekCode() - if err != nil { - return err - } - - var retErr error - if msgpackIsArray(code) { - crudErr := newErrorMany(r.rowType) - if err := d.Decode(&crudErr); err != nil { - return err - } - retErr = *crudErr - } else if code != msgpcode.Nil { - crudErr := newError(r.rowType) - if err := d.Decode(&crudErr); err != nil { + if arrLen > 1 { + code, err := d.PeekCode() + if err != nil { return err } - retErr = *crudErr - } else { - if err := d.DecodeNil(); err != nil { - return err + + if msgpackIsArray(code) { + crudErr := newErrorMany(r.rowType) + if err := d.Decode(&crudErr); err != nil { + return err + } + if crudErr != nil { + return *crudErr + } + } else if code != msgpcode.Nil { + crudErr := newError(r.rowType) + if err := d.Decode(&crudErr); err != nil { + return err + } + if crudErr != nil { + return *crudErr + } + } else { + if err := d.DecodeNil(); err != nil { + return err + } } } @@ -160,7 +165,7 @@ func (r *Result) DecodeMsgpack(d *msgpack.Decoder) error { } } - return retErr + return nil } // NumberResult describes CRUD result as an object containing number. @@ -175,18 +180,24 @@ func (r *NumberResult) DecodeMsgpack(d *msgpack.Decoder) error { return err } - if arrLen < 2 { - return fmt.Errorf("array len doesn't match: %d", arrLen) + if arrLen == 0 { + return fmt.Errorf("unexpected empty response array") } if r.Value, err = d.DecodeUint64(); err != nil { return err } - var crudErr *Error = nil + if arrLen > 1 { + var crudErr *Error = nil - if err := d.Decode(&crudErr); err != nil { - return err + if err := d.Decode(&crudErr); err != nil { + return err + } + + if crudErr != nil { + return crudErr + } } for i := 2; i < arrLen; i++ { @@ -195,10 +206,6 @@ func (r *NumberResult) DecodeMsgpack(d *msgpack.Decoder) error { } } - if crudErr != nil { - return crudErr - } - return nil } @@ -213,26 +220,31 @@ func (r *BoolResult) DecodeMsgpack(d *msgpack.Decoder) error { if err != nil { return err } - if arrLen < 2 { - if r.Value, err = d.DecodeBool(); err != nil { - return err - } - return nil + if arrLen == 0 { + return fmt.Errorf("unexpected empty response array") } - if _, err = d.DecodeInterface(); err != nil { + if r.Value, err = d.DecodeBool(); err != nil { return err } - var crudErr *Error = nil + if arrLen > 1 { + var crudErr *Error = nil - if err := d.Decode(&crudErr); err != nil { - return err + if err := d.Decode(&crudErr); err != nil { + return err + } + + if crudErr != nil { + return crudErr + } } - if crudErr != nil { - return crudErr + for i := 2; i < arrLen; i++ { + if err := d.Skip(); err != nil { + return err + } } return nil diff --git a/crud/tarantool_test.go b/crud/tarantool_test.go index 457cb2aa0..88572b56f 100644 --- a/crud/tarantool_test.go +++ b/crud/tarantool_test.go @@ -254,7 +254,7 @@ var testGenerateDataCases = []struct { }{ { "Insert", - 2, + 1, 1, crud.MakeInsertRequest(spaceName). Tuple(tuple). @@ -262,7 +262,7 @@ var testGenerateDataCases = []struct { }, { "InsertObject", - 2, + 1, 1, crud.MakeInsertObjectRequest(spaceName). Object(object). @@ -270,7 +270,7 @@ var testGenerateDataCases = []struct { }, { "InsertMany", - 2, + 1, 10, crud.MakeInsertManyRequest(spaceName). Tuples(tuples). @@ -278,7 +278,7 @@ var testGenerateDataCases = []struct { }, { "InsertObjectMany", - 2, + 1, 10, crud.MakeInsertObjectManyRequest(spaceName). Objects(objects). @@ -286,7 +286,7 @@ var testGenerateDataCases = []struct { }, { "Replace", - 2, + 1, 1, crud.MakeReplaceRequest(spaceName). Tuple(tuple). @@ -294,7 +294,7 @@ var testGenerateDataCases = []struct { }, { "ReplaceObject", - 2, + 1, 1, crud.MakeReplaceObjectRequest(spaceName). Object(object). @@ -302,7 +302,7 @@ var testGenerateDataCases = []struct { }, { "ReplaceMany", - 2, + 1, 10, crud.MakeReplaceManyRequest(spaceName). Tuples(tuples). @@ -310,7 +310,7 @@ var testGenerateDataCases = []struct { }, { "ReplaceObjectMany", - 2, + 1, 10, crud.MakeReplaceObjectManyRequest(spaceName). Objects(objects). @@ -318,7 +318,7 @@ var testGenerateDataCases = []struct { }, { "Upsert", - 2, + 1, 1, crud.MakeUpsertRequest(spaceName). Tuple(tuple). @@ -327,7 +327,7 @@ var testGenerateDataCases = []struct { }, { "UpsertObject", - 2, + 1, 1, crud.MakeUpsertObjectRequest(spaceName). Object(object). @@ -336,7 +336,7 @@ var testGenerateDataCases = []struct { }, { "UpsertMany", - 2, + 1, 10, crud.MakeUpsertManyRequest(spaceName). TuplesOperationsData(tuplesOperationsData). @@ -344,7 +344,7 @@ var testGenerateDataCases = []struct { }, { "UpsertObjectMany", - 2, + 1, 10, crud.MakeUpsertObjectManyRequest(spaceName). ObjectsOperationsData(objectsOperationData). @@ -475,7 +475,7 @@ func testCrudRequestCheck(t *testing.T, req tarantool.Request, // resp.Data[0] - CRUD res. // resp.Data[1] - CRUD err. - if expectedLen >= 2 { + if expectedLen >= 2 && resp.Data[1] != nil { if crudErr, err := getCrudError(req, resp.Data[1]); err != nil { t.Fatalf("Failed to get CRUD error: %#v", err) } else if crudErr != nil { From a81e8a23f90382c4d7c336d04c0cdf42ec680785 Mon Sep 17 00:00:00 2001 From: Georgy Moiseev Date: Fri, 6 Oct 2023 13:49:00 +0300 Subject: [PATCH 5/6] crud: support noreturn option `noreturn` option was introduced in crud 1.2.0 [1]. 1. https://github.com/tarantool/crud/pull/356/commits/af0ce90536aafd277608897eaf01c3ba42adeaec --- CHANGELOG.md | 1 + crud/example_test.go | 27 ++++++ crud/options.go | 35 ++++++-- crud/tarantool_test.go | 195 +++++++++++++++++++++++++++++++++++++++++ 4 files changed, 250 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1fc71aac7..2dc22ecd2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ Versioning](http://semver.org/spec/v2.0.0.html) except to the first release. - Support password and password file to decrypt private SSL key file (#319) - Support `operation_data` in `crud.Error` (#330) - Support `fetch_latest_metadata` option for crud requests with metadata (#335) +- Support `noreturn` option for data change crud requests (#335) ### Changed diff --git a/crud/example_test.go b/crud/example_test.go index 363d0570d..96185756c 100644 --- a/crud/example_test.go +++ b/crud/example_test.go @@ -187,6 +187,33 @@ func ExampleResult_many() { // [[2010 45 bla] [2011 4 bla]] } +// ExampleResult_noreturn demonstrates noreturn request: a data change +// request where you don't need to retrieve the result, just want to know +// whether it was successful or not. +func ExampleResult_noreturn() { + conn := exampleConnect() + req := crud.MakeReplaceManyRequest(exampleSpace). + Tuples([]crud.Tuple{ + []interface{}{uint(2010), nil, "bla"}, + []interface{}{uint(2011), nil, "bla"}, + }). + Opts(crud.ReplaceManyOpts{ + Noreturn: crud.MakeOptBool(true), + }) + + ret := crud.Result{} + if err := conn.Do(req).GetTyped(&ret); err != nil { + fmt.Printf("Failed to execute request: %s", err) + return + } + + fmt.Println(ret.Metadata) + fmt.Println(ret.Rows) + // Output: + // [] + // +} + // ExampleResult_error demonstrates how to use a helper type Result // to handle a crud error. func ExampleResult_error() { diff --git a/crud/options.go b/crud/options.go index 8a6997380..214417d1f 100644 --- a/crud/options.go +++ b/crud/options.go @@ -22,6 +22,7 @@ const ( afterOptName = "after" batchSizeOptName = "batch_size" fetchLatestMetadataOptName = "fetch_latest_metadata" + noreturnOptName = "noreturn" ) // OptUint is an optional uint. @@ -156,14 +157,18 @@ type SimpleOperationOpts struct { // the latest migration of the data format. Performance overhead is up to 15%. // Disabled by default. FetchLatestMetadata OptBool + // Noreturn suppresses successfully processed data (first return value is `nil`). + // Disabled by default. + Noreturn OptBool } // EncodeMsgpack provides custom msgpack encoder. func (opts SimpleOperationOpts) EncodeMsgpack(enc *msgpack.Encoder) error { - const optsCnt = 5 + const optsCnt = 6 names := [optsCnt]string{timeoutOptName, vshardRouterOptName, - fieldsOptName, bucketIdOptName, fetchLatestMetadataOptName} + fieldsOptName, bucketIdOptName, fetchLatestMetadataOptName, + noreturnOptName} values := [optsCnt]interface{}{} exists := [optsCnt]bool{} values[0], exists[0] = opts.Timeout.Get() @@ -171,6 +176,7 @@ func (opts SimpleOperationOpts) EncodeMsgpack(enc *msgpack.Encoder) error { values[2], exists[2] = opts.Fields.Get() values[3], exists[3] = opts.BucketId.Get() values[4], exists[4] = opts.FetchLatestMetadata.Get() + values[5], exists[5] = opts.Noreturn.Get() return encodeOptions(enc, names[:], values[:], exists[:]) } @@ -196,15 +202,18 @@ type SimpleOperationObjectOpts struct { // the latest migration of the data format. Performance overhead is up to 15%. // Disabled by default. FetchLatestMetadata OptBool + // Noreturn suppresses successfully processed data (first return value is `nil`). + // Disabled by default. + Noreturn OptBool } // EncodeMsgpack provides custom msgpack encoder. func (opts SimpleOperationObjectOpts) EncodeMsgpack(enc *msgpack.Encoder) error { - const optsCnt = 6 + const optsCnt = 7 names := [optsCnt]string{timeoutOptName, vshardRouterOptName, fieldsOptName, bucketIdOptName, skipNullabilityCheckOnFlattenOptName, - fetchLatestMetadataOptName} + fetchLatestMetadataOptName, noreturnOptName} values := [optsCnt]interface{}{} exists := [optsCnt]bool{} values[0], exists[0] = opts.Timeout.Get() @@ -213,6 +222,7 @@ func (opts SimpleOperationObjectOpts) EncodeMsgpack(enc *msgpack.Encoder) error values[3], exists[3] = opts.BucketId.Get() values[4], exists[4] = opts.SkipNullabilityCheckOnFlatten.Get() values[5], exists[5] = opts.FetchLatestMetadata.Get() + values[6], exists[6] = opts.Noreturn.Get() return encodeOptions(enc, names[:], values[:], exists[:]) } @@ -240,15 +250,18 @@ type OperationManyOpts struct { // the latest migration of the data format. Performance overhead is up to 15%. // Disabled by default. FetchLatestMetadata OptBool + // Noreturn suppresses successfully processed data (first return value is `nil`). + // Disabled by default. + Noreturn OptBool } // EncodeMsgpack provides custom msgpack encoder. func (opts OperationManyOpts) EncodeMsgpack(enc *msgpack.Encoder) error { - const optsCnt = 6 + const optsCnt = 7 names := [optsCnt]string{timeoutOptName, vshardRouterOptName, fieldsOptName, stopOnErrorOptName, rollbackOnErrorOptName, - fetchLatestMetadataOptName} + fetchLatestMetadataOptName, noreturnOptName} values := [optsCnt]interface{}{} exists := [optsCnt]bool{} values[0], exists[0] = opts.Timeout.Get() @@ -257,6 +270,7 @@ func (opts OperationManyOpts) EncodeMsgpack(enc *msgpack.Encoder) error { values[3], exists[3] = opts.StopOnError.Get() values[4], exists[4] = opts.RollbackOnError.Get() values[5], exists[5] = opts.FetchLatestMetadata.Get() + values[6], exists[6] = opts.Noreturn.Get() return encodeOptions(enc, names[:], values[:], exists[:]) } @@ -287,15 +301,19 @@ type OperationObjectManyOpts struct { // the latest migration of the data format. Performance overhead is up to 15%. // Disabled by default. FetchLatestMetadata OptBool + // Noreturn suppresses successfully processed data (first return value is `nil`). + // Disabled by default. + Noreturn OptBool } // EncodeMsgpack provides custom msgpack encoder. func (opts OperationObjectManyOpts) EncodeMsgpack(enc *msgpack.Encoder) error { - const optsCnt = 7 + const optsCnt = 8 names := [optsCnt]string{timeoutOptName, vshardRouterOptName, fieldsOptName, stopOnErrorOptName, rollbackOnErrorOptName, - skipNullabilityCheckOnFlattenOptName, fetchLatestMetadataOptName} + skipNullabilityCheckOnFlattenOptName, fetchLatestMetadataOptName, + noreturnOptName} values := [optsCnt]interface{}{} exists := [optsCnt]bool{} values[0], exists[0] = opts.Timeout.Get() @@ -305,6 +323,7 @@ func (opts OperationObjectManyOpts) EncodeMsgpack(enc *msgpack.Encoder) error { values[4], exists[4] = opts.RollbackOnError.Get() values[5], exists[5] = opts.SkipNullabilityCheckOnFlatten.Get() values[6], exists[6] = opts.FetchLatestMetadata.Get() + values[7], exists[7] = opts.Noreturn.Get() return encodeOptions(enc, names[:], values[:], exists[:]) } diff --git a/crud/tarantool_test.go b/crud/tarantool_test.go index 88572b56f..b361579ae 100644 --- a/crud/tarantool_test.go +++ b/crud/tarantool_test.go @@ -1012,6 +1012,201 @@ func TestFetchLatestMetadataOption(t *testing.T) { } } +var testNoreturnCases = []struct { + name string + req tarantool.Request +}{ + { + "Insert", + crud.MakeInsertRequest(spaceName). + Tuple(tuple). + Opts(crud.InsertOpts{ + Noreturn: crud.MakeOptBool(true), + }), + }, + { + "InsertObject", + crud.MakeInsertObjectRequest(spaceName). + Object(object). + Opts(crud.InsertObjectOpts{ + Noreturn: crud.MakeOptBool(true), + }), + }, + { + "InsertMany", + crud.MakeInsertManyRequest(spaceName). + Tuples(tuples). + Opts(crud.InsertManyOpts{ + Noreturn: crud.MakeOptBool(true), + }), + }, + { + "InsertObjectMany", + crud.MakeInsertObjectManyRequest(spaceName). + Objects(objects). + Opts(crud.InsertObjectManyOpts{ + Noreturn: crud.MakeOptBool(true), + }), + }, + { + "Replace", + crud.MakeReplaceRequest(spaceName). + Tuple(tuple). + Opts(crud.ReplaceOpts{ + Noreturn: crud.MakeOptBool(true), + }), + }, + { + "ReplaceObject", + crud.MakeReplaceObjectRequest(spaceName). + Object(object). + Opts(crud.ReplaceObjectOpts{ + Noreturn: crud.MakeOptBool(true), + }), + }, + { + "ReplaceMany", + crud.MakeReplaceManyRequest(spaceName). + Tuples(tuples). + Opts(crud.ReplaceManyOpts{ + Noreturn: crud.MakeOptBool(true), + }), + }, + { + "ReplaceObjectMany", + crud.MakeReplaceObjectManyRequest(spaceName). + Objects(objects). + Opts(crud.ReplaceObjectManyOpts{ + Noreturn: crud.MakeOptBool(true), + }), + }, + { + "Upsert", + crud.MakeUpsertRequest(spaceName). + Tuple(tuple). + Operations(operations). + Opts(crud.UpsertOpts{ + Noreturn: crud.MakeOptBool(true), + }), + }, + { + "UpsertObject", + crud.MakeUpsertObjectRequest(spaceName). + Object(object). + Operations(operations). + Opts(crud.UpsertObjectOpts{ + Noreturn: crud.MakeOptBool(true), + }), + }, + { + "UpsertMany", + crud.MakeUpsertManyRequest(spaceName). + TuplesOperationsData(tuplesOperationsData). + Opts(crud.UpsertManyOpts{ + Noreturn: crud.MakeOptBool(true), + }), + }, + { + "UpsertObjectMany", + crud.MakeUpsertObjectManyRequest(spaceName). + ObjectsOperationsData(objectsOperationData). + Opts(crud.UpsertObjectManyOpts{ + Noreturn: crud.MakeOptBool(true), + }), + }, + { + "Update", + crud.MakeUpdateRequest(spaceName). + Key(key). + Operations(operations). + Opts(crud.UpdateOpts{ + Noreturn: crud.MakeOptBool(true), + }), + }, + { + "Delete", + crud.MakeDeleteRequest(spaceName). + Key(key). + Opts(crud.DeleteOpts{ + Noreturn: crud.MakeOptBool(true), + }), + }, +} + +func TestNoreturnOption(t *testing.T) { + conn := connect(t) + defer conn.Close() + + for _, testCase := range testNoreturnCases { + t.Run(testCase.name, func(t *testing.T) { + for i := 1010; i < 1020; i++ { + req := tarantool.NewDeleteRequest(spaceName). + Key([]interface{}{uint(i)}) + conn.Do(req).Get() + } + + resp, err := conn.Do(testCase.req).Get() + if err != nil { + t.Fatalf("Failed to Do CRUD request: %s", err) + } + + if len(resp.Data) == 0 { + t.Fatalf("Expected explicit nil") + } + + if resp.Data[0] != nil { + t.Fatalf("Expected nil result, got %v", resp.Data[0]) + } + + if len(resp.Data) >= 2 && resp.Data[1] != nil { + t.Fatalf("Expected no returned errors, got %v", resp.Data[1]) + } + + for i := 1010; i < 1020; i++ { + req := tarantool.NewDeleteRequest(spaceName). + Key([]interface{}{uint(i)}) + conn.Do(req).Get() + } + }) + } +} + +func TestNoreturnOptionTyped(t *testing.T) { + conn := connect(t) + defer conn.Close() + + for _, testCase := range testNoreturnCases { + t.Run(testCase.name, func(t *testing.T) { + for i := 1010; i < 1020; i++ { + req := tarantool.NewDeleteRequest(spaceName). + Key([]interface{}{uint(i)}) + conn.Do(req).Get() + } + + resp := crud.Result{} + + err := conn.Do(testCase.req).GetTyped(&resp) + if err != nil { + t.Fatalf("Failed to Do CRUD request: %s", err) + } + + if resp.Rows != nil { + t.Fatalf("Expected nil rows, got %v", resp.Rows) + } + + if len(resp.Metadata) != 0 { + t.Fatalf("Expected no metadata") + } + + for i := 1010; i < 1020; i++ { + req := tarantool.NewDeleteRequest(spaceName). + Key([]interface{}{uint(i)}) + conn.Do(req).Get() + } + }) + } +} + // runTestMain is a body of TestMain function // (see https://pkg.go.dev/testing#hdr-Main). // Using defer + os.Exit is not works so TestMain body From b844e21cbc63e3d5df85d7b7badbab58143df506 Mon Sep 17 00:00:00 2001 From: Georgy Moiseev Date: Fri, 6 Oct 2023 18:20:32 +0300 Subject: [PATCH 6/6] doc: clarify crud error process decoding It is a bit confusing for non-experienced vmihailenco/msgpack users than `nil` values are actually parsed by `DecodeMapLen`, `DecodeUint64` and `DecodeBool` methods to some default value. So I decided to leave a note here. --- crud/result.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/crud/result.go b/crud/result.go index cc875a5de..7ae00e68f 100644 --- a/crud/result.go +++ b/crud/result.go @@ -79,6 +79,9 @@ func (r *Result) DecodeMsgpack(d *msgpack.Decoder) error { return fmt.Errorf("unexpected empty response array") } + // DecodeMapLen processes `nil` as zero length map, + // so in `return nil, err` case we don't miss error info. + // https://github.com/vmihailenco/msgpack/blob/3f7bd806fea698e7a9fe80979aa3512dea0a7368/decode_map.go#L79-L81 l, err := d.DecodeMapLen() if err != nil { return err @@ -184,6 +187,9 @@ func (r *NumberResult) DecodeMsgpack(d *msgpack.Decoder) error { return fmt.Errorf("unexpected empty response array") } + // DecodeUint64 processes `nil` as `0`, + // so in `return nil, err` case we don't miss error info. + // https://github.com/vmihailenco/msgpack/blob/3f7bd806fea698e7a9fe80979aa3512dea0a7368/decode_number.go#L91-L93 if r.Value, err = d.DecodeUint64(); err != nil { return err } @@ -225,6 +231,9 @@ func (r *BoolResult) DecodeMsgpack(d *msgpack.Decoder) error { return fmt.Errorf("unexpected empty response array") } + // DecodeBool processes `nil` as `false`, + // so in `return nil, err` case we don't miss error info. + // https://github.com/vmihailenco/msgpack/blob/3f7bd806fea698e7a9fe80979aa3512dea0a7368/decode.go#L367-L369 if r.Value, err = d.DecodeBool(); err != nil { return err }