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

Changed mapped composite handler to be lazily initialized #2483

Merged
merged 1 commit into from
Jun 2, 2019
Merged

Changed mapped composite handler to be lazily initialized #2483

merged 1 commit into from
Jun 2, 2019

Conversation

YohDeadfall
Copy link
Contributor

@YohDeadfall YohDeadfall commented May 23, 2019

Addresses #2476 (comment).

@YohDeadfall YohDeadfall added this to the 4.1 milestone May 23, 2019
@YohDeadfall YohDeadfall self-assigned this May 23, 2019
Copy link
Member

@roji roji left a comment

Choose a reason for hiding this comment

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

Thanks, see two nits but LGTM.

I guess next thing #2476 needs to be rebased?

length += member.ValidateAndGetLength(value, ref lengthCache);

return lengthCache.Lengths[position] = length;
}

public static MappedCompositeHandler<T> Create(PostgresCompositeType pgType, ConnectorTypeMapper typeMapper, INpgsqlNameTranslator nameTranslator)
CompositeMemberHandler<T>[] CreateMemberHandlers() => CreateMemberHandlers((PostgresCompositeType)PostgresType, _typeMapper, _nameTranslator);
Copy link
Member

Choose a reason for hiding this comment

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

nit: for extra cleanliness we could hide base.PostgresType with a typed new property:

new PostgresCompositeType PostgresType => (PostgresCompositeType)base.PostgresType;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

A nice idea. I will use it twice (am working on ctor support), so it makes sense.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do in the next PR since there are some changes in handler. So probably this property would be removed.

@YohDeadfall YohDeadfall merged commit 054b4e4 into npgsql:dev Jun 2, 2019
@YohDeadfall YohDeadfall deleted the lazy-composite-init branch June 2, 2019 19:34
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.

3 participants