Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add pg 16 to CI and remove :generic explain plan option #606

Merged
merged 2 commits into from
May 6, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,10 @@ jobs:
elixirbase:
- "1.11.4-erlang-21.3.8.24-alpine-3.13.3"
postgres:
- "15.0"
- "11.11"
- "9.6"
- "9.5"
- "16.2-alpine"
- "11.11-alpine"
- "9.6-alpine"
- "9.5-alpine"
steps:
- uses: earthly/actions-setup@v1
- uses: actions/checkout@v3
Expand Down
22 changes: 11 additions & 11 deletions Earthfile
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,10 @@ VERSION 0.6
all:
ARG ELIXIR_BASE=1.15.6-erlang-25.3.2.6-alpine-3.18.4
BUILD \
--build-arg POSTGRES=15.0 \
--build-arg POSTGRES=11.11 \
--build-arg POSTGRES=9.6 \
--build-arg POSTGRES=9.5 \
--build-arg POSTGRES=16.2-alpine \
--build-arg POSTGRES=11.11-alpine \
--build-arg POSTGRES=9.6-alpine \
--build-arg POSTGRES=9.5-alpine \
+integration-test-postgres

BUILD \
Expand Down Expand Up @@ -41,19 +41,19 @@ integration-test-postgres:
FROM +setup-base
ARG POSTGRES="11.11"

IF [ "$POSTGRES" = "9.5" ]
IF [ "$POSTGRES" = "9.5-alpine" ]
# for 9.5 we require a downgraded version of pg_dump;
# and in the 3.4 version, it is not included in postgresql-client but rather in postgresql
RUN echo 'http://dl-cdn.alpinelinux.org/alpine/v3.4/main' >> /etc/apk/repositories
RUN apk add postgresql=9.5.13-r0
ELSE IF [ "$POSTGRES" = "15.0" ]
# for 15.0 we need an upgraded version of pg_dump;
# alpine 3.17 does not come with the postgres 15 client by default;
ELSE IF [ "$POSTGRES" = "16.2-alpine" ]
# for 16 we need an upgraded version of pg_dump;
# alpine 3.16 does not come with the postgres 16 client by default;
# we must first update the public keys for the packages because they
# might have been rotated since our image was built
RUN apk add -X https://dl-cdn.alpinelinux.org/alpine/v3.17/main -u alpine-keys
RUN echo 'http://dl-cdn.alpinelinux.org/alpine/v3.17/main' >> /etc/apk/repositories
RUN apk add postgresql15-client
RUN apk add -X https://dl-cdn.alpinelinux.org/alpine/v3.19/main -u alpine-keys
RUN echo 'http://dl-cdn.alpinelinux.org/alpine/v3.19/main' >> /etc/apk/repositories
RUN apk add postgresql16-client
ELSE
RUN apk add postgresql-client
END
Expand Down
12 changes: 0 additions & 12 deletions integration_test/pg/explain_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -63,18 +63,6 @@ defmodule Ecto.Integration.ExplainTest do
assert explain =~ "(p0.title)::text = 'title'"
end

@tag :explain_generic
test "explain with generic plan" do
# when using generic plan, placeholders are used instead of values. i.e. $1 instead of 1
query = from p in Post, where: p.visits == ^1 and p.title == ^"title"

explain =
TestRepo.explain(:all, query, plan: :generic, analyze: true, verbose: true, timeout: 20000)

assert explain =~ "p0.visits = $1"
assert explain =~ "(p0.title)::text = $2"
end

test "explain MAP format" do
[explain] =
TestRepo.explain(:all, Post, analyze: true, verbose: true, timeout: 20000, format: :map)
Expand Down
10 changes: 3 additions & 7 deletions integration_test/pg/test_helper.exs
Original file line number Diff line number Diff line change
Expand Up @@ -102,22 +102,18 @@ excludes_above_9_5 = [:without_conflict_target]
excludes_below_9_6 = [:add_column_if_not_exists, :no_error_on_conditional_column_migration]
excludes_below_12_0 = [:plan_cache_mode]
excludes_below_15_0 = [:on_delete_nilify_column_list]
excludes_below_16_0 = [:explain_generic]

exclude_list = excludes ++ excludes_above_9_5

cond do
Version.match?(version, "< 9.6.0") ->
ExUnit.configure(exclude: exclude_list ++ excludes_below_9_6 ++ excludes_below_12_0 ++ excludes_below_15_0 ++ excludes_below_16_0)
ExUnit.configure(exclude: exclude_list ++ excludes_below_9_6 ++ excludes_below_12_0 ++ excludes_below_15_0)

Version.match?(version, "< 12.0.0") ->
ExUnit.configure(exclude: exclude_list ++ excludes_below_12_0 ++ excludes_below_15_0 ++ excludes_below_16_0)
ExUnit.configure(exclude: exclude_list ++ excludes_below_12_0 ++ excludes_below_15_0)

Version.match?(version, "< 15.0.0") ->
ExUnit.configure(exclude: exclude_list ++ excludes_below_15_0 ++ excludes_below_16_0)

Version.match?(version, "< 16.0.0") ->
ExUnit.configure(exclude: exclude_list ++ excludes_below_16_0)
ExUnit.configure(exclude: exclude_list ++ excludes_below_15_0)

true ->
ExUnit.configure(exclude: exclude_list)
Expand Down
10 changes: 2 additions & 8 deletions lib/ecto/adapters/postgres/connection.ex
Original file line number Diff line number Diff line change
Expand Up @@ -363,7 +363,8 @@ if Code.ensure_loaded?(Postgrex) do
~w[analyze verbose costs settings buffers timing summary format plan]a
)

fallback_generic? = explain_opts[:plan] == :fallback_generic
{plan_type, explain_opts} = Keyword.pop(explain_opts, :plan)
fallback_generic? = plan_type == :fallback_generic

result =
cond do
Expand All @@ -374,7 +375,6 @@ if Code.ensure_loaded?(Postgrex) do
"You may either change the plan type to `:custom` or remove the `:analyze` option."

fallback_generic? ->
explain_opts = Keyword.delete(explain_opts, :plan)
explain_queries = build_fallback_generic_queries(query, length(params), explain_opts)
fallback_generic_query(conn, explain_queries, opts)

Expand Down Expand Up @@ -465,12 +465,6 @@ if Code.ensure_loaded?(Postgrex) do
{:format, value}, acc ->
[String.upcase("#{format_to_sql(value)}") | acc]

{:plan, :generic}, acc ->
["GENERIC" | acc]

{:plan, _}, acc ->
acc

{opt, value}, acc ->
[String.upcase("#{opt} #{quote_boolean(value)}") | acc]
end)
Expand Down
12 changes: 6 additions & 6 deletions lib/ecto/adapters/sql.ex
Original file line number Diff line number Diff line change
Expand Up @@ -400,12 +400,12 @@ defmodule Ecto.Adapters.SQL do
* Postgrex: `:map`, `:yaml` and `:text`
* MyXQL: `:map` and `:text`

The `:plan` option in Postgres can take the values `:custom`, `:generic` or `:fallback_generic`.
When `:custom` is specified, the explain plan generated by Postgres will consider the specific values
of the query parameters that are supplied. When using `:generic` or `:fallback_generic`, the specific
values of the query parameters will be ignored. The difference between the two is that `:generic`
utilizes Postgres's built-in functionality (available since Postgres 16) and `:fallback_generic` is
a special implementation for earlier Postgres versions. Defaults to `:custom`.
The `:plan` option in Postgrex can take the values `:custom` or `:fallback_generic`. When `:custom`
is specified, the explain plan generated will consider the specific values of the query parameters
that are supplied. When using :fallback_generic`, the specific values of the query parameters will
be ignored. `:fallback_generic` does not use PostgreSQL's built-in support for a generic explain
plan (available as of PostgreSQL 16), but instead uses a special implmentation that works for PostgreSQL
versions 12 and above.
greg-rychlewski marked this conversation as resolved.
Show resolved Hide resolved

Any other value passed to `opts` will be forwarded to the underlying adapter query function, including
shared Repo options such as `:timeout`. Non built-in adapters may have specific behaviour and you should
Expand Down
Loading