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

Update TypeMapper.scala added support for Float data type #32

Merged
merged 5 commits into from
Sep 26, 2024

Conversation

rmorales-iaa
Copy link
Contributor

No description provided.

@lihaoyi
Copy link
Member

lihaoyi commented Sep 15, 2024

@rmorales-iaa seems like FloatType does not compile, might need some more tweaks

@rmorales-iaa
Copy link
Contributor Author

I have completed the definitions for the 'Float' data type, and it should compile now

@rmorales-iaa
Copy link
Contributor Author

Right now, I am importing GAIA_DR_3 (https://gea.esac.esa.int/archive/documentation/GDR3/Gaia_archive/chap_datamodel/sec_dm_main_source_catalogue/ssec_dm_gaia_source.html) into PostgreSQL 16.3 with the modified version that supports Floats. Inspecting the columns created in the database:

ra: float8
ra_error: float4

This corresponds to the definition in scalasql:

case class SourceMinimal[T[_]](
...
, ra: T[Double]
, ra_error: T[Float]
...
)

Here are the repositories with the source code for the import:

Importer:
https://gitlab.com/rmorales_iaa/henosis/-/blob/main/src/main/scala/com/henosis/database/gaia/gaiaDR_3/gaia_source/DB_Importer.scala?ref_type=heads

Case class:
https://gitlab.com/rmorales_iaa/common_scala/-/blob/main/database/postgreDB/gaia_dr_3/SourceMinimal.scala?ref_type=heads"

@lihaoyi
Copy link
Member

lihaoyi commented Sep 23, 2024

@rmorales-iaa can you run the autoformatting and autofixes according to https://github.com/com-lihaoyi/scalasql/blob/main/docs/developer.md#code-formatting-and-auto-fixes

@rmorales-iaa
Copy link
Contributor Author

Uhmm, the patch works for table insertion but not for table selection.

"could not find implicit value for parameter e: scalasql.Queryable.Row[scalasql.core.Expr[Float],Float]
object gaia_source_minimal extends TableSourceMinimal "

If I find a solution, I will update this pull request.

@rmorales-iaa
Copy link
Contributor Author

False alarm. I didn't properly update the dependencies to the new patch. My entire project with scalasql compiles now

@rmorales-iaa
Copy link
Contributor Author

I've some errors runnig

./mill -i -w __.fix + mill.scalalib.scalafmt.ScalafmtModule/reformatAll __.sources + "scalasql[2.13.12].test" + generateTutorial + generateReference

scalasql.ExampleTests.postgres 10ms
java.lang.NoClassDefFoundError: Could not initialize class scalasql.example.PostgresExample$
scalasql.ExampleTests$.$anonfun$tests$2(ExampleTests.scala:9)
java.lang.ExceptionInInitializerError: Exception java.lang.IllegalStateException: Could not find a valid Docker environment. Please see logs and check configuration [in thread "main"]
org.testcontainers.dockerclient.DockerClientProviderStrategy.lambda$getFirstValidStrategy$7(DockerClientProviderStrategy.java:277)
java.util.Optional.orElseThrow(Optional.java:408)
org.testcontainers.dockerclient.DockerClientProviderStrategy.getFirstValidStrategy(DockerClientProviderStrategy.java:268)
org.testcontainers.DockerClientFactory.getOrInitializeStrategy(DockerClientFactory.java:152)
org.testcontainers.DockerClientFactory.client(DockerClientFactory.java:193)
org.testcontainers.DockerClientFactory$1.getDockerClient(DockerClientFactory.java:106)
com.github.dockerjava.api.DockerClientDelegate.authConfig(DockerClientDelegate.java:109)
org.testcontainers.containers.GenericContainer.start(GenericContainer.java:333)
scalasql.example.PostgresExample$.postgres$lzycompute(PostgresExample.scala:22)
scalasql.example.PostgresExample$.postgres(PostgresExample.scala:19)
scalasql.example.PostgresExample$.(PostgresExample.scala:27)
scalasql.utils.PostgresSuite.$init$(ScalaSqlSuite.scala:45)
scalasql.postgres.DbApiTests$.(ConcreteTestSuites.scala:45)
jdk.internal.misc.Unsafe.ensureClassInitialized0(Unsafe.java:-2)
jdk.internal.misc.Unsafe.ensureClassInitialized(Unsafe.java:1044)
jdk.internal.reflect.UnsafeFieldAccessorFactory.newFieldAccessor(UnsafeFieldAccessorFactory.java:43)
jdk.internal.reflect.ReflectionFactory.newFieldAccessor(ReflectionFactory.java:186)
java.lang.reflect.Field.acquireFieldAccessor(Field.java:1105)
java.lang.reflect.Field.getFieldAccessor(Field.java:1086)
java.lang.reflect.Field.get(Field.java:418)
org.portablescala.reflect.LoadableModuleClass.loadModule(LoadableModuleClass.scala:17)
utest.PlatformShims$.loadModule(PlatformShims.scala:15)
utest.runner.BaseRunner.$anonfun$runSuite$4(BaseRunner.scala:110)

Tests: 680, Passed: 677, Failed: 3
1 targets failed
scalasql[2.13.12].test.test 95 tests failed:
scalasql.postgres.DbApiTests scalasql.postgres.DbApiTests.
scalasql.postgres.TransactionTests scalasql.postgres.TransactionTests.
scalasql.postgres.SelectTests scalasql.postgres.SelectTests.
scalasql.postgres.JoinTests scalasql.postgres.JoinTests.
scalasql.postgres.FlatJoinTests scalasql.postgres.FlatJoinTests.
and 90 more ...

@aboisvert
Copy link
Contributor

Regarding your last issue,

IllegalStateException: Could not find a valid Docker environment.

Do you have docker installed on your system and configured to use for the current user?

@rmorales-iaa
Copy link
Contributor Author

Right, right, I didn't see the precondition at the beginning of the page. As you'll see, I'm new to using 'mill'. After switching the repository to a computer with Docker, all tests were verified:

Tests: 1675, Passed: 1675, Failed: 0"

class FloatType extends TypeMapper[Float] {
def jdbcType = JDBCType.FLOAT
def get(r: ResultSet, idx: Int) = r.getFloat(idx)
def put(r: PreparedStatement, idx: Int, v: Float) = r.setDouble(idx, v)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rmorales-iaa should this be setFloat, for symmetry with the getFloat above?

@rmorales-iaa
Copy link
Contributor Author

"You're right, it should be 'r.setFloat(idx, v)' instead of 'r.setDouble(idx, v)'.
I have already made the correction, and I have also added some 'implicit def from(x: ): Expr[] = Expr(x)' that were missing for basic data types in the same file 'Dialect.java'.
I ran the tests and code formatting with mill, and I pushed it to the new pull request in the repository."

@lihaoyi
Copy link
Member

lihaoyi commented Sep 26, 2024

Looks good to me, @rmorales-iaa if you're happy with it I'll merge and cut a release

@rmorales-iaa
Copy link
Contributor Author

Nice!, please go ahead

@lihaoyi lihaoyi merged commit 3eab45e into com-lihaoyi:main Sep 26, 2024
6 checks passed
@lihaoyi
Copy link
Member

lihaoyi commented Sep 26, 2024

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

Successfully merging this pull request may close these issues.

3 participants