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

#3129 Fix for 15.x to verify DBJson dirtiness using same field orderi… #3402

Conversation

AntoineDuComptoirDesPharmacies

This pull request aim to fix a problem (#3129) where @DbJsonB fields are always considered dirty on load if the order of their properties do not match fields ordering from data stored in Postgres.

This is due to the fact that Postgres is re-ordering the fields if column type is a JsonB in order to optimize storage / index / etc...

To solve this problem, we parse/format the value coming from readSet method in order to be sure that comparison will be done using Java Jackson encoded JSON on both side.

…ld ordering on both sides (due to PostgreSQL reordering fields for JsonB types)
@AntoineDuComptoirDesPharmacies
Copy link
Author

Hi @rbygrave,

We do not know why the pipeline is not successful on kotlin build.
On our computer, everything compile fine and tests pass.

You will certainly have a better insight about this failure.

Thanks in advance.
Yours faithfully,
LCDP

@rbygrave rbygrave changed the base branch from ebean-15x to master May 15, 2024 20:32
@rbygrave rbygrave changed the base branch from master to ebean-15x May 15, 2024 20:33
@rbygrave
Copy link
Member

Ah right, a couple of things here.

  1. I broke the build - I broke the test kotlin module part of the build which needed a bump to the kotlin-querybean-generator
  2. That test / test-kotlin module is conditionally run ONLY when JDK 11 is used (so we can not notice the issue locally when we are not using JDK 11 which for me is most of the time as I'm 99% of the time locally on JDK 21 now).
  3. I fixed the build in master (but not in ebean-15x branch yet)

So your PR should be all good in the sense that the CI didn't pass because of my mistake.

Now, currently we operate with all changes going to master branch and then those changes are merged back to ebean-15x branch.

That is, any and all changes are applied to master and thus go into 14.x, 14.x-javax and 15.x ... so all new features and bug fixes go into master. When we release we release all 3 of these versions and they are all aligned in terms of features / bug fixes / version numbers ... with the exception that 15.x has some functionality removed.

So in general we want all PRs to be applied back to master. As part of the release I merge those changes to ebean-15x branch and resolve any conflicts as needed and release.

I'll look to merge that fix into the ebean-15x branch, but I think we really want this change to be merged back to master. I don't think we can just re-target this PR to master because it will then look to bring in the changes in ebean-15x, so I think we need a new PR based off master with these changes in it.

Can I get you to create a new PR based off master branch with these changes? (and looking to merge those changes into master instead of ebean-15x).

@AntoineDuComptoirDesPharmacies
Copy link
Author

Hi @rbygrave,

This make perfect sense ! In the future we will be careful using master only for contributions.
Here is the new PR starting and targeting master branch : #3405

Yours faithfully,
LCDP

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

Successfully merging this pull request may close these issues.

None yet

2 participants