Skip to content
This repository was archived by the owner on May 14, 2026. It is now read-only.

fix: use empty schema name by default - #32

Merged
AVaksman merged 14 commits into
mainfrom
get_table_names
Apr 6, 2021
Merged

fix: use empty schema name by default#32
AVaksman merged 14 commits into
mainfrom
get_table_names

Conversation

@IlyaFaer

@IlyaFaerIlyaFaer commented Mar 19, 2021

Copy link
Copy Markdown

The PR is based on another: #24

@google-clagoogle-claBot added the cla: yes This human has signed the Contributor License Agreement. label Mar 19, 2021
WHERE table_schema = '{}'
""".format(
schema
schema or ""

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

By default schema arg is None, so it tried to search for tables with schema 'None'. Not sure how I've written this, it's not smart at all. Fixing it.

Comment threadtest/test_suite.py
answer = ["users", "user_tmp", "email_addresses", "dingalings"]
eq_(table_names, answer)
else:
answer = ["dingalings", "email_addresses", "user_tmp", "users"]

@IlyaFaerIlyaFaerMar 19, 2021

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

user_tmp is a temporary table, adding it into the expected results

@IlyaFaer

IlyaFaer commented Mar 19, 2021

Copy link
Copy Markdown
Author

@larkee, this PR fixes two more reflection tests. Still, #24 better be merged first.

Безымянный

@IlyaFaer
IlyaFaer requested a review from larkeeMarch 19, 2021 10:35
@IlyaFaerIlyaFaer mentioned this pull request Mar 19, 2021

@larkeelarkee left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Changes introduced in this PR (i.e. schema name and table name test override) LGTM 👍

Approval pending the base PR

@AVaksman

Copy link
Copy Markdown
Contributor

Merging to unblock further work

@AVaksman
AVaksman merged commit f2af0b1 into mainApr 6, 2021
@IlyaFaer
IlyaFaer deleted the get_table_names branch April 6, 2021 10:44
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

cla: yesThis human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@IlyaFaer@AVaksman@larkee