Skip to content

Add format option for PostgreSQL explain#295

Merged
josevalim merged 1 commit intoelixir-ecto:masterfrom
pawurb:explain_format
Jan 11, 2021
Merged

Add format option for PostgreSQL explain#295
josevalim merged 1 commit intoelixir-ecto:masterfrom
pawurb:explain_format

Conversation

@pawurb
Copy link
Copy Markdown
Contributor

@pawurb pawurb commented Jan 7, 2021

This PR adds JSON and YAML format outputs to explain for PostgreSQL databases:

Repo.explain(:all, Area, format: :json) |> Jason.decode!()

[
  %{
    "Plan" => %{
      "Alias" => "a0",
      "Node Type" => "Seq Scan",
      "Parallel Aware" => false,
      "Plan Rows" => 70,
      "Plan Width" => 1064,
      "Relation Name" => "areas",
      "Startup Cost" => 0.0,
      "Total Cost" => 10.7
    }
  }
]

Repo.explain(:all, Area, format: :yaml) |> IO.puts

- Plan:
    Node Type: "Seq Scan"
    Parallel Aware: false
    Relation Name: "areas"
    Alias: "a0"
    Startup Cost: 0.00
    Total Cost: 10.70
    Plan Rows: 70
    Plan Width: 1064

Especially the json format is useful because it can be used with query visualizer tool for some advanced debugging.

Comment thread lib/ecto/adapters/postgres/connection.ex
Comment thread lib/ecto/adapters/postgres/connection.ex Outdated
@josevalim
Copy link
Copy Markdown
Member

Thanks @pawurb, this looks great!

I only have one question: the YAML format is still returned as text while JSON returns a map. This feels a bit inconsistent to me. I wonder if instead we should introduce something like format: :map?

/cc @leandrocp

@pawurb
Copy link
Copy Markdown
Contributor Author

pawurb commented Jan 11, 2021

@josevalim I'm afraid that parsing YAML would require adding another dependency. How about we stick to the text format for all the outputs and leave the parsing for the client?

@pawurb pawurb requested a review from josevalim January 11, 2021 08:50
@josevalim
Copy link
Copy Markdown
Member

@josevalim I'm afraid that parsing YAML would require adding another dependency. How about we stick to the text format for all the outputs and leave the parsing for the client?

That's another route. We keep it all as text and format: :map can be a convenience added later on.

Comment thread integration_test/pg/explain_test.exs Outdated
Comment thread lib/ecto/adapters/postgres/connection.ex Outdated
@leandrocp
Copy link
Copy Markdown
Contributor

How about we stick to the text format for all the outputs and leave the parsing for the client?

Agreed, IMO that's the best option 👍🏻

Comment thread lib/ecto/adapters/postgres/connection.ex
@pawurb pawurb requested a review from josevalim January 11, 2021 15:39
@josevalim
Copy link
Copy Markdown
Member

Yes, looks good to me, sorry for the confusion! I think we should not provide JSON then until we figure out the Jaosn dependency, so I will remove it post-merge. Thank you!

@josevalim josevalim merged commit fc6c7a5 into elixir-ecto:master Jan 11, 2021
@josevalim
Copy link
Copy Markdown
Member

💚 💙 💜 💛 ❤️

@pawurb pawurb deleted the explain_format branch January 12, 2021 16:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants