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

LPS-31133 and LPS-31033 #7812

Closed
wants to merge 5 commits into from

Conversation

shuyangzhou
Copy link

No description provided.

@brianchandotcom
Copy link
Owner

Hey Shuyang,

I merged your changes for LPS-31133 with some minor changes (please see Git log)

But I did not merge LPS-31033. I think the fix is incomplete at best. For example, in places where you put in "sqlQuery == true", you properly append a "_" to the parameters, but the select statement itself is not "appended".

See _FILTER_SQL_COUNT for an example. Technially, the primary key of the table could require a _

Thx

@brianchandotcom
Copy link
Owner

Also, I have a doubt that LPS-31033 should make StructurePKComparator break. Because even the order by references it by "id" and not by "id_"

Also, see LPS-18827's change to persistence_iml.ftl. It makes me wonder if we just need to fix it by changing it to DBName? Then it's a much smaller fix. But if we did, it makes me think that StructurePKComparator won't work...

So I feel we need to investigate this one more.

Thx!

@brianchandotcom
Copy link
Owner

Also see LPS-3461

@brianchandotcom
Copy link
Owner

Merged. Thank you.

View just my changes: shuyangzhou:852fa16b93...a836a48

@shuyangzhou
Copy link
Author

I will do more dig around to find a better solution for LPS-31033

@shuyangzhou
Copy link
Author

Why don't we just use DBName? SQL can not contain certain bad logic column name, but HQL can use all escaped SQL names. We can use name for model_.ftl to generate entity class, but for mapping and persistence_.ftl we just all use DBNames which are safe for both HQL and SQL. It may look ugly for HQL, but it's generated code, as long as the logic is working fine, who cares the names.

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