Skip to content
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

Audit tables column diffs type unexpectedly changed from JSON to TEXT #52

Closed
pniep opened this issue Oct 5, 2021 · 7 comments · Fixed by #170
Closed

Audit tables column diffs type unexpectedly changed from JSON to TEXT #52

pniep opened this issue Oct 5, 2021 · 7 comments · Fixed by #170
Labels
bug Something isn't working feedback needed Further information is requested

Comments

@pniep
Copy link

pniep commented Oct 5, 2021

Hi @DamienHarper,

After update doctrine wants to change type of all diffs columns in audit tables from json to text. [db: postgres, json support: yes]
I'm not sure that was intended.

Fragment below forces TEXT as the type of all JSON columns when database actually supports json columns .

if (DoctrineHelper::getDoctrineType('JSON') === $struct['type'] && $isJsonSupported) {

if (DoctrineHelper::getDoctrineType('JSON') === $struct['type'] && $isJsonSupported) {
    $type = DoctrineHelper::getDoctrineType('TEXT');
} else {
    $type = $struct['type'];
}

It looks like a bug. As commit message for these lines says the intent was to add compatibility for older versions of mariadb that don't support json.

@pniep pniep changed the title Columns type is unexpectedly changed from JSON to TEXT Audit tables column diffs type unexpectedly changed from JSON to TEXT Oct 5, 2021
@DamienHarper DamienHarper added bug Something isn't working feedback needed Further information is requested labels Oct 23, 2021
@DamienHarper
Copy link
Owner

@pniep thanks for the feedback.
In order to ease debugging/investigation, could you please provide me your current

  • PHP version
  • database type and version
  • auditor version

@dappa
Copy link

dappa commented Oct 29, 2021

I have the same problem. I suspect the problem is the check for $isJsonSupported should be !$isJsonSupported as the fallback to TEXT is for those who do not support json.

if (DoctrineHelper::getDoctrineType('JSON') === $struct['type'] && $isJsonSupported) {

@DamienHarper
Copy link
Owner

@dappa thanks for the feedback, looks like a bug. As I asked to @pniep, would you mind providing me your current

  • PHP version
  • database type and version
  • auditor version

@pniep
Copy link
Author

pniep commented Oct 29, 2021

@DamienHarper
PHP 8.0.12
PostgreSQL 13.2
auditor 1.3
auditor-bundle 4.2

doctrine/orm 2.10.1
doctrine/dbal 2.13.4

@dappa
Copy link

dappa commented Oct 29, 2021

@DamienHarper
PHP 7.4.3
mysql Ver 8.0.26
auditor 1.30

@dappa
Copy link

dappa commented Sep 23, 2022

Any chance to revisit this issue?
It seems to be present on 2.0.5 still.

@DamienHarper
Copy link
Owner

@dappa @pniep Fixed in master and 2.4.7.
Sorry it took so long.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working feedback needed Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants