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

IsNew always return false when PrimaryKey is typeof string and value is "" #538

Closed
ArgoZhang opened this issue May 24, 2019 · 4 comments

Comments

@ArgoZhang
Copy link
Contributor

commented May 24, 2019

hi, I found that IsNew function always return false when the poco's PrimaryKey is System.String type and value is "".
in line 2532 will return. so line 2535 will be never work.

if (type.IsGenericType && type.GetGenericTypeDefinition() == typeof(Nullable<>) || !type.IsValueType)
    return pk == null;

if (type == typeof(string))
    return string.IsNullOrEmpty((string) pk);

it will work fine when the poco PrimaryId value is null.
is it by design? or a bug?

@ArgoZhang

This comment has been minimized.

Copy link
Contributor Author

commented May 31, 2019

how about this? any question?

@ArgoZhang

This comment has been minimized.

Copy link
Contributor Author

commented Jun 17, 2019

still busy? really hope resolve IsNew and Mapper.Register issues. So many string type Primary key in my project. i do not want to create a new class inherit Database and override IsNew function. any question please let me known. thanks

ArgoZhang added a commit to ArgoZhang/PetaPoco that referenced this issue Aug 2, 2019

pleb added a commit that referenced this issue Aug 2, 2019

refactor(IsNew): check string type PrimaryKey first #538 (#542)
* refactor(IsNew): check string type PrimaryKey first #538

#Issue
link #538

* doc: change file endings
@pleb

This comment has been minimized.

Copy link
Member

commented Aug 2, 2019

@ArgoZhang, this is now in dev. Use the latest beta nuget package to try it out

@ArgoZhang

This comment has been minimized.

Copy link
Contributor Author

commented Aug 2, 2019

@pleb I've tested it. It's working very well.

@ArgoZhang ArgoZhang closed this Aug 2, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.