Skip to content

Implement the Union Shape#2019

Merged
jderusse merged 1 commit intoasync-aws:masterfrom
jderusse:union
Jan 16, 2026
Merged

Implement the Union Shape#2019
jderusse merged 1 commit intoasync-aws:masterfrom
jderusse:union

Conversation

@jderusse
Copy link
Member

fix #1900

This PR add support for union shape.

  • Use abstract class that return null for all membres
  • For each member, add a TheParentMemberTheChild class
  • Child class extends provide a single not null property (parent provides the rest of the props)
  • Populate methods returns the right child and throws exception when the child is unknown (all members are null)
  • static create methods return the right child and throws exception if the child is unknown

@jderusse jderusse force-pushed the union branch 6 times, most recently from e2b66b9 to 0bdd20d Compare December 26, 2025 16:58
@stof
Copy link
Member

stof commented Dec 28, 2025

  • static create methods return the right child and throws exception if the child is unknown

this would mean we throw an exception if the API answers with a new value in the union, which is something that AWS does not treat as a BC break.
The official AWS SDKs instead of a special member in the union UNKOWN_TO_SDK (the actual name differs between languages but they have such concept).

  • Use abstract class that return null for all membres

I don't think having all nullable properties is a good idea. It makes the SDK types less precise for dubious benefits. See my comment on the issue showing that your "simple" code justifying this choice is actually buggy code (which won't be detectable by static analysis in this solution, while it could be detected in others)

Copy link
Member

@stof stof left a comment

Choose a reason for hiding this comment

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

If you want the convenience API for cases with known types, maybe we should follow the lead of the Kotlin SDK.

@jderusse jderusse force-pushed the union branch 4 times, most recently from 56b8195 to db31b6d Compare January 3, 2026 21:03
@jderusse jderusse force-pushed the union branch 3 times, most recently from b364e0d to 7fecd69 Compare January 3, 2026 21:20
@jderusse jderusse force-pushed the union branch 3 times, most recently from 6b0f2ef to 3324be6 Compare January 3, 2026 22:07
@jderusse
Copy link
Member Author

jderusse commented Jan 3, 2026

The last remaining thing is deciding whether "abstract class returns null by default or not"

I need time to process this, and check if makes sense or not.

If you want the convenience API for cases with known types, maybe we should follow the lead of the Kotlin SDK.

I'm not sure to understand what you mean by this?
edit

@jderusse
Copy link
Member Author

jderusse commented Jan 5, 2026

after looking deeper, indeed, having null is a fortunate edge case for RDS results because null is a possible value of the union.

I've removed it from this PR

@jderusse jderusse force-pushed the union branch 6 times, most recently from b861147 to cd5bba1 Compare January 7, 2026 12:43
@jderusse jderusse force-pushed the union branch 3 times, most recently from acfc2c3 to f634c31 Compare January 16, 2026 13:44
@jderusse jderusse merged commit 2281f99 into async-aws:master Jan 16, 2026
12 checks passed
@jderusse jderusse deleted the union branch January 16, 2026 14:33
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.

Add support for union shapes in the code generator

2 participants

Comments