Skip to content

Bypass CompositeBuilder for parameterless composite types#6458

Open
yykkibbb wants to merge 1 commit intonpgsql:mainfrom
yykkibbb:bypass-composite-builder-for-parameterless-types
Open

Bypass CompositeBuilder for parameterless composite types#6458
yykkibbb wants to merge 1 commit intonpgsql:mainfrom
yykkibbb:bypass-composite-builder-for-parameterless-types

Conversation

@yykkibbb
Copy link
Copy Markdown

@yykkibbb yykkibbb commented Feb 19, 2026

Fixes #6452

When ConstructorParameters == 0, CompositeBuilder is unnecessary — it just creates the instance, boxes it, and calls field.Set() for each field. This adds a fast path that skips the builder entirely, avoiding per-read allocations.

Changes

  • Add ReadAndSet / SetDbNull methods to CompositeFieldInfo for direct field setting
  • Split CompositeConverter.Read() into ReadDirect (fast path) and ReadWithBuilder (existing path)
  • Extract ValidateOid as a shared static method
  • Add test for nullable property through the fast path

Skip CompositeBuilder when ConstructorParameters == 0 by reading
and setting field values directly on the instance via new
ReadAndSet/SetDbNull methods on CompositeFieldInfo.
@yykkibbb yykkibbb force-pushed the bypass-composite-builder-for-parameterless-types branch from 64b3667 to fde8d8d Compare February 19, 2026 14:19
@yykkibbb
Copy link
Copy Markdown
Author

yykkibbb commented Feb 19, 2026

This implements the approach @NinoFloris suggested in #6452. All existing composite tests pass along with a new test for nullable fields through the fast path.

@roji @vonzshik Open to any feedback on the implementation! — happy to adjust if there's a better approach or improvements to make.

Copy link
Copy Markdown
Member

@NinoFloris NinoFloris left a comment

Choose a reason for hiding this comment

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

Thanks @yykkibbb, though this shows that it can work I'd rather see a more minimal diff.
Ideally you would mostly keep the current api surface but allow the builder to be an instance of T as well. This would mean both paths incur an exact type type check (if builder is CompositeBuilder<T>) but that should be well predicted and cheap.

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.

Cache for CompositeBuilder<T>

2 participants