Skip to content

Commit

Permalink
Merge branch 'ankane:master' into master
Browse files Browse the repository at this point in the history
  • Loading branch information
andyatkinson authored Dec 5, 2023
2 parents cec9cf9 + 2c3b4a8 commit 8c42de4
Show file tree
Hide file tree
Showing 12 changed files with 78 additions and 13 deletions.
6 changes: 3 additions & 3 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,10 @@ jobs:
matrix:
include:
- ruby: 3.2
gemfile: gemfiles/activerecord71.gemfile
gemfile: Gemfile
postgres: 16
- ruby: 3.1
gemfile: Gemfile
gemfile: gemfiles/activerecord70.gemfile
postgres: 15
- ruby: "3.0"
gemfile: gemfiles/activerecord61.gemfile
Expand All @@ -22,7 +22,7 @@ jobs:
env:
BUNDLE_GEMFILE: ${{ matrix.gemfile }}
steps:
- uses: actions/checkout@v3
- uses: actions/checkout@v4
- uses: ruby/setup-ruby@v1
with:
ruby-version: ${{ matrix.ruby }}
Expand Down
7 changes: 6 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
## 3.3.4 (unreleased)
## 3.4.0 (2023-11-28)

- Added support for explaining normalized queries with Postgres 16
- Added Docker image for `linux/arm64`

## 3.3.4 (2023-09-05)

- Fixed support for aliases in config file

Expand Down
2 changes: 1 addition & 1 deletion Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ gemspec
gem "minitest", ">= 5"
gem "rake"

gem "activerecord", "~> 7.0.0"
gem "activerecord", "~> 7.1.0"
gem "combustion"
gem "pg"
gem "pg_query"
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ A performance dashboard for Postgres

:tangerine: Battle-tested at [Instacart](https://www.instacart.com/opensource)

[![Build Status](https://github.com/ankane/pghero/workflows/build/badge.svg?branch=master)](https://github.com/ankane/pghero/actions) [![Docker Pulls](https://img.shields.io/docker/pulls/ankane/pghero)](https://hub.docker.com/r/ankane/pghero)
[![Build Status](https://github.com/ankane/pghero/actions/workflows/build.yml/badge.svg)](https://github.com/ankane/pghero/actions)

## Documentation

Expand Down
10 changes: 8 additions & 2 deletions app/controllers/pg_hero/home_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -293,12 +293,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"}
Expand All @@ -307,6 +309,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
Expand All @@ -320,8 +324,10 @@ 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")
"Can't explain queries with bind parameters"
elsif message.include?("EXPLAIN options ANALYZE and GENERIC_PLAN cannot be used together")
"Can't analyze queries with bind parameters"
elsif message.start_with?("PG::SyntaxError")
"Syntax error with query"
elsif message.start_with?("PG::QueryCanceled")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ gemspec path: ".."
gem "minitest", ">= 5"
gem "rake"

gem "activerecord", github: "rails/rails"
gem "activerecord", "~> 7.0.0"
gem "combustion"
gem "pg"
gem "pg_query"
5 changes: 3 additions & 2 deletions lib/pghero/methods/explain.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,19 +12,20 @@ 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
end

# 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)
Expand Down
2 changes: 1 addition & 1 deletion lib/pghero/methods/query_stats.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion lib/pghero/version.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
module PgHero
VERSION = "3.3.3"
VERSION = "3.4.0"
end
39 changes: 39 additions & 0 deletions test/controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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"}
Expand All @@ -88,6 +100,18 @@ 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
if explain_normalized?
assert_match "Can't analyze queries with bind parameters", response.body
else
assert_match "Can't explain queries with bind parameters", response.body
end
end

def test_explain_analyze_timeout
with_explain("analyze") do
with_explain_timeout(0.01) do
Expand Down Expand Up @@ -122,6 +146,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
Expand Down
10 changes: 10 additions & 0 deletions test/explain_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions test/test_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit 8c42de4

Please sign in to comment.