Skip to content
This repository has been archived by the owner on Dec 3, 2024. It is now read-only.

Added SerializationHelper that handles PostgreSql serialization correctly & replaced all serialization function calls #63

Closed
wants to merge 20 commits into from
Closed

Added SerializationHelper that handles PostgreSql serialization correctly & replaced all serialization function calls #63

wants to merge 20 commits into from

Conversation

blazpezdir
Copy link
Contributor

@blazpezdir blazpezdir commented Nov 14, 2022 •
edited
Loading

Fixes #61

@blazpezdir blazpezdir mentioned this pull request Nov 15, 2022
Closed
@blazpezdir blazpezdir changed the title Added SerializationHelper that handles PostgreSql serialization correctly to avoid truncating string due to null byte values & replaced all serilazioation function calls Added SerializationHelper that handles PostgreSql serialization correctly to avoid truncating string due to null byte values & replaced all serialization function calls Nov 15, 2022
@blazpezdir blazpezdir changed the title Added SerializationHelper that handles PostgreSql serialization correctly to avoid truncating string due to null byte values & replaced all serialization function calls Added SerializationHelper that handles PostgreSql serialization correctly & replaced all serialization function calls Nov 15, 2022
Sammyjo20
Sammyjo20 suggested changes Nov 20, 2022
View reviewed changes
Copy link
Owner

@Sammyjo20 Sammyjo20 left a comment

Choose a reason for hiding this comment

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

Awesome PR, thank you! 🙌

My main question is around the use of DB::connection() - will it respect the overwritten DB connection on a per-model basis? I'm thinking of this PR

#60

src/Helpers/SerializationHelper.php
{
$serialized = serialize($value);

return DB::connection() instanceof PostgresConnection
Copy link
Owner

Choose a reason for hiding this comment

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

Should we be using the connection that has been specified on the model instead of the global DB connection? From Laravel's PR:

https://github.com/laravel/framework/pull/36081/files#diff-c46d3e14c232058da1ea59d469b8abc9866c1ced2104eece667c4077b54a5e22R304-R306

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I think you are right. This solution only covers the default scenario. I will look into it and make fixes accordingly.

@ksimenic ksimenic mentioned this pull request Feb 21, 2023
Merged
@Sammyjo20
Copy link
Owner

Merged with #69

@Sammyjo20 Sammyjo20 closed this Feb 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@Sammyjo20 Sammyjo20 Sammyjo20 requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

HaystackBale serialization not working with PostgreSql
2 participants
@blazpezdir @Sammyjo20