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

NPE in Spring-JDBC with Oracle and SimpleJdbcInsert [SPR-16495] #21038

Closed
spring-projects-issues opened this issue Feb 14, 2018 · 4 comments
Closed
Assignees
Labels
in: data status: backported type: regression
Milestone

Comments

@spring-projects-issues
Copy link
Collaborator

@spring-projects-issues spring-projects-issues commented Feb 14, 2018

John Ahlroos opened SPR-16495 and commented

When running the following code:

// Inserts a row and returns the generated id
public String performInsert()
{
  SimpleJdbcInsert insert = new SimpleJdbcInsert(dataSource)
                .withTableName(LOG_TABLE_NAME)
                .withSchemaName(SCHEMA_NAME)
                .usingGeneratedKeyColumns(ID_COLUMN)
                .usingColumns(VALUE_COLUMN);
        KeyHolder keyHolder = insert.executeAndReturnKeyHolder(Collections.singletonMap(VALUE_COLUMN, json));
        String id = (String) keyHolder.getKeys().get(ID_COLUMN);
        return id;
}

This code will work with Spring JDBC 4.3.6.RELEASE but if I change it to 4.3.7.RELEASE or above it will fail in:

Caused by: java.lang.NullPointerException
	at org.springframework.jdbc.core.metadata.OracleTableMetaDataProvider.lookupDefaultSchema(OracleTableMetaDataProvider.java:79)
	at org.springframework.jdbc.core.metadata.OracleTableMetaDataProvider.<init>(OracleTableMetaDataProvider.java:68)
	at org.springframework.jdbc.core.metadata.TableMetaDataProviderFactory$1.processMetaData(TableMetaDataProviderFactory.java:74)
	at org.springframework.jdbc.support.JdbcUtils.extractDatabaseMetaData(JdbcUtils.java:336)
	at org.springframework.jdbc.core.metadata.TableMetaDataProviderFactory.createMetaDataProvider(TableMetaDataProviderFactory.java:64)
	at org.springframework.jdbc.core.metadata.TableMetaDataContext.processMetaData(TableMetaDataContext.java:206)
	at org.springframework.jdbc.core.simple.AbstractJdbcInsert.compileInternal(AbstractJdbcInsert.java:280)
	at org.springframework.jdbc.core.simple.AbstractJdbcInsert.compile(AbstractJdbcInsert.java:266)
	at org.springframework.jdbc.core.simple.AbstractJdbcInsert.checkCompiled(AbstractJdbcInsert.java:313)
	at org.springframework.jdbc.core.simple.AbstractJdbcInsert.doExecuteAndReturnKeyHolder(AbstractJdbcInsert.java:396)
	at org.springframework.jdbc.core.simple.SimpleJdbcInsert.executeAndReturnKeyHolder(SimpleJdbcInsert.java:142)

Affects: 4.3.14

Issue Links:

  • #21022 SimpleJdbcCall can't access synonyms in Oracle database
  • #19234 Drop NativeJdbcExtractor mechanism in favor of java.sql.Connection.unwrap()

Referenced from: commits f2dc075, 766e602

Backported to: 4.3.15

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Feb 14, 2018

Juergen Hoeller commented

This turns out to be rather odd: The only reference that can be null add that point is the Connection returned from DatabaseMetaData.getConnection()... and I can't see any change in 4.3.7 that would have caused a regression here either. So we can be defensive against null there... but why is the metadata Connection not available in the first place?

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Feb 14, 2018

Juergen Hoeller commented

This seems to be a side effect of the #19234 refactoring: Instead of catching Exception as we did before, we changed it to SQLException... not covering an NPE anymore. Of course we never meant to cover an NPE in the first place, so by defensively checking whether the Connection is null there, we should be equally safe.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Feb 14, 2018

John Ahlroos commented

Here is the difference in the implementations of lookupDefaultSchema().

In 4.3.6 we consume the NPE in an empty catch clause:

private void lookupDefaultSchema(DatabaseMetaData databaseMetaData) {
		try {
			CallableStatement cstmt = null;
			try {
				cstmt = databaseMetaData.getConnection().prepareCall("{? = call sys_context('USERENV', 'CURRENT_SCHEMA')}");
				cstmt.registerOutParameter(1, Types.VARCHAR);
				cstmt.execute();
				this.defaultSchema = cstmt.getString(1);
			}
			finally {
				if (cstmt != null) {
					cstmt.close();
				}
			}
		}
		catch (Exception ignore) {
		}
	}

While in 4.3.7 we only catch the SQLExceptions and let the NPE pass:

private void lookupDefaultSchema(DatabaseMetaData databaseMetaData) {
		try {
			CallableStatement cstmt = null;
			try {
				cstmt = databaseMetaData.getConnection().prepareCall(
						"{? = call sys_context('USERENV', 'CURRENT_SCHEMA')}");
				cstmt.registerOutParameter(1, Types.VARCHAR);
				cstmt.execute();
				this.defaultSchema = cstmt.getString(1);
			}
			finally {
				if (cstmt != null) {
					cstmt.close();
				}
			}
		}
		catch (SQLException ex) {
			logger.debug("Encountered exception during default schema lookup", ex);
		}
	}

That is most likely why it stopped working.

Why the connection is null from DatabaseMetaData I don't know, it seems to be the case for both versions. The implementation (SerialDatabaseMetaData) I get there is coming from Weblogic's client library (wlfullclient.jar) so it might be a bug there. Anyway, I think it might be a good idea to revert the Exception handling back as this method is only a lookup method that tries the guess the schema.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Feb 14, 2018

Juergen Hoeller commented

We posted at almost the same time there, coming to the same conclusion :-)

Since we're generally trying to avoid accidental over-catching, I've added a not-null check for the Connection instead, logging a specific debug message in that case. A similar check applies to the initializeWithTableColumnMetaData implementation now when trying to include synonyms.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: data status: backported type: regression
Projects
None yet
Development

No branches or pull requests

2 participants