erlangbureau / jamdb_oracle

Oracle Database driver for Erlang
MIT License
103 stars 46 forks source link

Insert returning generated 'identity' id fails with 'badarg' #146

Closed dfrese closed 5 months ago

dfrese commented 9 months ago

Hi there!

I'm getting an error trying an Ecto.Repo.insert on a table with a generated id column. With the current code from master, against an Oracle 19c server.

The table was created like this:

CREATE TABLE xxx (id number(38,0) GENERATED by default on null as IDENTITY, yyy varchar(100))

The corresponding schema:

  defmodule DbSchema.Xxx do
    use Ecto.Schema

    @primary_key {:id, :id, autogenerate: true}

    schema "xxx" do
      field :yyy, :string
    end
  end

And the insert is done like this (Database is my Ecto.Repo)

    Database.insert!(%Xxx{yyy: "bar"}, returning: false)

The error details and log output I get are:

[debug] QUERY ERROR db=0.0ms
INSERT INTO xxx (yyy) VALUES (:1) RETURN id INTO :id ["bar"]

     ** (DBConnection.ConnectionError) :badarg
     code: Database.insert!(%DbSchema.Xxx{yyy: "bar"}, returning: false)
     stacktrace:
       (ecto_sql 3.10.2) lib/ecto/adapters/sql.ex:1047: Ecto.Adapters.SQL.raise_sql_call_error/1
       (ecto 3.10.3) lib/ecto/repo/schema.ex:764: Ecto.Repo.Schema.apply/4
       (ecto 3.10.3) lib/ecto/repo/schema.ex:377: anonymous fn/15 in Ecto.Repo.Schema.do_insert/4
       (ecto 3.10.3) lib/ecto/repo/schema.ex:273: Ecto.Repo.Schema.insert!/4
       test/oracle_test.exs:35: (test)

Same without the 'returning' option, or 'returning [:id]'.

Any help much appreciated!

dfrese commented 9 months ago

I traced it down into jamdb_oracle_conn.erl and the sql_query function:

    Data = lists:filtermap(fun(L) -> if hd(L) =:= $: -> {true, list_to_integer(tl(L))}; true -> false end end,
        string:tokens(Query," \t\r\n;,()=")),

Seems this tries to parse :id like a query parameter :1 which are usually integers.

vstavskyi commented 9 months ago

Hi! Returning param must be declared as :out in opts Repo.query("insert into xxx (yyy) values (:1) return id into :2 ", ["aaa", out: :integer])

I'll try to implement this Database.insert!(%Xxx{yyy: "bar"}, [returning: false, out: [:integer]])

dfrese commented 9 months ago

Thank you!

I'll try too, in the mean time.

dfrese commented 9 months ago

Did a quick&dirty fix, by adding a second variant of Jamdb.Oracle.Query/returning

  defp returning(_, []),
    do: []
  defp returning(header, fields) do
    returning = fields |> Enum.filter(& is_tuple(&1) == false)
    offset = Enum.count(header)
    [" RETURN ", intersperse_map(returning, ", ", &quote_name/1),
     " INTO ", intersperse_map(Enum.to_list(1..Enum.count(returning)), ", ", fn i -> [?: | "#{offset + i}"] end)]
  end   

which takes 'header' from insert to append the correct indices of out params after the other params.

Calling Repo.insert(..., out: [:integer]) worked then!

I didn't look into Repo.delete and Repo.update.

(Feel free to use it, in case this isn't as dirty as I think ;-) )

vstavskyi commented 9 months ago

fixed Old returning now works

dfrese commented 9 months ago

Thanks! It works; but I still need to specify out: [:integer].

Unfortunately, I get the same error now for 'sub entities', i.e. a 'has_many' relation, where afaik Ecto generates multiple insert statements for one Repo.insert. I don't think I can add 'out:' there. So adding 'out:' implicitly based on 'returning' would imho be needed to fix that.