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

BUGFIX: Remove default value for subtreetags to fix MySQL support #5253

Open
wants to merge 1 commit into
base: 9.0
Choose a base branch
from

Conversation

bwaidelich
Copy link
Member

Fixes: #5252

@bwaidelich bwaidelich marked this pull request as ready for review September 19, 2024 07:51
@dlubitz
Copy link
Contributor

dlubitz commented Sep 19, 2024

I wonder if we need to check the string if empty, as this wouldn't be a valid JSON anymore by default.

$subtreeTagsArray = json_decode($subtreeTagsJson, true, 512, JSON_THROW_ON_ERROR);

But I guess it's fine, as we ensure this in the projection:

$subtreeTagsJson = json_encode($this->subtreeTags, JSON_THROW_ON_ERROR | JSON_FORCE_OBJECT);

@bwaidelich
Copy link
Member Author

I wonder if we need to check the string if empty, as this wouldn't be a valid JSON anymore by default.

I wondered the same, but the tests are green so ¯_(ツ)_/¯

@mhsdesign
Copy link
Member

should we also add a test via this pr to ensure we dont break mysql again? :D

@bwaidelich
Copy link
Member Author

should we also add a test via this pr to ensure we dont break mysql again?

Right. We already have tests, but we don't run them on MySQL – I had added that as a todo for #4337

Copy link
Member

@mhsdesign mhsdesign left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks fine by reading but i didnt test *g

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix MySQL support
3 participants