MySQL Bugs: #52009: Query.explain() for NDB API wrapper returns null before query is invoked

Bug #52009	Query.explain() for NDB API wrapper returns null before query is invoked
Submitted:	12 Mar 2010 19:27	Modified:	5 May 2010 16:25
Reporter:	Todd Farmer (OCA)	Email Updates:
Status:	Closed	Impact on me:	None
Category:	MySQL Cluster: Cluster/J	Severity:	S2 (Serious)
Version:	7.1.1	OS:	Windows (XP)
Assigned to:	Craig Russell	CPU Architecture:	Any

Description:
There's no documentation to indicate appropriate usage of this method, but from looking at the code, it appears that access patterns are stored *during* execution of a Query, which may then be retrieved after execution.  This is exactly opposite of use case of "EXPLAIN" in a traditional MySQL sense, where one would use EXPLAIN to examine the proposed execution path chosen by the optimizer (without actually executing the query).

Because there is no documentation indicating the difference, I tried invoking explain() without actually executing the Query.  That throws no exception, but returns null instead of an empty Map.

How to repeat:
Call Query.explain() without executing the query.

Suggested fix:
1.  Never return null.  Either throw an exception, informing users that explain() may only be called after query execution, or return an empty map, or disassociate the generation of execution path data from actual query execution.

2.  Update documentation to indicate appropriate usage, expected results:

http://dev.mysql.com/doc/ndbapi/en/mccj-clusterj-query.html#mccj-clusterj-query-explain

3.  Consider changing return from Map<String, Object> to Map<String, String> if you intend to always use Strings as Map values.  If you intend to return some *other* type of Object, please create and return a wrapper class/interface to make it easy to print out the details from each entry.

I've added javadoc to explain the usage of this api.

clr% bzr commit -m "Bug 52009 Add information about Query.explain" clusterj-api/src/main/java/com/mysql/clusterj/Query.java
Committing to: /Users/clr/ndb/root/clusterj-working/                                                                                                                                                                                
modified clusterj-api/src/main/java/com/mysql/clusterj/Query.java
Committed revision 229.                                                                                                                                                                                                             
[CraigRussell:~/ndb/root/clusterj-working] clr% bzr push
Using saved push location: bzr+ssh://crussell@bk-internal.mysql.com/bzrroot/clusterj/
Pushed up to revision 229.                                                                                                                                                                                                          
[CraigRussell:~/ndb/root/clusterj-working] clr% pushd ../clusterj ; bzr pull ; popd
~/ndb/root/clusterj ~/ndb/root/clusterj-working 
Using saved parent location: bzr+ssh://crussell@bk-internal.mysql.com/bzrroot/clusterj/
 M  clusterj-api/src/main/java/com/mysql/clusterj/Query.java                                                                                                                                                                        
All changes applied successfully.                                                                                                                                                                                                   
Now on revision 229.

> 1.  Never return null.

This is a design choice, not a bug.

> 2.  Update documentation to indicate appropriate usage, expected results.

Done.

> 3.  Consider changing return from Map<String, Object> to Map<String, String>

Considered. The intent is to start with what we know, but extend in future as we have more feedback as to what might be useful to developers. In Java, unlike other languages, the standard behavior for Object .toString() is to return useful data. Objects that are returned from this method can safely have toString() called on them.

Instead of returning null, perhaps it would be appropriate to throw an Exception?  Since ultimately the method is being invoked at a time when it is unsupported.

Regarding the Object.toString() aspect, I understand this, but it's not universal behavior at all (try calling toString() on the returned Map object directly, for instance).  It wasn't documented what the Object actually was, so I had to write code to inspect the class of the Object to determine it was a String, rather than something else to manipulate to get useful information.  This was compounded by my expectation that explain() be independent of actual query execution, as it is in MySQL generally.

Query.explain can now be executed before the query is executed, as long as all parameters are bound. If parameters are not bound, throws ClusterJUserException.

The javadoc for Query describes the conditions for explain. The keys used in the explain Map 
are documented in the Query interface itself.

Thank you for your bug report. This issue has been committed to our source repository of that product and will be incorporated into the next release.

If necessary, you can access the source repository and build the latest available version, including the bug fix. More information about accessing the source trees is available at

    http://dev.mysql.com/doc/en/installing-source.html