diff --git a/internal/handlers/pg/msg_explain.go b/internal/handlers/pg/msg_explain.go index bf364bdc85b6..3959a71896c5 100644 --- a/internal/handlers/pg/msg_explain.go +++ b/internal/handlers/pg/msg_explain.go @@ -59,7 +59,6 @@ func (h *Handler) MsgExplain(ctx context.Context, msg *wire.OpMsg) (*wire.OpMsg, } qp.Explain = true - qp.DisablePushdown = h.DisablePushdown explain, err := common.GetRequiredParam[*types.Document](document, "explain") if err != nil { @@ -71,6 +70,10 @@ func (h *Handler) MsgExplain(ctx context.Context, msg *wire.OpMsg) (*wire.OpMsg, return nil, lazyerrors.Error(err) } + if h.DisablePushdown { + qp.Filter = nil + } + var queryPlanner *types.Document err = dbPool.InTransaction(ctx, func(tx pgx.Tx) error { var err error diff --git a/internal/handlers/pg/pg.go b/internal/handlers/pg/pg.go index 67c7bf80be16..23562d56b923 100644 --- a/internal/handlers/pg/pg.go +++ b/internal/handlers/pg/pg.go @@ -42,11 +42,12 @@ type Handler struct { // NewOpts represents handler configuration. type NewOpts struct { - PostgreSQLURL string - DisablePushdown bool + PostgreSQLURL string + L *zap.Logger Metrics *connmetrics.ConnMetrics StateProvider *state.Provider + DisablePushdown bool } // New returns a new handler. diff --git a/internal/handlers/pg/pgdb/query.go b/internal/handlers/pg/pgdb/query.go index bb405ea765d4..cdd08aac0956 100644 --- a/internal/handlers/pg/pgdb/query.go +++ b/internal/handlers/pg/pgdb/query.go @@ -42,12 +42,11 @@ type FetchedDocs struct { // QueryParam represents options/parameters used for SQL query. type QueryParam struct { // Query filter for possible pushdown; may be ignored in part or entirely. - Filter *types.Document - DB string - Collection string - Comment string - Explain bool - DisablePushdown bool + Filter *types.Document + DB string + Collection string + Comment string + Explain bool } // Explain returns SQL EXPLAIN results for given query parameters. @@ -84,14 +83,8 @@ func Explain(ctx context.Context, tx pgx.Tx, qp *QueryParam) (*types.Document, e query += ` FROM ` + pgx.Identifier{qp.DB, table}.Sanitize() - var args []any - - if qp.Filter != nil && !qp.DisablePushdown { - var where string - - where, args = prepareWhereClause(qp.Filter) - query += where - } + where, args := prepareWhereClause(qp.Filter) + query += where rows, err := tx.Query(ctx, query, args...) if err != nil { @@ -189,12 +182,11 @@ func queryById(ctx context.Context, tx pgx.Tx, schema, table string, id any) (*t // iteratorParams contains parameters for building an iterator. type iteratorParams struct { - schema string - table string - comment string - explain bool - disablePushdown bool - filter *types.Document + schema string + table string + comment string + explain bool + filter *types.Document } // buildIterator returns an iterator to fetch documents for given iteratorParams. @@ -207,7 +199,7 @@ func buildIterator(ctx context.Context, tx pgx.Tx, p *iteratorParams) (iterator. query += `SELECT _jsonb ` - if c := p.comment; c != "" && !p.disablePushdown { + if c := p.comment; c != "" { // prevent SQL injections c = strings.ReplaceAll(c, "/*", "/ *") c = strings.ReplaceAll(c, "*/", "* /") @@ -217,14 +209,8 @@ func buildIterator(ctx context.Context, tx pgx.Tx, p *iteratorParams) (iterator. query += ` FROM ` + pgx.Identifier{p.schema, p.table}.Sanitize() - var args []any - - if p.filter != nil && !p.disablePushdown { - var where string - - where, args = prepareWhereClause(p.filter) - query += where - } + where, args := prepareWhereClause(p.filter) + query += where rows, err := tx.Query(ctx, query, args...) if err != nil { diff --git a/internal/handlers/registry/registry.go b/internal/handlers/registry/registry.go index 414a709f7ce6..27c7db15591b 100644 --- a/internal/handlers/registry/registry.go +++ b/internal/handlers/registry/registry.go @@ -44,7 +44,7 @@ type NewHandlerOpts struct { Logger *zap.Logger Metrics *connmetrics.ConnMetrics StateProvider *state.Provider - DisablePushdown bool // defaults to false + DisablePushdown bool // for `pg` handler PostgreSQLURL string @@ -85,7 +85,8 @@ func init() { registry["pg"] = func(opts *NewHandlerOpts) (handlers.Interface, error) { handlerOpts := &pg.NewOpts{ - PostgreSQLURL: opts.PostgreSQLURL, + PostgreSQLURL: opts.PostgreSQLURL, + L: opts.Logger, Metrics: opts.Metrics, StateProvider: opts.StateProvider, diff --git a/internal/handlers/registry/tigris.go b/internal/handlers/registry/tigris.go index d7c03da4cad4..0d1f7d1edf27 100644 --- a/internal/handlers/registry/tigris.go +++ b/internal/handlers/registry/tigris.go @@ -25,13 +25,15 @@ import ( func init() { registry["tigris"] = func(opts *NewHandlerOpts) (handlers.Interface, error) { handlerOpts := &tigris.NewOpts{ - ClientID: opts.TigrisClientID, - ClientSecret: opts.TigrisClientSecret, - Token: opts.TigrisToken, - URL: opts.TigrisURL, - L: opts.Logger, - Metrics: opts.Metrics, - StateProvider: opts.StateProvider, + ClientID: opts.TigrisClientID, + ClientSecret: opts.TigrisClientSecret, + Token: opts.TigrisToken, + URL: opts.TigrisURL, + + L: opts.Logger, + Metrics: opts.Metrics, + StateProvider: opts.StateProvider, + DisablePushdown: opts.DisablePushdown, } return tigris.New(handlerOpts) } diff --git a/internal/handlers/tigris/msg_explain.go b/internal/handlers/tigris/msg_explain.go index 7617f0884c0b..b52069da36b5 100644 --- a/internal/handlers/tigris/msg_explain.go +++ b/internal/handlers/tigris/msg_explain.go @@ -60,11 +60,12 @@ func (h *Handler) MsgExplain(ctx context.Context, msg *wire.OpMsg) (*wire.OpMsg, return nil, lazyerrors.Error(err) } - var queryFilter string - if !h.DisablePushdown { - queryFilter = string(tigrisdb.BuildFilter(filter)) + if h.DisablePushdown { + filter = nil } + queryFilter := string(tigrisdb.BuildFilter(filter)) + queryPlanner := must.NotFail(types.NewDocument( "Filter", queryFilter, )) diff --git a/internal/handlers/tigris/tigris.go b/internal/handlers/tigris/tigris.go index e7ad95a252d5..98426d42ab26 100644 --- a/internal/handlers/tigris/tigris.go +++ b/internal/handlers/tigris/tigris.go @@ -32,14 +32,15 @@ import ( // NewOpts represents handler configuration. type NewOpts struct { - ClientID string - ClientSecret string - Token string - URL string - DisablePushdown bool + ClientID string + ClientSecret string + Token string + URL string + L *zap.Logger Metrics *connmetrics.ConnMetrics StateProvider *state.Provider + DisablePushdown bool } // Handler implements handlers.Interface on top of Tigris. diff --git a/internal/handlers/tigris/tigrisdb/query.go b/internal/handlers/tigris/tigrisdb/query.go index 2f97275a069f..25ca88866370 100644 --- a/internal/handlers/tigris/tigrisdb/query.go +++ b/internal/handlers/tigris/tigrisdb/query.go @@ -34,10 +34,9 @@ import ( // QueryParam represents options/parameters used by the fetch/query. type QueryParam struct { // Query filter for possible pushdown; may be ignored in part or entirely. - Filter *types.Document - DB string - Collection string - DisablePushdown bool + Filter *types.Document + DB string + Collection string } // QueryDocuments fetches documents from the given collection. @@ -68,10 +67,7 @@ func (tdb *TigrisDB) QueryDocuments(ctx context.Context, param *QueryParam) (ite return nil, lazyerrors.Error(err) } - var filter driver.Filter - if !param.DisablePushdown { - filter = BuildFilter(param.Filter) - } + filter := BuildFilter(param.Filter) tdb.l.Sugar().Debugf("Read filter: %s", filter)