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

Fix: allow (re)setting the current catalog #212

Merged
merged 2 commits into from Feb 28, 2020

Conversation

bpintea
Copy link
Collaborator

@bpintea bpintea commented Feb 28, 2020

This PR allows the application set the value of the current catalog attribute, in case this value is the same with what the driver had previously returned for this attribute.

Although Elasticsearch/SQL has no proper catalog support currently, this case should be supported as a safe use of the API.

This currently only works if the attribute had been retrieved before setting, on the same connection; specifically, it won't work when this would be set as a pre-connection attribute.

Closes #211.

This commit allows the application set the value of the current catalog
attribute, in case this value is the same with what the driver had
previously returned for this attribute.

This currently only works if the attribute had been retrieved before
setting, on the same connection; specifically, it won't work when this
would be set as a pre-connection attribute.
Copy link
Contributor

@droberts195 droberts195 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@@ -7,6 +7,11 @@
#include <gtest/gtest.h>
#include "connected_dbc.h"

/* */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: empty comment

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks, fixed.

- remove empty comment

Also apply change that was (somehow) not caught by the hooked formatter
previously.
@bpintea bpintea merged commit 279c485 into elastic:master Feb 28, 2020
@bpintea bpintea deleted the fix/allow_catalog_setting branch February 28, 2020 15:34
bpintea added a commit that referenced this pull request Feb 28, 2020
* allow (re)setting the current catalog

This commit allows the application set the value of the current catalog
attribute, in case this value is the same with what the driver had
previously returned for this attribute.

This currently only works if the attribute had been retrieved before
setting, on the same connection; specifically, it won't work when this
would be set as a pre-connection attribute.

(cherry picked from commit 279c485)
bpintea added a commit that referenced this pull request Feb 28, 2020
* allow (re)setting the current catalog

This commit allows the application set the value of the current catalog
attribute, in case this value is the same with what the driver had
previously returned for this attribute.

This currently only works if the attribute had been retrieved before
setting, on the same connection; specifically, it won't work when this
would be set as a pre-connection attribute.

(cherry picked from commit 279c485)
bpintea added a commit that referenced this pull request Feb 28, 2020
* allow (re)setting the current catalog

This commit allows the application set the value of the current catalog
attribute, in case this value is the same with what the driver had
previously returned for this attribute.

This currently only works if the attribute had been retrieved before
setting, on the same connection; specifically, it won't work when this
would be set as a pre-connection attribute.

(cherry picked from commit 279c485)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow setting the current catalog to cluster's name
2 participants