diff --git a/cmd/ferretdb/main.go b/cmd/ferretdb/main.go index 67ef971cd748..6e7b994ea780 100644 --- a/cmd/ferretdb/main.go +++ b/cmd/ferretdb/main.go @@ -86,9 +86,8 @@ var cli struct { Test struct { RecordsDir string `default:"" help:"Testing: directory for record files."` - DisableFilterPushdown bool `default:"false" help:"Experimental: disable filter pushdown."` - EnableUnsafeSortPushdown bool `default:"false" help:"Experimental: enable unsafe sort pushdown."` - EnableOplog bool `default:"false" help:"Experimental: enable capped collections, tailable cursors and OpLog." hidden:""` + DisableFilterPushdown bool `default:"false" help:"Experimental: disable filter pushdown."` + EnableOplog bool `default:"false" help:"Experimental: enable capped collections, tailable cursors and OpLog." hidden:""` //nolint:lll // for readability Telemetry struct { @@ -390,9 +389,8 @@ func run() { MySQLURL: mySQLFlags.MySQLURL, TestOpts: registry.TestOpts{ - DisableFilterPushdown: cli.Test.DisableFilterPushdown, - EnableUnsafeSortPushdown: cli.Test.EnableUnsafeSortPushdown, - EnableOplog: cli.Test.EnableOplog, + DisableFilterPushdown: cli.Test.DisableFilterPushdown, + EnableOplog: cli.Test.EnableOplog, }, }) if err != nil { diff --git a/integration/pushdown.go b/integration/pushdown.go index 21a4cb2492a5..767aef30b226 100644 --- a/integration/pushdown.go +++ b/integration/pushdown.go @@ -55,13 +55,7 @@ func (res resultPushdown) FilterPushdownExpected(t testtb.TB) bool { } // SortPushdownExpected returns true if sort pushdown is expected for currently running backend. -// It checks if pushdown is enabled by flag. -// For capped collection, pushdown for recordID is done even if pushdown is not enabled by flag. -func (res resultPushdown) SortPushdownExpected(t testtb.TB, cappedCollection bool) bool { - if !setup.UnsafeSortPushdownEnabled() && cappedCollection { - res = allPushdown - } - +func (res resultPushdown) SortPushdownExpected(t testtb.TB) bool { return res.pushdownExpected(t) } diff --git a/integration/query_compat_test.go b/integration/query_compat_test.go index 93975fb693b5..bc2c158763bb 100644 --- a/integration/query_compat_test.go +++ b/integration/query_compat_test.go @@ -253,12 +253,12 @@ func TestQueryCappedCollectionCompat(t *testing.T) { }, "Sort": { sort: bson.D{{"_id", int32(-1)}}, - sortPushdown: pgPushdown, + sortPushdown: allPushdown, }, "FilterSort": { filter: bson.D{{"v", int32(42)}}, sort: bson.D{{"_id", int32(-1)}}, - sortPushdown: pgPushdown, + sortPushdown: allPushdown, }, "MultipleSortFields": { sort: bson.D{{"v", 1}, {"_id", int32(-1)}}, @@ -289,7 +289,7 @@ func TestQueryCappedCollectionCompat(t *testing.T) { doc := ConvertDocument(t, explainRes) sortPushdown, _ := doc.Get("sortPushdown") - assert.Equal(t, tc.sortPushdown.SortPushdownExpected(t, true), sortPushdown) + assert.Equal(t, tc.sortPushdown.SortPushdownExpected(t), sortPushdown) findOpts := options.Find() if tc.sort != nil { diff --git a/integration/query_test.go b/integration/query_test.go index e763ed820c31..5fad00521df3 100644 --- a/integration/query_test.go +++ b/integration/query_test.go @@ -906,7 +906,7 @@ func TestQueryCommandLimitPushDown(t *testing.T) { limit: 2, len: 2, filterPushdown: noPushdown, - limitPushdown: allPushdown, + limitPushdown: noPushdown, }, "IDFilterSort": { filter: bson.D{{"_id", "array"}}, @@ -993,16 +993,11 @@ func TestQueryCommandLimitPushDown(t *testing.T) { assert.NoError(t, err) - var msg string - - if !setup.UnsafeSortPushdownEnabled() && tc.sort != nil { - tc.limitPushdown = noPushdown - msg = "Sort pushdown is disabled, but target resulted with limitPushdown" - } - doc := ConvertDocument(t, res) limitPushdown, _ := doc.Get("limitPushdown") - assert.Equal(t, tc.limitPushdown.SortPushdownExpected(t, false), limitPushdown, msg) + assert.Equal(t, tc.limitPushdown.SortPushdownExpected(t), limitPushdown) + + var msg string if setup.FilterPushdownDisabled() { tc.filterPushdown = noPushdown diff --git a/integration/setup/listener.go b/integration/setup/listener.go index dddaacda86bf..1471eee43b00 100644 --- a/integration/setup/listener.go +++ b/integration/setup/listener.go @@ -159,9 +159,8 @@ func setupListener(tb testtb.TB, ctx context.Context, logger *zap.Logger) string HANAURL: *hanaURLF, TestOpts: registry.TestOpts{ - DisableFilterPushdown: *disableFilterPushdownF, - EnableUnsafeSortPushdown: *enableUnsafeSortPushdownF, - EnableOplog: true, + DisableFilterPushdown: *disableFilterPushdownF, + EnableOplog: true, }, } h, closeBackend, err := registry.NewHandler(handler, handlerOpts) diff --git a/integration/setup/setup.go b/integration/setup/setup.go index 001ccbd5fff3..a938053ff27d 100644 --- a/integration/setup/setup.go +++ b/integration/setup/setup.go @@ -59,8 +59,7 @@ var ( debugSetupF = flag.Bool("debug-setup", false, "enable debug logs for tests setup") logLevelF = zap.LevelFlag("log-level", zap.DebugLevel, "log level for tests") - disableFilterPushdownF = flag.Bool("disable-filter-pushdown", false, "disable filter pushdown") - enableUnsafeSortPushdownF = flag.Bool("enable-unsafe-sort-pushdown", false, "enable unsafe sort pushdown") + disableFilterPushdownF = flag.Bool("disable-filter-pushdown", false, "disable filter pushdown") ) // Other globals. diff --git a/integration/setup/test_helpers.go b/integration/setup/test_helpers.go index 8c25202fef4c..32e294fa5317 100644 --- a/integration/setup/test_helpers.go +++ b/integration/setup/test_helpers.go @@ -85,8 +85,3 @@ func SkipForMongoDB(tb testtb.TB, reason string) { func FilterPushdownDisabled() bool { return *disableFilterPushdownF } - -// UnsafeSortPushdownEnabled returns true if unsafe sort pushdown is enabled. -func UnsafeSortPushdownEnabled() bool { - return *enableUnsafeSortPushdownF -} diff --git a/internal/handlers/registry/hana.go b/internal/handlers/registry/hana.go index d932d05eaa14..2eddce8c3868 100644 --- a/internal/handlers/registry/hana.go +++ b/internal/handlers/registry/hana.go @@ -42,9 +42,8 @@ func init() { ConnMetrics: opts.ConnMetrics, StateProvider: opts.StateProvider, - DisableFilterPushdown: opts.DisableFilterPushdown, - EnableUnsafeSortPushdown: opts.EnableUnsafeSortPushdown, - EnableOplog: opts.EnableOplog, + DisableFilterPushdown: opts.DisableFilterPushdown, + EnableOplog: opts.EnableOplog, } h, err := handler.New(handlerOpts) diff --git a/internal/handlers/registry/mysql.go b/internal/handlers/registry/mysql.go index da4d14bbb3a6..fd204e003f27 100644 --- a/internal/handlers/registry/mysql.go +++ b/internal/handlers/registry/mysql.go @@ -38,9 +38,8 @@ func init() { ConnMetrics: opts.ConnMetrics, StateProvider: opts.StateProvider, - DisableFilterPushdown: opts.DisableFilterPushdown, - EnableUnsafeSortPushdown: opts.EnableUnsafeSortPushdown, - EnableOplog: opts.EnableOplog, + DisableFilterPushdown: opts.DisableFilterPushdown, + EnableOplog: opts.EnableOplog, } h, err := handler.New(handlerOpts) diff --git a/internal/handlers/registry/postgresql.go b/internal/handlers/registry/postgresql.go index af2ed9b14303..9f8614db4d24 100644 --- a/internal/handlers/registry/postgresql.go +++ b/internal/handlers/registry/postgresql.go @@ -38,9 +38,8 @@ func init() { ConnMetrics: opts.ConnMetrics, StateProvider: opts.StateProvider, - DisableFilterPushdown: opts.DisableFilterPushdown, - EnableUnsafeSortPushdown: opts.EnableUnsafeSortPushdown, - EnableOplog: opts.EnableOplog, + DisableFilterPushdown: opts.DisableFilterPushdown, + EnableOplog: opts.EnableOplog, } h, err := handler.New(handlerOpts) diff --git a/internal/handlers/registry/registry.go b/internal/handlers/registry/registry.go index 6fa0727b9e10..7deded26bdd0 100644 --- a/internal/handlers/registry/registry.go +++ b/internal/handlers/registry/registry.go @@ -61,9 +61,8 @@ type NewHandlerOpts struct { // TestOpts represents experimental configuration options. type TestOpts struct { - DisableFilterPushdown bool - EnableUnsafeSortPushdown bool - EnableOplog bool + DisableFilterPushdown bool + EnableOplog bool } // NewHandler constructs a new handler. diff --git a/internal/handlers/registry/sqlite.go b/internal/handlers/registry/sqlite.go index 4c871066df59..02d4d6ed86b2 100644 --- a/internal/handlers/registry/sqlite.go +++ b/internal/handlers/registry/sqlite.go @@ -38,9 +38,8 @@ func init() { ConnMetrics: opts.ConnMetrics, StateProvider: opts.StateProvider, - DisableFilterPushdown: opts.DisableFilterPushdown, - EnableUnsafeSortPushdown: opts.EnableUnsafeSortPushdown, - EnableOplog: opts.EnableOplog, + DisableFilterPushdown: opts.DisableFilterPushdown, + EnableOplog: opts.EnableOplog, } h, err := handler.New(handlerOpts) diff --git a/internal/handlers/sqlite/msg_aggregate.go b/internal/handlers/sqlite/msg_aggregate.go index aefb93eb6ab6..b7587e35cc42 100644 --- a/internal/handlers/sqlite/msg_aggregate.go +++ b/internal/handlers/sqlite/msg_aggregate.go @@ -259,7 +259,7 @@ func (h *Handler) MsgAggregate(ctx context.Context, msg *wire.OpMsg) (*wire.OpMs var iter iterator.Interface[struct{}, *types.Document] if len(collStatsDocuments) == len(stagesDocuments) { - filter, sort := aggregations.GetPushdownQuery(aggregationStages) + filter, _ := aggregations.GetPushdownQuery(aggregationStages) // only documents stages or no stages - fetch documents from the DB and apply stages to them qp := new(backends.QueryParams) @@ -268,25 +268,6 @@ func (h *Handler) MsgAggregate(ctx context.Context, msg *wire.OpMsg) (*wire.OpMs qp.Filter = filter } - // Skip sorting if there are more than one sort parameters - if h.EnableUnsafeSortPushdown && sort.Len() == 1 { - var order types.SortType - - k := sort.Keys()[0] - v := sort.Values()[0] - - order, err = common.GetSortType(k, v) - if err != nil { - closer.Close() - return nil, err - } - - qp.Sort = &backends.SortField{ - Key: k, - Descending: order == types.Descending, - } - } - iter, err = processStagesDocuments(ctx, closer, &stagesDocumentsParams{c, qp, stagesDocuments}) } else { // TODO https://github.com/FerretDB/FerretDB/issues/2423 diff --git a/internal/handlers/sqlite/msg_explain.go b/internal/handlers/sqlite/msg_explain.go index 99b84d24a27d..839247b1bdcd 100644 --- a/internal/handlers/sqlite/msg_explain.go +++ b/internal/handlers/sqlite/msg_explain.go @@ -87,31 +87,11 @@ func (h *Handler) MsgExplain(ctx context.Context, msg *wire.OpMsg) (*wire.OpMsg, qp.Filter, params.Sort = aggregations.GetPushdownQuery(params.StagesDocs) } - // Skip sorting if there are more than one sort parameters - // TODO https://github.com/FerretDB/FerretDB/issues/3742 - if h.EnableUnsafeSortPushdown && params.Sort.Len() == 1 { - var order types.SortType - - k := params.Sort.Keys()[0] - v := params.Sort.Values()[0] - - order, err = common.GetSortType(k, v) - if err != nil { - return nil, err - } - - qp.Sort = &backends.SortField{ - Key: k, - Descending: order == types.Descending, - } - } - // Limit pushdown is not applied if: // - `filter` is set, it must fetch all documents to filter them in memory; - // - `sort` is set but `UnsafeSortPushdown` is not set, it must fetch all documents - // and sort them in memory; + // - `sort` is set, it must fetch all documents and sort them in memory; // - `skip` is non-zero value, skip pushdown is not supported yet. - if params.Filter.Len() == 0 && (params.Sort.Len() == 0 || h.EnableUnsafeSortPushdown) && params.Skip == 0 { + if params.Filter.Len() == 0 && params.Sort.Len() == 0 && params.Skip == 0 { qp.Limit = params.Limit } @@ -119,10 +99,6 @@ func (h *Handler) MsgExplain(ctx context.Context, msg *wire.OpMsg) (*wire.OpMsg, qp.Filter = nil } - if !h.EnableUnsafeSortPushdown { - qp.Sort = nil - } - res, err := coll.Explain(ctx, &qp) if err != nil { return nil, lazyerrors.Error(err) diff --git a/internal/handlers/sqlite/msg_find.go b/internal/handlers/sqlite/msg_find.go index f2065a796300..1ffffed3deec 100644 --- a/internal/handlers/sqlite/msg_find.go +++ b/internal/handlers/sqlite/msg_find.go @@ -112,31 +112,11 @@ func (h *Handler) MsgFind(ctx context.Context, msg *wire.OpMsg) (*wire.OpMsg, er qp.Filter = params.Filter } - // Skip sorting if there are more than one sort parameters - // TODO https://github.com/FerretDB/FerretDB/issues/3742 - if h.EnableUnsafeSortPushdown && params.Sort.Len() == 1 { - var order types.SortType - - k := params.Sort.Keys()[0] - v := params.Sort.Values()[0] - - order, err = common.GetSortType(k, v) - if err != nil { - return nil, err - } - - qp.Sort = &backends.SortField{ - Key: k, - Descending: order == types.Descending, - } - } - // Limit pushdown is not applied if: // - `filter` is set, it must fetch all documents to filter them in memory; - // - `sort` is set but `UnsafeSortPushdown` is not set, it must fetch all documents - // and sort them in memory; + // - `sort` is set, it must fetch all documents and sort them in memory; // - `skip` is non-zero value, skip pushdown is not supported yet. - if params.Filter.Len() == 0 && (params.Sort.Len() == 0 || h.EnableUnsafeSortPushdown) && params.Skip == 0 { + if params.Filter.Len() == 0 && params.Sort.Len() == 0 && params.Skip == 0 { qp.Limit = params.Limit } diff --git a/internal/handlers/sqlite/sqlite.go b/internal/handlers/sqlite/sqlite.go index 388060da3cee..259eab2706f9 100644 --- a/internal/handlers/sqlite/sqlite.go +++ b/internal/handlers/sqlite/sqlite.go @@ -48,9 +48,8 @@ type NewOpts struct { StateProvider *state.Provider // test options - DisableFilterPushdown bool - EnableUnsafeSortPushdown bool - EnableOplog bool + DisableFilterPushdown bool + EnableOplog bool } // New returns a new handler.