Review for 520387

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

Review for 520387

William Dazey
Hello,
I'd like to get a review for https://bugs.eclipse.org/bugs/show_bug.cgi?id=520387
I would also like to know if this use case should be valid for EclipseLink. Reading the JPA spec, section 11.1.20, it states:

The use of the GeneratedValue annotation is only required to be supported for simple primary keys.

Does my usecase qualify as a "simple primary key"? or is this a complex primary key. I was able to create a fix for the issue, but I am unsure as to the ramifications of such a change. I would really appreciate any feedback.

Thanks,
Will Dazey

_______________________________________________
eclipselink-dev mailing list
[hidden email]
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/eclipselink-dev
Reply | Threaded
Open this post in threaded view
|

Re: Review for 520387

Lukas Jungmann
Hi Will,


On 8/2/17 2:21 AM, William Dazey wrote:
> Hello,
> I'd like to get a review for
> https://bugs.eclipse.org/bugs/show_bug.cgi?id=520387
> I would also like to know if this use case should be valid for
> EclipseLink. Reading the JPA spec, section 11.1.20, it states:
>
> The use of the GeneratedValue annotation is only required to be
> supported for simple primary keys.
and section 11.1.17 says "The EmbeddedId annotation is applied to a
persistent field or property of an entity class or mapped
superclass to denote a composite primary key that is an embeddable class."
>
> Does my usecase qualify as a "simple primary key"?
No, as per 11.1.17 and also section 2.4: "The Id annotation or id XML
element must be used to denote a simple primary key." - I see no @Id in
your use-case.

> or is this a complex primary key. I was able to create a fix for the
> issue, but I am unsure as to the ramifications of such a change. I
> would really appreciate any feedback.
to me it looks like support for the usage of @GeneratedValue on non-@Id
fields is currently up to the provider but there is
https://github.com/javaee/jpa-spec/issues/113 - and in its context your
fix makes sense to me.

thanks,
--lukas

>
> Thanks,
> Will Dazey
>
>
> _______________________________________________
> eclipselink-dev mailing list
> [hidden email]
> To change your delivery options, retrieve your password, or unsubscribe from this list, visit
> https://dev.eclipse.org/mailman/listinfo/eclipselink-dev

_______________________________________________
eclipselink-dev mailing list
[hidden email]
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/eclipselink-dev
Reply | Threaded
Open this post in threaded view
|

Re: Review for 520387

William Dazey
Thanks for the feedback Lukas! The part that I get stuck on is that this seems to "mostly" work already in EclipseLink. Using an @GeneratedValue annotation within an @Embeddable class that is being used as an @EmbeddedId does seem to work as you may expect. However, the "bug" occurs because the @Embeddable class is reused across

The Embedded field is seen as a PK field by EclipseLink since it's used within the @EmbeddedId class. I really want to make sure tho if this is a either a bug or adding a new feature to EclipseLink. If EclipseLink is documented to support using @GeneratedValue within @EmbeddedId classes, then it seems to be a bug that needs to be fixed. If EclipseLink is not documented to support this tho, then it would seem to be a feature request / enhancement and I would prioritize this differently.

Thanks,
Will Dazey

On Wed, Aug 2, 2017 at 7:06 AM, Lukas Jungmann <[hidden email]> wrote:
Hi Will,


On 8/2/17 2:21 AM, William Dazey wrote:
Hello,
I'd like to get a review for https://bugs.eclipse.org/bugs/show_bug.cgi?id=520387
I would also like to know if this use case should be valid for EclipseLink. Reading the JPA spec, section 11.1.20, it states:

The use of the GeneratedValue annotation is only required to be supported for simple primary keys.
and section 11.1.17 says "The EmbeddedId annotation is applied to a persistent field or property of an entity class or mapped
superclass to denote a composite primary key that is an embeddable class."

Does my usecase qualify as a "simple primary key"?
No, as per 11.1.17 and also section 2.4: "The Id annotation or id XML element must be used to denote a simple primary key." - I see no @Id in your use-case.

or is this a complex primary key. I was able to create a fix for the issue, but I am unsure as to the ramifications of such a change. I would really appreciate any feedback.
to me it looks like support for the usage of @GeneratedValue on non-@Id fields is currently up to the provider but there is https://github.com/javaee/jpa-spec/issues/113 - and in its context your fix makes sense to me.

thanks,
--lukas

Thanks,
Will Dazey


_______________________________________________
eclipselink-dev mailing list
[hidden email]
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/eclipselink-dev

_______________________________________________
eclipselink-dev mailing list
[hidden email]
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/eclipselink-dev


_______________________________________________
eclipselink-dev mailing list
[hidden email]
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/eclipselink-dev
Reply | Threaded
Open this post in threaded view
|

Re: Review for 520387

William Dazey
Please disregard the previous email, it was sent incomplete.

Thanks for the feedback Lukas! The part that I get stuck on is that this seems to "mostly" work already in EclipseLink. Using an @GeneratedValue annotation within an @Embeddable class that is being used as an @EmbeddedId does seem to work as you may expect. However, the "bug" occurs because the @Embeddable class is nested across multiple @Embeddables and we only preprocess once.

The Embedded field is seen as a PK field by EclipseLink since it's used within the @EmbeddedId class. I really want to make sure tho if this is a either a bug or adding a new feature to EclipseLink. If EclipseLink is documented to support using @GeneratedValue within @EmbeddedId classes, then it seems to be a bug that needs to be fixed. If EclipseLink is not documented to support this tho, then it would seem to be a feature request / enhancement and I would prioritize this differently.

Thanks,
Will Dazey

On Wed, Aug 2, 2017 at 10:22 AM, William Dazey <[hidden email]> wrote:
Thanks for the feedback Lukas! The part that I get stuck on is that this seems to "mostly" work already in EclipseLink. Using an @GeneratedValue annotation within an @Embeddable class that is being used as an @EmbeddedId does seem to work as you may expect. However, the "bug" occurs because the @Embeddable class is reused across

The Embedded field is seen as a PK field by EclipseLink since it's used within the @EmbeddedId class. I really want to make sure tho if this is a either a bug or adding a new feature to EclipseLink. If EclipseLink is documented to support using @GeneratedValue within @EmbeddedId classes, then it seems to be a bug that needs to be fixed. If EclipseLink is not documented to support this tho, then it would seem to be a feature request / enhancement and I would prioritize this differently.

Thanks,
Will Dazey

On Wed, Aug 2, 2017 at 7:06 AM, Lukas Jungmann <[hidden email]> wrote:
Hi Will,


On 8/2/17 2:21 AM, William Dazey wrote:
Hello,
I'd like to get a review for https://bugs.eclipse.org/bugs/show_bug.cgi?id=520387
I would also like to know if this use case should be valid for EclipseLink. Reading the JPA spec, section 11.1.20, it states:

The use of the GeneratedValue annotation is only required to be supported for simple primary keys.
and section 11.1.17 says "The EmbeddedId annotation is applied to a persistent field or property of an entity class or mapped
superclass to denote a composite primary key that is an embeddable class."

Does my usecase qualify as a "simple primary key"?
No, as per 11.1.17 and also section 2.4: "The Id annotation or id XML element must be used to denote a simple primary key." - I see no @Id in your use-case.

or is this a complex primary key. I was able to create a fix for the issue, but I am unsure as to the ramifications of such a change. I would really appreciate any feedback.
to me it looks like support for the usage of @GeneratedValue on non-@Id fields is currently up to the provider but there is https://github.com/javaee/jpa-spec/issues/113 - and in its context your fix makes sense to me.

thanks,
--lukas

Thanks,
Will Dazey


_______________________________________________
eclipselink-dev mailing list
[hidden email]
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/eclipselink-dev

_______________________________________________
eclipselink-dev mailing list
[hidden email]
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/eclipselink-dev



_______________________________________________
eclipselink-dev mailing list
[hidden email]
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/eclipselink-dev
Reply | Threaded
Open this post in threaded view
|

Re: Review for 520387

Lukas Jungmann
On 8/2/17 5:24 PM, William Dazey wrote:

> Please disregard the previous email, it was sent incomplete.
>
> Thanks for the feedback Lukas! The part that I get stuck on is that
> this seems to "mostly" work already in EclipseLink. Using an
> @GeneratedValue annotation within an @Embeddable class that is being
> used as an @EmbeddedId does seem to work as you may expect. However,
> the "bug" occurs because the @Embeddable class is nested across
> multiple @Embeddables and we only preprocess once.
>
> The Embedded field is seen as a PK field by EclipseLink since it's
> used within the @EmbeddedId class. I really want to make sure tho if
> this is a either a bug or adding a new feature to EclipseLink. If
> EclipseLink is documented to support using @GeneratedValue within
> @EmbeddedId classes, then it seems to be a bug that needs to be fixed.
> If EclipseLink is not documented to support this tho, then it would
> seem to be a feature request / enhancement and I would prioritize this
> differently.
https://bugs.eclipse.org/bugs/show_bug.cgi?id=211329 - so it looks like
a bug to me even though I haven't found it documented anywhere :-/

thanks,
--lukas

>
> Thanks,
> Will Dazey
>
> On Wed, Aug 2, 2017 at 10:22 AM, William Dazey <[hidden email]
> <mailto:[hidden email]>> wrote:
>
>     Thanks for the feedback Lukas! The part that I get stuck on is
>     that this seems to "mostly" work already in EclipseLink. Using an
>     @GeneratedValue annotation within an @Embeddable class that is
>     being used as an @EmbeddedId does seem to work as you may expect.
>     However, the "bug" occurs because the @Embeddable class is reused
>     across
>
>     The Embedded field is seen as a PK field by EclipseLink since it's
>     used within the @EmbeddedId class. I really want to make sure tho
>     if this is a either a bug or adding a new feature to EclipseLink.
>     If EclipseLink is documented to support using @GeneratedValue
>     within @EmbeddedId classes, then it seems to be a bug that needs
>     to be fixed. If EclipseLink is not documented to support this tho,
>     then it would seem to be a feature request / enhancement and I
>     would prioritize this differently.
>
>     Thanks,
>     Will Dazey
>
>     On Wed, Aug 2, 2017 at 7:06 AM, Lukas Jungmann
>     <[hidden email] <mailto:[hidden email]>> wrote:
>
>         Hi Will,
>
>
>         On 8/2/17 2:21 AM, William Dazey wrote:
>
>             Hello,
>             I'd like to get a review for
>             https://bugs.eclipse.org/bugs/show_bug.cgi?id=520387
>             <https://bugs.eclipse.org/bugs/show_bug.cgi?id=520387>
>             I would also like to know if this use case should be valid
>             for EclipseLink. Reading the JPA spec, section 11.1.20, it
>             states:
>
>             The use of the GeneratedValue annotation is only required
>             to be supported for simple primary keys.
>
>         and section 11.1.17 says "The EmbeddedId annotation is applied
>         to a persistent field or property of an entity class or mapped
>         superclass to denote a composite primary key that is an
>         embeddable class."
>
>
>             Does my usecase qualify as a "simple primary key"?
>
>         No, as per 11.1.17 and also section 2.4: "The Id annotation or
>         id XML element must be used to denote a simple primary key." -
>         I see no @Id in your use-case.
>
>             or is this a complex primary key. I was able to create a
>             fix for the issue, but I am unsure as to the ramifications
>             of such a change. I would really appreciate any feedback.
>
>         to me it looks like support for the usage of @GeneratedValue
>         on non-@Id fields is currently up to the provider but there is
>         https://github.com/javaee/jpa-spec/issues/113
>         <https://github.com/javaee/jpa-spec/issues/113> - and in its
>         context your fix makes sense to me.
>
>         thanks,
>         --lukas
>
>
>             Thanks,
>             Will Dazey
>
>
>             _______________________________________________
>             eclipselink-dev mailing list
>             [hidden email]
>             <mailto:[hidden email]>
>             To change your delivery options, retrieve your password,
>             or unsubscribe from this list, visit
>             https://dev.eclipse.org/mailman/listinfo/eclipselink-dev
>             <https://dev.eclipse.org/mailman/listinfo/eclipselink-dev>
>
>
>         _______________________________________________
>         eclipselink-dev mailing list
>         [hidden email] <mailto:[hidden email]>
>         To change your delivery options, retrieve your password, or
>         unsubscribe from this list, visit
>         https://dev.eclipse.org/mailman/listinfo/eclipselink-dev
>         <https://dev.eclipse.org/mailman/listinfo/eclipselink-dev>
>
>
>
>
>
> _______________________________________________
> eclipselink-dev mailing list
> [hidden email]
> To change your delivery options, retrieve your password, or unsubscribe from this list, visit
> https://dev.eclipse.org/mailman/listinfo/eclipselink-dev

_______________________________________________
eclipselink-dev mailing list
[hidden email]
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/eclipselink-dev