Extract default column adapters to separate module #2121

veyndan · 2020-12-20T12:35:19Z

BREAKING CHANGE

These two issues had to be fixed in conjunction, per #2060 (comment).

Closes #2056.

Extracts the default column adapters for Boolean, Double, Float, Int, and Short into a new module named "primitive-adapters".

Closes #2060.

Removes the default import in the SQL file for types: Boolean, Short, Int, Integer, Long, Float, Double, String, ByteArray.

veyndan · 2020-12-20T12:36:17Z

adapters/primitive-adapters/build.gradle

@@ -0,0 +1,12 @@
+apply plugin: 'org.jetbrains.kotlin.jvm'
+
+archivesBaseName = 'sqldelight-primitive-adapters'


I'm not sure how I feel about the name "primitive" tbh, as Kotlin doesn't have primitive types.

Perhaps we should name it sqldelight-sqlite-adapters per https://github.com/cashapp/sqldelight/pull/2121/files#r546376383?

yes, given our discussion yesterday the idea is that dialects will have their own repo, and part of it would be a set of adapters - and in this case these are the SQLite adapters, where something like postgres will have none of these since you should be using the correct column type 👍

veyndan · 2020-12-20T12:41:25Z

adapters/primitive-adapters/gradle.properties

@@ -0,0 +1,4 @@
+POM_ARTIFACT_ID=primitive-adapters


I'd kinda prefer the name adapters-primitive as it'd look better when importing dependencies (similar to the Retrofit adapters):

implementation "com.squareup.sqldelight:adapters-primitive:1.4.4" implementation "com.squareup.sqldelight:adapters-postgresql:1.4.4"

Compared to what we have now:

implementation "com.squareup.sqldelight:primitive-adapters:1.4.4" implementation "com.squareup.sqldelight:postgresql-adapters:1.4.4"

I went with this approach as we seem to have this style for the other modules (e.g., jdbc-driver instead of driver-jdbc)

whatever works, I think before 2.0 this would become something like "app.cash.sqldelight-sqlite:adapters"

or if it stays in this repo "app.cash.sqldelight:sqlite-adapters", where we're doing target first since the target is how we're grouping similar libs

veyndan · 2020-12-20T12:44:39Z

...e-adapters/src/main/kotlin/com/squareup/sqldelight/adapter/primitive/BooleanColumnAdapter.kt

@@ -0,0 +1,9 @@
+package com.squareup.sqldelight.adapter.primitive


So this probably should be com.squareup.sqldelight.primitive.adapter if we're following the approach of the other modules (e.g., JdbcDriver has the import com.squareup.sqldelight.jdbc.driver), but personally I'd find the grouping in the imports to make a lot more sense here than having the adapter name first.

similar to above, I'd say keep whatever you've already done and then we can change when we split out the dialects

veyndan · 2020-12-20T13:20:36Z

docs/common/custom_column_types.md

+## Primitives
+
+A sibling module that adapts primitives for your convenience.
+
+```groovy
+dependencies {
+  implementation "com.squareup.sqldelight:primitive-adapters:{{ versions.sqldelight }}"
+}
+```
+
+The following adapters exist:
+
+- `BooleanColumnAdapter` — Retrieves `kotlin.Boolean` for an SQL type implicitly stored as `kotlin.Long`
+- `DoubleColumnAdapter` — Retrieves `kotlin.Double` for an SQL type implicitly stored as `kotlin.Long`
+- `FloatColumnAdapter` — Retrieves `kotlin.Float` for an SQL type implicitly stored as `kotlin.Long`
+- `IntColumnAdapter` — Retrieves `kotlin.Int` for an SQL type implicitly stored as `kotlin.Long`
+- `ShortColumnAdapter` — Retrieves `kotlin.Short` for an SQL type implicitly stored as `kotlin.Long`
+


Idk if this should be promoted for use in dialects other than SQLite, as HSQL, MySQL, and PostgreSQL all have native SQL types that map to the above (e.g., in PostgreSQL, a user shouldn't do INT AS kotlin.Short and use ShortColumnAdapter as PostgreSQL has the type SMALLINT which natively decodes to kotlin.Short). Should this be moved to the SQLite specific documentation (even though it can technically be used by other dialects)?

SQLite specific

veyndan · 2020-12-20T13:40:02Z

Not too sure why the CI is failing. It looks like an OutOfMemoryError.

Caused by: java.lang.OutOfMemoryError: Metaspace

AlecKazakova

would it be problematic to have these adapters live in runtime instead of a new module? that's what we currently do for EnumColumnAdapter

veyndan · 2021-02-21T14:41:48Z

@AlecStrong I moved it into a separate module per your comment #2056 (comment) (lmk if I misunderstood it).

Also, per #2121 (comment) I'm wondering whether we should promote it for usage in non-SQLite dialects as I'd imagine most other dialects shouldn't need it. Theoretically, most other dialects shouldn't need to use EnumColumnAdapter as there are native enum types in those dialects (but we can't support them atm due to limitations in the abstraction which can be solved in 2.0). Please let me know your thoughts regarding the other comments on the PR as they all are about this issue 😄

AlecKazakova · 2022-01-18T13:56:07Z

resolved comments, once its rebased I'll review all the code. And yea this should all be a separate module assuming that we go down a road where we have

sqlite-dialect
--adapters
--runtime
--compiler

and potentially we just fold adapters into runtime

veyndan · 2022-02-17T13:30:31Z

@AlecStrong Took a while to fix because of bit rot and some code that was just wrong, but I've updated the PR with the following:

Fixed adapters as they were always being encoded to kotlin.Long and not kotlin.Double in the case of REAL. Removed DoubleColumnAdapter as it doesn't really make sense.
Moved documentation of primitive adapters to be SQLite specific.
Made primitive-adapters multiplatform.
Forbid encoding to kotlin.Boolean if database value isn't 0 or 1.

I left the primitive-adapters name per #2121 (comment).

AlecKazakova

looks great, thank you

Zlash92 · 2022-03-07T08:16:27Z

@AlecStrong Are these changes available in a release/RC yet?

AlecKazakova · 2022-03-07T13:27:24Z

no, I was thinking of doing a 2.0 alpha release though - I'll do that later today

Zlash92 · 2022-03-07T13:47:09Z

Ok nice. Will it be publicly available? :)

AlecKazakova · 2022-03-07T13:56:05Z

yep yep it'll be an official release

Zlash92 · 2022-03-07T13:56:52Z

Sweet. Thanks.

Zlash92 · 2022-03-08T13:11:00Z

So I can just watch out for it here: https://github.com/cashapp/sqldelight/releases ?

Zlash92 · 2022-03-08T14:35:21Z

Any thought of when it will be available? 😇 Sorry about the constant questions, but it would be really useful if it's near rc-ready.

veyndan commented Dec 20, 2020

View reviewed changes

AlecKazakova added this to the API Breaking milestone Feb 21, 2021

AlecKazakova approved these changes Feb 21, 2021

View reviewed changes

AlecKazakova mentioned this pull request Jan 18, 2022

[Runtime Error] Not able to map Boolean type while using PostgreSQL #2141

Closed

Extract default column adapters to separate module

b477808

AlecKazakova approved these changes Feb 17, 2022

View reviewed changes

AlecKazakova merged commit 9e6d10a into sqldelight:master Feb 17, 2022

veyndan deleted the 2056-2060/extract-default-column-adapters branch February 17, 2022 14:44

AlecKazakova pushed a commit that referenced this pull request Mar 8, 2022

Extract default column adapters to separate module (#2121)

4b27809

Satook pushed a commit to SuppApp/sqldelight that referenced this pull request Mar 21, 2022

Extract default column adapters to separate module (sqldelight#2121)

8321b14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Extract default column adapters to separate module #2121

Extract default column adapters to separate module #2121

veyndan commented Dec 20, 2020 •

edited

Loading

veyndan Dec 20, 2020

veyndan Dec 20, 2020

AlecKazakova Jan 18, 2022

veyndan Dec 20, 2020 •

edited

Loading

AlecKazakova Jan 18, 2022

AlecKazakova Jan 18, 2022

veyndan Dec 20, 2020 •

edited

Loading

AlecKazakova Jan 18, 2022

veyndan Dec 20, 2020

AlecKazakova Jan 18, 2022

veyndan commented Dec 20, 2020

AlecKazakova left a comment

veyndan commented Feb 21, 2021

AlecKazakova commented Jan 18, 2022

veyndan commented Feb 17, 2022

AlecKazakova left a comment

Zlash92 commented Mar 7, 2022 •

edited

Loading

AlecKazakova commented Mar 7, 2022

Zlash92 commented Mar 7, 2022

AlecKazakova commented Mar 7, 2022

Zlash92 commented Mar 7, 2022

Zlash92 commented Mar 8, 2022

Zlash92 commented Mar 8, 2022 •

edited

Loading

		@@ -0,0 +1,12 @@
		apply plugin: 'org.jetbrains.kotlin.jvm'

		archivesBaseName = 'sqldelight-primitive-adapters'

		@@ -0,0 +1,9 @@
		package com.squareup.sqldelight.adapter.primitive

Extract default column adapters to separate module #2121

Extract default column adapters to separate module #2121

Conversation

veyndan commented Dec 20, 2020 • edited Loading

BREAKING CHANGE

Closes #2056.

Closes #2060.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

veyndan Dec 20, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

veyndan Dec 20, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

veyndan commented Dec 20, 2020

AlecKazakova left a comment

Choose a reason for hiding this comment

veyndan commented Feb 21, 2021

AlecKazakova commented Jan 18, 2022

veyndan commented Feb 17, 2022

AlecKazakova left a comment

Choose a reason for hiding this comment

Zlash92 commented Mar 7, 2022 • edited Loading

AlecKazakova commented Mar 7, 2022

Zlash92 commented Mar 7, 2022

AlecKazakova commented Mar 7, 2022

Zlash92 commented Mar 7, 2022

Zlash92 commented Mar 8, 2022

Zlash92 commented Mar 8, 2022 • edited Loading

veyndan commented Dec 20, 2020 •

edited

Loading

veyndan Dec 20, 2020 •

edited

Loading

veyndan Dec 20, 2020 •

edited

Loading

Zlash92 commented Mar 7, 2022 •

edited

Loading

Zlash92 commented Mar 8, 2022 •

edited

Loading