-
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 code generation add-on for Gradle, with reflection from SQL DDL files #11
Conversation
f715102
to
86a1483
Compare
This looks promising. I guess this only works without having any CrateDB-specific schema definitions since it has to pass the jOQL parser and will then be created in a H2 memory database. |
Can we then create the domain model based on the code generation from the SQL files ? |
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. | ||
- `jOOQ's code generator`_ currently does not work with directly connecting to |
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.
I would probably mention that since we are using the psql jdbc driver and we are bound to JOOQ parser, we can only use sql-standard/psql based definitions.
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.
I guess this only works without having any CrateDB-specific schema definitions since it has to pass the jOQL parser and will then be created in a H2 memory database.
I would probably mention that since we are using the psql jdbc driver and we are bound to JOOQ parser, we can only use sql-standard/psql based definitions.
You are right. Thanks!
11:06:01 SEVERE DDLDatabase Error : Your SQL string could not be parsed or interpreted. This may have a variety of reasons, including:
- The jOOQ parser doesn't understand your SQL
- The jOOQ DDL simulation logic (translating to H2) cannot simulate your SQL
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.
I've improved the "Caveats" section with 15ae4cd. Thank you again.
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.
Hi again. I've diverted the documentation updates to GH-12 after the review.
@@ -17,13 +15,6 @@ | |||
/** | |||
* This class is generated by jOOQ. | |||
*/ | |||
@Generated( |
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.
I think having the code generated is ok as long as it's possible to do so out of the box.
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.
As the description stated, all these files are generated now. For whatever reason these comments don't get generated anymore.
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.
ok, i did not know this.
@@ -40,7 +40,7 @@ public void testExampleWithDynamicSchema() throws SQLException, IOException { | |||
|
|||
// Check number of records. | |||
DSLContext db = app.getDSLContext(); | |||
int count = db.fetchCount(DSL.selectFrom("testdrive.book")); |
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.
Maybe a few more advanced queries would be nice.
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.
Thanks. I've planned to exercise more advanced queries within a subsequent iteration.
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.
👍
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.
The backlog now enumerates a number of items which may be addressed on behalf of later iterations.
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.
Cool stuff, just some more minor suggestions.
- We have not been able to make multiple SQL DDL statements work within a | ||
single SQL bootstrap file at ``src/main/resources/bootstrap.sql``. |
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.
Is this really a caveat? Isn't defining dedicated SQL files for each table even a cleaner way?
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.
I am not sure. I think when using a framework managing your data migrations, there is usually one file per schema-change iteration, containing multiple ALTER TABLE
statements. This caveat is here as a reminder that I discovered issues in this area, and that I would like to revisit the topic on a subsequent iteration.
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.
See also #11 (review) below.
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.
This statement has been removed. Apparently, it was based on a quirk on my side, and I have not been able to observe a problem in this area again.
property { | ||
key = 'unqualifiedSchema' | ||
value = 'none' |
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.
I think this confuses a bit as in the examples, no schema is used. I think this works as the testdrive
is passed into the connection string as the database parameter which CrateDB then uses as the default schema name. Maybe rather use testdrive
here and not inside the connection string and/or use the schema also in the application?
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.
I already tried to use a real value there, but the setting apparently only accepts public
or none
.
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.
See also #12 (review).
86a1483
to
15ae4cd
Compare
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.
👍
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.
Very nice 👍
15ae4cd
to
ab9187e
Compare
by-language/java-jooq/src/main/java/io/crate/demo/jooq/Application.java
Outdated
Show resolved
Hide resolved
This time, the code within the `src/generated` folder has truly been generated using jOOQ 3.17.7. - The code generator used for that is `DDLDatabase` [1]. - The relevant upstream documentation is [2]. [1] org.jooq.meta.extensions.ddl.DDLDatabase [2] https://www.jooq.org/doc/latest/manual/code-generation/codegen-ddl/
ab9187e
to
6870718
Compare
About
As a followup to GH-9, this patch adds the Gradle-based jOOQ code generation add-on. It can be invoked like:
The style of code generation exercised here works by reflecting the database schema from existing SQL DDL files.
✅ Contrary to GH-10, this works well. ✅
Details
This time, the code within the
src/generated
folder has truly been generated using jOOQ 3.17.7.org.jooq.meta.extensions.ddl.DDLDatabase
.