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

Composite Type's oid resolving as generic "record" when param to Ecto SELECTs with WHERE #506

Closed
donaldguy opened this issue Apr 16, 2020 · 4 comments

Comments

@donaldguy
Copy link

donaldguy commented Apr 16, 2020

TL;DR

with an Ecto.Type which corresponds to a composite type, Repo.insert and Repo.all work fine, but as far as I can tell any attempt to select on the composite type column using Repo.one, Repo.get_by crashes as:

** (DBConnection.EncodeError) cannot encode anonymous tuple {"foo"}. Please define a custom Postgrex extension that matches on its underlying type:

    use Postgrex.BinaryExtension, type: "typeinthedb"

    (postgrex 0.15.3) lib/postgrex/type_module.ex:148: Postgrex.DefaultTypes.encode_tuple/3
    (postgrex 0.15.3) lib/postgrex/type_module.ex:897: Postgrex.DefaultTypes.encode_params/3
    (postgrex 0.15.3) lib/postgrex/query.ex:75: DBConnection.Query.Postgrex.Query.encode/3
    (db_connection 2.2.1) lib/db_connection.ex:1148: DBConnection.encode/5
    (db_connection 2.2.1) lib/db_connection.ex:1246: DBConnection.run_prepare_execute/5
    (db_connection 2.2.1) lib/db_connection.ex:1342: DBConnection.run/6
    (db_connection 2.2.1) lib/db_connection.ex:539: DBConnection.parsed_prepare_execute/5
    (db_connection 2.2.1) lib/db_connection.ex:532: DBConnection.prepare_execute/4

This seems to be a result of param_oids in the Query ending up with a consistently overly-general and non-working type for the relevant parameter

I tried to run this down further than that, but I could not for the life of me find my way through the describes and recvs and such in protocol.ex to answer the question of how param_oids are determined.

Full Repro

#!/bin/bash
asdf local erlang 22.3
asdf local elixir 1.10.2
mix new composite_fail &> /dev/null
mv .tool-versions composite_fail
cd composite_fail
sed -i '' -e 's/ # {:dep_from_hexpm, "~> 0.3.0"},/{:ecto_sql, "~> 3.1"},{:postgrex, ">= 0.0.0"}/' mix.exs
mix deps.get
mix deps.compile

mix ecto.gen.repo -r CompositeFail.Repo
cat << EOF > config/config.exs
import Config

config :composite_fail,
      ecto_repos: [CompositeFail.Repo]

config :composite_fail, CompositeFail.Repo,
  username: "postgres",
  password: "postgres",
  database: "composite_fail",
  hostname: "localhost",
  show_sensitive_data_on_connection_error: true,
  pool_size: 10
EOF

mix ecto.create

mix ecto.gen.migration  create

cat <<EOF > priv/repo/migrations/*create*
defmodule CompositeFail.Repo.Migrations.Create do
  use Ecto.Migration

  def change do
     execute("CREATE TYPE eg_t AS (d text);", "DROP TYPE eg_t")

     create table(:things) do
       add :eg_col, :eg_t
     end
  end
end
EOF

mix ecto.migrate

psql -d composite_fail -c "INSERT INTO things (eg_col) VALUES ('(\"example\")') ;"

cat <<EOF  > lib/composite_fail/eg_t.ex
defmodule CompositeFail.EgT do
  use Ecto.Type

  def type, do: :eg_t

  def cast(s), do: {:ok, s}

  def dump(s) when is_binary(s), do: {:ok, {s}}
  def dump(_), do: :error

  def load({s}), do: {:ok, s}

  def equal?(a,b), do: a == b
end
EOF

cat <<EOF > lib/composite_fail/thing.ex
defmodule CompositeFail.Thing do
  use Ecto.Schema

  schema "things" do
    field :eg_col, CompositeFail.EgT
  end
end
EOF

thereupon into iex -S mix

Erlang/OTP 22 [erts-10.7] [source] [64-bit] [smp:16:16] [ds:16:16:10] [async-threads:1] [hipe]

Interactive Elixir (1.10.2) - press Ctrl+C to exit (type h() ENTER for help)
iex(1)> alias CompositeFail.{Repo,Thing}
[CompositeFail.Repo, CompositeFail.Thing]
iex(2)> Repo.start_link()
{:ok, #PID<0.241.0>}
iex(3)> Repo.all(Thing)

00:51:00.869 [debug] %Postgrex.Query{cache: :reference, columns: nil, name: "ecto_460", param_formats: nil, param_oids: nil, param_types: nil, ref: nil, result_formats: nil, result_oids: nil, result_types: nil, statement: "SELECT t0.\"eg_col\" FROM \"things\" AS t0", types: nil} uses unknown oid(s) 21965forcing us to reload type information from the database. This is expected behaviour whenever you migrate your database.

00:51:00.876 [debug] QUERY OK source="things" db=0.3ms decode=0.7ms queue=11.4ms idle=6896.1ms
SELECT t0."eg_col" FROM "things" AS t0 []
[
  %CompositeFail.Thing{
    __meta__: #Ecto.Schema.Metadata<:loaded, "things">,
    eg_col: "example"
  }
]
iex(4)> #and 
nil
iex(5)> Repo.insert(%Thing{eg_col: "different"})

01:00:42.235 [debug] QUERY OK db=3.1ms queue=0.9ms idle=9988.0ms
INSERT INTO "things" ("eg_col") VALUES ($1) RETURNING "id" [{"different"}]
{:ok,
 %CompositeFail.Thing{
   __meta__: #Ecto.Schema.Metadata<:loaded, "things">,
   eg_col: "different",
   id: 2
 }}
iex(6)> #BUT!!!
nil
iex(7)> import Ecto.Query
Ecto.Query
iex(8)> Repo.one(from t in Thing, where: t.eg_col == ^"example")

01:02:07.851 [debug] QUERY ERROR source="things" db=3.4ms queue=1.6ms idle=9603.0ms
SELECT t0."id", t0."eg_col" FROM "things" AS t0 WHERE (t0."eg_col" = $1) [{"example"}]
** (DBConnection.EncodeError) cannot encode anonymous tuple {"example"}. Please define a custom Postgrex extension that matches on its underlying type:

    use Postgrex.BinaryExtension, type: "typeinthedb"

    (postgrex 0.15.3) lib/postgrex/type_module.ex:148: Postgrex.DefaultTypes.encode_tuple/3
    (postgrex 0.15.3) lib/postgrex/type_module.ex:897: Postgrex.DefaultTypes.encode_params/3
    (postgrex 0.15.3) lib/postgrex/query.ex:75: DBConnection.Query.Postgrex.Query.encode/3
    (db_connection 2.2.1) lib/db_connection.ex:1148: DBConnection.encode/5
    (db_connection 2.2.1) lib/db_connection.ex:1246: DBConnection.run_prepare_execute/5
    (db_connection 2.2.1) lib/db_connection.ex:1342: DBConnection.run/6
    (db_connection 2.2.1) lib/db_connection.ex:539: DBConnection.parsed_prepare_execute/5
    (db_connection 2.2.1) lib/db_connection.ex:532: DBConnection.prepare_execute/4

Digging further into that

iex(8)> require IEx
IEx
iex(9)> IEx.break! DBConnection.Query.Postgrex.Query.encode/3

iex(10)> Repo.one(from t in Thing, where: t.eg_col == ^"example")
Break reached: DBConnection.Query.Postgrex.Query.encode/3 (deps/postgrex/lib/postgrex/query.ex:72)

   70:   end
   71:
   72:   def encode(query, params, _) do
   73:     %{param_types: param_types, types: types} = query
   74:

pry(2)> query.param_oids
[2249]

pry(3)> query.param_types
[
  {Postgrex.Extensions.Record, nil,
   {Postgrex.DefaultTypes, #Reference<0.680458661.533331973.143696>}}
]

this is:

❯ psql -d composite_fail -c 'SELECT typname FROM pg_type WHERE oid = 2249'
 typname
---------
 record
(1 row)

whereas we'd want it to be (and it otherwise is here)

❯ psql -d composite_fail -c "SELECT oid FROM pg_type WHERE typname = 'eg_t'"
  oid
-------
 21965
(1 row)
@donaldguy donaldguy changed the title Composite type oid detected as generic "record" in Ecto Query where, crashing query Composite type oid detected as generic "record" in Ecto Queries, crashing query Apr 16, 2020
@donaldguy donaldguy changed the title Composite type oid detected as generic "record" in Ecto Queries, crashing query Composite type oid detected as generic "record" in Ecto SELECTs Apr 16, 2020
@donaldguy donaldguy changed the title Composite type oid detected as generic "record" in Ecto SELECTs Composite type oid detected as generic "record" in Ecto SELECTs with WHERE Apr 16, 2020
@donaldguy donaldguy changed the title Composite type oid detected as generic "record" in Ecto SELECTs with WHERE Composite type oid detected as generic "record" in Ecto SELECTs with WHERE on field Apr 16, 2020
@donaldguy
Copy link
Author

Also happens for e.g. Repo.get_by(Thing, %{eg_col: "different"})

@donaldguy donaldguy changed the title Composite type oid detected as generic "record" in Ecto SELECTs with WHERE on field Composite Type's oid resolving as generic "record" when param to Ecto SELECTs with WHERE Apr 16, 2020
@josevalim
Copy link
Member

Yes, please see #353. They are not currently supported. Contributions are welcome!

@donaldguy
Copy link
Author

donaldguy commented Apr 16, 2020

@josevalim I did see that issue fairly early on, but insofar as it describes specifically "If a composite type, such as a table row, changes its types" and talks at length in the frame of types whose definitions are changing,

I wasn't worried about that edge case and thought that surely a stable schema should be fine. It was not remotely clear to me that the upshot of that issue was that they are blanket unsupported in this way.

I'd consider at least removing the word "safely" from the title of that issue.

And ideally add some language about this being unsupported in the README? (the existing section there about OID decimal vs string encoding hints at issues like this, but its implication was deeply unclear to me)

I did not know this was not supposed to work. I spent fully 3 hours trying to make sure I wasn't making a mistake in my implementation


I also considered (obviously) following the advice of the error message and implementing a full extension as a workaround, but found how to do that altogether less documented than made that seem viable. I could not find a good example anywhere via Google or Github search.

Insofar as Postgrex seems in my limited experience to be the go to adapter for Ecto, I think some user friendliness work here could go a long way, though I understand how it gets pushed back.

Do you think the EEF would support this work?

Anyway, thanks

@josevalim
Copy link
Member

Fantastic feedback. I have updated the README and linked issued.

Do you think the EEF would support this work?

Someone could definitely write a proposal, the biggest question though is finding someone who would do the sponsored work.

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

No branches or pull requests

2 participants