Skip to content
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

Add database columns with CURRENT_TIMESTAMP support #30

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

slankka
Copy link

@slankka slankka commented Oct 27, 2019

New Feature

  1. If using DATETIME with current_timestamp, the corresponding data field need not set, it will generated by database.

Robustness Improvement

  1. change maven plugin artifact to jpa-entity-generator-maven-plugin. Because calling it maven-<yourplugin>-plugin is an infringement of the Apache Maven Trademark maven plugin development guide
  2. forcely write bytes as UTF-8
  3. support com.mysql.cj.jdbc.Driver. If not set catalog explicitly, it will get all tables from all databases, and if we get columns without catalog, it will may get duplicated columns from tables (from different catalogs) whose name are same.(Tested in mysql connector 8.0.18).

2. support com.mysql.cj.jdbc.Driver. If not set catalog explicitly, it will get all tables from all databases, and if we get columns without catalog, it will may get duplicated columns from tables (from different catalogs) whose name are same.(test in mysql connector 8.0.18).
@slankka
Copy link
Author

slankka commented Oct 28, 2019

It takes me some time to find out why the new entity files are missing in sample folder.
finally, Unfortunately I'm not familiar with gradle, 😭 ,and all the files are generated in gradle dist folder, such as

gradle.user.home\daemon\4.10.2\src\main\java\
%USERPROFILE%\.gradle\daemon\4.10.2\src\main\java\

I will commit these files tonight. 😃

update

It only works for gradlew entityGen in sample folder, If I change CodeGenerator:213:

String workingDir = System.getProperty("user.dir");
Path path = Paths.get(workingDir.concat(File.separator) + filepath);

I don't know why if not doing like this, the entity files are generated in gradle dist folders.

Copy link
Collaborator

@seratch seratch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, the changes look to me. I left some comments. If other maintainers have some opinions on this PR, please feel free to join the review.

pom.xml Show resolved Hide resolved
// return "Boolean";
return "boolean";
// my sql
return "Boolean";
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be a breaking change. If I remember correctly, setting primitive type here means the value will be false if absent. Is there any reason you changed this?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When the database has a nullable column, primitive type field of the entity can't store null, NPE will occur. Wrapped Type can represent true/false and not set

Copy link
Author

@slankka slankka Nov 15, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assuming we force make all boolean fields to be not null.
There is an another situation:

Sometimes, we use ExampleMatcher of JPA, and telling JPA to ignore the null value field:

ExampleMatcher.matchingAll().withIgnoreNullValues()

or mybatis :

<if test="aNullableBooleanState != null">
a_nullable_boolean_state= #{aNullableBooleanState, jdbcType="BIT"}
</if>

If the field type is primitive, we will always query data with and field =true/false, can't leave this query without this condition.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@slankka
I'm terribly sorry for my belated response here.

That makes sense. That said, I'd like to avoid bringing any breaking changes to this library. This library is widely used at SmartNews and I am no longer working for the company.

If you're still interested in working this PR and have time for it, could you introduce a flag in the configuration allowing users to choose Boolean over the primitive type plus set it as false by default?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants