-
Notifications
You must be signed in to change notification settings - Fork 95
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
Do not dump Babelfish initialize user #1841
Merged
shardgupta
merged 7 commits into
babelfish-for-postgresql:BABEL_3_X_DEV
from
amazon-aurora:jira-babel-4404
Sep 19, 2023
Merged
Do not dump Babelfish initialize user #1841
shardgupta
merged 7 commits into
babelfish-for-postgresql:BABEL_3_X_DEV
from
amazon-aurora:jira-babel-4404
Sep 19, 2023
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
rishabhtanwar29
force-pushed
the
jira-babel-4404
branch
3 times, most recently
from
September 13, 2023 17:27
9674c3e
to
a653693
Compare
Signed-off-by: Rishabh Tanwar <[email protected]>
Signed-off-by: Rishabh Tanwar <[email protected]>
rishabhtanwar29
force-pushed
the
jira-babel-4404
branch
from
September 13, 2023 18:02
a653693
to
55b0f70
Compare
Signed-off-by: Rishabh Tanwar <[email protected]>
rishabhtanwar29
force-pushed
the
jira-babel-4404
branch
from
September 13, 2023 19:02
fea1e6b
to
43455a4
Compare
Signed-off-by: Rishabh Tanwar <[email protected]>
Signed-off-by: Rishabh Tanwar <[email protected]>
rishabhtanwar29
force-pushed
the
jira-babel-4404
branch
from
September 14, 2023 05:41
9a38e5d
to
9270bfa
Compare
rishabhtanwar29
changed the title
Do not dump babelfish initialize user
Do not dump Babelfish initialize user
Sep 14, 2023
Signed-off-by: Rishabh Tanwar <[email protected]>
rishabhtanwar29
force-pushed
the
jira-babel-4404
branch
from
September 18, 2023 06:20
65dcea0
to
328f027
Compare
HarshLunagariya
approved these changes
Sep 18, 2023
shardgupta
approved these changes
Sep 19, 2023
shardgupta
pushed a commit
to babelfish-for-postgresql/postgresql_modified_for_babelfish
that referenced
this pull request
Sep 19, 2023
When Babelfish initialize user is same between source and target servers for dump/restore, the restore throws errors like "role ... already exists" and "duplicate key value violates unique constraint..." etc while inserting catalog entry for initialize user in babelfish_authid_login_ext catalog. These errors are actually expected and makes sense as we know the user do already exists on the target server but the whole restore gets rolled back in case the restore process has been made transactional (`--single-transaction` option is used) and there is no way for a user to fix this manually. To overcome the above issue, this commit implements the logic to skip dumping Babelfish initialize user so that there is no conflict when initialize user is same or different between source and target servers. This is safe since no T-SQL objects will have direct reference to initialize user and they will adapt to the new initialize user on the target server. The only case we have to handle is the "owner" name in `sys.babelfish_sysdatabases` catalog table, which references to initialize user. For this, we will also skip dumping the "owner" column and re-populate this column during restore with the owner of current database (which is going to be initialize user of target Babelfish database). Task: BABEL-4404 Signed-off-by: Rishabh Tanwar <[email protected]> Extensions PR: babelfish-for-postgresql/babelfish_extensions#1841
Sairakan
pushed a commit
to amazon-aurora/postgresql_modified_for_babelfish
that referenced
this pull request
Nov 16, 2023
When Babelfish initialize user is same between source and target servers for dump/restore, the restore throws errors like "role ... already exists" and "duplicate key value violates unique constraint..." etc while inserting catalog entry for initialize user in babelfish_authid_login_ext catalog. These errors are actually expected and makes sense as we know the user do already exists on the target server but the whole restore gets rolled back in case the restore process has been made transactional (`--single-transaction` option is used) and there is no way for a user to fix this manually. To overcome the above issue, this commit implements the logic to skip dumping Babelfish initialize user so that there is no conflict when initialize user is same or different between source and target servers. This is safe since no T-SQL objects will have direct reference to initialize user and they will adapt to the new initialize user on the target server. The only case we have to handle is the "owner" name in `sys.babelfish_sysdatabases` catalog table, which references to initialize user. For this, we will also skip dumping the "owner" column and re-populate this column during restore with the owner of current database (which is going to be initialize user of target Babelfish database). Task: BABEL-4404 Signed-off-by: Rishabh Tanwar <[email protected]> Extensions PR: babelfish-for-postgresql/babelfish_extensions#1841
priyansx
pushed a commit
to amazon-aurora/postgresql_modified_for_babelfish
that referenced
this pull request
Nov 22, 2023
When Babelfish initialize user is same between source and target servers for dump/restore, the restore throws errors like "role ... already exists" and "duplicate key value violates unique constraint..." etc while inserting catalog entry for initialize user in babelfish_authid_login_ext catalog. These errors are actually expected and makes sense as we know the user do already exists on the target server but the whole restore gets rolled back in case the restore process has been made transactional (`--single-transaction` option is used) and there is no way for a user to fix this manually. To overcome the above issue, this commit implements the logic to skip dumping Babelfish initialize user so that there is no conflict when initialize user is same or different between source and target servers. This is safe since no T-SQL objects will have direct reference to initialize user and they will adapt to the new initialize user on the target server. The only case we have to handle is the "owner" name in `sys.babelfish_sysdatabases` catalog table, which references to initialize user. For this, we will also skip dumping the "owner" column and re-populate this column during restore with the owner of current database (which is going to be initialize user of target Babelfish database). Task: BABEL-4404 Signed-off-by: Rishabh Tanwar <[email protected]> Extensions PR: babelfish-for-postgresql/babelfish_extensions#1841
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.
Description
When Babelfish initialize user is same between source and target servers
for dump/restore, the restore throws errors like "role ... already exists" and
"duplicate key value violates unique constraint..." etc while inserting catalog
entry for initialize user in babelfish_authid_login_ext catalog. These errors are
actually expected and makes sense as we know the user do already exists on
the target server but the whole restore gets rolled back in case the restore
process has been made transactional (
--single-transaction
option is used)and there is no way for a user to fix this manually.
To overcome the above issue, the engine commit implements the logic to skip
dumping Babelfish initialize user as well as the "owner" column of
sys.babelfish_sysdatabases
catalog table.This commit implements logic to re-populate "owner" column of
sys.babelfish_sysdatabases
catalog table during restore with the owner ofcurrent database (which is going to be initialize user of target Babelfish database).
Test changes:
both data formats.
--single-transaction
option for restore so that any error during restorewould result in restore getting rolledback and failed.
Task: BABEL-4404
Signed-off-by: Rishabh Tanwar [email protected]
Engine PR: babelfish-for-postgresql/postgresql_modified_for_babelfish#216
Test Scenarios Covered
Use case based -
Boundary conditions -
Arbitrary inputs -
Negative test cases -
Minor version upgrade tests -
Major version upgrade tests -
Performance tests -
Tooling impact -
Client tests -
Check List
By submitting this pull request, I confirm that my contribution is under the terms of the Apache 2.0 and PostgreSQL licenses, and grant any person obtaining a copy of the contribution permission to relicense all or a portion of my contribution to the PostgreSQL License solely to contribute all or a portion of my contribution to the PostgreSQL open source project.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.