From cf13e5cd171918fae9ea7c1eff68171a1c0fd718 Mon Sep 17 00:00:00 2001 From: Aman Mangal Date: Thu, 3 Aug 2023 22:25:49 +0530 Subject: [PATCH] feat(query): add feature flag normalize-compatibility-mode (#8845) This feature flag allows configuring @normalize in the query language such that if there are multiple fields with same alias, they could be returned as a list or non-list. There was a breaking change at some point with the PR https://github.com/dgraph-io/dgraph/pull/7639. Some customers do not prefer a list response in case of @normalize. We would like to not make any breaking changes going forward, and hence, introducing this feature flag enables users to make that choice. --- dgraph/cmd/alpha/run.go | 12 +++ dgraph/cmd/zero/pgp_test.go | 2 - dgraphtest/config.go | 14 +++- dgraphtest/dgraph.go | 4 + query/normalize_feature_flag_test.go | 121 +++++++++++++++++++++++++++ query/outputnode.go | 9 +- worker/server_state.go | 3 +- x/config.go | 3 + 8 files changed, 159 insertions(+), 9 deletions(-) create mode 100644 query/normalize_feature_flag_test.go diff --git a/dgraph/cmd/alpha/run.go b/dgraph/cmd/alpha/run.go index 68493a57add..9e10cf51416 100644 --- a/dgraph/cmd/alpha/run.go +++ b/dgraph/cmd/alpha/run.go @@ -260,6 +260,13 @@ they form a Raft group and provide synchronous replication. Flag("size", "The audit log max size in MB after which it will be rolled over."). String()) + + flag.String("feature-flags", worker.FeatureFlagsDefaults, z.NewSuperFlagHelp(worker.FeatureFlagsDefaults). + Head("Feature flags to enable various experimental features"). + Flag("normalize-compatibility-mode", "configure @normalize response formatting."+ + " 'v20': returns values with repeated key for fields with same alias (same as v20.11)."+ + " For more details, see https://github.com/dgraph-io/dgraph/pull/7639"). + String()) } func setupCustomTokenizers() { @@ -737,6 +744,11 @@ func run() { } edgraph.Init() + // feature flags + featureFlagsConf := z.NewSuperFlag(Alpha.Conf.GetString("feature-flags")).MergeAndCheckDefault( + worker.FeatureFlagsDefaults) + x.Config.NormalizeCompatibilityMode = featureFlagsConf.GetString("normalize-compatibility-mode") + x.PrintVersion() glog.Infof("x.Config: %+v", x.Config) glog.Infof("x.WorkerConfig: %+v", x.WorkerConfig) diff --git a/dgraph/cmd/zero/pgp_test.go b/dgraph/cmd/zero/pgp_test.go index efcaeb5714e..ead939872f5 100644 --- a/dgraph/cmd/zero/pgp_test.go +++ b/dgraph/cmd/zero/pgp_test.go @@ -1,5 +1,3 @@ -//go:build integration - package zero import ( diff --git a/dgraphtest/config.go b/dgraphtest/config.go index 1fa0fe3ad56..336c647076a 100644 --- a/dgraphtest/config.go +++ b/dgraphtest/config.go @@ -92,9 +92,10 @@ type ClusterConfig struct { refillInterval time.Duration uidLease int // exposed port offset for grpc/http port for both alpha/zero - portOffset int - bulkOutDir string - lambdaURL string + portOffset int + bulkOutDir string + lambdaURL string + featureFlags []string } func NewClusterConfig() ClusterConfig { @@ -182,7 +183,14 @@ func (cc ClusterConfig) WithBulkLoadOutDir(dir string) ClusterConfig { return cc } +// WithGraphqlLambdaURL sets the URL to lambda server for alpha func (cc ClusterConfig) WithGraphqlLambdaURL(url string) ClusterConfig { cc.lambdaURL = url return cc } + +// WithNormalizeCompatibilityMode sets the normalize-compatibility-mode feature flag for alpha +func (cc ClusterConfig) WithNormalizeCompatibilityMode(mode string) ClusterConfig { + cc.featureFlags = append(cc.featureFlags, fmt.Sprintf("normalize-compatibility-mode=%v", mode)) + return cc +} diff --git a/dgraphtest/dgraph.go b/dgraphtest/dgraph.go index 9c23ab04a58..68c616915fb 100644 --- a/dgraphtest/dgraph.go +++ b/dgraphtest/dgraph.go @@ -22,6 +22,7 @@ import ( "os" "path/filepath" "strconv" + "strings" "time" "github.com/docker/docker/api/types/mount" @@ -246,6 +247,9 @@ func (a *alpha) cmd(c *LocalCluster) []string { if c.conf.lambdaURL != "" { acmd = append(acmd, fmt.Sprintf(`--graphql=lambda-url=%s`, c.conf.lambdaURL)) } + if len(c.conf.featureFlags) > 0 { + acmd = append(acmd, fmt.Sprintf("--feature-flags=%v", strings.Join(c.conf.featureFlags, ";"))) + } return acmd } diff --git a/query/normalize_feature_flag_test.go b/query/normalize_feature_flag_test.go new file mode 100644 index 00000000000..7a1d6d3fcdc --- /dev/null +++ b/query/normalize_feature_flag_test.go @@ -0,0 +1,121 @@ +//go:build integration2 + +/* + * Copyright 2023 Dgraph Labs, Inc. and Contributors * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package query + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "github.com/dgraph-io/dgo/v230/protos/api" + "github.com/dgraph-io/dgraph/dgraphtest" +) + +func TestNormalizeDirectiveWithNoListResponse(t *testing.T) { + conf := dgraphtest.NewClusterConfig().WithNumAlphas(1).WithNumZeros(1). + WithReplicas(1).WithNormalizeCompatibilityMode("v20") + c, err := dgraphtest.NewLocalCluster(conf) + require.NoError(t, err) + defer func() { c.Cleanup(t.Failed()) }() + require.NoError(t, c.Start()) + + gc, cleanup, err := c.Client() + require.NoError(t, err) + defer cleanup() + require.NoError(t, c.AssignUids(gc.Dgraph, 100)) + + const dataSchema = ` + friend : [uid] @reverse @count . + name : string @index(term, exact, trigram) @count @lang . + dob : dateTime @index(year) .` + require.NoError(t, gc.SetupSchema(dataSchema)) + + triples := []byte(` + <1> <23> . + <1> <24> . + <1> <25> . + <1> <31> . + <1> <101>. + <23> <1> . + <31> <1> . + <31> <25> . + <1> "1910-01-01" . + <23> "1910-01-02" . + <24> "1909-05-05" . + <25> "1909-01-10" . + <31> "1901-01-15" . + <1> "Michonne" . + <23> "Rick Grimes" . + <24> "Glenn Rhee" .`) + _, err = gc.Mutate(&api.Mutation{SetNquads: triples, CommitNow: true}) + require.NoError(t, err) + + query := ` + { + me(func: uid(0x01)) @recurse @normalize { + n: name + d: dob + friend + } + }` + js, err := gc.Query(query) + require.NoError(t, err) + require.JSONEq(t, ` + { + "me": [ + { + "n": "Michonne", + "d": "1910-01-01T00:00:00Z", + "n": "Rick Grimes", + "d": "1910-01-02T00:00:00Z", + "n": "Michonne", + "d": "1910-01-01T00:00:00Z" + }, + { + "n": "Michonne", + "d": "1910-01-01T00:00:00Z", + "n": "Glenn Rhee", + "d": "1909-05-05T00:00:00Z" + }, + { + "n": "Michonne", + "d": [ + "1910-01-01T00:00:00Z", + "1909-01-10T00:00:00Z" + ] + }, + { + "n": "Michonne", + "d": [ + "1910-01-01T00:00:00Z", + "1901-01-15T00:00:00Z" + ], + "n": "Michonne", + "d": "1910-01-01T00:00:00Z" + }, + { + "n": "Michonne", + "d": [ + "1910-01-01T00:00:00Z", + "1901-01-15T00:00:00Z", + "1909-01-10T00:00:00Z" + ] + } + ] + }`, string(js.Json)) +} diff --git a/query/outputnode.go b/query/outputnode.go index 1f27927c8f2..9b06bf36061 100644 --- a/query/outputnode.go +++ b/query/outputnode.go @@ -978,9 +978,12 @@ func (enc *encoder) normalize(fj fastJsonNode) ([]fastJsonNode, error) { } for i, slice := range parentSlice { - // sort the fastJson list - // This will ensure that nodes with same attribute name comes together in response - enc.MergeSort(&parentSlice[i]) + if x.Config.NormalizeCompatibilityMode == "" { + // sort the fastJson list. This will ensure that nodes + // with same attribute name comes together in response + enc.MergeSort(&parentSlice[i]) + } + // From every list we need to remove node with attribute "uid". var prev, cur fastJsonNode cur = slice diff --git a/worker/server_state.go b/worker/server_state.go index 839adec4c6d..208ff105f22 100644 --- a/worker/server_state.go +++ b/worker/server_state.go @@ -52,7 +52,8 @@ const ( ZeroLimitsDefaults = `uid-lease=0; refill-interval=30s; disable-admin-http=false;` GraphQLDefaults = `introspection=true; debug=false; extensions=true; poll-interval=1s; ` + `lambda-url=;` - CacheDefaults = `size-mb=1024; percentage=0,65,35;` + CacheDefaults = `size-mb=1024; percentage=0,65,35;` + FeatureFlagsDefaults = `normalize-compatibility-mode=` ) // ServerState holds the state of the Dgraph server. diff --git a/x/config.go b/x/config.go index d6df6847389..ba86b5a92b3 100644 --- a/x/config.go +++ b/x/config.go @@ -67,6 +67,9 @@ type Options struct { // poll-interval duration - The polling interval for graphql subscription. GraphQL *z.SuperFlag GraphQLDebug bool + + // feature flags + NormalizeCompatibilityMode string } // Config stores the global instance of this package's options.