Merge pull request #246 from ydb-platform/keep-in-cache

Enabled by default keep-in-cache

Author: asmyasnikov

CHANGELOG.md

@@ -1,3 +1,7 @@
+* Marked as deprecated `session.Prepare` callback
+* Marked as deprecated `options.WithQueryCachePolicyKeepInCache` and `options.WithQueryCachePolicy` options
+* Added `options.WithKeepInCache` option
+* Enabled by default keep-in-cache policy for data queries
 * Fixed bug with convertation `time.Duration` from/to YDB value
 * Removed from `ydb.Connection` embedding of `grpc.ClientConnInterface`
 * Fixed stopping of repeater

internal/table/keep_in_cache_test.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"testing"
 
+	"github.com/stretchr/testify/require"
 	"google.golang.org/protobuf/proto"
 
 	"github.com/ydb-platform/ydb-go-genproto/protos/Ydb_Table"
@@ -15,18 +16,28 @@ import (
 
 func TestQueryCachePolicyKeepInCache(t *testing.T) {
 	for _, test := range [...]struct {
-		name                   string
-		queryCachePolicyOption []options.QueryCachePolicyOption
+		name                    string
+		executeDataQueryOptions []options.ExecuteDataQueryOption
+		keepInCache             bool
 	}{
+		{
+			name:                    "no options",
+			executeDataQueryOptions: nil,
+			keepInCache:             true,
+		},
 		{
 			name: "with server cache",
-			queryCachePolicyOption: []options.QueryCachePolicyOption{
-				options.WithQueryCachePolicyKeepInCache(),
+			executeDataQueryOptions: []options.ExecuteDataQueryOption{
+				options.WithKeepInCache(true),
 			},
+			keepInCache: true,
 		},
 		{
-			name:                   "no server cache",
-			queryCachePolicyOption: []options.QueryCachePolicyOption{},
+			name: "no server cache",
+			executeDataQueryOptions: []options.ExecuteDataQueryOption{
+				options.WithKeepInCache(false),
+			},
+			keepInCache: false,
 		},
 	} {
 		t.Run(test.name, func(t *testing.T) {
@@ -41,15 +52,7 @@ func TestQueryCachePolicyKeepInCache(t *testing.T) {
 								if !ok {
 									t.Fatalf("cannot cast request '%T' to *Ydb_Table.ExecuteDataQueryRequest", request)
 								}
-								if len(test.queryCachePolicyOption) > 0 {
-									if !r.QueryCachePolicy.GetKeepInCache() {
-										t.Fatalf("keep-in-cache policy must be true, got: %v", r.QueryCachePolicy.GetKeepInCache())
-									}
-								} else {
-									if r.QueryCachePolicy.GetKeepInCache() {
-										t.Fatalf("keep-in-cache policy must be false, got: %v", r.QueryCachePolicy.GetKeepInCache())
-									}
-								}
+								require.Equal(t, test.keepInCache, r.QueryCachePolicy.GetKeepInCache())
 								return &Ydb_Table.ExecuteQueryResult{
 									TxMeta: &Ydb_Table.TransactionMeta{
 										Id: "",
@@ -79,7 +82,7 @@ func TestQueryCachePolicyKeepInCache(t *testing.T) {
 				),
 				"SELECT 1",
 				table.NewQueryParameters(),
-				options.WithQueryCachePolicy(test.queryCachePolicyOption...),
+				test.executeDataQueryOptions...,
 			)
 			if err != nil {
 				t.Fatal(err)

internal/table/session.go

@@ -685,6 +685,9 @@ func (s *session) executeDataQuery(
 		TxControl:  tx.Desc(),
 		Parameters: params.Params(),
 		Query:      &query.query,
+		QueryCachePolicy: &Ydb_Table.QueryCachePolicy{
+			KeepInCache: true,
+		},
 		OperationParams: operation.Params(
 			ctx,
 			s.config.OperationTimeout(),

table/options/options.go

@@ -511,16 +511,42 @@ type (
 	QueryCachePolicyOption func(*queryCachePolicy)
 )
 
+// WithKeepInCache manages keep-in-cache flag in query cache policy
+//
+// By default all data queries executes with keep-in-cache policy
+func WithKeepInCache(keepInCache bool) ExecuteDataQueryOption {
+	return withQueryCachePolicy(
+		withQueryCachePolicyKeepInCache(keepInCache),
+	)
+}
+
+// WithQueryCachePolicyKeepInCache manages keep-in-cache policy
+//
+// Deprecated: data queries always executes with enabled keep-in-cache policy.
+// Use WithKeepInCache for disabling keep-in-cache policy
 func WithQueryCachePolicyKeepInCache() QueryCachePolicyOption {
+	return withQueryCachePolicyKeepInCache(true)
+}
+
+func withQueryCachePolicyKeepInCache(keepInCache bool) QueryCachePolicyOption {
 	return func(p *queryCachePolicy) {
-		p.KeepInCache = true
+		p.KeepInCache = keepInCache
 	}
 }
 
+// WithQueryCachePolicy manages query cache policy
+//
+// Deprecated: use WithKeepInCache for disabling keep-in-cache policy
 func WithQueryCachePolicy(opts ...QueryCachePolicyOption) ExecuteDataQueryOption {
+	return withQueryCachePolicy(opts...)
+}
+
+func withQueryCachePolicy(opts ...QueryCachePolicyOption) ExecuteDataQueryOption {
 	return func(d *ExecuteDataQueryDesc) {
 		if d.QueryCachePolicy == nil {
-			d.QueryCachePolicy = new(Ydb_Table.QueryCachePolicy)
+			d.QueryCachePolicy = &Ydb_Table.QueryCachePolicy{
+				KeepInCache: true,
+			}
 		}
 		for _, opt := range opts {
 			opt((*queryCachePolicy)(d.QueryCachePolicy))

table/table.go

@@ -69,69 +69,86 @@ type Session interface {
 		path string,
 		opts ...options.CreateTableOption,
 	) (err error)
+
 	DescribeTable(
 		ctx context.Context,
 		path string,
 		opts ...options.DescribeTableOption,
 	) (desc options.Description, err error)
+
 	DropTable(
 		ctx context.Context,
 		path string,
 		opts ...options.DropTableOption,
 	) (err error)
+
 	AlterTable(
 		ctx context.Context,
 		path string,
 		opts ...options.AlterTableOption,
 	) (err error)
+
 	CopyTable(
 		ctx context.Context,
 		dst, src string,
 		opts ...options.CopyTableOption,
 	) (err error)
+
 	Explain(
 		ctx context.Context,
 		query string,
 	) (exp DataQueryExplanation, err error)
+
+	// Prepare prepares query for executing in the future
+	//
+	// Deprecated: use Execute with KeepInCache policy option
 	Prepare(
 		ctx context.Context,
 		query string,
 	) (stmt Statement, err error)
+
 	Execute(
 		ctx context.Context,
 		tx *TransactionControl,
 		query string,
 		params *QueryParameters,
 		opts ...options.ExecuteDataQueryOption,
 	) (txr Transaction, r result.Result, err error)
+
 	ExecuteSchemeQuery(
 		ctx context.Context,
 		query string,
 		opts ...options.ExecuteSchemeQueryOption,
 	) (err error)
+
 	DescribeTableOptions(
 		ctx context.Context,
 	) (desc options.TableOptionsDescription, err error)
+
 	StreamReadTable(
 		ctx context.Context,
 		path string,
 		opts ...options.ReadTableOption,
 	) (r result.StreamResult, err error)
+
 	StreamExecuteScanQuery(
 		ctx context.Context,
 		query string,
 		params *QueryParameters,
 		opts ...options.ExecuteScanQueryOption,
 	) (_ result.StreamResult, err error)
+
 	BulkUpsert(
 		ctx context.Context,
 		table string,
 		rows types.Value,
 	) (err error)
+
 	BeginTransaction(
 		ctx context.Context,
 		tx *TransactionSettings,
 	) (x Transaction, err error)
+
 	KeepAlive(
 		ctx context.Context,
 	) error

table/table_e2e_test.go

@@ -380,9 +380,6 @@ func TestTable(t *testing.T) {
 					table.ValueParam("$seasonID", types.Uint64Value(1)),
 					table.ValueParam("$episodeID", types.Uint64Value(1)),
 				),
-				options.WithQueryCachePolicy(
-					options.WithQueryCachePolicyKeepInCache(),
-				),
 			)
 			if err != nil {
 				return err
@@ -419,9 +416,6 @@ func TestTable(t *testing.T) {
 					table.ValueParam("$episodeID", types.Uint64Value(1)),
 					table.ValueParam("$views", types.Uint64Value(views+1)), // increment views
 				),
-				options.WithQueryCachePolicy(
-					options.WithQueryCachePolicyKeepInCache(),
-				),
 			)
 			if err != nil {
 				return err
@@ -467,9 +461,6 @@ func TestTable(t *testing.T) {
 					table.ValueParam("$seasonID", types.Uint64Value(1)),
 					table.ValueParam("$episodeID", types.Uint64Value(1)),
 				),
-				options.WithQueryCachePolicy(
-					options.WithQueryCachePolicyKeepInCache(),
-				),
 			)
 			if err != nil {
 				return err
@@ -777,9 +768,6 @@ func executeDataQuery(ctx context.Context, t *testing.T, c table.Client, folderA
 				table.NewQueryParameters(
 					table.ValueParam("$seriesID", types.Uint64Value(1)),
 				),
-				options.WithQueryCachePolicy(
-					options.WithQueryCachePolicyKeepInCache(),
-				),
 				options.WithCollectStatsModeBasic(),
 			)
 			if err != nil {