-
Notifications
You must be signed in to change notification settings - Fork 30
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
DSN parameter: support for "index_include_frozen" #151
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Introduce new ConnectedDBC class member function to validate the generated request; thus removing code duplication in tests and allowing for easier change of the request format when introducing new request settings.
Form the expected JSON request using the same defs as used in the driver.
Introduce new request setting "to allow all queries inside one connection to include frozen indices. If [false], the user would have to manually mention this through INCLUDE FROZEN and FROZEN".
droberts195
approved these changes
May 10, 2019
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
bpintea
added a commit
that referenced
this pull request
May 18, 2019
* C2SQL conversion tests: new class fn for requests Introduce new ConnectedDBC class member function to validate the generated request; thus removing code duplication in tests and allowing for easier change of the request format when introducing new request settings. * use JSON keys defined in driver for unit testing Form the expected JSON request using the same defs as used in the driver. * introduce new "index_include_frozen" request param Introduce new request setting "to allow all queries inside one connection to include frozen indices. If [false], the user would have to manually mention this through INCLUDE FROZEN and FROZEN". (cherry picked from commit d35ae29)
bpintea
added a commit
to bpintea/elasticsearch-sql-odbc
that referenced
this pull request
Jul 2, 2019
Partially revert elastic#151, removing the support for the "index_include_frozen". Close elastic#159
bpintea
added a commit
that referenced
this pull request
Jul 2, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds the support for the configuration of the
index_include_frozen
parameter of ES/SQL (elastic/elasticsearch#41558): the DSN parameter is calledIndexIncludeFrozen
.The PR also changes the unit tests to centralize the way the generated JSON request is verified against an expected value: the ConnectedDBC class now has two new function members and these will construct the expected JSON object. This allows to easily support addition of any request configuration parameters, besides removing quite some code duplication.