AugustNagro / magnum

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

added support for column and table names in repos #8

Closed lbialy closed 1 year ago

lbialy commented 1 year ago

This PR introduces column names and table name into ImmutableRepo and Repo allowing user to build more future-proof queries (addresses #4). For this to work it was also necessary to modify the logic of sql interpolator so that it works in two stages (it was initially considering everything that went into the interpolation as value input to be interpolated into PreparedStatement) - first it interpolates identifiers into the SQL query string and only then, after removing identifier type interpolation values goes to value embedding. I have also found out that despite table entity classes needing derives DbCodec it was impossible to use these classes in interpolation so I have fixed that in one breath and now it's completely legit to write sql"INSERT INTO $table $insertColumns VALUES ($p)" where p is a case class instance for which there's a DbCodec instance. This in turn replaces the idea for passing of the parameter strings (useless anyhow, it would require custom handling to properly fire DbCodec on such query anyhow, current solution is safer) mentioned in #4. There are tests for all of the supported databases but I have a M-macbook and can't run Oracle so fingers crossed that it doesn't blow up in CI.

AugustNagro commented 1 year ago

now it's completely legit to write sql"INSERT INTO $table $insertColumns VALUES ($p)" where p is a case class instance for which there's a DbCodec instance

Ok.. that's awesome.

can't run Oracle

I tested and it's green.

I've thought a little bit more about the goal of 'future-proof queries'.

The first thing is that future-proof queries have the benefit of greatly reducing boilerplate. They also minimize the need to make dozens of changes should a column name change.

A downside is that it's much more work to copy-paste into a SQL editor like DbBeaver. One other problem with the current implementation is that it doesn't totally eliminate the need to write sql identifiers. For example, top_speed in the added test:

val sql = sql"SELECT ${*} FROM $table WHERE top_speed > $speed"

Queries having multiple joins would be even worse. Also, summoning RepoDefault instances for the other tables in every method is probably bad for performance, since the deriving method is complex.

If we're going to offer a future-proof query dsl, we should go all-in.

I propose the following api:


case class User(id: Long, firstName: String, age: Int) derives DbCodec

case class UserCreator(firstName: String, age: Int) derives DbCodec

object Schema:
  val user = DbSchema[UserCreator, User, Long]

object UserRepo extends Repo[UserCreator, User, Long]:
    def firstNamesForLast(lastName: String)(using DbCon): Vector[String] =
    import Schema.user
    sql"""
      SELECT DISTINCT ${user.firstName}
      FROM $user
      WHERE ${user.lastName} = $lastName
    """.run

   def insertIgnoreDuplicates(insert: ProjectInsert)(using DbCon): Int =
      import Schema.projects
      sql"""
         INSERT OR IGNORE INTO $projects ${projects.insertCols}
         VALUES ($insert)
         """.update.run()

You might find it interesting that I partially implemented this many months ago:

https://github.com/AugustNagro/magnum/blob/d4eade8f17f4f46c8de8ad044358d210338723d7/README.md

It works by using structural types + transparent macro.

But I found that in my projects, renaming columns / tables happens very rarely. Meanwhile, having to copy queries to sql-editors like DbBeaver is super common. I also hadn't discovered the 'given RepoDefaults' pattern yet. So it was more verbose. One other downside is that IDEA doesn't support auto-completion for the structural accessors like userSchema.lastName (although metals does). It also doesn't work if you add an explicit type to val user since it loses the structural type.

So I'm pretty torn on the overall pros/cons. But there are many situations where having future-proof statements is a big win. So lets have it and let users decide.

When you address the minor comments I will merge your PR, then this weekend I'll do some refactoring and bring back DbSchema. Then I'll request you to review that PR.

AugustNagro commented 1 year ago

Actually, no need to make changes. I'll just merge now, then make the PR as I said.