Skip to content

Commit 0044e03

Browse files
authored
fix: Improvements to SET search_path for postgres explain plans (#5422)
### Brief description of Pull Request During dogfooding we discovered some cases where explain plans were failing to run due to missing or ambiguous schema names. This PR addresses several small issues with the `SET search_path` command which is meant to address these issues. ### Pull Request Details - Sets the search path first, before any other requests are issued, including creating the prepared statement. - Places the datname in double quotes to prevent syntax errors - Explicitly sets the search path for SESSION rather than depending upon the default. ### PR Checklist - [x] Tests updated
1 parent 79e138d commit 0044e03

File tree

2 files changed

+8
-8
lines changed

2 files changed

+8
-8
lines changed

internal/component/database_observability/postgres/collector/explain_plans.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -568,6 +568,11 @@ func (c *ExplainPlans) fetchExplainPlanJSON(ctx context.Context, qi queryInfo) (
568568
}
569569
defer conn.Close()
570570

571+
setSearchPathStatement := fmt.Sprintf("SET SESSION search_path TO \"%s\", public", qi.datname)
572+
if _, err := conn.ExecContext(ctx, setSearchPathStatement); err != nil {
573+
return nil, fmt.Errorf("failed to set search path: %w", err)
574+
}
575+
571576
preparedStatementName := strings.ReplaceAll(fmt.Sprintf("explain_plan_%s", qi.queryId), "-", "_")
572577
preparedStatementText := fmt.Sprintf("PREPARE %s AS %s", preparedStatementName, qi.queryText)
573578
logger := log.With(c.logger, "query_id", qi.queryId, "datname", qi.datname, "preparedStatementName", preparedStatementName, "preparedStatementText", preparedStatementText)
@@ -581,11 +586,6 @@ func (c *ExplainPlans) fetchExplainPlanJSON(ctx context.Context, qi queryInfo) (
581586
}
582587
}()
583588

584-
setSearchPathStatement := fmt.Sprintf("SET search_path TO %s, public", qi.datname)
585-
if _, err := conn.ExecContext(ctx, setSearchPathStatement); err != nil {
586-
return nil, fmt.Errorf("failed to set search path: %w", err)
587-
}
588-
589589
if _, err := conn.ExecContext(ctx, "SET plan_cache_mode = force_generic_plan"); err != nil {
590590
return nil, fmt.Errorf("failed to set plan cache mode: %w", err)
591591
}

internal/component/database_observability/postgres/collector/explain_plans_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2915,8 +2915,8 @@ func TestExplainPlanFetchExplainPlans(t *testing.T) {
29152915
require.Equal(t, "complex_aggregation_with_case.json", jsonFile.Name)
29162916
jsonData := jsonFile.Data
29172917

2918+
mock.ExpectExec("SET SESSION search_path TO \"testdb\", public").WillReturnResult(sqlmock.NewResult(0, 1))
29182919
mock.ExpectExec("PREPARE explain_plan_123456 AS select * from some_table where id = $1").WillReturnResult(sqlmock.NewResult(0, 1))
2919-
mock.ExpectExec("SET search_path TO testdb, public").WillReturnResult(sqlmock.NewResult(0, 1))
29202920
mock.ExpectExec("SET plan_cache_mode = force_generic_plan").WillReturnResult(sqlmock.NewResult(0, 1))
29212921
mock.ExpectQuery("EXPLAIN (FORMAT JSON) EXECUTE explain_plan_123456(null)").WillReturnRows(sqlmock.NewRows([]string{"json"}).AddRow(jsonData))
29222922
mock.ExpectExec("DEALLOCATE explain_plan_123456").WillReturnResult(sqlmock.NewResult(0, 1))
@@ -2982,8 +2982,8 @@ func TestExplainPlanFetchExplainPlans(t *testing.T) {
29822982
require.Equal(t, "complex_aggregation_with_case.json", jsonFile.Name)
29832983
jsonData := jsonFile.Data
29842984

2985+
mock.ExpectExec("SET SESSION search_path TO \"testdb\", public").WillReturnResult(sqlmock.NewResult(0, 1))
29852986
mock.ExpectExec("PREPARE explain_plan_123456 AS with cte as (select * from some_table where id = $1) select * from cte").WillReturnResult(sqlmock.NewResult(0, 1))
2986-
mock.ExpectExec("SET search_path TO testdb, public").WillReturnResult(sqlmock.NewResult(0, 1))
29872987
mock.ExpectExec("SET plan_cache_mode = force_generic_plan").WillReturnResult(sqlmock.NewResult(0, 1))
29882988
mock.ExpectQuery("EXPLAIN (FORMAT JSON) EXECUTE explain_plan_123456(null)").WillReturnRows(sqlmock.NewRows([]string{"json"}).AddRow(jsonData))
29892989
mock.ExpectExec("DEALLOCATE explain_plan_123456").WillReturnResult(sqlmock.NewResult(0, 1))
@@ -3049,8 +3049,8 @@ func TestExplainPlanFetchExplainPlans(t *testing.T) {
30493049
require.Equal(t, "complex_aggregation_with_case.json", jsonFile.Name)
30503050
jsonData := jsonFile.Data
30513051

3052+
mock.ExpectExec("SET SESSION search_path TO \"testdb\", public").WillReturnResult(sqlmock.NewResult(0, 1))
30523053
mock.ExpectExec("PREPARE explain_plan_123456 AS select * from some_table").WillReturnResult(sqlmock.NewResult(0, 1))
3053-
mock.ExpectExec("SET search_path TO testdb, public").WillReturnResult(sqlmock.NewResult(0, 1))
30543054
mock.ExpectExec("SET plan_cache_mode = force_generic_plan").WillReturnResult(sqlmock.NewResult(0, 1))
30553055
mock.ExpectQuery("EXPLAIN (FORMAT JSON) EXECUTE explain_plan_123456").WillReturnRows(sqlmock.NewRows([]string{"json"}).AddRow(jsonData))
30563056
mock.ExpectExec("DEALLOCATE explain_plan_123456").WillReturnResult(sqlmock.NewResult(0, 1))

0 commit comments

Comments
 (0)