field/3's :writable seems to ignore the :insert option

[lucavenir]

Elixir version

1.16.2

Database and Version

PostgreSQL 16.1

Ecto Versions

3.12.4

Database Adapter and Versions (postgrex, myxql, etc)

postgrex 0.19.1

Current behavior

I wanted to apply this :writable option mentioned in field/3' documentation. Unluckily writable: insert seems to be ignored.

Given this schema:

defmodule MyApp.Accounts.Account do
  use Ecto.Schema
  import Ecto.Changeset

  schema "accounts" do
    field :firebase_uid, :string, writable: :insert
    field :deactivated_at, :utc_datetime

    timestamps(type: :utc_datetime)
  end

  @doc false
  def changeset(account, attrs) do
    account
    |> cast(attrs, [:firebase_uid, :deactivated_at])
    |> validate_required([:firebase_uid])
  end
end

This tests fails:

test "update_account/2 doesn't allow firebase_uid to change" do
  account = account_fixture()
  invalid_attrs = %{ firebase_uid: "another different uid" }

  # this wont work
  # assert {:error, %Ecto.Changeset{}} = Accounts.update_account(account, invalid_attrs)
  # instead, these pass (???)
  {:ok, account} = Accounts.update_account(account, invalid_attrs)
  assert account.firebase_uid == "another different uid"  # !!
end

Expected behavior

I'd expect the previous test to have the commented parts to pass, and the uncommented parts to fail, i.e. I'd expect this test to pass:

test "update_account/2 doesn't allow firebase_uid to change" do
  account = account_fixture()
  invalid_attrs = %{ firebase_uid: "another different uid" }

  assert {:error, %Ecto.Changeset{}} = Accounts.update_account(account, invalid_attrs)
  # or
  # {:ok, account} = Accounts.update_account(account, invalid_attrs)
  # assert account.firebase_uid != "another different uid" 
end

(meaning: I'm not sure if the :insert option ignores the write or returns an error, but surely it shouldn't allow an edit!)

[greg-rychlewski]

Hi @lucavenir ,

Thank you for the report. I believe the update is silently not happening. We just filter the non-updatable fields out before performing the operation. Are you able to select from the database afterwards to see what the real value is? In the test I just did, this is the case.

Then I guess the question is should do something more than silently ignore.

[lucavenir]

Hello @gentoid,

Silently ignoring is coherent with much of Ecto's behaviors AFAIK. So it's fine for me.

But in the commented test above, I check for the updated struct, and I find it to be updated even though it should silently ignore it. I tried that as well and the test fails.

So you're not able to reproduce this? If that's the case I'll try and reproduce this in a sandbox.

[greg-rychlewski]

You might have to pull the data again with Repo.one/Repo.all, etc... instead of looking at just the return from the update.

If you are not using the returning option or you are using that option and nothing is updated, you will just see the changeset you proposed for update. Not necessarily what's in the database.

[lucavenir]

Hello @greg-rychlewski,

Indeed performing another Repo.one after Repo.update returns the untouched struct, so at a DB level that property works.

Now what's left (in my mind) is how to determine if this is expected behavior, or rather, a desired behavior.

Let me preface that I'm a 110% newbie on Ecto, but the APIs I've used up until now always return a struct representing the current state of the db after any operation. In other words, AFAICT, receiveing {:ok, struct} after a Repo call means: "here the new state on the database".
Sure, with :returning I can filter which fields I want, but I never ever touched that option.

About returning (update/3), I read:

Be aware that the fields returned from the database overwrite what was supplied by the user. Any field not returned by the database will be present with the original value supplied by the user.

If so, why wouldn't the database return firebase_uid in this case?
By the way, isn't returning: true a defult?

[josevalim]

Let me preface that I'm a 110% newbie on Ecto, but the APIs I've used up until now always return a struct representing the current state of the db after any operation.

When you have a changeset, we only send the fields you changed to the database. If any of the fields in the database have been changed meanwhile, that will actually not be reflected on the struct, unless you set read_after_writes for some particular fields (or via the :returning option). In other words, returning does not default to true (and it is not even supported by all databases).

There may still be arguments for changing the behaviour here though. Unsure. :)

[lucavenir]

Yeah I guess I didn't read the documentation well enough. Thanks for the quick response! 💕

I understand what the current behavior is, so, it's expected behavior. I fail to understand what value does this behavior bring. But I accept it now.

What my simple grug mind would think (or desire?) is this process:

• hey ecto, here's some changes, I validated them via a changeset
• great, I trust a changeset, but turns out this field is read-only, here's what has actually changed
• nice, this struct you've given me now represents the current state of the system

Instead, what grug receives now is: "here's the fields that effectively changed PLUS whatever you passed me in that might not have changed". Am I understanding this right? I'm not sure what I can make of this (again, consider that I'm a 110% newb).

I'd leave the debate to people way smarter than me, I just wanted to share my 2 cents as a person in a learning process.

In any case, knowing that returning: true isn't a default, I simply used in Repo.update/3 with such explicit option, and I'm good to go! It works and my use case is solved like that.

[josevalim]

In your particular case, I'd say the simplest solution would be to not cast firebase_uid at all during updates, but I believe you are right that we should not apply that field at all as part of changes. It may be that we are filtering it too late? So it doesn't make its way to the database, but it still makes its way to the record.

[lucavenir]

not cast firebase_uid at all during updates

Namely, defining a different changeset for updates, right? The (great, btw!!) community on discord gave me this first answer, but I wanted to explore and learn all the possibilities.

In your opinion, what's the most idiomatic way of solving my particular case? Having two changesets or exploiting this :writable prop?

[josevalim]

For now, the changeset approach is the way to go but we plan to fix what you reported so it just works. :)

[greg-rychlewski]

@lucavenir Thank you for the report and discussion. This never even crossed my mind.

[lucavenir]

Thank you guys!

You're breathtaking!

The fact that you've discussed and addressed this in less than 24h is awesome, beyond any possible imagination. Have a great day! 👍