Add support for conflict_target: {:unsafe_fragment, _}

[ewildgoose]

Sqlite does support ON CONFLICT DO upsert feature, eg: https://www.sqlite.org/lang_UPSERT.html

As part of this I synced a couple of function definitions to match current upstream postgrex as part of this. This is a bit stark as they use a different formatting option. Does this fit with your goals?

Note my goal here was to be able to do something like:

Repo.insert(
  %__MODULE__{
    device_id: device_id,
    sim_id: sim_id,
    device_type: device_type
  },
  on_conflict: {:replace_all_except, [:id]},
  conflict_target: {:unsafe_fragment, ~s<("device_id",coalesce("sim_id", ""))>},
  returning: true

Where the goal is to simulate a unique constraint on a column which accepts nulls (default for sqlite is to allow multiple "unique" rows in a nullable column)

[warmwaffles]

I approved the test suite to run. @ewildgoose feel free to push changes and what not.

[ewildgoose]

OK, so this looks like I've introduced some bug, probably syncing with (what I'll call upstream)

Error is: :erlang.iolist_to_binary([[], ["UPDATE ", [[34, "users", 34]], " AS ", [117 | "0"], " SET "], [[:name, " = ", 63]], [], []])

I assume it's the improper list [117 | "0"] which is breaking things?

I've copied a couple of what I thought were indifferent changes from "upstream" where they appear to do a list cons when returning a value, rather than returning the longer list. I would hazard a guess there is somewhere sending a value not wrapped in a list, which then gets turned into an improper list

I could remove some of the changes, but I wonder if you can guide on what you see as "upstream"? Is it work a slightly larger sync change to make our code look more substantially similar to postgrex? (ease of comparison/digression being the only motivation)

[warmwaffles]

Can you link me to the sections you copied upstream? I want to take a look at the blame and do some digging.

[ewildgoose]

Hi, see comment above. Main link is this: https://github.com/elixir-ecto/ecto_sql/blob/master/lib/ecto/adapters/postgres/connection.ex#L176

I had to merge it because of the extra clause we have forbidding using named contraints.

I also added the error message at the base of the file.

Then I jumped to a wrong conclusion, which is why I synced "update_fields" as well.

The only useful functional line that I've added is line 667:

  defp conflict_target({:unsafe_fragment, fragment}),
    do: [fragment, ?\s]

The rest is just syncing with ecto_sql (which may or may not be desired?) Edit: Sorry, I keep saying "postgrex", when I really mean ecto_sql/postgres

[warmwaffles]

Sorry, I keep saying "postgrex", when I really mean ecto_sql/postgres

Don't worry it confuses me as well.

[ewildgoose]

I presume it's this change causing the issue (or the update_fields() change, but can't see how that causes it?)

-    [
-      cte,
-      prefix,
-      fields,
-      join,
-      where,
-      returning(query, sources)
-    ]
+    [cte, prefix, fields, join, where | returning(query, sources)]