-
Notifications
You must be signed in to change notification settings - Fork 1
Serializable value binding #25
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
base: main
Are you sure you want to change the base?
Conversation
| @PublishedApi | ||
| internal val AnyType = typeOf<Any>() | ||
|
|
||
| // FIXME This is not ideal, but KProperty1<T, out V> doesn't have the right | ||
| // variance to prevent Any from satisfying V. | ||
| /** | ||
| * Check that we aren't coercing types to Any at runtime. | ||
| */ | ||
| @PublishedApi | ||
| internal inline fun <reified T> checkedTypeOf(): KType { | ||
| val type = typeOf<T>() | ||
| if (type == AnyType) { | ||
| throw IllegalArgumentException( | ||
| "Type was coerced to Any; did you pass the correct value?" | ||
| ) | ||
| } | ||
| return type | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI this doesn't work because we fail in KProperty1.builder() before hitting checkedTypeOf.
| is ByteArray -> bindBytes(value) | ||
| is Double -> bindDouble(value) | ||
| is Number -> bindLong(value.toLong()) | ||
| is Long -> bindLong(value) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whoops, this can revert.
| val otherValue: Int | ||
| ) | ||
|
|
||
| @Serializable |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@chrisjenx I think this is what you were running into with @SerialName; enums require @Serializable or else @SerialName is ignored.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, to which the Serialization team still need to fix/follow up
| if (element) { | ||
| when (it.valueDescriptor.kind) { | ||
| StructureKind.LIST -> "$prefix${it.propertyName}[%]" | ||
| PolymorphicKind.SEALED -> "$prefix${it.propertyName}[1]" | ||
| else -> "$prefix${it.propertyName}" | ||
| } | ||
| } else { | ||
| "$prefix${it.propertyName}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The element arg is a hack. Sometimes we want the "outer" collection path, e.g. $.property, for example when comparing via eq:
TestObject::list eq listOf("one", "two", "three")Other times we want the "inner" path, e.g. $.property[%], for example when filtering by contains:
TestObject::list contains "one"(Note that I added contains, which is the opposite of inList)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm still confused by contains vs inList. In sql you would write that as 'one' IN column.name
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe that was the source of my confusion. I would expect inList to produce a clause like column IN (?, ?, ?).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh I think I see what you are saying, if the field is a list, inList doesn't make sense as thats checking the entire contents of that fields vs a list to list match
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Exactly, and I think it's going to be hard to make the DSL reject nonsensical queries like that. Ultimately I think we'll need to do some code-generation to build a schema that the DSL operates on, instead of trying to make Kotlin's type system reject this.
|
All in all, I don't think this approach is viable because object-valued field types can't be compared unless you rely on the field ordering in source; this can change between builds of the program. If the field order changes, you can still compare values in Kotlin but your comparisons would break in SQL because it's all string comparisons, and SQLite doesn't have a |
|
Yeah, I have a compiler plugin branch, that will probably fix allot of this for you, as that will not only give us the correct serial names, but it will also give us the defined type so we can support/generate type matching, I think I got most of it working, just got pulled off to something else before finishing |
|
Looking through the PR I've tried quite a few things you have done lol, it's annoying kotlin is so close to give us nearly everything we need, but then it misses just a few things to make it work without needing to write extra stuff on top |
This is an experiment to allow building queries using Kotlin types instead of their equivalent representation in SQL. For example:
AutoIncrementSqlPreparedStatement.bindValuenow serializes value arguments to JSON. Several queries now usejson_quoteto normalize values passed via prepared statement bindings.