AugustNagro / magnum

A 'new look' for database access in Scala
Apache License 2.0
153 stars 10 forks source link

Support for Arrays of Enums (Postgres) #36

Open Nexus6 opened 1 month ago

Nexus6 commented 1 month ago

Version: magnum/magnumpg 1.2.1

Let's say I have a Scala enum of

enum Color derives DbCodec:
  case Red, Green, Blue

I've been experimenting with Magnum+PG, and it seems like Table column types of Color work fine - serialization to strings in the db is not a problem. Where I have db tables with column types like text[], using Scala types like List[String] to represent them also seems to work fine (I've only tested queries so far, not inserts/updates) with Magnum. But trying to specify column types like List[Color], as in

@Table(PostgresDbType, SqlNameMapper.CamelToSnakeCase)
final case class Rainbow(
  @Id id:Int,
  colors:List[Color]
) derives DbCodec

...gets me a compile error: "Cannot find a DbCodec instance for scala.collection.immutable.List[Color] ".

Since enum support is partially there (at least for things that can be serialized to strings), and Array support is there, it would make sense to me that they work together.

Is this something that just isn't implemented yet, or something that won't be available with Magnum out of the box? Are there any work-arounds that come to mind?

Even better than having the above working, would be having it work with PG's native enum types and Array types, eg. color[] columns where 'color' is a user-defined PG enum type.

AugustNagro commented 4 weeks ago

Is this something that just isn't implemented yet, or something that won't be available with Magnum out of the box

Thanks for filing; support just isn't implemented yet. Please review & try https://github.com/AugustNagro/magnum/pull/37 when you get a chance.

Even better than having the above working, would be having it work with PG's native enum types and Array types, eg. color[] columns where 'color' is a user-defined PG enum type.

In the linked MR, you will see there's tests for both

Array of Pg Enum <-> Seq of Scala Enum Array of Pg Varchar <-> Seq of Scala Enum

They require different DbCodec givens, since the JDBC array types are different for each case.

Nexus6 commented 4 weeks ago

Fantastic, thanks for the quick feature-add, I'll test #37 in the next few days. If it works I'll be able to remove some hacky work-around code from my project!

Related: will this new feature also support the "Array of Pg text" <-> Seq of Scala Enum" case? I suspect the answer is 'yes' and if so it might be nice to have that in the docs somewhere simply because it's probably common for devs to use 'text' types instead of 'varchar' types when they need to store strings in PG, including in arrays.

AugustNagro commented 4 weeks ago

will this new feature also support the "Array of Pg text" <-> Seq of Scala Enum" case

Yes; it will definitely support this; JDBC considers both text and varchar to be JDBCType.VARCHAR. I added to the readme:

If instead your Postgres type is an array of varchar or text, use the following import: 
Nexus6 commented 3 weeks ago

I gave the new branch a test-drive last night. There's something lacking in my SBT test environment. First, running "sbt test" resulted in a bunch of error messages about a missing .sbt/.credentials file. Second, despite having docker installed on my dev machine (I use it to run my dev postgresql server and a few other apps), there were a bunch of errors like ==> X PgCodecTests.beforeAllFixture(ForAllTestContainers) 0.004s java.lang.IllegalStateException: Could not find a valid Docker environment. Please see logs and check configuration

I was able to build and deploy the library successfully so I decided to just hack-in some test code on one of my projects and test with that. Am hitting some snags, in part probably because I'm trying a few things that maybe weren't planned for. So a few Qs for the moment:

  1. I'm testing mainly using SQL fragments and interpolation, haven't tried Repos yet in these tests. Should I expect these both (Repos and interpolation) to work right now?
  2. It seems like I've seen runtime errors caused by Magnum not respecting @SqlName annotations on enums. For example if I have a ColorModel enum backed by a Pg color[] enum column, and annotated with @SqlName("color"), Magnum still goes looking for a colormodel[] pg column and of course there's an error when it doesn't find it.
  3. Should there be an @Enum annotation, analogous to @Table, that would encapsulate information about the nature of the enum as represented in the db?
  4. Just to cause trouble, I decided to create a table with two enum columns, one backed by a text[] column in the db, and one backed by an enum[] column. This seemed to cause a lot of problems, either at compile time or runtime depending on which imports I included. Should this be expected to work now? I would imagine for folks who inherited db schemas that they're not allowed to change, it might be important to have enum array support in Magnum that can handle disparate array column types (varchar[], text[], enum[]) within the same table. Maybe that @Enum annotation could help here?
AugustNagro commented 3 weeks ago

Thanks for trying it out and this is great feedback. I'll be able to take action & respond in detail next week when things are less busy.

On Mon, Aug 19, 2024, 1:15 PM Eric Anderson @.***> wrote:

I gave the new branch a test-drive last night. There's something lacking in my SBT test environment. First, running "sbt test" resulted in a bunch of error messages about a missing .sbt/.credentials file. Second, despite having docker installed on my dev machine (I use it to run my dev postgresql server and a few other apps), there were a bunch of errors like ==> X PgCodecTests.beforeAllFixture(ForAllTestContainers) 0.004s java.lang.IllegalStateException: Could not find a valid Docker environment. Please see logs and check configuration

I was able to build and deploy the library successfully so I decided to just hack-in some test code on one of my projects and test with that. Am hitting some snags, in part probably because I'm trying a few things that maybe weren't planned for. So a few Qs for the moment:

  1. I'm testing mainly using SQL fragments and interpolation, haven't tried Repos yet in these tests. Should I expect these both (Repos and interpolation) to work right now?
  2. It seems like I've seen runtime errors caused by Magnum not respecting @SqlName annotations on enums. For example if I have a ColorModel enum backed by a Pg color[] enum column, and annotated with @SqlName("color"), Magnum still goes looking for a colormodel[] pg column and of course there's an error when it doesn't find it.
  3. Should there be an @Enum annotation, analogous to @Table, that would encapsulate information about the nature of the enum as represented in the db?
  4. Just to cause trouble, I decided to create a table with two enum columns, one backed by a text[] column in the db, and one backed by an enum[] column. This seemed to cause a lot of problems, either at compile time or runtime depending on which imports I included. Should this be expected to work now? I would imagine for folks who inherited db schemas that they're not allowed to change, it might be important to have enum array support in Magnum that can handle disparate array column types (varchar[], text[], enum[]) within the same table. Maybe that @Enum annotation could help here?

— Reply to this email directly, view it on GitHub https://github.com/AugustNagro/magnum/issues/36#issuecomment-2297370418, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABQOKNQ72RA5G265V2KLP7DZSJG5XAVCNFSM6AAAAABMQ6H7JKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEOJXGM3TANBRHA . You are receiving this because you commented.Message ID: @.***>

Nexus6 commented 3 weeks ago

Re: my previous comment...

I've opened a new issue #41 for the .sbt/.credentials warnings. The docker+testcontainers test dependency was a bigger PITA but I think I've got a working setup now, the evidence being that "sbt test" now completes successfully on my dev machine. [info] Passed: Total 6, Failed 0, Errors 0, Passed 6 [info] Passed: Total 203, Failed 0, Errors 0, Passed 203

This area - (Magnum) developer docs - could use a lot of improvement, there's just one sentence devoted to it now unless I've missed something. I should probably open a separate Issue on this.

Nexus6 commented 2 weeks ago

Regarding my problem with getting @SqlName to work with enums: the problem seems to go away - @SqlName annotations on enums are respected, when I do not also annotate the enum with @Table.

The following works - it both compiles and passes the project's PgCodecTests, when all Color->ColorModel references are changed of course::

import com.augustnagro.magnum.{DbCodec, SqlName}

@SqlName("Color")
enum ColorModel derives DbCodec:
  case Red, Green, Blue

The following:,

import com.augustnagro.magnum.*

@Table(PostgresDbType, SqlNameMapper.CamelToSnakeCase)
@SqlName("Color")
enum ColorModel derives DbCodec:
  case Red, Green, Blue

fails tests with errors like:

PgCodecTests:
==> X PgCodecTests.select all MagUser  0.413s com.augustnagro.magnum.SqlException: Error executing query:
SELECT id, name, friends, matrix, test, dates, bx, c, iv, l, lSeg, p, pnt, poly, colors, colorMap FROM mag_user
With message:
Red not convertible to ColorModel

You might ask why I was inclined to add the @Table annotation to the Scala enum definition in the first place. The answer is that the example in the documentation, in the "Scala 3 Enum & NewType Support" section, shows exactly this, so I thought @Table was some kind of required magic to make PG enum support work. Instead, it's not necessary at all, and actually breaks @SqlName functionality on enums.