Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 333715 - MySQLPlatform.computeMaxRowsForSQL computes incorrect value
Summary: MySQLPlatform.computeMaxRowsForSQL computes incorrect value
Status: CLOSED FIXED
Alias: None
Product: z_Archived
Classification: Eclipse Foundation
Component: Eclipselink (show other bugs)
Version: unspecified   Edit
Hardware: All All
: P3 major (vote)
Target Milestone: ---   Edit
Assignee: Nobody - feel free to take it CLA
QA Contact:
URL: http://dev.mysql.com/doc/refman/5.5/e...
Whiteboard: submitted_patch
Keywords:
Depends on:
Blocks:
 
Reported: 2011-01-06 20:01 EST by David Green CLA
Modified: 2022-06-09 10:33 EDT (History)
4 users (show)

See Also:


Attachments
patch that fixes the problem (2.39 KB, patch)
2011-01-06 20:03 EST, David Green CLA
no flags Details | Diff
mylyn/context/zip (5.27 KB, application/octet-stream)
2011-01-06 20:03 EST, David Green CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description David Green CLA 2011-01-06 20:01:27 EST
According to "the MySQL 5.5 documentation":http://dev.mysql.com/doc/refman/5.5/en/select.html the LIMIT clause is specified as follows:

pre. 
[LIMIT {[offset,] row_count | row_count OFFSET offset}]

the documentation also states:

bq. 
With two arguments, the first argument specifies the offset of the first row to return, and the second specifies the maximum number of rows to return. The offset of the initial row is 0 (not 1): 
SELECT * FROM tbl LIMIT 5,10;  # Retrieve rows 6-15

MySQLPlatform.computeMaxRowsForSQL incorrectly calculates these values, causing Query.setMaxResults(int) to behave unexpectedly.

this bug is related to bug 256790: MySQL LIMIT Clause in generated queries
Comment 1 David Green CLA 2011-01-06 20:03:12 EST
Created attachment 186240 [details]
patch that fixes the problem

attached a patch that fixes the problem.  Note that I wasn't able to run the test case.
Comment 2 David Green CLA 2011-01-06 20:03:15 EST
Created attachment 186241 [details]
mylyn/context/zip
Comment 3 Tom Ware CLA 2011-01-13 11:18:43 EST
Setting target and priority.  See the following page for the meanings of these fields:

http://wiki.eclipse.org/EclipseLink/Development/Bugs/Guidelines
Comment 4 David Green CLA 2011-03-04 13:24:01 EST
A workaround for this issue is to provide your own subclass of the MySQLPlatform class:

bc. 
public class MySQL55Platform extends MySQLPlatform {
	@Override
	public int computeMaxRowsForSQL(int firstResultIndex, int maxResults) {
		// fix bug 333715 in superclass: see http://dev.mysql.com/doc/refman/5.5/en/select.html
		return maxResults;
	}
}

in your persistence.xml you can reference your platform as follows:

bc. 
<property name="eclipselink.target-database" value="com.company.project.common.jpa.MySQL55Platform"/>
Comment 5 Tom Ware CLA 2011-06-01 14:46:12 EDT
Investigating for 2.3.1
Comment 6 Tom Ware CLA 2011-06-01 15:24:04 EDT
I do not see the described problem when I test.

here is my test:

        EntityManager em = createEntityManager();
        List results = em.createQuery("Select Object(Emp) from Employee Emp").setFirstResult(2).setMaxResults(5).getResultList();

When I run with current EclipseLink code, I get:

SELECT <selectFields> FROM CMP3_EMPLOYEE t1 LEFT OUTER JOIN CMP3_DEPT t0 ON (t0.ID = t1.DEPT_ID), CMP3_SALARY t2 WHERE (t2.EMP_ID = t1.EMP_ID) LIMIT ?, ?
	bind => [2, 5]

When I run with the patch, I get:

SELECT <selectfields> FROM CMP3_EMPLOYEE t1 LEFT OUTER JOIN CMP3_DEPT t0 ON (t0.ID = t1.DEPT_ID), CMP3_SALARY t2 WHERE (t2.EMP_ID = t1.EMP_ID) LIMIT ?, ?
	bind => [2, 7]


The current SQL is correct according to the quote above.

Please reopen if you can show me the bug here.
Comment 7 Tom Ware CLA 2011-06-01 15:39:27 EDT
FYI:  Your test case is setup using EclipseLink API rather than JPA API, as a result, the expectations are a little different.

For backward compatility reasons maxRows ends up being firstIndex + maxRows.

The reason for this is that in old versions of EclipseLink we used the JDBC constructs on Statement.   In pure JDBC, to implement firstResult, you have to read a bunch or rows and then set the cursor within the result set.  In order to give people expected numbers of results, we had to adjust by the number of first results.

Now that we use LIMIT to get the values, there is likely some value in experimenting with how to line up these values a bit better, but that will require some JPA changes as well to ensure JPA queries function properly.

I'll "unclose" this issue, but the patch will likely be more extensive than the one provided.
Comment 8 Clint Morgan CLA 2012-05-29 15:06:17 EDT
I think this can be closed. 

I am working with David, and realized that we were calling 
@ReadQuery.setMaxRows@ with the *pageSize*. However it looks like that method expects the max result index (i.e., offset + pageSize). This mistake was an easy one to make because the jpa @Query.setMaxResults@ takes the page size.

We have updated our usage of @ReadQuery@ and now no longer need the workaround that David suggests here.
Comment 9 Tom Ware CLA 2012-05-29 15:09:53 EDT
Closing based on feedback above.
Comment 10 David Green CLA 2012-05-29 15:50:29 EDT
Thanks for catching this one Clint.
Comment 11 Eclipse Webmaster CLA 2022-06-09 10:33:44 EDT
The Eclipselink project has moved to Github: https://github.com/eclipse-ee4j/eclipselink