Skip to content

Commit

Permalink
GOCBC-1549: Allow both positional and named parameters to be set in q…
Browse files Browse the repository at this point in the history 
…uery

Motivation
==========
The server allows both to be set, so we shouldn't return an invalid argument error when that happens

Changes
=======
* Remove check where an invalidArgumentsError is returned when both PositionalParameters and NamedParameters are set in QueryOptions
* Add tests for queries with both positional and named parameters

Results
=======
Query tests pass

Change-Id: Ie87912e4385beb2fa1c8958a5ddecf23c993a33f
Reviewed-on: https://review.couchbase.org/c/gocb/+/202622
Reviewed-by: Charles Dixon <chvckd@gmail.com>
Tested-by: Dimitris Christodoulou <dimitris.christodoulou@couchbase.com>
  • Loading branch information
@DemetrisChr
DemetrisChr committed Dec 20, 2023
1 parent e2153b3 commit 9d30c13
Show file tree
Hide file tree
Showing 3 changed files with 83 additions and 32 deletions.
93 changes: 69 additions & 24 deletions cluster_query_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,18 +30,21 @@ func (suite *IntegrationTestSuite) TestQuery() {
suite.Run("TestClusterQuery", func() {
suite.runClusterQueryPositionalTest(n, true)
suite.runClusterQueryNamedTest(n, true)
suite.runClusterQueryBothPositionalAndNamedTest(n, true)
})
suite.Run("TestClusterQueryNoMetrics", func() {
suite.runClusterQueryPositionalTest(n, false)
suite.runClusterQueryNamedTest(n, false)
suite.runClusterQueryBothPositionalAndNamedTest(n, false)
})
suite.Run("TestClusterPreparedQuery", func() {
suite.runClusterPreparedQueryPositionalTest(n)
suite.runClusterPreparedQueryNamedTest(n)
suite.runClusterPreparedQueryBothPositionalAndNamedTest(n)
})
}

func (suite *IntegrationTestSuite) runPreparedQueryTest(n int, query, bucket, scope string, queryFn queryIface, params interface{}) {
func (suite *IntegrationTestSuite) runPreparedQueryTest(n int, query, bucket, scope string, queryFn queryIface, positionalParams []interface{}, namedParams map[string]interface{}) {
suite.skipIfUnsupported(ClusterLevelQueryFeature)

deadline := time.Now().Add(60 * time.Second)
Expand All @@ -50,14 +53,10 @@ func (suite *IntegrationTestSuite) runPreparedQueryTest(n int, query, bucket, sc
globalMeter.Reset()
contextID := "contextID"
opts := &QueryOptions{
Timeout: 5 * time.Second,
ClientContextID: contextID,
}
switch p := params.(type) {
case []interface{}:
opts.PositionalParameters = p
case map[string]interface{}:
opts.NamedParameters = p
Timeout: 5 * time.Second,
ClientContextID: contextID,
PositionalParameters: positionalParams,
NamedParameters: namedParams,
}
result, err := queryFn.Query(query, opts)
suite.Require().Nil(err, "Failed to execute query %v", err)
Expand Down Expand Up @@ -127,14 +126,21 @@ func (suite *IntegrationTestSuite) runClusterPreparedQueryPositionalTest(n int)
suite.skipIfUnsupported(ClusterLevelQueryFeature)

query := fmt.Sprintf("SELECT `%s`.* FROM `%s` WHERE service=? LIMIT %d;", globalBucket.Name(), globalBucket.Name(), n)
suite.runPreparedQueryTest(n, query, "", "", globalCluster, []interface{}{"query"})
suite.runPreparedQueryTest(n, query, "", "", globalCluster, []interface{}{"query"}, nil)
}

func (suite *IntegrationTestSuite) runClusterPreparedQueryNamedTest(n int) {
suite.skipIfUnsupported(ClusterLevelQueryFeature)

query := fmt.Sprintf("SELECT `%s`.* FROM `%s` WHERE service=$service LIMIT %d;", globalBucket.Name(), globalBucket.Name(), n)
suite.runPreparedQueryTest(n, query, "", "", globalCluster, map[string]interface{}{"service": "query"})
suite.runPreparedQueryTest(n, query, "", "", globalCluster, nil, map[string]interface{}{"service": "query"})
}

func (suite *IntegrationTestSuite) runClusterPreparedQueryBothPositionalAndNamedTest(n int) {
suite.skipIfUnsupported(ClusterLevelQueryFeature)

query := fmt.Sprintf("SELECT `%s`.* FROM `%s` WHERE service=$service AND type=? LIMIT %d;", globalBucket.Name(), globalBucket.Name(), n)
suite.runPreparedQueryTest(n, query, "", "", globalCluster, []interface{}{"brewery"}, map[string]interface{}{"service": "query"})
}

func (suite *IntegrationTestSuite) TestClusterQueryImprovedErrorsDocNotFound() {
Expand Down Expand Up @@ -180,7 +186,7 @@ func (suite *IntegrationTestSuite) TestClusterQueryImprovedErrorsDocNotFound() {
suite.Assert().Equal(float64(17012), qErr.Errors[0].Reason["code"])
}

func (suite *IntegrationTestSuite) runQueryTest(n int, query, bucket, scope string, queryFn queryIface, withMetrics bool, params interface{}) {
func (suite *IntegrationTestSuite) runQueryTest(n int, query, bucket, scope string, queryFn queryIface, withMetrics bool, positionalParams []interface{}, namedParams map[string]interface{}) {
suite.skipIfUnsupported(ClusterLevelQueryFeature)

deadline := time.Now().Add(60 * time.Second)
Expand All @@ -189,16 +195,12 @@ func (suite *IntegrationTestSuite) runQueryTest(n int, query, bucket, scope stri
globalMeter.Reset()
contextID := "contextID"
opts := &QueryOptions{
Timeout: 5 * time.Second,
Adhoc: true,
Metrics: withMetrics,
ClientContextID: contextID,
}
switch p := params.(type) {
case []interface{}:
opts.PositionalParameters = p
case map[string]interface{}:
opts.NamedParameters = p
Timeout: 5 * time.Second,
Adhoc: true,
Metrics: withMetrics,
ClientContextID: contextID,
PositionalParameters: positionalParams,
NamedParameters: namedParams,
}
result, err := queryFn.Query(query, opts)
suite.Require().Nil(err, "Failed to execute query %v", err)
Expand Down Expand Up @@ -267,14 +269,21 @@ func (suite *IntegrationTestSuite) runClusterQueryPositionalTest(n int, withMetr
suite.skipIfUnsupported(ClusterLevelQueryFeature)

query := fmt.Sprintf("SELECT `%s`.* FROM `%s` WHERE service=? LIMIT %d;", globalBucket.Name(), globalBucket.Name(), n)
suite.runQueryTest(n, query, "", "", globalCluster, withMetrics, []interface{}{"query"})
suite.runQueryTest(n, query, "", "", globalCluster, withMetrics, []interface{}{"query"}, nil)
}

func (suite *IntegrationTestSuite) runClusterQueryNamedTest(n int, withMetrics bool) {
suite.skipIfUnsupported(ClusterLevelQueryFeature)

query := fmt.Sprintf("SELECT `%s`.* FROM `%s` WHERE service=$service LIMIT %d;", globalBucket.Name(), globalBucket.Name(), n)
suite.runQueryTest(n, query, "", "", globalCluster, withMetrics, map[string]interface{}{"service": "query"})
suite.runQueryTest(n, query, "", "", globalCluster, withMetrics, nil, map[string]interface{}{"service": "query"})
}

func (suite *IntegrationTestSuite) runClusterQueryBothPositionalAndNamedTest(n int, withMetrics bool) {
suite.skipIfUnsupported(ClusterLevelQueryFeature)

query := fmt.Sprintf("SELECT `%s`.* FROM `%s` WHERE service=$service AND type=? LIMIT %d;", globalBucket.Name(), globalBucket.Name(), n)
suite.runQueryTest(n, query, "", "", globalCluster, withMetrics, []interface{}{"brewery"}, map[string]interface{}{"service": "query"})
}

func (suite *IntegrationTestSuite) setupClusterQuery() int {
Expand Down Expand Up @@ -932,6 +941,42 @@ func (suite *UnitTestSuite) TestQueryPositionalParams() {
suite.Require().NotNil(result)
}

func (suite *UnitTestSuite) TestQueryBothPositionalAndNamedParams() {
reader := new(mockQueryRowReader)

statement := "SELECT * FROM dataset"
positionalParams := []interface{}{float64(1), "imafish"}
namedParams := map[string]interface{}{
"num": 1,
"imafish": "namedbarry",
"$cilit": "bang",
}

cluster := suite.queryCluster(false, reader, func(args mock.Arguments) {
opts := args.Get(1).(gocbcore.N1QLQueryOptions)

var actualOptions map[string]interface{}
err := json.Unmarshal(opts.Payload, &actualOptions)
suite.Require().Nil(err)

if suite.Assert().Contains(actualOptions, "args") {
suite.Require().Equal(positionalParams, actualOptions["args"])
}

suite.Assert().Equal(float64(1), actualOptions["$num"])
suite.Assert().Equal("namedbarry", actualOptions["$imafish"])
suite.Assert().Equal("bang", actualOptions["$cilit"])
})

result, err := cluster.Query(statement, &QueryOptions{
PositionalParameters: positionalParams,
NamedParameters: namedParams,
Adhoc: true,
})
suite.Require().Nil(err)
suite.Require().NotNil(result)
}

func (suite *UnitTestSuite) TestQueryClientContextID() {
reader := new(mockQueryRowReader)

Expand Down
4 changes: 0 additions & 4 deletions query_options.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -133,10 +133,6 @@ func (opts *QueryOptions) toMap() (map[string]interface{}, error) {
execOpts["readonly"] = opts.Readonly
}

if opts.PositionalParameters != nil && opts.NamedParameters != nil {
return nil, makeInvalidArgumentsError("Positional and named parameters must be used exclusively")
}

if opts.PositionalParameters != nil {
execOpts["args"] = opts.PositionalParameters
}
Expand Down
18 changes: 14 additions & 4 deletions scope_query_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,22 +53,32 @@ func (suite *IntegrationTestSuite) setupScopeQuery() int {

func (suite *IntegrationTestSuite) runScopePreparedQueryPositionalTest(n int) {
query := fmt.Sprintf("SELECT `%s`.* FROM `%s` WHERE service=? LIMIT %d;", globalCollection.Name(), globalCollection.Name(), n)
suite.runPreparedQueryTest(n, query, globalBucket.Name(), globalScope.Name(), globalScope, []interface{}{"scopequery"})
suite.runPreparedQueryTest(n, query, globalBucket.Name(), globalScope.Name(), globalScope, []interface{}{"scopequery"}, nil)
}

func (suite *IntegrationTestSuite) runScopePreparedQueryNamedTest(n int) {
query := fmt.Sprintf("SELECT `%s`.* FROM `%s` WHERE service=$service LIMIT %d;", globalCollection.Name(), globalCollection.Name(), n)
suite.runPreparedQueryTest(n, query, globalBucket.Name(), globalScope.Name(), globalScope, map[string]interface{}{"service": "scopequery"})
suite.runPreparedQueryTest(n, query, globalBucket.Name(), globalScope.Name(), globalScope, nil, map[string]interface{}{"service": "scopequery"})
}

func (suite *IntegrationTestSuite) runScopePreparedQueryBothPositionalAndNamedTest(n int, withMetrics bool) {
query := fmt.Sprintf("SELECT `%s`.* FROM `%s` WHERE service=$service AND type=? LIMIT %d;", globalBucket.Name(), globalBucket.Name(), n)
suite.runPreparedQueryTest(n, query, globalBucket.Name(), globalScope.Name(), globalScope, []interface{}{"brewery"}, map[string]interface{}{"service": "query"})
}

func (suite *IntegrationTestSuite) runScopeQueryPositionalTest(n int, withMetrics bool) {
query := fmt.Sprintf("SELECT `%s`.* FROM `%s` WHERE service=? LIMIT %d;", globalCollection.Name(), globalCollection.Name(), n)
suite.runQueryTest(n, query, globalBucket.Name(), globalScope.Name(), globalScope, withMetrics, []interface{}{"scopequery"})
suite.runQueryTest(n, query, globalBucket.Name(), globalScope.Name(), globalScope, withMetrics, []interface{}{"scopequery"}, nil)
}

func (suite *IntegrationTestSuite) runScopeQueryNamedTest(n int, withMetrics bool) {
query := fmt.Sprintf("SELECT `%s`.* FROM `%s` WHERE service=$service LIMIT %d;", globalCollection.Name(), globalCollection.Name(), n)
suite.runQueryTest(n, query, globalBucket.Name(), globalScope.Name(), globalScope, withMetrics, map[string]interface{}{"service": "scopequery"})
suite.runQueryTest(n, query, globalBucket.Name(), globalScope.Name(), globalScope, withMetrics, nil, map[string]interface{}{"service": "scopequery"})
}

func (suite *IntegrationTestSuite) runScopeQueryBothPositionalAndNamedTest(n int, withMetrics bool) {
query := fmt.Sprintf("SELECT `%s`.* FROM `%s` WHERE service=$service AND type=? LIMIT %d;", globalBucket.Name(), globalBucket.Name(), n)
suite.runQueryTest(n, query, globalBucket.Name(), globalScope.Name(), globalScope, withMetrics, []interface{}{"brewery"}, map[string]interface{}{"service": "query"})
}

func (suite *UnitTestSuite) queryScope(prepared bool, reader queryRowReader, runFn func(args mock.Arguments)) *Scope {
Expand Down

0 comments on commit 9d30c13

Please sign in to comment.