Bug fixes for 2.4.0 in sqllin-dsl and sqllin-processor - #124
Merged
Merged
Conversation
The annotation's parameter was named `isAutoincrement` while parts of the documentation referred to it as `autoIncrement`, so code copied from the docs failed to compile with "Cannot find a parameter with this name". `autoIncrement` is the better of the two names: Kotlin's `is` prefix convention applies to properties rather than annotation parameters, none of the other annotations (`@CompositeUnique`, `@ForeignKey`, `@References`, `@Default`) carry such a prefix, and `isAutoincrement` was itself inconsistent in its casing. This is a source-incompatible rename, so call sites passing the argument by name have to be updated. The processor reads the argument positionally, so the generated DDL is unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…t (B3) The processor always emitted a `public` table object, so an `internal` @DBRow class failed to compile with EXPOSED_SUPER_CLASS, EXPOSED_FUNCTION_RETURN_TYPE and EXPOSED_RECEIVER_TYPE. Keeping a data layer internal therefore forced the entities to be public, which on iOS also pushes them into the generated ObjC header. Since every generated member lives inside that object, narrowing the object alone narrows all of them; the `override`s cannot be narrowed individually anyway, as Kotlin forbids reducing an override's visibility. A @DBRow class that is neither public nor internal is now reported through KSPLogger instead of producing code that cannot compile, because the generated object lives in a different file and cannot reference a private entity. Covered by a new `internal` test entity: without the fix, the test module no longer compiles. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The class KDoc said statements are executed in batch when the scope exits, but
its example then read a query's results inside the scope:
val adults = PersonTable SELECT WHERE(age GTE 18) LIMIT 10
`adults` is a statement, not a list, and calling `getResults()` on it there
throws IllegalStateException. The example now keeps the statement in a variable
declared outside the scope and reads it afterwards, and the KDoc states the rule
explicitly, including its consequence that a read-modify-write cannot be
expressed in a single scope.
The example also used bare column names outside the table object's scope, where
they do not resolve, so it would not have compiled as written. It is now
wrapped in `PersonTable { table -> ... }`; the whole example was transcribed
into the test module and compiled to confirm it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The `CREATE_INDEX` and `CREATE_UNIQUE_INDEX` KDoc examples were written as
User::class.table.CREATE_INDEX("idx_user_email", User::email)
but no `KClass.table` extension exists anywhere in the library, and the columns
are not Kotlin property references either — they are accessors on the generated
table object. Both examples now use the form the tests already exercise:
UserTable.CREATE_INDEX("idx_user_email", UserTable.email)
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`ALERT_ADD_COLUMN` and `ALERT_RENAME_TABLE_TO` misspelled the SQL keyword `ALTER`. They are renamed to `ALTER_ADD_COLUMN` and `ALTER_RENAME_TABLE_TO`, and the internal `Alert` operation object to `Alter`, along with every reference in the documentation, the KDoc and the tests. This is a source-incompatible rename of public API. The 2.2.0 entry in the change log still says `ALERT`, which is what that version actually shipped, so it is left as it is. Note that the operations still emit the invalid keyword "ALERT TABLE" and therefore still fail at runtime; that is a separate defect, fixed in the next commit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…sts (B10) `Alter.sqlStr` produced the invalid keyword `ALERT TABLE`, so every ALTER operation — `ALTER_ADD_COLUMN`, `ALTER_RENAME_TABLE_TO` (both overloads), `RENAME_COLUMN` (both overloads) and `DROP_COLUMN` — failed at runtime and had never worked. The existing tests hid this. Each of their seven cases wrapped the operation in try/catch, swallowed the exception, and then asserted only that the rows were still present, so none of them asserted anything about the operation itself. Every case was also built on a statement that was invalid to begin with: adding a column that already existed, renaming a table to its own name, or renaming a column onto an existing column's name. They could not have passed even with the correct keyword. They are replaced by a single migration test that drives 'alter_target' from the shape of `AlterBefore` to the shape of `AlterAfter`, reading the table back through the entity that matches the shape it should have at each point, so a step that does not run fails the test instead of passing quietly. `AlterWithLegacy` serves as a probe for whether the dropped column is really gone. Two platform details shape the test. DROP COLUMN requires SQLite 3.35, which the Android framework bundles only from API 34 on, so it runs last and its effect is asserted only where the statement actually executes. The helper reads the query results rather than merely executing the statement, because the Android driver's `rawQuery` is lazy: a missing table or column surfaces only once the cursor is read. Verified on jvmTest, testAndroidHostTest (Robolectric API 26 and 37) and macosArm64Test. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The processor decided whether a generated `SetClause` property is nullable by
reading `ColumnConstraintParser.isRowId`, a parser-level flag that is set once
the `@PrimaryKey` column has been parsed and never reset. Every column declared
after a `Long?` primary key was therefore generated as nullable, whatever the
entity declared. For
data class PersonWithId(@PrimaryKey val id: Long?, val name: String, val age: Age)
`name` and `age` were generated as `String?` and `Int?`, so
`UPDATE SET { name = null }` compiled against a NOT NULL column and failed only
at runtime. The behaviour also depended on the order the properties were
declared in.
The flag was redundant even for the key itself, whose `Long?` type already makes
it nullable, so the branch is removed and each property takes the nullability of
its own column.
A compile-time check in the test module assigns the generated properties to
non-null variables; it fails to compile without this fix.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Every `@PrimaryKey` property was required to be nullable, unconditionally. That
contradicted the annotation's own KDoc, which says a key of any type other than
Long must be non-null, and the error message for it, "The primary key must be
not-null.", said the opposite of what the check enforced. The test suite had
followed the check rather than the documentation (`@PrimaryKey val sku: String?`).
The cost was more than an inconvenience. A forced-nullable String key generated
`sku TEXT PRIMARY KEY`, and on a rowid table SQLite does not let PRIMARY KEY
imply NOT NULL for anything but an INTEGER PRIMARY KEY, so such a key accepted
NULL in any number of rows. Several tests inserted products with a NULL SKU.
In standard SQL a primary key is NOT NULL whoever supplies it; what differs is
only whether an INSERT may leave it out for the database to assign, and only a
rowid alias can be assigned. The Kotlin `?` therefore expresses "not assigned
yet", not "may be NULL", and that is what it now means:
- `Long?`: an INTEGER PRIMARY KEY the database assigns; a plain INSERT omits it.
- `Long`: still an INTEGER PRIMARY KEY, and still a rowid alias, but supplied by
the caller and written by every INSERT. This is new, and replaces the
single-column @CompositePrimaryKey that a caller-supplied numeric key used to
need, which produced `BIGINT ... PRIMARY KEY(id)`, not a rowid alias.
- any other type: supplied by the caller, must be non-null, and is declared
`PRIMARY KEY NOT NULL`.
`Long` and `Long?` keys produce the same DDL, so switching between them needs no
migration.
`autoIncrement = true` now requires a `Long?` key. A nullable key of any other
type is rejected, which also covers `ULong?`: it maps to BIGINT, was treated as a
rowid because the check accepted BIGINT, and so was left out of INSERT although
nothing assigned it, storing NULL.
`PrimaryKeyInfo.isRowId` is renamed to `isGeneratedByDatabase`, since a non-null
Long key is a rowid alias that the database does not generate. Its KDoc had
always described this meaning.
The rejection paths were verified by compiling entities that declare a `String?`
key, a `ULong?` key and an `autoIncrement` non-null `Long` key.
This is a source-incompatible change: drop the `?` from any non-Long @PrimaryKey.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Standard SQL accepts a one-column table constraint, `PRIMARY KEY(col)`, and it means the same as declaring `PRIMARY KEY` on the column itself, so this is not a question of SQL validity. It is one of API shape: `@CompositePrimaryKey` is documented as a key that "consists of multiple columns", and a single-column key already has `@PrimaryKey`. Allowing both was not harmless. Whether SQLite makes a single-column key a rowid alias depends only on its declared type being exactly INTEGER, and the two paths disagreed on that for a Long: `@PrimaryKey` maps it to INTEGER, a rowid alias, while `@CompositePrimaryKey` maps it to BIGINT, which is not. The same intent, a numeric key the caller supplies, therefore produced two different storage layouts depending on which annotation was picked. That was the only way to express such a key before `@PrimaryKey val id: Long` became possible. The processor now rejects a `@CompositePrimaryKey` that ends up with exactly one column, pointing to `@PrimaryKey`. The count is only known once every property has been parsed, so the check runs when the primary key metadata is generated. This is a source-incompatible change: replace a lone `@CompositePrimaryKey` with `@PrimaryKey`. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A `@CompositePrimaryKey` produced, for example,
enrollment(studentId BIGINT,courseId BIGINT,...,PRIMARY KEY(studentId,courseId))
On a rowid table SQLite, unlike standard SQL, does not let a table-level PRIMARY
KEY imply NOT NULL, so these columns accepted NULL, and with it the key stopped
identifying rows: two rows with the key (NULL, 101) are both accepted, because a
unique index treats every NULL as distinct.
SQLlin itself cannot write those NULLs. The key columns are non-null Kotlin
types, the generated SetClause properties are non-null since the previous fix,
and the processor already refuses an ON DELETE SET NULL foreign key on a
non-null column. Anything else writing to the database can, though, and SQLlin
then reads such a row back without complaint, as 0 or an empty string, so two
rows keyed (NULL, 101) surface as two entities keyed (0, 101).
NOT NULL used to be appended in two places: inside the @PrimaryKey branch, and
in a final `else if` that composite key columns never reached. It is now one
rule after the branch: every non-null column is declared NOT NULL except a rowid
alias, which is the only column SQLite itself keeps from being NULL. Across all
33 tables generated by the test module, the only DDL that changes is that of the
two composite-key tables.
This only affects tables created from now on. An existing table keeps its
schema, since SQLite cannot add NOT NULL to an existing column.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ON DELETE / ON UPDATE SET DEFAULT writes the column's default value, which is NULL when the column declares none. The documentation, in both user guides and in the KDoc of @default, has always said that a default is required for these triggers, but the processor enforced something else on each of its two paths. On @references the check was inverted. It read check(isNotNull || hasDefaultValue) { "The column must be nullable or have a default value ..." } so it accepted a non-null column without a default, whose parent row then could not be deleted (NOT NULL constraint failed), and rejected a nullable one, while its own message described the opposite rule. On a @ForeignKeyGroup there was no check at all. Both paths now require @default. A nullable column without one is rejected too: setting it to its default would only set it to NULL, which is what ON_DELETE_SET_NULL already says, so it is most likely a forgotten @default. The group path cannot check where it reads @foreignkey, as its SET NULL check does, because @default may come later among the property's annotations, as it does in the test entity DefaultFKChild. The check is made once every annotation of the property has been read, so it holds whichever order they are written in. Verified by compiling entities that cover both paths, nullable and non-null columns, and @default before and after the foreign key annotation. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The processor collected a class's properties with getAllProperties(), which
includes computed properties that have no backing field, such as
val title: String get() = "$name by $author"
kotlinx.serialization doesn't serialize those, yet each one was given a column,
NOT NULL when its type was non-null, and an accessor that looked the column up
by its index in the serializer's descriptor. INSERT writes only the serialized
properties, so it never filled that column and every insert failed with "NOT
NULL constraint failed", and the accessor's index ran past the end of the
descriptor.
The property list now keeps only properties backed by a field, besides leaving
out @transient ones, so it matches exactly what the serializer writes, in the
same order. A body property with an initializer has a backing field, is
serialized, and still gets its column.
Book now declares a computed `title`. Without this fix, four tests fail with
"NOT NULL constraint failed: book.title".
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The processor skipped a property whose type it couldn't map to a column, such as a List, without saying anything. The property was left out of CREATE TABLE, but its serializer still wrote and read it, so INSERT failed at runtime with "has no column named" and SELECT with "no such column". When it was the last property, the comma already written after the previous column stayed in place, so CREATE TABLE itself failed with a syntax error. Such a property is now a compile-time error that names it, gives its type with its nullability, lists the supported types, and suggests @transient to keep it out of the table. Every unsupported property of a class is reported at once, with its location, and no table file is generated for the class. The check relies on the previous fix: a computed property isn't serialized, so it isn't checked, whatever its type. TestPrimitiveTypeForKSP now has a @transient List and a computed List, so the test module stops compiling if either exclusion breaks. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…DSL markers (B11)
SQLlin's four @DslMarker annotations are applied to functions, properties and
enum entries. That is where IntelliJ IDEA looks for them when it gives DSL
calls one of its highlighting styles, which is what they are for. It is not
where the compiler's DSL scope control applies, which needs them on types, and
since Kotlin 2.3.20 the compiler warns about this use (KT-81567): 157 warnings
in sqllin-dsl, and two per column in every generated table, which land in the
build of each module that uses SQLlin.
The markers stay, since they do their job. The warning is suppressed:
- on each generated table object, as generated code is compiled in the
user's module;
- in sqllin-dsl, at the narrowest scope that doesn't repeat itself: on a file
or class where more than half of the declarations carry a marker, and on
each such declaration otherwise.
The KDoc of the markers said they prevent implicit receiver nesting, which they
never did. It now describes what they are for.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The setter generated for an enum column's SetClause property always appended `value?.ordinal`. The type of `value` is the property's type, which, since the fix to that property's nullability (B12), is non-null whenever the column is. For such a column the safe call is unnecessary, and the module compiling the generated code reported it, once per non-null enum column. B12 made this more common: before it, every column declared after a `Long?` primary key had been generated as nullable, which happened to make the safe call necessary. The setter is now given the same nullability that decides the property's type, and only a nullable enum column keeps the safe call. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…B17) Every SQL function in Function.kt carries @FunctionDslMaker, which is how IntelliJ IDEA gives their calls a DSL highlighting style, except the seven string functions added in 2.2.0: substr, trim, ltrim, rtrim, replace, instr and printf. Their calls were therefore not highlighted like the rest. They now carry the marker too. The annotation has no effect at runtime, and the file already suppresses the compiler's warning about markers applied to functions, so nothing else changes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…og (B13) The entry for b15d963 called it a fix, but one of the cases it now rejects used to work: a nullable column in a @ForeignKeyGroup with an ON ... SET DEFAULT trigger and no @default compiled, and deleting the parent row set the column to NULL. Such code no longer compiles, so the entry is now marked as a breaking change and says how to migrate. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The installation guide added the generated directory to commonMain's sources but never made the tasks that read it depend on kspCommonMainKotlinMetadata. Gradle fails the build when a task reads another task's output without depending on it, and the generated sources are read by every Kotlin compilation and, once a module also runs another KSP processor such as Room or Koin Annotations, by that processor's KSP tasks as well, which a rule matching only compilation tasks does not cover. SQLlin's own sample and test builds have always declared the rule that covers both, matching compilation tasks by type and KSP tasks by name. The guide now shows exactly that rule, in English and in Chinese, and says why it is needed. It also states that each @DBRow class gets an object named after the class with a Table suffix, whatever the table is called, which the guide only implied through its examples. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
qiaoyuang
added a commit
that referenced
this pull request
Oct 2, 2026
Revert "Bug fixes for 2.4.0 in sqllin-dsl and sqllin-processor (#124)"
qiaoyuang
added a commit
that referenced
this pull request
Oct 2, 2026
Bug fixes for 2.4.0 in sqllin-dsl and sqllin-processor (re-land of #124)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bug fixes for 2.4.0 in
sqllin-dslandsqllin-processor, one commit per issue. Each commit message explains its issue in detail, andCHANGELOG.mdhas an entry for each change.Breaking changes
These may stop existing code from compiling. Migration is described in
CHANGELOG.md.@PrimaryKey(isAutoincrement = ...)is renamed toautoIncrement, the name the documentation already used. Positional@PrimaryKey(true)is unaffected. (06bcbb7)ALERT_ADD_COLUMN/ALERT_RENAME_TABLE_TOare renamed toALTER_ADD_COLUMN/ALTER_RENAME_TABLE_TO. (26cee52)@PrimaryKey's nullability now decides who supplies its value.Long?is assigned by the database, as before. A non-nullLongkey is now allowed: it stays anINTEGER PRIMARY KEY(a rowid alias) but is written by everyINSERT. A key of any other type must be non-null and is declaredNOT NULL.autoIncrement = truerequiresLong?, andULong?is rejected. Previously every@PrimaryKeywas forced to be nullable, against the annotation's own KDoc, so aStringkey could holdNULLin any number of rows. (e599cb2)@CompositePrimaryKeyon a single property is rejected; use@PrimaryKey. For aLongkey it producedBIGINT ... PRIMARY KEY(id), which is not a rowid alias. (096cafb)ON ... SET DEFAULTforeign key requires@Default, as the documentation says. The@Referencescheck was inverted, and@ForeignKeyGrouphad none. Most newly rejected cases failed at runtime anyway, but a nullable@ForeignKeyGroupcolumn without a default used to compile and work, settingNULL; it now has to declare@Defaultor useSET_NULL. (b15d963; c0d02e2 marks it as breaking inCHANGELOG.md, where it was first listed as a fix)Fixes
ALERT TABLEand had never worked. Their tests swallowed the exception; they are rewritten as one migration test with real assertions. (9582b25)internal@DBRowclass didn't compile, because the generated table object was alwayspublic. (4231b08)SetClauseproperty after aLong?primary key was generated as nullable, soUPDATE SET { name = null }compiled against aNOT NULLcolumn. (a278016)NOT NULL. SQLite doesn't imply it on rowid tables, so(NULL, 101)could be stored twice. This only affects newly created tables. (ed000c4)NOT NULLcolumn thatINSERTnever wrote, so every insert failed. (30b52a6)List, was silently dropped fromCREATE TABLE, soINSERT/SELECTfailed at runtime. As the last property it produced invalid DDL. It is now a compile-time error. (5e6a202)DSL_MARKER_APPLIED_TO_WRONG_TARGETwarning per column in every module using SQLlin (Kotlin 2.3.20+, KT-81567). The markers are there for IntelliJ's DSL highlighting, so the warning is suppressed, in generated code and insqllin-dsl. (7514270)@FunctionDslMaker, so they weren't highlighted like the other SQL functions. (cb5d95c)DatabaseScope's KDoc now says execution is deferred to scope exit, and its example no longer reads results inside the scope (1e066a7). The index examples no longer use a non-existentKClass.table(e7747e9). The installation guide now declares the task dependencies the generated code needs, the same rule SQLlin's own builds use, without which Gradle fails the build, in particular alongside another KSP processor; it also states that each generated object is named<ClassName>Tableafter its class (531f7f2).Testing
jvmTest(45 tests) andtestAndroidHostTeston Robolectric API 26 and 37 (90 tests) pass locally, rerun at every commit that changes code.macosArm64Testonly compiles. CI on this PR is the first native run of these changes. The commit message of 9582b25 says it was verified onmacosArm64Test, which is not accurate: the tests were only compiled there.🤖 Generated with Claude Code