HHH-18723 Support @SQLRestriction in class marked as @MappedSuperclass #9085

a-e-tsvetkov · 2024-10-12T06:38:14Z

I have several entities with common super class SoftDeleteSupport marked with @MappedSuperclass annotation. It have boolean rmv; field.

Currently I have to put @SQLRestriction in each entity.

I propose to support @SQLRestriction on classes marked with @MappedSuperclass t reduce code duplication.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license
and can be relicensed under the terms of the LGPL v2.1 license in the future at the maintainers' discretion.
For more information on licensing, please check here.

https://hibernate.atlassian.net/browse/HHH-18723

gavinking

Not sure about the requested feature.

gavinking · 2024-10-12T08:44:59Z

hibernate-core/src/main/java/org/hibernate/annotations/SQLRestriction.java

 */
 @Target({TYPE, METHOD, FIELD})
 @Retention(RUNTIME)
+@Inherited


I'm not yet convinced that we should support the requested feature.

But if we decided to, this isn't the right fix, since it implies inheritance even from things which aren't @MappedSuperclasses.

Fair enough. I will fix implementation.

If I do so what will be the process of approving this feature? What is your reservations against it?

@gavinking
I found that similar annotation SoftDelete is supporting MappedSuperclass.

Will this convince you to accept my improvement?

Will this convince you to accept my improvement?

I don't hate it. Let's see what others think.

Well it certainly can't be merged in its current form because it breaks the overridability of @SQLRestriction, as I commented yesterday.

But if what you're saying is that this functionality is really not very important to you after all, then yes, please just close the PR.

gavinking

SQLRestriction is an overridable annotation, and this proposed implementation breaks that.

gavinking · 2024-11-30T10:03:42Z

hibernate-core/src/main/java/org/hibernate/boot/model/internal/EntityBinder.java

-		final SQLRestriction restriction = getOverridableAnnotation( annotatedClass, SQLRestriction.class, context );
+		final SQLRestriction restriction = extractSQLRestriction( annotatedClass, context );


I have added this test: 78cd996 to demonstrate the problem with this change.

I updated your test.
Now it fails if @DialectOverride.SQLRestriction is ignored.

I will work on fixing it.

I think I solved that problem.

SvenBunge · 2025-02-01T10:48:37Z

Looking for the same feature. IT seems this PR has been closed without merging it into main and without commenting the actual state. The Jira-Ticket is on review. Can somebody enlights me about the latest development?

See https://hibernate.atlassian.net/browse/HHH-18723

beikov · 2025-02-11T10:51:57Z

It just seems @a-e-tsvetkov stopped working on it. If you care about this, you can create a new PR.

a-e-tsvetkov · 2025-02-12T20:19:17Z

I closed PR after I was asked to do so. вт, 11 февр. 2025 г. в 13:52, Christian Beikov ***@***.***>:

…

It just seems @a-e-tsvetkov <https://github.com/a-e-tsvetkov> stopped working on it. If you care about this, you can create a new PR. — Reply to this email directly, view it on GitHub <#9085 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AFJP2IF2RGPG4XPKKYRZLZT2PHI6JAVCNFSM6AAAAABP2D4PV6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDMNJQGQ2DMNBSG4> . You are receiving this because you were mentioned.Message ID: ***@***.***>

gavinking · 2025-02-12T22:00:58Z

I closed PR after I was asked to do so.

FTR all I said was that it could not be merged "in its current form", because in the current implementation your change is breaking a different feature of @SQLRestriction.

I'm not completely sure what the second half of what I wrote above (on Nov 30) was in response to, since the comment I was replying to appears to have been deleted (I suppose you deleted it). My best reconstruction of events is that I guess you must have implied in the deleted comment that you weren't prepared to make the requested changes in order to not break existing functionality.

a-e-tsvetkov · 2025-02-13T15:43:58Z

In the deleted comment I asked about status, I posted it before I saw your other comment about problems with my code. Looks like there is some misunderstanding on my part. Let me summarize the situation. In the current state of PR problems with initial code are fixed. If anyone is interested in this change I can restore my PR and bring it up to date. To do so I need to understand the process of approval and approximate time frame. Please let me know if you want me to restore the PR. чт, 13 февр. 2025 г. в 01:01, Gavin King ***@***.***>:

…

I closed PR after I was asked to do so. FTR all I said was that it could not be merged *"in its current form"*, because in the current implementation your change is breaking a different feature of @SQLRestriction. I'm not completely sure what the second half of what I wrote above (on Nov 30) was in response to, since the comment I was replying to appears to have been deleted (I suppose you deleted it). My best reconstruction of events is that I guess you must have implied in the deleted comment that you weren't prepared to make the requested changes in order to not break existing functionality. — Reply to this email directly, view it on GitHub <#9085 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AFJP2IC4X35O6N6Y2F6ZJNT2PPADFAVCNFSM6AAAAABP2D4PV6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDMNJUHEZTMNRVGE> . You are receiving this because you were mentioned.Message ID: ***@***.***>

beikov · 2025-02-13T15:48:51Z

There is no "timeframe" that we can guarantee. We all just try our best to not have you wait for too long, but as you can imagine, things might get in between.

If you're willing to address issues that we point out, please reopen the PR.

a-e-tsvetkov · 2025-02-13T16:02:10Z

I reopened the PR.
Let me know if you find any other problems with it.

a-e-tsvetkov · 2025-02-13T16:06:39Z

Apparently I can't reopen it because I force-pushed in the branch.
Should I create new one or there is a way to update this PR?

beikov · 2025-02-13T16:12:45Z

Please create a new PR then.

a-e-tsvetkov · 2025-02-13T16:22:39Z

Here is new PR #9744

gavinking requested changes Oct 12, 2024

View reviewed changes

a-e-tsvetkov marked this pull request as draft October 12, 2024 09:34

a-e-tsvetkov force-pushed the feature/HHH-18723 branch from c3d61f0 to b77345b Compare October 12, 2024 10:12

a-e-tsvetkov marked this pull request as ready for review October 12, 2024 10:13

a-e-tsvetkov requested a review from gavinking October 12, 2024 11:43

gavinking requested changes Nov 28, 2024

View reviewed changes

a-e-tsvetkov force-pushed the feature/HHH-18723 branch from b77345b to 69e2670 Compare November 30, 2024 08:38

gavinking reviewed Nov 30, 2024

View reviewed changes

a-e-tsvetkov force-pushed the feature/HHH-18723 branch 3 times, most recently from 0b49682 to 8673922 Compare December 5, 2024 17:04

HHH-18723 Support @SQLRestriction in class marked as @MappedSuperclass

9780730

a-e-tsvetkov force-pushed the feature/HHH-18723 branch from 8673922 to 9780730 Compare December 7, 2024 06:13

a-e-tsvetkov closed this Dec 7, 2024

		final SQLRestriction restriction = getOverridableAnnotation( annotatedClass, SQLRestriction.class, context );
		final SQLRestriction restriction = extractSQLRestriction( annotatedClass, context );

Uh oh!

HHH-18723 Support @SQLRestriction in class marked as @MappedSuperclass #9085

HHH-18723 Support @SQLRestriction in class marked as @MappedSuperclass #9085

Uh oh!

Conversation

a-e-tsvetkov commented Oct 12, 2024 • edited by hibernate-github-bot bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gavinking left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gavinking Oct 12, 2024

Choose a reason for hiding this comment

Uh oh!

a-e-tsvetkov Oct 12, 2024

Choose a reason for hiding this comment

Uh oh!

a-e-tsvetkov Oct 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gavinking Oct 15, 2024

Choose a reason for hiding this comment

Uh oh!

gavinking Nov 30, 2024

Choose a reason for hiding this comment

Uh oh!

gavinking left a comment

Choose a reason for hiding this comment

Uh oh!

gavinking Nov 30, 2024

Choose a reason for hiding this comment

Uh oh!

a-e-tsvetkov Dec 5, 2024

Choose a reason for hiding this comment

Uh oh!

a-e-tsvetkov Dec 5, 2024

Choose a reason for hiding this comment

Uh oh!

SvenBunge commented Feb 1, 2025

Uh oh!

beikov commented Feb 11, 2025

Uh oh!

a-e-tsvetkov commented Feb 12, 2025 via email

Uh oh!

gavinking commented Feb 12, 2025

Uh oh!

a-e-tsvetkov commented Feb 13, 2025 via email

Uh oh!

beikov commented Feb 13, 2025

Uh oh!

a-e-tsvetkov commented Feb 13, 2025

Uh oh!

a-e-tsvetkov commented Feb 13, 2025

Uh oh!

beikov commented Feb 13, 2025

Uh oh!

a-e-tsvetkov commented Feb 13, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

a-e-tsvetkov commented Oct 12, 2024 •

edited by hibernate-github-bot bot

Loading

gavinking left a comment •

edited

Loading

a-e-tsvetkov Oct 12, 2024 •

edited

Loading