Frost / isn

Postgrex.Extension and Ecto.Type for PostgreSQL isn module
10 stars 4 forks source link

Postgrex error on faulty ISBN #52

Open ArthurClemens opened 3 months ago

ArthurClemens commented 3 months ago

When importing a CSV with a list of books, I am getting a Postgrex error for an ISBN that contains a typo:

%Postgrex.Error{
  message: nil,
  postgres: %{
    code: :invalid_text_representation,
    line: "891",
    message: "invalid check digit for ISBN number: \"9783038440926\", should be 5",
    file: "isn.c",
    unknown: "ERROR",
    where: "unnamed portal parameter $7 = '...'",
    severity: "ERROR",
    pg_code: "22P02",
    routine: "string2ean"
  },
  connection_id: 71842,
  query: nil
}

Because the Ecto.Multi aborts the entire transaction, I've tried to catch the problem earlier, inside the changeset, where I am executing the query "SELECT '#{isbn}'::isbn13" to find any Postgrex errors.

Perhaps there is a more elegant way to do this?

Frost commented 3 months ago

Hi,

I'm on a vacation at the moment, and don't have a lot of time for coding, so it might take a while for me to rely or fix things right now.

I agree there should be a more elegant way to do that. I am, however, not sure if there is one at the moment.

It could possibly be nicer to get it as an ecto error instead.

This error stems directly from the postgres isn extension, and there is a mechanism in that extension to circumvent this check in cases where you know this is the correct ISBN and the check fails. There are some ISBN that are real but where the checksum for match.

I haven't gotten around to implementing something for that part yet, unfortunately. And it is of course not always what you want to do, because if it is a manually typed ISBN, there might be an actual typo, and in that case, there should be a more elegant way to handle the error.

If you have any suggestions on how you'd like that to look, I'm open to suggestions. :)

ArthurClemens commented 2 months ago

There's a couple of scenarios to handle:

  1. Input by user: validate and show any errors in the interface
  2. Batch insert: with 2 preferences a. Use weak mode to mark invalid entries b. Void invalid entries

Solutions:

  1. Use Repo.query("SELECT '#{isbn}'::isbn13") to detect wrong input, used in validate_isbn13 below.
  2. . a. To use weak mode, I've tried to add a question mark to the ISBN as the examples imply, but I get a different Postgres error: :numeric_value_out_of_range, message: "value \"9780679416289?\" is out of range for ISBN type", b. I'm using this changeset validation function that optionally ignores (and removes) wrong entries:
defp validate_isbn13(changeset, opts) do
  isbn = get_change(changeset, :isbn13)

  if is_nil(isbn) do
    changeset
  else
    validate_isbn13(changeset, isbn, opts)
  end
end

defp validate_isbn13(changeset, isbn, opts) do
  ignore_invalid_isbn = Keyword.get(opts, :ignore_invalid_isbn, false)

  cond do
    valid_isbn13?(isbn) ->
      changeset

    ignore_invalid_isbn ->
      delete_change(changeset, :isbn13)

    true ->
      add_error(changeset, :isbn13, dgettext("errors", "Invalid ISBN"))
  end
end

defp valid_isbn13?(isbn) do
  query = "SELECT '#{isbn}'::isbn13"

  case Repo.query(query) do
    {:ok, _} -> true
    _ -> false
  end
end