diff --git a/CHANGELOG.md b/CHANGELOG.md index 7c02f1399..eb14e0949 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ ## 3.4.0 (unreleased) +- Added support for explaining normalized queries with Postgres 16 - Added Docker image for `linux/arm64` ## 3.3.4 (2023-09-05) diff --git a/app/controllers/pg_hero/home_controller.rb b/app/controllers/pg_hero/home_controller.rb index c7e1c4aee..d5adc4119 100644 --- a/app/controllers/pg_hero/home_controller.rb +++ b/app/controllers/pg_hero/home_controller.rb @@ -292,12 +292,14 @@ def explain # need to prevent CSRF and DoS if request.post? && @query.present? begin + generic_plan = @database.server_version_num >= 160000 && @query.include?("$1") + explain_options = case params[:commit] when "Analyze" {analyze: true} when "Visualize" - if @explain_analyze_enabled + if @explain_analyze_enabled && !generic_plan {analyze: true, costs: true, verbose: true, buffers: true, format: "json"} else {costs: true, verbose: true, format: "json"} @@ -306,6 +308,8 @@ def explain {} end + explain_options[:generic_plan] = true if generic_plan + if explain_options[:analyze] && !@explain_analyze_enabled render_text "Explain analyze not enabled", status: :bad_request return @@ -319,7 +323,7 @@ def explain @error = if message == "Unsafe statement" "Unsafe statement" - elsif message.start_with?("PG::ProtocolViolation: ERROR: bind message supplies 0 parameters") + elsif message.start_with?("PG::UndefinedParameter") || message.include?("EXPLAIN options ANALYZE and GENERIC_PLAN cannot be used together") "Can't explain queries with bind parameters" elsif message.start_with?("PG::SyntaxError") "Syntax error with query" diff --git a/lib/pghero/methods/explain.rb b/lib/pghero/methods/explain.rb index b2d94cdbe..f997eeddc 100644 --- a/lib/pghero/methods/explain.rb +++ b/lib/pghero/methods/explain.rb @@ -12,7 +12,7 @@ def explain(sql) if (sql.sub(/;\z/, "").include?(";") || sql.upcase.include?("COMMIT")) && !explain_safe? raise ActiveRecord::StatementInvalid, "Unsafe statement" end - explanation = select_all("EXPLAIN #{sql}").map { |v| v[:"QUERY PLAN"] }.join("\n") + explanation = execute("EXPLAIN #{sql}").map { |v| v["QUERY PLAN"] }.join("\n") end explanation @@ -20,11 +20,12 @@ def explain(sql) # TODO rename to explain in 4.0 # note: this method is not affected by the explain option - def explain_v2(sql, analyze: nil, verbose: nil, costs: nil, settings: nil, buffers: nil, wal: nil, timing: nil, summary: nil, format: "text") + def explain_v2(sql, analyze: nil, verbose: nil, costs: nil, settings: nil, generic_plan: nil, buffers: nil, wal: nil, timing: nil, summary: nil, format: "text") options = [] add_explain_option(options, "ANALYZE", analyze) add_explain_option(options, "VERBOSE", verbose) add_explain_option(options, "SETTINGS", settings) + add_explain_option(options, "GENERIC_PLAN", generic_plan) add_explain_option(options, "COSTS", costs) add_explain_option(options, "BUFFERS", buffers) add_explain_option(options, "WAL", wal) diff --git a/lib/pghero/methods/query_stats.rb b/lib/pghero/methods/query_stats.rb index 40fbe19b4..c86cd6390 100644 --- a/lib/pghero/methods/query_stats.rb +++ b/lib/pghero/methods/query_stats.rb @@ -319,7 +319,7 @@ def combine_query_stats(grouped_stats) end def explainable?(query) - query =~ /select/i && !query.include?("?)") && !query.include?("= ?") && !query.include?("$1") && query !~ /limit \?/i + query =~ /select/i && (server_version_num >= 160000 || (!query.include?("?)") && !query.include?("= ?") && !query.include?("$1") && query !~ /limit \?/i)) end # removes comments diff --git a/test/controller_test.rb b/test/controller_test.rb index 728c26825..97f50942d 100644 --- a/test/controller_test.rb +++ b/test/controller_test.rb @@ -62,6 +62,18 @@ def test_explain_only refute_match /Execution Time/i, response.body end + def test_explain_only_normalized + post pg_hero.explain_path, params: {query: "SELECT $1"} + assert_response :success + if explain_normalized? + assert_match "Result (cost=0.00..0.01 rows=1 width=32)", response.body + refute_match /Planning Time/i, response.body + refute_match /Execution Time/i, response.body + else + assert_match "Can't explain queries with bind parameters", response.body + end + end + def test_explain_only_not_enabled with_explain(false) do post pg_hero.explain_path, params: {query: "SELECT 1"} @@ -88,6 +100,14 @@ def test_explain_analyze assert_match /Execution Time/i, response.body end + def test_explain_analyze_normalized + with_explain("analyze") do + post pg_hero.explain_path, params: {query: "SELECT $1", commit: "Analyze"} + end + assert_response :success + assert_match "Can't explain queries with bind parameters", response.body + end + def test_explain_analyze_timeout with_explain("analyze") do with_explain_timeout(0.01) do @@ -122,6 +142,21 @@ def test_explain_visualize_analyze assert_match "Actual Total Time", response.body end + def test_explain_visualize_normalized + with_explain("analyze") do + post pg_hero.explain_path, params: {query: "SELECT $1", commit: "Visualize"} + end + assert_response :success + + if explain_normalized? + assert_match "https://tatiyants.com/pev/#/plans/new", response.body + assert_match ""Node Type": "Result"", response.body + refute_match "Actual Total Time", response.body + else + assert_match "Can't explain queries with bind parameters", response.body + end + end + def test_tune get pg_hero.tune_path assert_response :success diff --git a/test/explain_test.rb b/test/explain_test.rb index f28b72163..13e4a49ab 100644 --- a/test/explain_test.rb +++ b/test/explain_test.rb @@ -54,6 +54,16 @@ def test_explain_v2_analyze end end + def test_explain_v2_generic_plan + assert_raises(ActiveRecord::StatementInvalid) do + database.explain_v2("SELECT $1") + end + + if explain_normalized? + assert_match "Result", database.explain_v2("SELECT $1", generic_plan: true) + end + end + def test_explain_v2_format_text assert_match "Result (cost=", database.explain_v2("SELECT 1", format: "text") end diff --git a/test/test_helper.rb b/test/test_helper.rb index 775bf8daa..9f39481f9 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -21,6 +21,10 @@ def with_explain_timeout(value) yield end end + + def explain_normalized? + database.server_version_num >= 160000 + end end logger = ActiveSupport::Logger.new(ENV["VERBOSE"] ? STDERR : nil)