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

Incorrect value for field #2

Open
EvgenyAfanasev opened this issue Jun 24, 2023 · 9 comments
Open

Incorrect value for field #2

EvgenyAfanasev opened this issue Jun 24, 2023 · 9 comments

Comments

@EvgenyAfanasev
Copy link
Contributor

Hi. I have next model:

  @Table(PostgresDbType, SqlNameMapper.CamelToSnakeCase)
  final case class Modules(
      @Id id: Int,
      title: String,
      parentId: Int,
      description: Option[String]
  )

With next values:

Modules(1, "Module title", 2, Some("Module Description"))

And with next table definition:

id INT
title VARCHAR(255)
description VARCHAR(255)
course_id INT

And when I tried add entity using insertReturning method I received error

Incorrect value for int: Module Description

I checked columns in my table definition and see problem with order of model attributes. I have to create ordering of case class properties like in database for working. Is it correct behavior ? IMHO, developer hasn't to think about ordering tables columns for creating dao layer

@AugustNagro
Copy link
Owner

We can change that by using the ResultSet columnLabel methods instead of columnIndex. Like getInt(String) instead of getInt(int)

Another option is to add an @Order(int) annotation and then put documentation in the readme.

I think I like Option 1 better. That's what spring data does and it's very user friendly, despite being less efficient (ResultSet javadoc says "In general, using the column index will be more efficient")

WDYT?

@EvgenyAfanasev
Copy link
Contributor Author

Yeah, 1 option is more friendly

@AugustNagro
Copy link
Owner

Yep agreed. I think we will want to add readSingle(rs: ResultSet, columnName: String): E to DbCodec as well.

@EvgenyAfanasev
Copy link
Contributor Author

But what do think about separation DbCodec on two parts: DbCodec as reader by column names and DbIndexedCodec as reader by index ? I can take it to work

@AugustNagro
Copy link
Owner

AugustNagro commented Jun 25, 2023 via email

@lbialy
Copy link
Contributor

lbialy commented Jun 26, 2023

@EvgenyAfanasev just a quick remark - tuples don't make much sense with named args but they do make a lot of sense with positional args. I guess derivation will have to make that choice and go with labels for case classes and indexes for tuples.

@AugustNagro
Copy link
Owner

Right. If we end up going with readSingle(rs: ResultSet, columnName: String): E I think it's ok to throw UnsupportedOperationException for Tuples. We should also add a check in the RepoDefaults macro to make sure each case class field's dbCodec.cols.length == 1, since nesting gets weird when using names, and it's a good practice for entity classes to be flat anyway.

@lbialy
Copy link
Contributor

lbialy commented Jun 29, 2023

I'd still support tuples, they are awesome for arbitrary queries that pull small pieces of larger tables. Is it a big deal to support both labeled and positional writing/reading in DbCodec?

@cornerman
Copy link

Just read this thread, and thought that the proposal for named tuples in scala might relevant here as well: scala/improvement-proposals#72

I think, it would be great to support them when they are available.

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

No branches or pull requests

4 participants