diff --git a/integration/query_array_compat_test.go b/integration/query_array_compat_test.go index 926af6112232..c6fcefc7f2c5 100644 --- a/integration/query_array_compat_test.go +++ b/integration/query_array_compat_test.go @@ -84,14 +84,12 @@ func testQueryArrayCompatDotNotation() map[string]queryCompatTestCase { resultType: emptyResult, }, "Field": { - filter: bson.D{{"v.array", int32(42)}}, - skipForTigris: "Tigris does not support language keyword 'array' as field name", - resultPushdown: true, + filter: bson.D{{"v.array", int32(42)}}, + skipForTigris: "Tigris does not support language keyword 'array' as field name", }, "FieldPosition": { - filter: bson.D{{"v.array.0", int32(42)}}, - skipForTigris: "Tigris does not support language keyword 'array' as field name", - resultPushdown: true, + filter: bson.D{{"v.array.0", int32(42)}}, + skipForTigris: "Tigris does not support language keyword 'array' as field name", }, "FieldPositionQuery": { filter: bson.D{{"v.array.0", bson.D{{"$gte", int32(42)}}}}, @@ -102,44 +100,32 @@ func testQueryArrayCompatDotNotation() map[string]queryCompatTestCase { resultType: emptyResult, }, "DocumentDotNotationArrayDocument": { - filter: bson.D{{"v.0.foo.0.bar", "hello"}}, - skipForTigris: "No suitable Tigris-compatible provider to test this data", - resultPushdown: true, + filter: bson.D{{"v.0.foo.0.bar", "hello"}}, + skipForTigris: "No suitable Tigris-compatible provider to test this data", }, "DocumentDotNotationArrayDocumentNoIndex": { filter: bson.D{{"v.foo.bar", "hello"}}, skip: "https://github.com/FerretDB/FerretDB/issues/1828", }, "FieldArrayIndex": { - filter: bson.D{{"v.foo[0]", int32(42)}}, - skipForTigris: "Tigris does not support characters as field name", - resultPushdown: true, + filter: bson.D{{"v.foo[0]", int32(42)}}, + skipForTigris: "Tigris does not support characters as field name", }, "FieldArrayAsterix": { - filter: bson.D{{"v.foo[*]", int32(42)}}, - skipForTigris: "Tigris does not support characters as field name", - resultPushdown: true, + filter: bson.D{{"v.foo[*]", int32(42)}}, + skipForTigris: "Tigris does not support characters as field name", }, "FieldAsterix": { - filter: bson.D{{"v.*", int32(42)}}, - skipForTigris: "Tigris does not support characters as field name", - resultPushdown: true, + filter: bson.D{{"v.*", int32(42)}}, + skipForTigris: "Tigris does not support characters as field name", }, "FieldAt": { - filter: bson.D{{"v.@", int32(42)}}, - skipForTigris: "Tigris does not support characters as field name", - resultPushdown: true, + filter: bson.D{{"v.@", int32(42)}}, + skipForTigris: "Tigris does not support characters as field name", }, "FieldComma": { - filter: bson.D{{"v.f,oo", int32(42)}}, - skipForTigris: "Tigris does not support characters as field name", - resultPushdown: true, - }, - "FieldDollarSign": { - filter: bson.D{{"v.$", int32(42)}}, - skipForTigris: "Tigris does not support characters as field name", - resultPushdown: true, - resultType: emptyResult, + filter: bson.D{{"v.f,oo", int32(42)}}, + skipForTigris: "Tigris does not support characters as field name", }, } diff --git a/integration/query_comparison_compat_test.go b/integration/query_comparison_compat_test.go index 343c385c2c82..26a03b346769 100644 --- a/integration/query_comparison_compat_test.go +++ b/integration/query_comparison_compat_test.go @@ -47,14 +47,12 @@ func testQueryComparisonCompatImplicit() map[string]queryCompatTestCase { resultType: emptyResult, }, "DocumentDotNotation": { - filter: bson.D{{"v.foo", int32(42)}}, - skipForTigris: "No suitable Tigris-compatible provider to test this data", - resultPushdown: true, + filter: bson.D{{"v.foo", int32(42)}}, + skipForTigris: "No suitable Tigris-compatible provider to test this data", }, "DocumentDotNotationNoSuchField": { - filter: bson.D{{"no-such-field.some", 42}}, - resultType: emptyResult, - resultPushdown: true, + filter: bson.D{{"no-such-field.some", 42}}, + resultType: emptyResult, }, "ArrayNoSuchField": { filter: bson.D{{"no-such-field", bson.A{42}}}, diff --git a/internal/handlers/pg/pgdb/query.go b/internal/handlers/pg/pgdb/query.go index ada2366a412d..f55b2e0faa9f 100644 --- a/internal/handlers/pg/pgdb/query.go +++ b/internal/handlers/pg/pgdb/query.go @@ -251,11 +251,6 @@ func prepareWhereClause(sqlFilters *types.Document) (string, []any, error) { return "", nil, lazyerrors.Error(err) } - keyOperator := "->" // keyOperator is the operator that is used to access the field. (->/#>) - - var key any = k // key can be either a string '"v"' or PostgreSQL path '{v,foo}' - var prefix string // prefix is the first key in path, if the filter key is not a path - the prefix is empty - if k != "" { // skip $comment if k[0] == '$' { @@ -267,16 +262,14 @@ func prepareWhereClause(sqlFilters *types.Document) (string, []any, error) { return "", nil, lazyerrors.Error(err) } - // If the key is in dot notation use path operator (#>) + // TODO dot notation https://github.com/FerretDB/FerretDB/issues/2069 if path.Len() > 1 { - keyOperator = "#>" - key = path.Slice() // '{v,foo}' - prefix = path.Prefix() // 'v' + continue } } // Handle _id with a simpler query, as it can't be an array - if k == "_id" || prefix == "_id" { + if k == "_id" { switch v := v.(type) { case *types.Document, *types.Array, types.Binary, bool, time.Time, types.NullType, types.Regex, types.Timestamp: // type not supported for pushdown @@ -284,14 +277,13 @@ func prepareWhereClause(sqlFilters *types.Document) (string, []any, error) { // TODO $gt and $lt https://github.com/FerretDB/FerretDB/issues/1875 case float64, string, types.ObjectID, int32, int64: // Select if value under the key is equal to provided value. - sql := `((_jsonb%[1]s%[2]s)::jsonb = %[3]s)` + sql := `(_jsonb->%[1]s)::jsonb = %[2]s` - // operator is -> for non-array, and #> for array // placeholder p.Next() returns SQL argument references such as $1, $2 to prevent SQL injections. - // placeholder $1 is used for field key or it's path, + // placeholder $1 is used for field key, // placeholder $2 is used for field value v. - filters = append(filters, fmt.Sprintf(sql, keyOperator, p.Next(), p.Next())) - args = append(args, key, string(must.NotFail(pjson.MarshalSingleValue(v)))) + filters = append(filters, fmt.Sprintf(sql, p.Next(), p.Next())) + args = append(args, k, string(must.NotFail(pjson.MarshalSingleValue(v)))) default: panic(fmt.Sprintf("Unexpected type of value: %v", v)) @@ -311,14 +303,13 @@ func prepareWhereClause(sqlFilters *types.Document) (string, []any, error) { // Select if value under the key is equal to provided value. // If the value under the key is not equal to v, // but the value under the key k is an array - select if it contains the value equal to v. - sql := `((_jsonb%[1]s%[2]s)::jsonb = %[3]s) OR (_jsonb%[1]s%[2]s)::jsonb @> %[3]s` + sql := `(_jsonb->%[1]s)::jsonb = %[2]s OR (_jsonb->%[1]s)::jsonb @> %[2]s` - // operator is -> for non-array, and #> for array // placeholder p.Next() returns SQL argument references such as $1, $2 to prevent SQL injections. - // placeholder $1 is used for field key or it's path, + // placeholder $1 is used for field key, // placeholder $2 is used for field value v. - filters = append(filters, fmt.Sprintf(sql, keyOperator, p.Next(), p.Next())) - args = append(args, key, string(must.NotFail(pjson.MarshalSingleValue(v)))) + filters = append(filters, fmt.Sprintf(sql, p.Next(), p.Next())) + args = append(args, k, string(must.NotFail(pjson.MarshalSingleValue(v)))) default: panic(fmt.Sprintf("Unexpected type of value: %v", v)) diff --git a/internal/handlers/pg/pgdb/query_test.go b/internal/handlers/pg/pgdb/query_test.go index 634d95730670..4519017b385e 100644 --- a/internal/handlers/pg/pgdb/query_test.go +++ b/internal/handlers/pg/pgdb/query_test.go @@ -289,7 +289,7 @@ func TestPrepareWhereClause(t *testing.T) { objectID := types.ObjectID{0x62, 0x56, 0xc5, 0xba, 0x0b, 0xad, 0xc0, 0xff, 0xee, 0xff, 0xff, 0xff} // WHERE clauses occurring frequently in tests - whereEq := " WHERE ((_jsonb->$1)::jsonb = $2)" + whereEq := " WHERE (_jsonb->$1)::jsonb = $2" whereEqOrContain := whereEq + " OR (_jsonb->$1)::jsonb @> $2" for name, tc := range map[string]struct { @@ -339,16 +339,13 @@ func TestPrepareWhereClause(t *testing.T) { expected: whereEq, }, "IDDotNotation": { - filter: must.NotFail(types.NewDocument("_id.doc", "foo")), - expected: " WHERE ((_jsonb#>$1)::jsonb = $2)", + filter: must.NotFail(types.NewDocument("_id.doc", "foo")), }, "DotNotation": { - filter: must.NotFail(types.NewDocument("v.doc", "foo")), - expected: " WHERE ((_jsonb#>$1)::jsonb = $2) OR (_jsonb#>$1)::jsonb @> $2", + filter: must.NotFail(types.NewDocument("v.doc", "foo")), }, "DotNotationArrayIndex": { - filter: must.NotFail(types.NewDocument("v.arr.0", "foo")), - expected: " WHERE ((_jsonb#>$1)::jsonb = $2) OR (_jsonb#>$1)::jsonb @> $2", + filter: must.NotFail(types.NewDocument("v.arr.0", "foo")), }, } { name, tc := name, tc diff --git a/internal/handlers/tigris/tigrisdb/query.go b/internal/handlers/tigris/tigrisdb/query.go index f36bea180095..25641429a3ac 100644 --- a/internal/handlers/tigris/tigrisdb/query.go +++ b/internal/handlers/tigris/tigrisdb/query.go @@ -18,7 +18,6 @@ import ( "context" "encoding/json" "fmt" - "strconv" "time" "github.com/tigrisdata/tigris-client-go/driver" @@ -93,8 +92,6 @@ func BuildFilter(filter *types.Document) (string, error) { res := map[string]any{} for k, v := range filter.Map() { - key := k // key can be either a single key string '"v"' or Tigris dot notation '"v.foo"' - // TODO https://github.com/FerretDB/FerretDB/issues/1940 if v == "" { continue @@ -106,7 +103,6 @@ func BuildFilter(filter *types.Document) (string, error) { continue } - // If the key is in dot notation translate it to a tigris dot notation var path types.Path var err error @@ -114,22 +110,10 @@ func BuildFilter(filter *types.Document) (string, error) { return "", lazyerrors.Error(err) } + // TODO dot notation https://github.com/FerretDB/FerretDB/issues/2069 + // TODO https://github.com/FerretDB/FerretDB/issues/1914 if path.Len() > 1 { - indexSearch := false - - // TODO https://github.com/FerretDB/FerretDB/issues/1914 - for _, k := range path.Slice() { - if _, err := strconv.Atoi(k); err == nil { - indexSearch = true - break - } - } - - if indexSearch { - continue - } - - key = path.String() // '"v.foo"' + continue } } @@ -143,7 +127,7 @@ func BuildFilter(filter *types.Document) (string, error) { return "", lazyerrors.Error(err) } - res[key] = json.RawMessage(rawValue) + res[k] = json.RawMessage(rawValue) default: panic(fmt.Sprintf("Unexpected type of field %s: %T", k, v)) } diff --git a/internal/handlers/tigris/tigrisdb/query_test.go b/internal/handlers/tigris/tigrisdb/query_test.go index 6b23899153e4..bf3eeed4cb06 100644 --- a/internal/handlers/tigris/tigrisdb/query_test.go +++ b/internal/handlers/tigris/tigrisdb/query_test.go @@ -275,12 +275,10 @@ func TestBuildFilter(t *testing.T) { expected: `{"_id":"foo"}`, }, "IDDotNotation": { - filter: must.NotFail(types.NewDocument("_id.doc", "foo")), - expected: `{"_id.doc":"foo"}`, + filter: must.NotFail(types.NewDocument("_id.doc", "foo")), }, "DotNotation": { - filter: must.NotFail(types.NewDocument("v.doc", "foo")), - expected: `{"v.doc":"foo"}`, + filter: must.NotFail(types.NewDocument("v.doc", "foo")), }, "DotNotationArrayIndex": { filter: must.NotFail(types.NewDocument("v.arr.0", "foo")),