-
Notifications
You must be signed in to change notification settings - Fork 556
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
No-op flags #2030
No-op flags #2030
Conversation
@@ -118,14 +118,22 @@ public PreparedStatement prepareStatement(String sql) throws SQLException { | |||
@Override | |||
public CallableStatement prepareCall(String sql) throws SQLException { | |||
checkOpen(); | |||
throw new SQLFeatureNotSupportedException("CallableStatement not supported", ExceptionUtils.SQL_STATE_FEATURE_NOT_SUPPORTED); | |||
if (!config.isIgnoreUnsupportedRequests()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would think this is totally valid exception because if this is called then NPE will be thrown anyway.
In most cases we can ignore parameters, but not a whole functionality.
throw new SQLFeatureNotSupportedException("nativeSQL not supported", ExceptionUtils.SQL_STATE_FEATURE_NOT_SUPPORTED); | ||
} | ||
|
||
return null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
may be return the same sql? + javadoc that it is not implemented according to the spec if flag is true
throw new SQLFeatureNotSupportedException("CallableStatement not supported", ExceptionUtils.SQL_STATE_FEATURE_NOT_SUPPORTED); | ||
} | ||
|
||
return null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the same - this is whole functionality and it will be NPE anyway. so Exception should be thrown all the time
throw new SQLFeatureNotSupportedException("getTypeMap not supported", ExceptionUtils.SQL_STATE_FEATURE_NOT_SUPPORTED); | ||
} | ||
|
||
return null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This kind of missing functionality. I would return an empty map at least to not cause NPE.
But we need to create an issue and fill this properly
} | ||
|
||
@Override | ||
public int getNetworkTimeout() throws SQLException { | ||
//TODO: Should this be supported? | ||
throw new SQLFeatureNotSupportedException("getNetworkTimeout not supported", ExceptionUtils.SQL_STATE_FEATURE_NOT_SUPPORTED); | ||
if (!config.isIgnoreUnsupportedRequests()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems something missing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean it may be a useful timeout - we should take it from client.
Quality Gate failedFailed conditions |
Summary
We should obey the no-op flag for unsupported features where possible.
Checklist
Delete items not relevant to your PR: