-
Notifications
You must be signed in to change notification settings - Fork 8
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
Java/jOOQ: Add basic demo application and testing rig #9
Conversation
73313b3
to
00dd60a
Compare
Demo application using CrateDB with jOOQ and the PostgreSQL JDBC driver.
jOOQ's code generator [1] takes your database schema and reverse- engineers it into a set of Java classes. This feature currently does not work with CrateDB yet. The code provided within the `src/generated` directory has not been derived by reflecting the database schema from CrateDB. Hereby, it is provided statically. [1] https://www.jooq.org/doc/latest/manual/code-generation/
00dd60a
to
eb951b9
Compare
@amotl Are you sure that you want to check in autogenerated code ? Would it be easier to generate the code add-hoc as part of the build process ? |
@mkleen: Thanks for your suggestion. However, it is currently not possible to generate the corresponding code, see the "Caveat" section above. However, I will do it quickly if you can fix crate/crate#12544 right away? ;] |
I am referring to the generated domain model such as Lines 18 to 23 in eb951b9
|
jOOQ's code generator runs a CTE with |
I am so serious about that, because I am tired of example apps which don't follow best practices. They are painful to maintain and also painful for the users because they will start on a weak foundation. |
What's the best practice with jOOQ ? Is it possible to use it without source generation with a normal domain model ? |
Why so serious? Here we go, the intention is clearly stated in the README:
|
This patch is here to demonstrate just that, right. And to open the door for subsequent refinements. Best practice, and main intention, is definitively to use it with code generation, according to upstream docs. Personally, I think the other details of jOOQ are equally powerful, even without using code generation. |
I think if we provide an example with CrateDB and jOOQ then this should work out of the box. If the standard workflow is to generate the domain model from the database schema then this should work too. Imagine to explain to a user that he first has to generate the domain model from another database and then modify it by hand to make it work with CrateDB. |
I am not sure that committing the generated domain model is not following best practices in this particular case. In order to generate the code, you will need a running database upfront. That would defeat using the domain model within unit tests which use mocking-based techniques right away, essentially rendering the whole application test suite completely useless. When assembling this code, I followed the same practice of providing the generated domain model classes within the source code repository like seen at 1. This was invaluable information for me to get started directly using CrateDB, without being able to use the code generator. I think it is a good start into this topic and will help others to get started quickly as well. I think it does not mislead the user in any way, and follows the style of the example code as provided by the upstream author(s). I would clearly like to see any gaps being closed on behalf of subsequent iterations, in order to leverage more of what jOOQ has to offer.
It does! ;]
I don't think it has to be strictly like that, based on my arguments. However, your question will clearly spawn a corresponding research. Footnotes |
Please also have a look at the "dynamic schema" example code, which also demonstrates a well-supported subset of jOOQ's feature set. cratedb-examples/by-language/java-jooq/src/main/java/io/crate/demo/jooq/Application.java Lines 132 to 191 in eb951b9
|
Ok, seems like the sample app from jooq also checks in the generated sources |
Out of curiosity, is this the only missing feature? also wondering if generation would work, if specified as |
Code generation is advertised as one of the major features of jOOQ. Personally, I think using jOOQ will become really powerful when eventually supporting CrateDB's container data types Working on those details would also be an exercise for subsequent iterations, this patch is only intended to set the stage, by exercising very basic operations.
I haven't, but I don't think that pgjdbc generates this query, never heard about that. If you think it will give more insights, I can also try the other driver. |
Thanks for the suggestion, I will try. |
It also croaks, see #10 (comment). |
@amotl I spent a bit more time on reading on jOQL. There is the option to use it without code generation. I think I would prefer if we set up an example for jOQL to use an option which the user can reproduce it without any hacks. I would also spend a little bit more time on advanced functionality to see if this works end-to-end. Let's make sure this ages well. |
However, if crate/crate#12544 is the blocker to get jOQl properly working, I would probably add an issue and then wait for feedback from the community. Then we can support the feature once it gets upvotes. |
Hi @mkleen, thanks for your feedback. Maybe GH-10 already addresses some of your suggestions. I hope that this patch will only be the baseline, and would like to evaluate other features and more advanced operations than just the basic ones demonstrated here, on behalf of subsequent patches. The two most prominent topics for me would be those: With kind regards, Footnotes |
/** | ||
* This class is generated by jOOQ. | ||
*/ | ||
@Generated( | ||
value = { | ||
"https://www.jooq.org", | ||
"jOOQ version:3.17.7" | ||
}, | ||
comments = "This class is generated by jOOQ." | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@seut mentioned that the pseudo-generated classes should not contain comments like this.
Instead, the comments should be more clear about that they have been hand-written / derived from other examples, eventually citing the README:
cratedb-examples/by-language/java-jooq/README.rst
Lines 46 to 52 in eb951b9
Caveats | |
======= | |
- `jOOQ's code generator`_ takes your database schema and reverse-engineers it | |
into a set of Java classes. This feature currently does not work with CrateDB | |
yet. The code provided within the ``src/generated`` directory has not been | |
derived by reflecting the database schema from CrateDB. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How would a solution look like without generated code ? Would that result in the same kind of classes ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mkleen Afaik this can be seen here
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GH-11 takes a better approach on this. Instead of leaving us or any reader hanging on this detail, it uses the existing SQL DDL scripts to reflect the database schema, see DDLDatabase: Code generation from SQL files 1. Let me know what you think about it.
As outlined within the upstream documentation, and accordingly cited within the README now, this fits well with the very reasonable scenario that your SQL DDL statements are usually maintained in form of multiple incremental migration scripts anyway, when writing an application where schema changes are important to be tracked within the source code repository. Within this scenario, there is no need to connect to an existing database for reflecting its schema.
jOOQ even promises it could cope well with CrateDB-specific extensions to the CREATE TABLE
DDL statements, which definitively matters.
While the script uses pretty standard SQL constructs, you may well use some vendor-specific extensions, and even DML statements in between to set up your schema - it doesn't matter. By ignoring unsupported content, the jOOQ SQL parser supports parsing everything that is representable through the jOOQ API, as well as some well known vendor-specific syntax, [and even unknown vendor-specific syntax by adding correspdonding hints].
Hi there,
after learning that evaluating CrateDB together with jOOQ is a long cherished wish, this patch finally stages a corresponding testing rig.
The program and test suite can be invoked like:
With kind regards,
Andreas.
/cc @hlcianfagna, @matriv, @mkleen, @proddata, @seut, @hammerhead
About
The idea of jOOQ is to generate typesafe code based on the SQL schema. Then, accessing a database table using the jOOQ DSL API looks like this:
cratedb-examples/by-language/java-jooq/src/main/java/io/crate/demo/jooq/Application.java
Lines 110 to 115 in 278b0f8
Caveat
This feature currently does not work with CrateDB yet. The code provided within the
src/generated
directory has not been derived by reflecting the database schema from CrateDB. In order to make it work out of the box using jOOQ's PostgreSQL dialect, CrateDB would need to supportWITH RECURSIVE
CTEs.RECURSIVE
support to common table expressions (WITH) crate#12544