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

workarounds for selecting ElementCollection attributes #30581

Conversation

njr-11
Copy link
Contributor

@njr-11 njr-11 commented Jan 17, 2025

EclipseLink has wrong behavior when ElementCollection attributes are selected, as shown by #30575
This PR puts in place the following workarounds, to the degree that we are able,

When a single result of a selected ElementCollection is returned, we can sometimes implement a workaround to identify the mismatch and generate the correct type of collection, populated with the results.

When multiple results of a selected ElementCollection are returned, EclipseLink combines the values into a single vector where it is impossible to split them back out into the separate results that they are. We attempt to detect this scenario and raise our own UnsupportedOperationException in order to spare the user from unexpected wrong behavior.

There are also some improvements in this area to type conversions.

@njr-11
Copy link
Contributor Author

njr-11 commented Jan 17, 2025

#build (view Open Liberty Personal Build - ❌ completed with errors/failures.)

Note: Target locations of links might be accessible only to IBM employees.

@LibbyBot
Copy link

Code analysis and actions

DO NOT DELETE THIS COMMENT.
  • 6 FAT files were changed, added, or removed.

  • Check that the build did not break the affected FAT suite(s).

  • 3 product code files were changed.

  • Please describe in a separate comment how you tested your changes.

  • 1 messages files were changed and need an L2 review.

  • @OpenLiberty/message-reviewer Please review.

    • dev/io.openliberty.data.internal.persistence/resources/io/openliberty/data/internal/persistence/resources/CWWKDMessages.nlsprops
  • 1 NLS files were changed and need an ID review.

  • @OpenLiberty/message-reviewer Please review.

    • dev/io.openliberty.data.internal.persistence/resources/io/openliberty/data/internal/persistence/resources/CWWKDMessages.nlsprops

@KyleAure
Copy link
Member

KyleAure commented Jan 17, 2025

Database Rotation SOE for this PR: https://libh-proxy1.fyre.ibm.com/cognitive-dev/pipelineAnalysis.html?pipelineId=24ac3a31-414e-4ba4-825c-bcf390c9866e

Note: Target locations of links might be accessible only to IBM employees.

@njr-11
Copy link
Contributor Author

njr-11 commented Jan 17, 2025

@is273 thank you for reviewing the messages. All of the updates you asked for are included in a363dac
Please add your approval once you confirm.

@njr-11
Copy link
Contributor Author

njr-11 commented Jan 17, 2025

New build needed due to additional commit with NLS message updates:
(view Open Liberty Personal Build - ❌ completed with errors/failures.)
#build
The other build for database rotation should still be fine because NLS updates don't make difference to what is being covered there.

Note: Target locations of links might be accessible only to IBM employees.

@LibbyBot
Copy link

Code analysis and actions

DO NOT DELETE THIS COMMENT.
  • 6 FAT files were changed, added, or removed.

  • Check that the build did not break the affected FAT suite(s).

  • 3 product code files were changed.

  • Please describe in a separate comment how you tested your changes.

  • 1 messages files were changed and need an L2 review.

  • @OpenLiberty/message-reviewer Please review.

    • dev/io.openliberty.data.internal.persistence/resources/io/openliberty/data/internal/persistence/resources/CWWKDMessages.nlsprops
  • 1 NLS files were changed and need an ID review.

  • @OpenLiberty/message-reviewer Please review.

    • dev/io.openliberty.data.internal.persistence/resources/io/openliberty/data/internal/persistence/resources/CWWKDMessages.nlsprops

@njr-11
Copy link
Contributor Author

njr-11 commented Jan 21, 2025

New build needed due to additional commit with NLS message updates: (view Open Liberty Personal Build - ❌ completed with errors/failures.) #build The other build for database rotation should still be fine because NLS updates don't make difference to what is being covered there.
(view Open Liberty Personal Build - ❌ completed with errors/failures.)

Note: Target locations of links might be accessible only to IBM employees.

The build has 1 failure, which is a preexisting intermittent issue in unrelated test bucket com.ibm.ws.jbatch.rest.api_fat

Note: Target locations of links might be accessible only to IBM employees.

@njr-11 njr-11 merged commit 56d85ff into OpenLiberty:integration Jan 21, 2025
2 of 3 checks passed
@LibbyBot
Copy link

Code analysis and actions

DO NOT DELETE THIS COMMENT.
  • 6 FAT files were changed, added, or removed.

  • Check that the build did not break the affected FAT suite(s).

  • 3 product code files were changed.

  • Please describe in a separate comment how you tested your changes.

  • 1 messages files were changed and need an L2 review.

  • @OpenLiberty/message-reviewer Please review.

    • dev/io.openliberty.data.internal.persistence/resources/io/openliberty/data/internal/persistence/resources/CWWKDMessages.nlsprops
  • 1 NLS files were changed and need an ID review.

  • @OpenLiberty/message-reviewer Please review.

    • dev/io.openliberty.data.internal.persistence/resources/io/openliberty/data/internal/persistence/resources/CWWKDMessages.nlsprops

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.

4 participants