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

Returned entity contains nonexistent field #2699

Closed
mguimas opened this issue Sep 21, 2018 · 2 comments · Fixed by #2702
Closed

Returned entity contains nonexistent field #2699

mguimas opened this issue Sep 21, 2018 · 2 comments · Fixed by #2702
Labels

Comments

@mguimas
Copy link

mguimas commented Sep 21, 2018

Environment

Ecto 2.2.10
Elixir 1.7.3
macOS High Sierra 10.13.6

Current behavior

Trying to change a nonexistent field in a post seems fine:

iex(56)> %Post{} |> change(%{nonexistent_field: "thing"}) |> apply_changes
%EctoAssoc.Post{
  __meta__: #Ecto.Schema.Metadata<:built, "posts">,
  body: nil,
  header: nil,
  id: nil,
  lock_version: 1,
  tags: #Ecto.Association.NotLoaded<association :tags is not loaded>,
  user: #Ecto.Association.NotLoaded<association :user is not loaded>,
  user_id: nil
}

Now doing similarly but this time inserting the post, the insert succeeds but
the returned struct contains the nonexistent_field field:

iex(57)> %Post{} |> change(%{nonexistent_field: "thing"}) |> Repo.insert!

14:44:41.979 [debug] QUERY OK db=1.7ms
begin []

14:44:41.983 [debug] QUERY OK db=3.1ms
INSERT INTO "posts" ("lock_version") VALUES ($1) RETURNING "id" [1]

14:44:41.985 [debug] QUERY OK db=2.6ms
commit []
%{
  __meta__: #Ecto.Schema.Metadata<:loaded, "posts">,
  __struct__: EctoAssoc.Post,
  body: nil,
  header: nil,
  id: 6,
  lock_version: 1,
  nonexistent_field: "thing",
  tags: #Ecto.Association.NotLoaded<association :tags is not loaded>,
  user: #Ecto.Association.NotLoaded<association :user is not loaded>,
  user_id: nil
}
iex(58)>

Expected behavior

Either one of these:

  1. Passing a nonexisting field to change or cast should raise an Ecto.UnknownFieldError

  2. The Repo operations must not add nonexisting fields to their results.

  3. some other solution

I prefer the first because a program using an unkown field clearly has a bug.

@qcam
Copy link
Member

qcam commented Sep 22, 2018

I could work on this if it has not been taken, based on the proposed behaviors above.

@josevalim
Copy link
Member

@qcam yes please!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants