feat: implements Sqlite 'WITHOUT ROWID' table modifier

[bnegrao]

This new feature adds a 'withoutRowid' option to EntityOptions in order to
enable Sqlite 'WITHOUT ROWID' modifier to the 'CREATE TABLE' statement.
See https://www.sqlite.org/withoutrowid.html

Closes: #3330

[pleerock]

Thanks for contribution!

Looks like you don't track changes of withoutRowid (let's say it was set to true, then suddenly you decided to set to false) and schema sync won't catch up this change and your database won't be synchronized

[bnegrao]

Hello Umed, thank you for reviewing.

Can you point me to some code that does that tracking so I can have that as a guide in my implementation?

[pleerock]

its in the same sqlite query runner file, you need to figure solution for your feature.

[bnegrao]

Changing the withoutRowid option would be a "secret" change in the primary key of the table. Why secret? Because if you don't give that option to CREATE TABLE, SQlite will secretly create an INTEGER primary key for the table and what you defined as the primary key will be secretly indexed.

The current implementation of AbstractSqliteQueryRunner.ts does nothing when there is an update in the primary keys, see:

/**
 * Updates composite primary keys.
 */
async updatePrimaryKeys(tableOrName: Table|string, columns: TableColumn[]): Promise<void> {
    await Promise.resolve();
}

The reason for the "do nothing" implementation is probably because SQlite does not support an "alter table" command that would change the primary keys after the table is created.

Given that I believe it does not make sense to track for changes for that property. Agree?

[pleerock]

Right, sqlite doesn't support alter table command, that's why we always re-create table (and backup and restore all the data) after anything changed in your schema.

Example:

• you added a new column in entity
• we read columns we have in the database
• we read your entity columns
• we find a difference - a new column added
• since we can't alter we re-create table with a new column added
  (for other dbs we just call alter)
• that's it

same process with just everything to make sure your code in the entity always match your database representation. Same you need to do - read information about WITHOUT ROWID check if it exist or not in the table and in the entity, compare, do the job.

[bnegrao]

But the problem with the withoutRowid option is it is an invisible change in the schema, that option defines the creation of the rowid column that is not declared in your Entity class.

This method from RdbmsSchemaBuilder.ts will never be able to detect that there should be a modification in the table schema since there is no column added or removed:

/**
 * Updates composite primary keys.
 */
protected async updatePrimaryKeys(): Promise<void> {
    await PromiseUtils.runInSequence(this.entityToSyncMetadatas, async metadata => {
        const table = this.queryRunner.loadedTables.find(table => table.name === metadata.tablePath);
        if (!table)
            return;

        const primaryMetadataColumns = metadata.columns.filter(column => column.isPrimary);
        const primaryTableColumns = table.columns.filter(column => column.isPrimary);
        if (primaryTableColumns.length !== primaryMetadataColumns.length && primaryMetadataColumns.length > 1) {
            const changedPrimaryColumns = primaryMetadataColumns.map(primaryMetadataColumn => {
                return new TableColumn(TableUtils.createTableColumnOptions(primaryMetadataColumn, this.connection.driver));
            });
            await this.queryRunner.updatePrimaryKeys(table, changedPrimaryColumns);
        }
    });
}

That last line await this.queryRunner.updatePrimaryKeys(table, changedPrimaryColumns); would never be invoked with the change of the withoutRowid option.

Am I mistaken? do you have any ideas?

[bnegrao]

And I don't understand why are you asking to implement that right now if the current implementation of updatePrimaryKeys in AbstractSqliteQueryRunner.ts does nothing:

/**
 * Updates composite primary keys.
 */
async updatePrimaryKeys(tableOrName: Table|string, columns: TableColumn[]): Promise<void> {
    await Promise.resolve();
}

If the users of typeorm + SQLite were happy with that implementation why do we need to worry now?

[pleerock]

Okay let's merge it, since this feature most likely will be used during first-time table creation its okay for now not to have auto-syncing feature.

[olosegres]

@pleerock Please tell me, is code from this PR lets use withoutRowid option with EntitySchema?

[pthrasher]

@olosegres I've opened a PR to add the support you're requesting here: #8432