zio / zio-quill

Compile-time Language Integrated Queries for Scala
https://zio.dev/zio-quill
Apache License 2.0
2.15k stars 348 forks source link

onConflictUpdate ignores schema #1527

Closed alexandru closed 5 years ago

alexandru commented 5 years ago

I may have hit a variation on #766 but I'm not sure.

Consider this schema:

  lazy val subscriberSearchViewTable = quote {
    querySchema[DBMyType](
      "my_table",
      _.id -> "id",
     //...
      _.envISO639Language -> "env_iso639_language",
    )
  }

And the type is nothing out of the ordinary:

final case class DBMyType(
  id: UUID,
  //...
  envISO639Language: Option[ISO639Language],
  //...
)

Things work (e.g. filter, insert, etc), except for onConflictUpdate:

    quote {
      subscriberSearchViewTable
        .insert(lift(row))
        .onConflictUpdate(_.id)(
          _.envISO639Language -> _.envISO639Language
        )
    }

The error this fails with is:

ERROR: column excluded.env_i_s_o639_language does not exist

What Quill does is to apply the NamingStrategy instead of my custom schema when updating that field.

The issue can be worked-around by renaming that field from envISO639Language to envIso639Language. But that's not possible in my case.

Version: 3.3.0

deusaquilus commented 5 years ago

Yes, this is the same as #766

mdedetrich commented 5 years ago

@deusaquilus The PR that was meant to fix #766 has been merged however this issue is still cropping up with quill 3.4.3, is there something additional that needs to be done to fix the .onConflictUpdate?

mdedetrich commented 5 years ago

@alexandru Fix is now in quill latest snapshot, will also be available in next release.

alexandru commented 5 years ago

Thanks a lot @mdedetrich, you've just made my day 🙂 since we had to change field names for working around this.

Cheers,

deusaquilus commented 5 years ago

This is awesome! Thank you @mdedetrich!

-Alex

On Fri, Sep 6, 2019 at 4:51 AM Alexandru Nedelcu notifications@github.com wrote:

Thanks a lot @mdedetrich https://github.com/mdedetrich, you've just made my day 🙂 since we had to change field names for working around this.

Cheers,

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/getquill/quill/issues/1527?email_source=notifications&email_token=AAKOLCA3I2DJCTU3SEK56I3QIIK2BA5CNFSM4IE2Z7F2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD6CGR5I#issuecomment-528771317, or mute the thread https://github.com/notifications/unsubscribe-auth/AAKOLCHVKKPRUYHMLXZMH2DQIIK2BANCNFSM4IE2Z7FQ .

kubukoz commented 5 years ago

So good to see this is fixed! Just hit that issue :D