Skip to content

Commit

Permalink
JDBC-231 DatabaseMetaData fix inconsistent use of quoted/unquoted ide…
Browse files Browse the repository at this point in the history
…ntifiers

Stricter interpretation of JDBC requirements.

* Patterns are used as is (case sensitive),
* We no longer strip double quotes,
* `null` is always interpreted as `"%"` (or 'all') for pattern,
* Empty string is no longer interpreted as `"%"` (or 'all') unless explicitly allowed by the javadoc,
* Name parameters (non-patterns) no longer strip backslashes,
* We no longer try the uppercase variant if nothing is found.

The exception is the OO protocol which sometimes will try the uppercase variant (this might change in the future).
  • Loading branch information
mrotteveel committed Jul 6, 2016
1 parent 724363b commit 211a0be
Show file tree
Hide file tree
Showing 18 changed files with 609 additions and 844 deletions.
61 changes: 61 additions & 0 deletions src/documentation/release_notes.md
Expand Up @@ -399,6 +399,67 @@ _Unless explicitly indicated, changes also apply to `CallableStatement`_
`NVARCHAR/NCHAR/NCLOB` support, it is only provided for compatibility
purposes.

### DatabaseMetaData ###

The `java.sql.DatabaseMetaData` implementation has been changed to follow the
JDBC requirements for object name pattern or object name parameters (referred to
as _patterns_ in the rest of this section).

These changes affect all methods that accept one or more pattern or name
parameters and return a `ResultSet`. This includes, but is not limited to,
`getTables` and `getColumns`.

In Firebird unquoted object names are stored upper case, while quoted object
names are stored as is. The JDBC specification states that the pattern must
match the object name as stored in the database. This means the pattern should
be case sensitive.

The changes made are as follows:

* `null` will always be interpreted as `"%"` (before this rule was applied
inconsistently)
* Empty string will no longer match (ie they are no longer interpreted as
`"%"`) unless explicitly allowed by the method javadoc (usually only the
`catalogPattern` and `schemaPattern`, which are always ignored by Jaybird)
* Double quotes around a pattern will no longer be stripped
* The driver will no longer try the uppercase variant of the provided
pattern(s) if the original value(s) did not yield a result
* Object name parameters that are not patterns (as indicated by the absence of
`Pattern` in the parameter name) will no longer have backslashes removed

Review your `DatabaseMetaData` usage and make the following changes:

* Empty string parameters: replace with `"%"` (or `null`)
* Double quotes around patterns: remove the double quotes
* Casing of patterns should be reviewed for correctness
* Non-pattern parameters containing `\` should be reviewed for correctness

Some examples:

~~~ {.sql}
CREATE TABLE tablename ( -- tablename is stored as TABLENAME in metadata!
   column1 INTEGER,
   "column2" INTEGER
);
~~~

In Jaybird 2.2 using `getColumns(null, null, "tablename", "column%")` returns
`COLUMN1`(!). In Jaybird 3.0 this produces **no rows** as `tablename` does not
match `TABLENAME`.

Changing the query to `getColumns(null, null, "TABLENAME", "column%")` in
Jaybird 2.2 and 3.0 produces only one row (`column2`), as `COLUMN1` does not
match `column%`.

In Jaybird 2.2 using `getColumns(null, null, "\"TABLENAME\"", "column%")`
returns `column2`, in Jaybird 3.0 this produces **no rows** as quotes are no
longer stripped.

In Jaybird 2.2 using `getColumns(null, null, "TABLENAME", "")` returns all
columns of the table, in Jaybird 3.0 this produces **no rows** as empty string
does not match any column. Instead, you should use
`getColumns(null, null, "TABLENAME", "%")`.

**TODO: Add other changes**

Character set OCTETS handled as JDBC (VAR)BINARY
Expand Down
26 changes: 24 additions & 2 deletions src/main/org/firebirdsql/jdbc/AbstractGeneratedKeysQuery.java
Expand Up @@ -40,6 +40,8 @@
*/
public abstract class AbstractGeneratedKeysQuery {

// TODO Add caching for column info

private static final Logger logger = LoggerFactory.getLogger(AbstractGeneratedKeysQuery.class);
private static final int QUERY_TYPE_KEEP_UNMODIFIED = 1;
private static final int QUERY_TYPE_ADD_ALL_COLUMNS = 2;
Expand Down Expand Up @@ -308,7 +310,7 @@ private void updateQuery() throws SQLException {
private void addAllColumns() throws SQLException {
DatabaseMetaData metaData = getDatabaseMetaData();
List<String> columns = new ArrayList<>();
try (ResultSet rs = metaData.getColumns(null, null, statementModel.getTableName(), null)) {
try (ResultSet rs = metaData.getColumns(null, null, normalizeObjectName(statementModel.getTableName()), null)) {
while (rs.next()) {
// Need to quote columns for mixed case columns
columns.add('"' + rs.getString(IDX_COLUMN_NAME) + '"');
Expand All @@ -329,7 +331,7 @@ private void addIndexedColumns() throws SQLException {
DatabaseMetaData metaData = getDatabaseMetaData();
Arrays.sort(columnIndexes);
List<String> columns = new ArrayList<>();
try (ResultSet rs = metaData.getColumns(null, null, statementModel.getTableName(), null)) {
try (ResultSet rs = metaData.getColumns(null, null, normalizeObjectName(statementModel.getTableName()), null)) {
while (rs.next()) {
if (Arrays.binarySearch(columnIndexes, rs.getInt(IDX_ORDINAL_POSITION)) >= 0) {
// Need to quote columns for mixed case columns
Expand All @@ -341,6 +343,26 @@ private void addIndexedColumns() throws SQLException {
addReturningClause();
}

/**
* Normalizes an object name from the parser.
* <p>
* Like-wildcard characters are escaped, and unquoted identifiers are uppercased, and quoted identifiers are
* returned with the quotes stripped.
* </p>
*
* @param objectName Object name
* @return Normalized object name
*/
private String normalizeObjectName(String objectName) {
if (objectName == null) return null;
objectName = objectName.trim();
objectName = FBDatabaseMetaData.escapeWildcards(objectName);
if (objectName.length() > 2 && objectName.charAt(0) == '"' && objectName.charAt(objectName.length() - 1) == '"') {
return objectName.substring(1, objectName.length() - 1);
}
return objectName.toUpperCase();
}

/**
* Adds the columns in columnNames to the query as generated keys.
*/
Expand Down
9 changes: 7 additions & 2 deletions src/main/org/firebirdsql/jdbc/FBConnection.java
Expand Up @@ -169,8 +169,11 @@ public void setManagedConnection(FBManagedConnection mc) {
synchronized (getSynchronizationObject()) {
//close any prepared statements we may have executed.
if (this.mc != mc && metaData != null) {
metaData.close();
metaData = null;
try {
metaData.close();
} finally {
metaData = null;
}
}
this.mc = mc;
}
Expand Down Expand Up @@ -513,7 +516,9 @@ public void close() throws SQLException {
synchronized (getSynchronizationObject()) {
try {
freeStatements();
if (metaData != null) metaData.close();
} finally {
metaData = null;
if (mc != null) {
// leave managed transactions alone, they are normally
// committed after the Connection handle is closed.
Expand Down

0 comments on commit 211a0be

Please sign in to comment.