Project

General

Profile

Actions

Bug #105693

open

DB field without TCA breaks saving be_users

Added by Philipp Kitzberger 4 months ago. Updated 2 months ago.

Status:
Under Review
Priority:
Should have
Assignee:
Category:
Database API (Doctrine DBAL)
Target version:
-
Start date:
2024-11-25
Due date:
% Done:

0%

Estimated time:
TYPO3 Version:
12
PHP Version:
Tags:
Complexity:
Is Regression:
Sprint Focus:

Description

When having a table column in be_users that doesn't correspond to a TCA field definition it's not possible anymore to update a BE user record due to a ColumnDoesNotExistException: There is no column with name "zzz_deleted_cruser_id" on table "be_users"

This can happen after doing DB compare and having fields renamed to zzz_deleted_...

After dropping them the problem is gone.

We noticed this problem first in version 12.4.22. But it's occurring on the current main branch as well!

How to reproduce:
  • ALTER TABLE be_users ADD column zzz_deleted_cruser_id INT(11);
  • Try and save a BE user record

It's not reproducible with other tables such as tt_content!


Files


Related issues 2 (0 open2 closed)

Related to TYPO3 Core - Feature #99062: Native JSON field support in Doctrine DBALClosedBenni Mack2022-11-11

Actions
Related to TYPO3 Core - Feature #99226: Introduce dbType json for TCA type userClosed2022-11-30

Actions
Actions #1

Updated by Philipp Kitzberger 4 months ago

  • Description updated (diff)
Actions #2

Updated by Philipp Kitzberger 4 months ago

  • Description updated (diff)
Actions #3

Updated by Philipp Kitzberger 4 months ago

  • Description updated (diff)
Actions #4

Updated by Philipp Kitzberger 4 months ago

  • Description updated (diff)
Actions #5

Updated by Philipp Kitzberger 4 months ago

  • Subject changed from DB field without TCA breaks saving record process to DB field without TCA breaks saving be_users
  • Description updated (diff)
Actions #6

Updated by Philipp Kitzberger 4 months ago

  • Description updated (diff)
Actions #7

Updated by Garvin Hicking 4 months ago

  • Category set to Database API (Doctrine DBAL)
Actions #8

Updated by Stefan Bürk 4 months ago

  • Assignee set to Stefan Bürk
Actions #9

Updated by Stefan Bürk 4 months ago

  • Status changed from New to Needs Feedback

@Philipp Kitzberger thansk for reporting this. Sadly, I cannot reproduce this,
and from my side this does not make much sense.

The database schema used within the DataHandler is a cached form, which
is cleared after DataBase Analyzer operations to ensure information not
to be stalled. And this works correctly, even if I directly switch to
the backend user edit one and save it again.

If you alter the not tca managed field yourself, you need to ensure that
the system caches are flushed correctly - otherwise the database schema
data does not know anything for the renamed database field.

Use known cache flush methods for this.

As I tried different approaches to reproduce this (main, 13.4 and 12.4)
without any luck (except executing the alter manually without clearing the
cache) we need more (exact) description how you managed to get this state
and how to reproduce it.

Currently I would say no need to change anything due to missing cache clear
after a manual database operation.

Can you help us to understand and reproduce this issue ? Otherwise I would go
to close this issue in a couple of days as not beeing relevant.

Actions #10

Updated by Garvin Hicking 4 months ago

Specifically, are any custom cache configs involved or maybe mutagen/ddev that could "lag" the cache?

Actions #11

Updated by Philipp Kitzberger 4 months ago · Edited

It's true: after flushing all caches the problem is gone.

But still, why does a new field (that's not been created via "the system") disrupt the DataHandler this much?

Update: and I'm able to reproduce it with tt_content as well.

Actions #12

Updated by Stefan Bürk 4 months ago · Edited

But still, why does a new field (that's not been created via "the system") disrupt the DataHandler this much?

Again, the DataHandler works with a cache of the database schema. If you add or change the database, the cache data does not reflect these changes. In some places, these information are required to do data conversions.

However, if you open a record ALL fields are retrieved from the database, for example with the "added" field. When you click safe, the value is passed to the DataHandler. As it is not included within TCA, code paths scanning this would not do TCA based things with it. That specific code path uses the cached database information, and the field is not contained for that table in the cache. The exception thrown is from the classes populated from cache and not from real database, and thus emitting the exception.

Will have a look to harden that in some way, now that it is clear how this happend. Should not crash, still need tho thing about it how to deal with it which may be not that easy. Using uncached schema information is not reasonable, as schema information is quite happy and redoing that for multiple fields and tables is a real performance hog and the reason the schema has been cached (and cleared on TYPO3 controller database changes).

In general, I still postulate that schema should not changed outside / without TYPO3. But we will see to harden it at that specific place.

Clearing caches on system changes is a common required task, and changeing database should be taken as a system change anyway.

To summarize this:

  • Database Analyzer changes are safe as the caches are correctly cleared.
  • This specific (rare) case happens if database structure is changed with plain DDL either by code or from the outside without clearing the database structure cache.
  • We need decide how we can deal with this at the concrete place WITHOUT reading uncached schema information at that place. Not sure if data could be dropped silently, which may introduce other headache (for example if field is not nullabe etc).

Thanks for the feedback (and yes, it is clear that this is not bound to a specific table, just for changing database structure from outside without lushing the database schema cache.

Which should reduces the occurance of this issue a little bit, as most system would use the database analyzer to change the database scheme.

Actions #13

Updated by Stefan Bürk 4 months ago

  • Status changed from Needs Feedback to Accepted
Actions #14

Updated by Philipp Kitzberger 4 months ago

@Stefan Bürk, a big thank you for taking time to explain this matter!

However, if you open a record ALL fields are retrieved from the database, ...

It's happening as well when just toggling the hidden flag of a record btw.

Actions #15

Updated by Stefan Bürk 4 months ago

Philipp Kitzberger wrote in #note-14:

@Stefan Bürk, a big thank you for taking time to explain this matter!

However, if you open a record ALL fields are retrieved from the database, ...

It's happening as well when just toggling the hidden flag of a record btw.

Again, it does not matter which field is changed - the whole retrieved record is transfered witht the field added without updateing the cached database schema information.
Trying to mitigate that in some way without loosing the cache and circumstance the fact that someone is altering the TYPO3 managed database from the outside without taking care to clear the caches afterwards.

Actions #16

Updated by Garvin Hicking 4 months ago

How could we proceed with this? Add a note in the DB analyzer that says "Analysis works on internally cached information. Caches need to be flushed to recognize external changes to the database."?

Actions #17

Updated by Garvin Hicking 4 months ago

  • Status changed from Accepted to Needs Feedback
Actions #18

Updated by Garvin Hicking 3 months ago

  • Status changed from Needs Feedback to Closed

Thinking about this, I believe it is an edge case to modify data outside of TYPO3, and the "cache flush" dance should be implied when doing this. Adding a note for this might make the DB compare popup just too crowded and possibly confusing for most people who don't need to pay attention on this.

Therefore I'd like to close this issue and hope you agree. If not, please let me know how and where you'd improve the documentation/output and we can discuss. Thanks!

Actions #19

Updated by Stefan Bürk 3 months ago · Edited

  • Status changed from Closed to Accepted

Garvin Hicking wrote in #note-18:

Thinking about this, I believe it is an edge case to modify data outside of TYPO3, and the "cache flush" dance should be implied when doing this. Adding a note for this might make the DB compare popup just too crowded and possibly confusing for most people who don't need to pay attention on this.

Therefore I'd like to close this issue and hope you agree. If not, please let me know how and where you'd improve the documentation/output and we can discuss. Thanks!

Reopening this issue. I guess we should at least add an important rst file (or add it to exiting one if suitable) to make this clear, introduced with TYPO3 v12 for TCA json field support.

Additionally, I'mg going to have a look if we can avoid the dbal exception in DataHandler and "passthrough" the value unhandled in this case.

Adding a note in the DB compare does not make much sense. Changeing outside TCA manually does not enforce that people look into the GUI. And since typo3console allows to automate this many developers even more misses to visit the gui tool anyway. On the other side, the DB tool does not have an issue here when cache data is not refreshed. In anycase, if any DB Analyzer aciton is executed AFTER the manual change the schema state is invalidated and refreshed anyway.

Actions #20

Updated by Stefan Bürk 2 months ago

  • Related to Feature #99062: Native JSON field support in Doctrine DBAL added
  • Related to Feature #99226: Introduce dbType json for TCA type user added
Actions #21

Updated by Gerrit Code Review 2 months ago

  • Status changed from Accepted to Under Review

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

Actions #22

Updated by Gerrit Code Review 2 months ago

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

Actions #23

Updated by Gerrit Code Review 2 months ago

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

Actions #24

Updated by Gerrit Code Review 2 months ago

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

Actions #25

Updated by Gerrit Code Review 2 months ago

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

Actions

Also available in: Atom PDF