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.

Sign up for GitHub

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

Jump to bottom

Update PdoRepository::initSchema() to create table with profile as LONGTEXT. #474

Closed
wants to merge 1 commit into from
Closed

Update PdoRepository::initSchema() to create table with profile as LONGTEXT. #474

wants to merge 1 commit into from

Conversation

Charl13
Copy link

@Charl13 Charl13 commented Mar 2, 2022 •
edited by glensc
Loading

In larger applications profile column of type TEXT exceeds maximum allowed characters of 65535 characters. This leads to data not showing in UI since json_decode() returns null on truncated JSON string.

Replaces #437

Update PdoRepository::initSchema() to create table with profile as LO…
75ea31b 
…NGTEXT.

In larger applications profile column of type TEXT exceeds maximum
allowed characters of 65535 characters. This leads to data not showing
in UI since json_decode() returns null on truncated JSON string.
@Charl13 Charl13 mentioned this pull request Mar 2, 2022
Closed
@glensc
Copy link
Contributor

glensc commented Mar 2, 2022 •
edited
Loading

The exact same problem as the previous one (where you came from, but didn't link here):

This is not portable for all storage providers! And marking as Draft to indicate this is far from ready for merge.

@glensc glensc marked this pull request as draft March 2, 2022 16:42
@Charl13
Copy link
Author

Charl13 commented Mar 2, 2022 •
edited by glensc
Loading

Right, missed that one.

What do you suggest? Extract initSchema() and make an adapter for it based on the driver name?

@glensc
Copy link
Contributor

glensc commented Mar 2, 2022

Simplest would be adopt based on driver name

@Charl13
Copy link
Author

Charl13 commented Apr 13, 2022

Can I use doctrine/dbal to refactor the PdoRepository? This allows us to be more flexible on the database platform.

@glensc
Copy link
Contributor

glensc commented Apr 13, 2022

Read the comments in the previous PR, especially:

and use the new property to make conditions in code.

doctrine/dbal seems currently way overkill and considering they've made changes in minor versions such way that multiple of my projects just failed and had to StackOverflow to solve the problems, I personally wouldn't want to use doctrine/* at all at this time.

so let's leave bigger refactoring to the future.

@glensc
Copy link
Contributor

glensc commented Apr 19, 2022 •
edited
Loading

Adding check for $this->driverName is pretty trivial. No longer interested of this?

@glensc glensc mentioned this pull request Oct 7, 2022
Closed
@Mmasson-01 Mmasson-01 mentioned this pull request Dec 21, 2022
Merged
@glensc
Copy link
Contributor

glensc commented Dec 21, 2022

Replaced by #487

@glensc glensc closed this Dec 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Charl13 @glensc