-
Notifications
You must be signed in to change notification settings - Fork 24.9k
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
SQL: Use ResultSets over exceptions in metadata #40641
Conversation
Pinging @elastic/es-search |
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.
LGTM
@@ -30,17 +30,23 @@ | |||
class JdbcHttpClient { | |||
private final HttpClient httpClient; | |||
private final JdbcConfiguration conCfg; | |||
private final InfoResponse serverInfo; | |||
private InfoResponse serverInfo; |
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.
It can still be final if there is an else { serverInfo = null }
in the ctor.
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.
But then this won't work.
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.
LGTM
Changed the JDBC metadata to return empty results sets instead of throwing SQLFeatureNotSupported as it seems a more safer/compatible approach for consumers. Fix elastic#40533
Since #40583 has a discussion around it, I've rebased the latest commit to be able to merge this. |
…e-unsafe-publication * elastic/master: Update contributing docs to reflect JDK 11 (elastic#40955) Docs: Simplifying setup by using module configuration variant syntax (elastic#40879) Unmute CreateIndexIT testCreateAndDeleteIndexConcurrently CI failures (elastic#40960) Revert "Short-circuit rebalancing when disabled (elastic#40942)" Mute EnableAllocationShortCircuitTests SQL: Refactor args verification of In & conditionals (elastic#40916) Avoid sharing source directories as it breaks intellij (elastic#40877) Short-circuit rebalancing when disabled (elastic#40942) SQL: Prefer resultSets over exceptions in metadata (elastic#40641) Mute ClusterPrivilegeTests.testThatSnapshotAndRestore Fix Race in AsyncTwoPhaseIndexerTests.testStateMachine (elastic#40947) Relax Overly Strict Assertion in TransportShardBulkAction (elastic#40940)
Changed the JDBC metadata to return empty results sets instead of throwing SQLFeatureNotSupported as it seems a more safer/compatible approach for consumers. Fix elastic#40533
Changed the JDBC metadata to return empty results sets instead of
throwing SQLFeatureNotSupported as it seems a more safer/compatible
approach for consumers.
Fix #40533
It's a follow-up to #40583 hence only the latest commit is the one that counts.