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

issue 2751 make sure drop schema drops everything #2932

Merged
merged 7 commits into from
Nov 4, 2021
Merged

Conversation

punktilious
Copy link
Collaborator

@punktilious punktilious commented Nov 1, 2021

Signed-off-by: Robin Arnold robin.arnold@ibm.com

In addition to making sure we clean up the new WHOLE_SYSTEM_VERSION and CONTROL tables, we also now do a final check on the cleaned schema to make sure it is really clean and contains no tables.

The drop is now idempotent (dropping a second time will be a NOP) and if the drop is successful, the VERSION_HISTORY data will be cleared for the dropped schema, allowing the same schema name to be reused immediately.

Signed-off-by: Robin Arnold <robin.arnold@ibm.com>
Signed-off-by: Robin Arnold <robin.arnold@ibm.com>
Signed-off-by: Robin Arnold <robin.arnold@ibm.com>
+ " WHERE LOWER(table_schema) = LOWER('" + schemaName + "') "
+ " AND LOWER(table_name) = LOWER('" + tableName + "')";
+ " WHERE table_schema = LOWER('" + schemaName + "') "
+ " AND table_name = LOWER('" + tableName + "')";
Copy link
Contributor

Choose a reason for hiding this comment

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

I think initially our Postgres code allowed upper case input. I know Postgres smacks it to LOWER internally.
Is this truly backwards compatible with our older versions?

Please confirm

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

PostgreSQL lower-cases identifiers (like table-names) unless they are quoted (which we don't...because it makes things ugly and unreadable). Also, putting LOWER on the columns in that way prevents use of any indexes which could lead to much worse performance for bigger schemas.

Copy link
Contributor

@prb112 prb112 left a comment

Choose a reason for hiding this comment

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

LGTM - left comments

punktilious and others added 3 commits November 3, 2021 15:18
Signed-off-by: Robin Arnold <robin.arnold@ibm.com>
but really just to force a rebuild...

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
@lmsurpre lmsurpre merged commit b19bda2 into main Nov 4, 2021
@lmsurpre lmsurpre deleted the issue-2751-fix2 branch November 4, 2021 12:54
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.

3 participants