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

JSON_TO_ABAP: fix compressed JSON array to sorted/hashed table keeping components problem #182

Merged

Conversation

thorsten-wolf-neptune
Copy link
Contributor

This PR reproduces the issue that is tracked #181 and contains a fix proposal.

@larshp
Copy link
Collaborator

larshp commented Jul 3, 2024

perhaps move the CLEAR up to below code, to have less impact on performance

image

@thorsten-wolf-neptune
Copy link
Contributor Author

tried that at first but that is outside of the loop of the array items so it wouldn't work
3bbb2d7#diff-018e6f26075d703b00b41a45a1c76360a17a8ce9846d7314b7b71c9a151f71b5R856

@larshp
Copy link
Collaborator

larshp commented Jul 8, 2024

@sbcgua

Copy link
Owner

@sbcgua sbcgua left a comment

Choose a reason for hiding this comment

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

Good catch, thank you for the analysis and fix. And good UTs!

I would just suggest moving the clear a bit above to if at 887 line. Cannot add a commit directly to your branch

image

@thorsten-wolf-neptune
Copy link
Contributor Author

Thx :-) i only noticed it because in abapgit we have "skip initial fields" set to true for some TABU entries and when pulling it to a new system the tables where completely messed up :-)

@larshp once that is merged/fixed abapgit would need to also get the mirror updated or? Should i create an issue there too?

Copy link
Owner

@sbcgua sbcgua left a comment

Choose a reason for hiding this comment

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

👍

@sbcgua sbcgua merged commit 9636661 into sbcgua:master Jul 9, 2024
2 checks passed
@larshp
Copy link
Collaborator

larshp commented Jul 9, 2024

abapGit/abapGit#6977

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.

3 participants