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

HHH-9271 - IdentityGenerator broken with quoted identifiers #1301

Merged
merged 1 commit into from
Mar 22, 2016

Conversation

vladmihalcea
Copy link
Contributor

Fix for HHH-9271

"unrecognized id type : " + type.getName() + " -> " + clazz.getName()
);
catch (SQLException e) {
if(Identifier.isQuoted( identifier )) {
Copy link
Member

Choose a reason for hiding this comment

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

Unless I am mistaken... that identifier, if quoted, would be surrounded by the Dialect's quoting chars. Identifier#isQuoted is checking the "source form" or quoting (with one additional pattern check). In other words, Identifier#isQuoted will match name, [name] and "name". But it would not match, e.g., 'name'. I think it might be better to check here using Dialect#openQuote() and Dialect#closeQuote()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I checked it and the Identifier#isQuoted verifies all the currently supported quoting characters: `, [, and ". As far as I know, there cannot be a 'name' because there is no Dialect to use such a quoting syntax. More, the single quote syntax is reserved for Strings in SQL.

@vladmihalcea vladmihalcea force-pushed the HHH-9271 branch 2 times, most recently from 7249bc2 to c374703 Compare March 22, 2016 10:28
@vladmihalcea vladmihalcea merged commit c62c367 into hibernate:master Mar 22, 2016
@vladmihalcea vladmihalcea deleted the HHH-9271 branch March 22, 2016 10:31
@sebersole
Copy link
Member

Ok, when a new/custom Dialect uses a different quote character and we get a
request to add the patterns recognized by that Dialect into
Identifier#isQuoted, we'll be sure that gets asigned to you :)

On Tue, Mar 22, 2016 at 5:31 AM Vlad Mihalcea notifications@github.com
wrote:

Merged #1301 #1301.


You are receiving this because you commented.

Reply to this email directly or view it on GitHub
#1301 (comment)

@vladmihalcea
Copy link
Contributor Author

But there are other places where Identifier#isQuoted is also used, so maybe we should add a new task that replaces this statis method with a component that is aware of the current Dialect. I agree that we should change it, but I think we should do that for all occurences, nit just for this particular use case.

@sebersole
Copy link
Member

Show me where Identifier#isQuoted is used to check names that Dialect has
already quoted?

You have to remember that there are 2 forms of "quoted".. (1) the source
form, what we find in XML or annotations and (2) database quoted form

On Tue, Mar 22, 2016, 8:12 AM Vlad Mihalcea notifications@github.com
wrote:

But there are other places where Identifier#isQuoted is also used, so
maybe we should add a new task that replaces this statis method with a
component that is aware of the current Dialect. I agree that we should
change it, but I think we should do that for all occurences, nit just for
this particular use case.


You are receiving this because you commented.

Reply to this email directly or view it on GitHub
#1301 (comment)

@vladmihalcea
Copy link
Contributor Author

I'm going to change it to use what you've suggested previously. But in the case a new Dialect comes with a new quote symbol, is it true that we will not add it to the Identifier#isQuoted too?

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.

2 participants