Something (JDK 7?) caused a type-mapping regression in DDR generation #39
Comments
What the TypeMapper was doing was correct for a function return value, matching the return value to the narrowest mapped type that the actual return value could be assigned to. It was not correct for a function parameter, which needs to be matched to the widest mapped type that could be assigned to the parameter. Jens, if you ever read this, forgive me. At least I caught it quickly.
|
The line where |
|
I did find some related reports in the Java bug database: http://bugs.java.com/bugdatabase/view_bug.do?bug_id=8038455 8038455 is written as an 'enhancement' (due for Java 9) and I didn't see anything clearly reflecting that they knew stuff had broken in 7 that worked in 6. (I also found a machine that still had 6 on it, and confirmed it's still not broken in 6.) So I made another report they will (I hope) at least check, to make sure that this issue is the same as what they're enhancing for 9.... I can't give that bug number yet ... their system never has given a number at submission time, it has to go through one review queue before they even decide it gets a number. |
The annotation processor in javac runs in multiple 'rounds', as long as generated source files keep appearing, and one final round after they don't. This code used to save some mapping of Class objects to the javax.lang.model objects (TypeElement/TypeMirror) until the final round, which worked in Java 6, but as of 7 one gets back different model objects in different rounds, for the same types, and they don't match, so type mapping breaks. This workaround moves all those lookups to before or during round 1, when a consistent set of model objects can be looked up. So, it will work as long as all the source files that might contain pljava annotations will be found in round 1. That should always be the case unless someone is using a very fancy build with _other_ annotation processors generating new source files with pljava annotations that have to be processed in additional rounds. For now, that won't work, because their types won't seem to match what was computed in round 1. So don't do that. http://bugs.java.com/bugdatabase/view_bug.do?bug_id=8038455 might refer to this problem, and promises a fix in Java 9, for what it's worth.
Workaround #39 breakage seen with Java 7
|
The workaround added in #42 is probably going to be the last word on this until at least Java 9. Closing. |
Certain later-detected annotation errors were being output with no source location info, probably because of that javac (new with 7? see also #39) habit of throwing away symbol tables between rounds. Merge branch 'workaround/REL1_5_STABLE-BASE/sourcelocs' into REL1_5_STABLE.
Certain later-detected annotation errors were being output with no source location info, probably because of that javac (new with 7? see also #39) habit of throwing away symbol tables between rounds. Merge branch 'workaround/REL1_5_STABLE-BASE/sourcelocs'
This is a story that has to be pieced together out of commits and pulls, because there wasn't an issue filed (until now) to report the regression.
In August 2014, sptrakesh was working on Java 7 and clang support, and made a commit f49cf50 that commented
pljava-examplesout of the build inpom.xml, apparently because at least one example had stopped working.In October 2014, fbroda made a commit bd6f79c that re-enabled
pljava-examplesand added an explicitcomplexTypeannotation to(and then a later commit 7b5f28d that corrected the
complexType).Apparently the regression was that
Iterator<String>was no longer automatically recognized asRETURNS SETOF VARCHAR, as had been tested and known to work in Java 6.The real problem is devilishly subtle and I'm not sure what to do about it yet. The annotation framework in javac calls the annotation processor in more than one "round" - we currently collect information about all the SQL-related annotations in the non-final round(s), then generate the DDR file in the final round.
What has started happening in Java 7 is that getElementUtils().getTypeElement(x) will always return the same
TypeElementinstance for a given namexwithin one round, but will now return a differentTypeElementinstance for the same type namexfrom one round to the next. UltimatelyDDRProcessorImpl.TypeMapper.getSQLTypeends up calling isAssignable(tm,ktm) to comparetm(java.lang.Stringfrom theIteratortype seen in earlier round) andktm(java.lang.Stringmapped toVARCHAR, in current round), andisAssignablereturns false because the twoTypeElementinstances representingjava.lang.Stringare not the same!I don't know if this is a Java 7 bug. Seems as if it might be ... at the very least, it is highly counterintuitive and seems likely to break stuff. I haven't stumbled easily on any other reports of it though.
I'm still looking at ways to get it working again. I'll add more when I learn more.
Meanwhile, there's kind of a moral in this issue. As pointed out in #11, at the moment the examples are about the nearest thing we have to regression testing. If at some point an example stops working, we need to report an issue to find out why it stopped working and solve the regression - not commit a change to the example that hides the problem (like the explicit annotation in this case). Hiding the problem in the example isn't going to stop anybody's real code from breaking in the same way. If it had been reported as an issue back in August 2014 there'd have been an extra year to get to the bottom of it.
The text was updated successfully, but these errors were encountered: