Skip to content

Commit 8bcda67

Browse files
gavinkinggavinking
committed
HHH-16516 don't quote $ unnecessarily
it's only needed on HSQLDB and remove unnecessary logging
1 parent 297db57 commit 8bcda67

File tree

6 files changed

+127
-106
lines changed

6 files changed

+127
-106
lines changed

‎hibernate-community-dialects/src/main/java/org/hibernate/community/dialect/HSQLLegacyDialect.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -927,6 +927,7 @@ public String translateExtractField(TemporalUnit unit) {
927927
public IdentifierHelper buildIdentifierHelper(IdentifierHelperBuilder builder, DatabaseMetaData dbMetaData)
928928
throws SQLException {
929929
builder.setAutoQuoteInitialUnderscore(true);
930+
builder.setAutoQuoteDollar(true);
930931
return super.buildIdentifierHelper(builder, dbMetaData);
931932
}
932933

‎hibernate-core/src/main/java/org/hibernate/boot/model/naming/Identifier.java‎

Lines changed: 58 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
package org.hibernate.boot.model.naming;
66

77
import java.util.Locale;
8+
import java.util.Objects;
89

910
import org.hibernate.dialect.Dialect;
1011

@@ -67,19 +68,19 @@ public static Identifier toIdentifier(String text, boolean quote) {
6768
/**
6869
* Means to generate an {@link Identifier} instance from its simple text form.
6970
* <p>
70-
* If passed text is {@code null}, {@code null} is returned.
71+
* If passed {@code text} is {@code null}, {@code null} is returned.
7172
* <p>
72-
* If passed text is surrounded in quote markers, the generated Identifier
73-
* is considered quoted. Quote markers include back-ticks (`),
74-
* double-quotes (") and brackets ([ and ]).
73+
* If passed {@code text} is surrounded in quote markers, the returned Identifier
74+
* is considered quoted. Quote markers include back-ticks (`), double-quotes ("),
75+
* and brackets ([ and ]).
7576
*
7677
* @param text The text form
7778
* @param quote Whether to quote unquoted text forms
78-
* @param quoteOnNonIdentifierChar Controls whether to treat the result as quoted if text contains characters that are invalid for identifiers
79+
* @param autoquote Whether to quote the result if it contains special characters
7980
*
8081
* @return The identifier form, or {@code null} if text was {@code null}
8182
*/
82-
public static Identifier toIdentifier(String text, boolean quote, boolean quoteOnNonIdentifierChar) {
83+
public static Identifier toIdentifier(String text, boolean quote, boolean autoquote) {
8384
if ( isBlank( text ) ) {
8485
return null;
8586
}
@@ -102,24 +103,40 @@ public static Identifier toIdentifier(String text, boolean quote, boolean quoteO
102103
end--;
103104
quote = true;
104105
}
105-
else if ( quoteOnNonIdentifierChar && !quote ) {
106-
// Check the letters to determine if we must quote the text
107-
char c = text.charAt( start );
108-
if ( !isLetter( c ) && c != '_' ) {
109-
// SQL identifiers must begin with a letter or underscore
110-
quote = true;
111-
}
112-
else {
113-
for ( int i = start + 1; i < end; i++ ) {
114-
c = text.charAt( i );
115-
if ( !isLetterOrDigit( c ) && c != '_' ) {
116-
quote = true;
117-
break;
118-
}
106+
else if ( autoquote && !quote ) {
107+
quote = autoquote( text, start, end );
108+
}
109+
return new Identifier( text.substring( start, end ), quote );
110+
}
111+
112+
private static boolean autoquote(String text, int start, int end) {
113+
// Check the letters to determine if we must quote the text
114+
if ( !isLegalFirstChar( text.charAt( start ) ) ) {
115+
// SQL identifiers must begin with a letter or underscore
116+
return true;
117+
}
118+
else {
119+
for ( int i = start + 1; i < end; i++ ) {
120+
if ( !isLegalChar( text.charAt( i ) ) ) {
121+
return true;
119122
}
120123
}
121124
}
122-
return new Identifier( text.substring( start, end ), quote );
125+
return false;
126+
}
127+
128+
private static boolean isLegalChar(char current) {
129+
return isLetterOrDigit( current )
130+
// every database also allows _ here
131+
|| current == '_'
132+
// every database except HSQLDB also allows $ here
133+
|| current == '$';
134+
}
135+
136+
private static boolean isLegalFirstChar(char first) {
137+
return isLetter( first )
138+
// many databases also allow _ here
139+
|| first == '_';
123140
}
124141

125142
/**
@@ -141,21 +158,22 @@ public static boolean isQuoted(String name) {
141158

142159
public static boolean isQuoted(String name, int start, int end) {
143160
if ( start + 2 < end ) {
144-
switch ( name.charAt( start ) ) {
145-
case '`':
146-
return name.charAt( end - 1 ) == '`';
147-
case '[':
148-
return name.charAt( end - 1 ) == ']';
149-
case '"':
150-
return name.charAt( end - 1 ) == '"';
151-
}
161+
final char first = name.charAt( start );
162+
final char last = name.charAt( end - 1 );
163+
return switch ( first ) {
164+
case '`' -> last == '`';
165+
case '[' -> last == ']';
166+
case '"' -> last == '"';
167+
default -> false;
168+
};
169+
}
170+
else {
171+
return false;
152172
}
153-
return false;
154173
}
155174

156175
public static String unQuote(String name) {
157176
assert isQuoted( name );
158-
159177
return name.substring( 1, name.length() - 1 );
160178
}
161179

@@ -236,11 +254,9 @@ public String toString() {
236254
}
237255

238256
@Override
239-
public boolean equals(Object o) {
240-
if ( !(o instanceof Identifier that) ) {
241-
return false;
242-
}
243-
return getCanonicalName().equals( that.getCanonicalName() );
257+
public boolean equals(Object object) {
258+
return object instanceof Identifier that
259+
&& getCanonicalName().equals( that.getCanonicalName() );
244260
}
245261

246262
public boolean matches(String name) {
@@ -251,16 +267,13 @@ public boolean matches(String name) {
251267

252268
@Override
253269
public int hashCode() {
254-
return isQuoted ? text.hashCode() : text.toLowerCase( Locale.ENGLISH ).hashCode();
270+
return isQuoted
271+
? text.hashCode()
272+
: text.toLowerCase( Locale.ENGLISH ).hashCode();
255273
}
256274

257275
public static boolean areEqual(Identifier id1, Identifier id2) {
258-
if ( id1 == null ) {
259-
return id2 == null;
260-
}
261-
else {
262-
return id1.equals( id2 );
263-
}
276+
return Objects.equals( id1, id2 );
264277
}
265278

266279
public static Identifier quote(Identifier identifier) {
@@ -270,7 +283,7 @@ public static Identifier quote(Identifier identifier) {
270283
}
271284

272285
@Override
273-
public int compareTo(Identifier o) {
274-
return getCanonicalName().compareTo( o.getCanonicalName() );
286+
public int compareTo(Identifier identifier) {
287+
return getCanonicalName().compareTo( identifier.getCanonicalName() );
275288
}
276289
}

‎hibernate-core/src/main/java/org/hibernate/dialect/HSQLDialect.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -712,6 +712,7 @@ public String translateExtractField(TemporalUnit unit) {
712712
public IdentifierHelper buildIdentifierHelper(IdentifierHelperBuilder builder, DatabaseMetaData dbMetaData)
713713
throws SQLException {
714714
builder.setAutoQuoteInitialUnderscore( true );
715+
builder.setAutoQuoteDollar( true );
715716
return super.buildIdentifierHelper( builder, dbMetaData );
716717
}
717718

‎hibernate-core/src/main/java/org/hibernate/engine/jdbc/env/internal/NormalizingIdentifierHelperImpl.java‎

Lines changed: 26 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,10 @@
1515
import org.hibernate.engine.jdbc.env.spi.JdbcEnvironment;
1616
import org.hibernate.engine.jdbc.env.spi.NameQualifierSupport;
1717

18-
import org.jboss.logging.Logger;
19-
2018
/**
2119
* @author Steve Ebersole
2220
*/
2321
public class NormalizingIdentifierHelperImpl implements IdentifierHelper {
24-
private static final Logger log = Logger.getLogger( NormalizingIdentifierHelperImpl.class );
2522

2623
private final JdbcEnvironment jdbcEnvironment;
2724

@@ -30,6 +27,7 @@ public class NormalizingIdentifierHelperImpl implements IdentifierHelper {
3027
private final boolean globallyQuoteIdentifiersSkipColumnDefinitions;
3128
private final boolean autoQuoteKeywords;
3229
private final boolean autoQuoteInitialUnderscore;
30+
private final boolean autoQuoteDollar;
3331
private final TreeSet<String> reservedWords;
3432
private final IdentifierCaseStrategy unquotedCaseStrategy;
3533
private final IdentifierCaseStrategy quotedCaseStrategy;
@@ -41,6 +39,7 @@ public NormalizingIdentifierHelperImpl(
4139
boolean globallyQuoteIdentifiersSkipColumnDefinitions,
4240
boolean autoQuoteKeywords,
4341
boolean autoQuoteInitialUnderscore,
42+
boolean autoQuoteDollar,
4443
TreeSet<String> reservedWords, //careful, we intentionally omit making a defensive copy to not waste memory
4544
IdentifierCaseStrategy unquotedCaseStrategy,
4645
IdentifierCaseStrategy quotedCaseStrategy) {
@@ -50,39 +49,33 @@ public NormalizingIdentifierHelperImpl(
5049
this.globallyQuoteIdentifiersSkipColumnDefinitions = globallyQuoteIdentifiersSkipColumnDefinitions;
5150
this.autoQuoteKeywords = autoQuoteKeywords;
5251
this.autoQuoteInitialUnderscore = autoQuoteInitialUnderscore;
52+
this.autoQuoteDollar = autoQuoteDollar;
5353
this.reservedWords = reservedWords;
5454
this.unquotedCaseStrategy = unquotedCaseStrategy == null ? IdentifierCaseStrategy.UPPER : unquotedCaseStrategy;
5555
this.quotedCaseStrategy = quotedCaseStrategy == null ? IdentifierCaseStrategy.MIXED : quotedCaseStrategy;
5656
}
5757

5858
@Override
5959
public Identifier normalizeQuoting(Identifier identifier) {
60-
log.tracef( "Normalizing identifier quoting [%s]", identifier );
61-
6260
if ( identifier == null ) {
6361
return null;
6462
}
65-
66-
if ( identifier.isQuoted() ) {
63+
else if ( identifier.isQuoted() ) {
6764
return identifier;
6865
}
69-
70-
if ( globallyQuoteIdentifiers ) {
71-
log.tracef( "Forcing identifier [%s] to quoted for global quoting", identifier );
66+
else if ( mustQuote( identifier ) ) {
7267
return Identifier.toIdentifier( identifier.getText(), true );
7368
}
74-
75-
if ( autoQuoteKeywords && isReservedWord( identifier.getText() ) ) {
76-
log.tracef( "Forcing identifier [%s] to quoted as recognized reserved word", identifier );
77-
return Identifier.toIdentifier( identifier.getText(), true );
78-
}
79-
80-
if ( autoQuoteInitialUnderscore && identifier.getText().startsWith("_") ) {
81-
log.tracef( "Forcing identifier [%s] to quoted due to initial underscore", identifier );
82-
return Identifier.toIdentifier( identifier.getText(), true );
69+
else {
70+
return identifier;
8371
}
72+
}
8473

85-
return identifier;
74+
private boolean mustQuote(Identifier identifier) {
75+
return globallyQuoteIdentifiers
76+
|| autoQuoteKeywords && isReservedWord( identifier.getText() )
77+
|| autoQuoteInitialUnderscore && identifier.getText().startsWith( "_" )
78+
|| autoQuoteDollar && identifier.getText().contains( "$" );
8679
}
8780

8881
@Override
@@ -110,10 +103,7 @@ public boolean isReservedWord(String word) {
110103

111104
@Override
112105
public String toMetaDataCatalogName(Identifier identifier) {
113-
log.tracef( "Normalizing identifier quoting for catalog name [%s]", identifier );
114-
115106
if ( !nameQualifierSupport.supportsCatalogs() ) {
116-
log.trace( "Environment does not support catalogs; returning null" );
117107
// null is used to tell DatabaseMetaData to not limit results based on catalog.
118108
return null;
119109
}
@@ -133,53 +123,30 @@ private String toMetaDataText(Identifier identifier) {
133123
throw new IllegalArgumentException( "Identifier cannot be null; bad usage" );
134124
}
135125

126+
final String text = identifier.getText();
136127
if ( identifier instanceof DatabaseIdentifier ) {
137-
return identifier.getText();
128+
return text;
138129
}
139-
140-
if ( identifier.isQuoted() ) {
141-
switch ( quotedCaseStrategy ) {
142-
case UPPER: {
143-
log.tracef( "Rendering quoted identifier [%s] in upper case for use in DatabaseMetaData", identifier );
144-
return identifier.getText().toUpperCase( Locale.ROOT );
145-
}
146-
case LOWER: {
147-
log.tracef( "Rendering quoted identifier [%s] in lower case for use in DatabaseMetaData", identifier );
148-
return identifier.getText().toLowerCase( Locale.ROOT );
149-
}
150-
default: {
151-
// default is mixed case
152-
log.tracef( "Rendering quoted identifier [%s] in mixed case for use in DatabaseMetaData", identifier );
153-
return identifier.getText();
154-
}
155-
}
130+
else if ( identifier.isQuoted() ) {
131+
return switch ( quotedCaseStrategy ) {
132+
case UPPER -> text.toUpperCase( Locale.ROOT );
133+
case LOWER -> text.toLowerCase( Locale.ROOT );
134+
case MIXED -> text; // default
135+
};
156136
}
157137
else {
158-
switch ( unquotedCaseStrategy ) {
159-
case MIXED: {
160-
log.tracef( "Rendering unquoted identifier [%s] in mixed case for use in DatabaseMetaData", identifier );
161-
return identifier.getText();
162-
}
163-
case LOWER: {
164-
log.tracef( "Rendering unquoted identifier [%s] in lower case for use in DatabaseMetaData", identifier );
165-
return identifier.getText().toLowerCase( Locale.ROOT );
166-
}
167-
default: {
168-
// default is upper case
169-
log.tracef( "Rendering unquoted identifier [%s] in upper case for use in DatabaseMetaData", identifier );
170-
return identifier.getText().toUpperCase( Locale.ROOT );
171-
}
172-
}
138+
return switch ( unquotedCaseStrategy ) {
139+
case MIXED -> text;
140+
case LOWER -> text.toLowerCase( Locale.ROOT );
141+
case UPPER -> text.toUpperCase( Locale.ROOT ); // default
142+
};
173143
}
174144
}
175145

176146
@Override
177147
public String toMetaDataSchemaName(Identifier identifier) {
178-
log.tracef( "Normalizing identifier quoting for schema name [%s]", identifier );
179-
180148
if ( !nameQualifierSupport.supportsSchemas() ) {
181149
// null is used to tell DatabaseMetaData to not limit results based on schema.
182-
log.trace( "Environment does not support catalogs; returning null" );
183150
return null;
184151
}
185152

@@ -195,8 +162,6 @@ public String toMetaDataSchemaName(Identifier identifier) {
195162

196163
@Override
197164
public String toMetaDataObjectName(Identifier identifier) {
198-
log.tracef( "Normalizing identifier quoting for object name [%s]", identifier );
199-
200165
if ( identifier == null ) {
201166
// if this method was called, the value is needed
202167
throw new IllegalArgumentException( "null was passed as an object name" );

‎hibernate-core/src/main/java/org/hibernate/engine/jdbc/env/spi/IdentifierHelperBuilder.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ public class IdentifierHelperBuilder {
4040
private boolean skipGlobalQuotingForColumnDefinitions = false;
4141
private boolean autoQuoteKeywords = true;
4242
private boolean autoQuoteInitialUnderscore = false;
43+
private boolean autoQuoteDollar = false;
4344
private IdentifierCaseStrategy unquotedCaseStrategy = IdentifierCaseStrategy.UPPER;
4445
private IdentifierCaseStrategy quotedCaseStrategy = IdentifierCaseStrategy.MIXED;
4546

@@ -150,6 +151,10 @@ public void setAutoQuoteInitialUnderscore(boolean autoQuoteInitialUnderscore) {
150151
this.autoQuoteInitialUnderscore = autoQuoteInitialUnderscore;
151152
}
152153

154+
public void setAutoQuoteDollar(boolean autoQuoteDollar) {
155+
this.autoQuoteDollar = autoQuoteDollar;
156+
}
157+
153158
public NameQualifierSupport getNameQualifierSupport() {
154159
return nameQualifierSupport;
155160
}
@@ -215,6 +220,7 @@ public IdentifierHelper build() {
215220
skipGlobalQuotingForColumnDefinitions,
216221
autoQuoteKeywords,
217222
autoQuoteInitialUnderscore,
223+
autoQuoteDollar,
218224
reservedWords,
219225
unquotedCaseStrategy,
220226
quotedCaseStrategy

0 commit comments

Comments
 (0)