derby fails with duplicate primary key(s) in group by list

• Key: OPENJPA-312
• Type: Bug
• Status: Resolved
• Resolution: Fixed
• Priority: Minor
• Assignee: Daniel Lee
• Reporter: Daniel Lee
• Component/s: sql
• Affects Version/s: 1.0.0
• Fix Version/s: 1.0.0

Description

derby fails with duplicate primary key(s) in group by list

With query "select o.customer, avg(o.amount) from Order o group by o.customer" the push-down query contains duplicate columns in the group by clause. This is okay when DB2 and other DB that tolerate the duplicates but Derby returns error.

Of course, we can ask fix on Derby but we can also easy fix in OpenJPA to avoid duplicates in the group by list. Please refer to the following for the error result and the attach patch for the fix.

Output from running the query that generate duplicate in the group by list:
6429 demo TRACE [main] openjpa.Query - Executing query: select o.customer, avg(o.amount) from Order o group by o.customer
6639 demo TRACE [main] openjpa.jdbc.SQL - <t 1094861122, conn 1639735740> executing prepstmnt 1405375428 SELECT t1.countryCode, t1.id, t1.version, t1.city, t1.state, t1.street, t1.zip, t1.creditRating, t1.name, AVG(t0.amount) FROM Order t0 INNER JOIN Customer t1 ON t0.customer_countryCode = t1.countryCode AND t0.customer_id = t1.id GROUP BY t1.countryCode, t1.id, t1.version, t1.countryCode, t1.id, t1.city, t1.state, t1.street, t1.zip, t1.countryCode, t1.id, t1.creditRating, t1.name

Comments

[Daniel Lee]

The fix for OpenJPA-312.

[Kevin Sutter]

Daniel,
The basic idea of the patch looks okay, but the code changes do not look consistent. In some cases, you use "sql" and in others, you are using "sql.getSQL()". I would prefer just using "sql".

Also, you check for an empty _grouping after you check if the desired group sub-string is already present in the _grouping. This seems backwards. I know it's only being used to determine whether to use a comma or not. So, instead of doing these extra empty checks, could we do something along the following. I think this accomplishes the same goal.

         getJoins(joins, true);
         if (_grouping == null) {
             _grouping = new SQLBuffer(_dict);
             _grouping.append(sql);
        }
        else if (_grouping.getSQL().indexOf(sql) < 0)
             _grouping.append(", " + sql);

Thanks,
Kevin

new fix

[Kevin Sutter]

Daniel,
The code is much cleaner now. Thanks. But, I don't think the performance of StringTokenizer is worth any advantages it might provide. Is there some reason why the previous mechanism of just using indexOf() wasn't sufficient? I like the idea of isolating this check to a single boolean method like you have, but I would like to see a better performing implementation.

Thanks,
Kevin

[Daniel Lee]

Per discussion conclusion, attached here is the patch of fix using a ListArray to keep track of the columns in _grouping. StringTokenizer has been removed.

Thanks,
Daniel

[Kevin Sutter]

Daniel, Thanks for the updated patch. Per our side discussion, I am making a couple of slight changes to your patch, but the intent is still there. Thanks for resolving this issue. I will be committing the changes shortly.

Kevin

[Kevin Sutter]

Resolved via r566381.