Hello Wonseok,
I think it's a great idea. I'm not sure what the CascadeAggregateDelete is, but I believe it might be good to also include a hint for CascadePrivateParts. While CascadePrivateParts isn't relevant with exclusive JPA relations, some users have requested privately owned functionality/features, which would be used with CascadePrivateParts. Users could mark relations as privately owned directly through TopLink apis, and so it would be nice for them to be able to use this hint on their queries when required.
As far as I know (but I'm working to verify it), CascadeAggregateDelete and the other cascade options apply to writeObjectQuery types such as Delete and UpdateObjectQuery instances too. Since these queries require setting the target object on the query (instead of parameters), there is no way for users to execute them using JPA, so the query hints will only apply to read/refresh type queries. I'll try and confirm this though with Gordon and get back to you on Monday. The query hint name will need to be approved, but I like the options.
Best Regards,
Chris
----- Original Message -----
From: Wonseok Kim
To: Christopher Delahunt ; persistence_at_glassfish.dev.java.net
Sent: Friday, June 22, 2007 10:37 AM
Subject: Re: Code review for issue 1324: toplink.refresh query hint does not cascade
Hi Chris,
I have an idea of new query hint to modify cascade policy. This enables application use dynamic refresh policy such as no-cascading or cascade-all in addition to default cascade-by-mapping. With this hint application can refresh related entities or do not cascade at all when required regardless of static "cascade=REFRESH" option. This could be done with TopLink customizer or TopLink API, but query hint would be more convenient.
When I look into DatabaseQuery several cascade policies are defined like below, but I'm not sure we should support all this policies. For refresh read query NoCascading, CascadeAllParts and CascadeByMapping seem to be meaningful.
-----
/* Used as default for read, means shallow write for modify. */
public static final int NoCascading = 1;
/* Used as default for write, used for refreshing to refresh the whole object. */
public static final int CascadePrivateParts = 2;
/* Currently not supported, used for deep write/refreshes/reads in the future. */
public static final int CascadeAllParts = 3;
/* Used by the unit of work. */
public static final int CascadeDependentParts = 4;
/* Used by aggregate Collections: As aggregates delete at update time, cascaded deletes
* must know to stop when entering postDelete for a particular mapping. Only used by the
* aggregate collection when update is occuring in a UnitOfWork
* CR 2811
*/
public static final int CascadeAggregateDelete = 5;
/*
* Used when refreshing should check the mappings to determin if a particular
* mapping should be cascaded.
*/
public static final int CascadeByMapping = 6;
-----
I'm curious this policy could be exploited for queries other than read query with refresh hint.
Depending on the usability of the cascade we could could either "toplink.cascade-policy " or "toplink.refresh.cascade-policy" as a hint name.
What do you think?
Regards,
Wonseok
On 6/22/07, Christopher Delahunt <christopher.delahunt_at_oracle.com> wrote:
Looks good.
Thanks Wonseok for getting this in.
Best Regards,
Chris
----- Original Message -----
From: Wonseok Kim
To: Christopher Delahunt
Sent: Thursday, June 21, 2007 11:06 AM
Subject: Re: Code review for issue 1324: toplink.refresh query hint does not cascade
OK, then I will apply this to EJBQueryImpl.buildEJBQLDatabaseQuery as below diff.
What do you think?
EJBQueryImpl.java
===================================================================
RCS file: /cvs/glassfish/entity-persistence/src/java/oracle/toplink/essentials/internal/ejb/cmp3/base/EJBQueryImpl.java,v
retrieving revision 1.39
diff -u -r1.39 EJBQueryImpl.java
--- src/java/oracle/toplink/essentials/internal/ejb/cmp3/base/EJBQueryImpl.java 22 May 2007 23:54:23 -0000 1.39
+++ src/java/oracle/toplink/essentials/internal/ejb/cmp3/base/EJBQueryImpl.java 21 Jun 2007 15:04:01 -0000
@@ -221,7 +221,13 @@
parseTree.addParametersToQuery(databaseQuery);
((EJBQLCallQueryMechanism)databaseQuery.getQueryMechanism()).getEJBQLCall().setIsParsed(true);
databaseQuery.setFlushOnExecute(flushOnExecute);
-
+
+ //GF#1324 toplink.refresh query hint does not cascade
+ //cascade by mapping as default for read query
+ if(databaseQuery.isReadQuery ()) {
+ databaseQuery.cascadeByMapping();
+ }
+
// apply any query hints
applyHints(hints, databaseQuery);
Regards,
Wonseok
On 6/21/07, Christopher Delahunt <christopher.delahunt_at_oracle.com> wrote:
Hi Wonseok,
Someone mentioned to me that we will be/are reusing the parser for other things in the future (cmp2 support etc) so its better if the cascadeByMapping is only in the JPA classes, rather than in the select node. buildEJBQLDatabaseQuery is probably the best spot, with a check if the query is a read query.
Best Regards,
Chris
----- Original Message -----
From: Wonseok Kim
To: persistence_at_glassfish.dev.java.net
Sent: Thursday, June 21, 2007 10:20 AM
Subject: Re: Code review for issue 1324: toplink.refresh query hint does not cascade
Hi Chris,
I applied cascadeByMapping() as default like you mentioned when creating ReportQuery in SelectNode. I think this cascade policy should be applied only to Read query. Please review again the attached fix.
Thanks,
Wonseok
On 6/21/07, Christopher Delahunt < christopher.delahunt_at_oracle.com> wrote:
Hello Wonseok,
My concern is that by setting the cascadeByMapping option with the hint, it will override any settings users might have set manually (through customizers or direct toplink apis) on the query still wishing to set the query to refresh dynamically. It might be better to always have the JPQL queries use the cascadeByMapping in the create or buildEJBQLDatabaseQuery method, as this will ensure that all refresh queries use this option unless a user specifically changes it.
Best Regards,
Chris
----- Original Message -----
From: Wonseok Kim
To: persistence_at_glassfish.dev.java.net
Sent: Wednesday, June 20, 2007 11:15 AM
Subject: Code review for issue 1324: toplink.refresh query hint does not cascade
Hi Tom, Chris
I'm fixing the following issue. Please review the attached fix and let me know if you have any concern.
https://glassfish.dev.java.net/issues/show_bug.cgi?id=1324
I confirmed cascade refresh works as expected in my manual local test(modified data manually between transactions and confirmed the result of the query with refresh hint).
Also, I added some assertion code in entity-persistence-tests to check the DatabaseQuery's cascade policy.
Thanks,
Wonseok