Bug #90754

Database Compare with SQLite can not be finished

Added by Daniel Siepmann 2 months ago. Updated about 2 months ago.

Status:
Closed
Priority:
Should have
Assignee:
-
Category:
Database API (Doctrine DBAL)
Target version:
-
Start date:
2020-03-13
Due date:
% Done:

100%

TYPO3 Version:
10
PHP Version:
7.3
Tags:
Complexity:
Is Regression:
Sprint Focus:

Description

With current master, and since some v10 releases already, I can't finish the database compare using SQLite.

I've attached a screenshot of the result when executing the compare, together with a file containing the output of the compare beforehand:

liayn1 8 minutes ago
I added a dummy column to the pages table locally and did a compare.
it showed me the option to rename the column to zzz_, this worked without errors.
Now I see the same suggestion again and additionally the statements to drop the zzz field.
So this is some different bug indeed

Screenshot from 2020-03-13 09-29-37.png View (128 KB) Daniel Siepmann, 2020-03-13 10:01

compare.txt View (111 KB) Daniel Siepmann, 2020-03-13 10:01


Related issues

Related to TYPO3 Core - Bug #90758: Renaming of column does work in sqlite with db compare Closed 2020-03-13
Follows TYPO3 Core - Bug #90751: database compare shows differences for fresh installation Closed 2020-03-12

Associated revisions

Revision decea21a (diff)
Added by Markus Klein 2 months ago

[BUGFIX] Make SQL schema migrations working on SQLite

This patch fixes three things:

1) SQLite can handle a primary key, the special handling
can be removed. The field names must be quoted though.
A lot of duplicate tests can be removed here.

2) The handcrafted arrays passed on to the TableDiff class
need to be indexed by the column or index name, otherwise
the SQLite driver fails to identify them correctly.

3) Allow only one column operation per table.
Having multiple column adjustments (rename, removal)
within one run generates wrong migration SQL.
This is due to the possibility of the Install Tool
to be able to select on a "per column" level.
(Keep in mind that in SQLite one operation needs at
least 5 SQL statements, contrary to other DMBS)

Resolves: #90758
Resolves: #90754
Resolves: #90751
Releases: master, 9.5
Change-Id: I972346fae3cc97b877e20033b466345fa1c0a83d
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/63702
Tested-by: TYPO3com <>
Tested-by: Daniel Siepmann <>
Tested-by: Georg Ringer <>
Tested-by: Benni Mack <>
Reviewed-by: Georg Ringer <>
Reviewed-by: Benni Mack <>

Revision b71024c2 (diff)
Added by Markus Klein 2 months ago

[BUGFIX] Make SQL schema migrations working on SQLite

This patch fixes three things:

1) SQLite can handle a primary key, the special handling
can be removed. The field names must be quoted though.
A lot of duplicate tests can be removed here.

2) The handcrafted arrays passed on to the TableDiff class
need to be indexed by the column or index name, otherwise
the SQLite driver fails to identify them correctly.

3) Allow only one column operation per table.
Having multiple column adjustments (rename, removal)
within one run generates wrong migration SQL.
This is due to the possibility of the Install Tool
to be able to select on a "per column" level.
(Keep in mind that in SQLite one operation needs at
least 5 SQL statements, contrary to other DMBS)

Resolves: #90758
Resolves: #90754
Resolves: #90751
Releases: master, 9.5
Change-Id: I972346fae3cc97b877e20033b466345fa1c0a83d
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/63745
Tested-by: TYPO3com <>
Tested-by: Benni Mack <>
Reviewed-by: Benni Mack <>

History

#1 Updated by Gerrit Code Review 2 months ago

  • Status changed from New to Under Review

Patch set 1 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/63702

#2 Updated by Markus Klein 2 months ago

  • Follows Bug #90751: database compare shows differences for fresh installation added

#3 Updated by Markus Klein 2 months ago

The proposed patch solves the problem when a single field is to be renamed for a table, but does not solve the issue if multiple fields are to be changed.

The reason is hidden in
\TYPO3\CMS\Core\Database\Schema\ConnectionMigrator::getUnusedFieldUpdateSuggestions

There it says "// Treat each changed column with a new diff to get a dedicated suggestions just for this single column."

This works for DB-Systems which need only 1 statement to adjust the schema for each column, but fails on SQLite where a column rename is a series of at least 5 statements (via temporary table).

If one than more column for the same table needs to be renamed the subsequent statements are calculated without knowing about the changes from the first column to change. This leads to absolute chaos.

Two possible solutions for SQLite therefore are:
  1. Allow only a single field per table to be renamed at once
  2. Rename all fields per table at once (no individual choice possible for single fields)

#4 Updated by Markus Klein 2 months ago

  • Related to Bug #90758: Renaming of column does work in sqlite with db compare added

#5 Updated by Mathias Brodala 2 months ago

  • Description updated (diff)

#6 Updated by Mathias Brodala 2 months ago

  • Description updated (diff)

#8 Updated by Gerrit Code Review 2 months ago

Patch set 3 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/63702

#9 Updated by Gerrit Code Review 2 months ago

Patch set 1 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/63712

#10 Updated by Gerrit Code Review 2 months ago

Patch set 2 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/63712

#11 Updated by Gerrit Code Review 2 months ago

Patch set 3 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/63712

#12 Updated by Gerrit Code Review 2 months ago

Patch set 6 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/63702

#13 Updated by Gerrit Code Review 2 months ago

Patch set 1 for branch 9.5 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/63745

#14 Updated by Gerrit Code Review 2 months ago

Patch set 2 for branch 9.5 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/63745

#15 Updated by Markus Klein 2 months ago

  • Status changed from Under Review to Resolved
  • % Done changed from 0 to 100

#16 Updated by Benni Mack about 2 months ago

  • Status changed from Resolved to Closed

Also available in: Atom PDF