-
Notifications
You must be signed in to change notification settings - Fork 11
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
DbCodec is not recursively applied #24
Comments
Hi, nested / embedded entities are not supported. I added a FAQ section to the readme in the linked MR, please review and let me know what you think. Cheers, |
The PR looks great and provides an alternative solution and you kind of answered my follow up question: whether this is a design decision or a technical limitation. It seems like its the former and I want to ask: do you see this feature at some point in the library? would you accept pull requests? |
I am biased against it; although I do agree it is a nice face-level developer experience, there's a lot of complexity introduced that can confuse users. For example, what if users want to make a table that embeds the same type multiple times? @Table(H2DbType, SqlNameMapper.CamelToSnakeCase)
case class Triangle(
pointA: Point,
pointB: Point,
pointC: Point
) derives DbCodec
case class Point(
x: Int,
y: Int
) derives DbCodec How would we handle the column naming? Or what about embeddings within embeddings within embeddings (etc)? (hard to reason about) Or what about when a class is embedded in multiple entity classes, but you refactor the column name for just one of those tables? (hard to maintain). For example see the hibernate docs on Embeddable: https://docs.jboss.org/hibernate/orm/6.5/userguide/html_single/Hibernate_User_Guide.html#embeddables I think that, since SQL itself does not support embeddings, we should not try to make entity tables appear like they're object-oriented. |
hey @AugustNagro , I wanna be a little bit annoying here because I think this feature would play very nicely with the overall design of your library. Think about this scenario case class Persisted[T](
@Id id: Long,
company: Company // as above
)
object CompanyRepo extends Repo[Company, Persisted[Company], String] now, the elegant solution here is to not having to redefine your entire entity as a creator, but rather compose the repo requirements. even in a generic way, useable throughout the whole app. I want to briefly address the other issues you mentioned, just for the sake of completeness
I completely understand you, as maintainer, not wanting to open that can of worms but imho the above scenario makes a strong case for this feature |
No worries, and I appreciate the feedback.. I'll do my best to be open minded and continue thinking about this usecase, especially in conjunction with adding UDT, JSON, & XML support. There's definitely pros to supporting embeddings, which is why Hibernate and others do so. Like you say, Persisted[T] would scrap the boilerplate when defining Entity/EntityBuilder pairs. I do like that example a lot. The downside is that it is a BIG feature to do right, and it's bringing us a lot closer to ORM-territory. So, I think the requirements would be:
|
I can help conceptually as I do not consider myself a proficient scala library developer. Few points of feedback:
I thank you for spending a few thoughts on this. I feel like this feature would be a win for the library without completely landing in orm land (with one-to-manies, lazy loading, etc...). Furthermore, I want to add that "repo" is, at least to me, a high level concept. an abstraction that allows interaction with the database without knowing too much about the database. currently it feels more like a "dao" which is closer to db (again, imho). This change, I'd say, would move us closer to "repo" as it can handle scala models as you would design them (more or less) without thinking in "rows". |
First of all, I am not a scala expert so it might be I am missing something obvious here.
I assumed the following would work (embedding values in entities)
but the address does not get "expanded" in the geneerated insert statement
again - if I am missing something obvious, I apologize, in that case consider this issue a suggestion for documentation
The text was updated successfully, but these errors were encountered: