Skip to content

Fix session schema in testRenameTableAcrossSchema#11978

Merged
ebyhr merged 1 commit intotrinodb:masterfrom
ebyhr:ebi/test-table-exists
Apr 16, 2022
Merged

Fix session schema in testRenameTableAcrossSchema#11978
ebyhr merged 1 commit intotrinodb:masterfrom
ebyhr:ebi/test-table-exists

Conversation

@ebyhr
Copy link
Member

@ebyhr ebyhr commented Apr 15, 2022

Description

Fix session schema in testRenameTableAcrossSchema. The previous logic tried to check the below table:
"catalog"."schema"."new_schema.renamed_table"

It should be:
"catalog"."new_schema"."renamed_table"

Documentation

(x) No documentation is needed.

Release notes

(x) No release notes entries required.

@ebyhr ebyhr added the no-release-notes This pull request does not require release notes entry label Apr 15, 2022
@cla-bot cla-bot bot added the cla-signed label Apr 15, 2022
@ebyhr ebyhr force-pushed the ebi/test-table-exists branch from b5ae8e7 to 2ac48e0 Compare April 15, 2022 10:11
@ebyhr ebyhr requested review from findepi and hashhar April 15, 2022 10:22
Copy link
Member

Choose a reason for hiding this comment

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

Unrelated (should be separate commit)

Also, while format is better for general string manipulation, direct + concatenation allows IntelliJ to apply syntax highlighting. At least my IntelliJ version does SQL syntax highlihting for the code before the change, and doesn't do after.
So maybe just drop the change?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't have a strong opinion, but I feel 3 concatenation for simple SQL is less readable than format even if the former has syntax highlight. Anyway, reverted to the original style.

"ALTER TABLE " + tableName + " RENAME TO " + schemaName + "." + renamedTable

format("ALTER TABLE %s RENAME TO %s.%s", tableName, schemaName, renamedTable)

The previous logic tried to check the below table:
"catalog"."schema"."new_schema.renamed_table"

It should be:
"catalog"."new_schema"."renamed_table"
@ebyhr ebyhr force-pushed the ebi/test-table-exists branch from 2ac48e0 to 7d02885 Compare April 16, 2022 00:02
@ebyhr ebyhr merged commit 0e1a4da into trinodb:master Apr 16, 2022
@ebyhr ebyhr deleted the ebi/test-table-exists branch April 16, 2022 01:59
@github-actions github-actions bot added this to the 378 milestone Apr 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla-signed no-release-notes This pull request does not require release notes entry

Development

Successfully merging this pull request may close these issues.

2 participants