-
Notifications
You must be signed in to change notification settings - Fork 77
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature/master/sqlgen #10
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Simply move needed sources to their new places in the mavenized layout, and delete those no longer needed given the Java 6 built-in annotation support. An intermediate commit guaranteed not to build.
Paste in the newer copyright notice, recently added in pljava-examples, that gives an explicit BSD 3-clause link and has room for other contributors.
Remove the dependency on org.objectweb.asm in favor of the annotation processing facilities now built in to javac as of Java 6. Add annotations to override the auto-determined type for function parameters and/or supply default values, and to supply arbitrary SQL commands to be included in the generated deployment descriptor.
Match indentation of relocated code to new surroundings.
The META-INF/services/javax.annotation.processing.Processor entry is what tells javac there is an annotation processor present in the jar, making the magic happen. The ddr file's name (unless overridden with -Addr.output=) is pljava.ddr, and is relative to the root of the classfiles destination tree.
Moving the annotation examples into pljava-examples creates a jar with two deployment descriptors, the old one for the non-annotated examples and the generated one. That's actually ok per SQL/JRT 2003, but small changes to Commands.java and a new sqlj.jar_descriptors table are needed to remove the former arbitary restriction to one descriptor only. Eventually the non-annotated examples could all be annotated, leaving the generated descriptor as the only one. But as the limit to one descriptor was at variance with the standard anyway, it seemed worth removing.
The annotation processor service configuration file can't be in resources/ because maven copies those resources to the target directory before compilation, and in a clean build javac will fail trying to instantiate a processor it hasn't compiled yet. Moved the config file to a new directory late-added-resources/ with an addition to pom.xml to do the copy only just before producing the jar.
For now, nothing done but to dump the annotated examples in with the other ones (and change the name of one method that duplicated one of the non-annotated ones), and further annotate the trigger example to create the tables it refers to. (Also, one example referred to an example.properties file, so one has been created.) For now, the result is a jar with two deployment descriptors (which is supposed to be ok per SQL/JRT 2003, so is now accepted): the original one and the generated one. Notes on things I know aren't perfect yet: 1. Maven doesn't put things in the jar in a deterministic order. The generated ddr depends on the javatest schema and _properties type created in the old ddr, but sometimes "mvn clean install" will put the two ddrs in the jar in the other order. This is just a maven issue; pljava correctly follows the spec and uses the order of the ddrs found in the jar. It wouldn't be a problem if the two ddrs had no interdependencies, or if the maven build could be tweaked to force a certain order in the jar, or if the old ddr were done away with entirely by annotating all the old examples so the generator generates everything. 2. The generated ddr only reflects annotations seen in the compile run. If a non-clean build run only runs javac on a few changed files, the generated ddr can be incomplete. So for now it's best to run javac on everything (or do a mvn clean before building). There may be a way to improve the annotation processor to save a record of all the files that would need to be reexamined in a partial rebuild. What I don't know yet is how (or whether) that can be communicated back to javac to get those files examined in a later round. 3. Naturally until issue tada#8 is fixed, none of the no-arg example functions can actually be used. 4. The old examples.ddr contains a set search_path making javatest the current schema, which causes deployRemove to fail later. The trouble is that if deployRemove alters the search path on the current schema, it will try to restore the old value after undeploying, and that fails if the undeploy made that schema go away. Possibly deployRemove could be made to just fail silently if that happens, but that's an issue for another day.
The jar_repository and jar_entry table names aren't plural, so jar_descriptor should not be either.
This looks really good. I'm merging it in since I'm quite sure this is something that we really want. The only thing missing here are tests but hey, we don't have tests for much of the other code either. Requires a different issue for sure. |
jcflack
added a commit
to jcflack/pljava
that referenced
this pull request
Jul 13, 2015
Ken Olson caught this a year ago in his pull request tada#29, but it was my oversight; when rearranging the sqlj relations back in tada#10 to accommodate more than one deployment descriptor, I updated the sql in install_jar and remove_jar (and deployInstall and deployRemove), but I just totally overlooked replace_jar. Oops.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Updates the annotation-driven SQL-generating code under src/java5 from a while back to remove the dependency on org.objectweb.asm and instead use the annotation processing now built into javac as of Java 6. New annotations for including arbitrary SQL commands to go along with autogenerated function and trigger declarations. As discussed on pljava-dev.
After thinking of a few possible organizations, I ultimately moved both the annotation interfaces and the annotation processor into pljava-api. That way user code can be compiled with nothing more than pljava-api.jar on the classpath and everything works including descriptor generation, and nothing was complicated in the maven setup.