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

Do not dump Babelfish initialize user #216

Conversation

rishabhtanwar29
Copy link
Contributor

@rishabhtanwar29 rishabhtanwar29 commented Sep 13, 2023

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, 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

Check List

  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is under the terms of the PostgreSQL license, 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.

@rishabhtanwar29 rishabhtanwar29 changed the title Do not dump babelfish initialize user Do not dump Babelfish initialize user Sep 14, 2023
src/bin/pg_dump/dump_babel_utils.c Outdated Show resolved Hide resolved
src/bin/pg_dump/dump_babel_utils.c Show resolved Hide resolved
src/bin/pg_dump/dump_babel_utils.c Outdated Show resolved Hide resolved
Signed-off-by: Rishabh Tanwar <[email protected]>
@shardgupta shardgupta merged commit a211b57 into babelfish-for-postgresql:BABEL_3_X_DEV__PG_15_X Sep 19, 2023
@shardgupta shardgupta deleted the jira-babel-4404 branch September 19, 2023 07:03
shardgupta pushed a commit to babelfish-for-postgresql/babelfish_extensions 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, 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 of current database (which is going to be initialize user of target Babelfish database).

#### Test changes:
* Updated github dump/restore action to take two more options as input:
  1. dump-data-as: Dump table data using COPY or INSERT
  2. dump-format: Dump format (plain/custom/tar/directory)
* Updated dump-restore configurations to test all four dump formats and both data formats.
* Enabled `--single-transaction` option for restore so that any error during restore would result in restore getting rolledback and failed.
* Added TAP test coverage for pg_restore.
* Fixed BABEL-1206-vu-prepare test to insert valid data into varchar column. 

Task: BABEL-4404 
Signed-off-by: Rishabh Tanwar <[email protected]>

Engine PR: babelfish-for-postgresql/postgresql_modified_for_babelfish#216
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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants