openjpa-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Abe White (JIRA)" <>
Subject [jira] Commented: (OPENJPA-168) sql optimize n rows query hint
Date Tue, 27 Mar 2007 14:36:32 GMT


Abe White commented on OPENJPA-168:

Something is wrong with my email system so instead of replying to the SVN commit message,
I'm posting my comments on the commit of this patch here.

I didn't go through the entire commit, but what I saw looked really good.  There are, however,
a few problems I'd like to see fixed:

1. The formatting is messed up (in that it is unlike all our other formatting) in many places.
 Indentation is off, spaces are missing before opening braces or added before line-ending
semicolons, etc.  Can you change the formatting to be consistent with the rest of the codebase?

2.  The point of making the optimize hint string a static constant is not only to avoid typo
errors when we use it internally, but also to make it available to users.  In this way it
is exactly like the constants in org.apache.openjpa.kernel.QueryFlushModes, for example. 
I'd like to see the constant removed from AbstractStoreQuery.  Instead, I'd like to see an
org.apache.openjpa.kernel.QueryHints interface with a HINT_RESULT_COUNT defined, and I'd like
to see org.apache.openjpa.persistence.OpenJPAQuery implement this interface, just as it does
for QueryFlushModes.  This will allow both our internal code -- through QueryHints -- and
user code -- through OpenJPAQuery -- to use the constant.

3. This is much more of a matter of personal opinion, but I think the added code is over-commented.
 The new SelectExecutor.setExpectedResultCount method should be thoroughly documented, but
IMO we don't need comments explaining what we're doing every time we invoke the method.  The
code speaks for itself, and over-commenting always runs the danger of the comments falling
out of synch with the code.

> sql optimize n rows query hint
> ------------------------------
>                 Key: OPENJPA-168
>                 URL:
>             Project: OpenJPA
>          Issue Type: New Feature
>            Reporter: David Wisneski
>         Assigned To: David Wisneski
>         Attachments: OPENJPA-168.patch.txt, OPENJPA-168.patch2.txt
> There werre various comments from Patrick, Abe and Kevin Sutter about the code that I
checked related to Optimize hint.  So I have gone back and relooked at this and wil be making
some changes.  At Kevin's suggestion I will do this through a JIRA feature so that folks will
have opportunity to comment on this before the code is actually done and checked in.

This message is automatically generated by JIRA.
You can reply to this email to add a comment to the issue online.

View raw message