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

BUG: read_sql_table unable to read views #54185

Merged
merged 1 commit into from
Jul 20, 2023

Conversation

mroeschke
Copy link
Member

@mroeschke mroeschke commented Jul 19, 2023

Looks like read_sql_table isn't implemented for sqllite, but this PR enable read_sql to work with views

@mroeschke mroeschke added the IO SQL to_sql, read_sql, read_sql_query label Jul 19, 2023
@mroeschke mroeschke added this to the 2.1 milestone Jul 19, 2023
@@ -2411,7 +2413,15 @@ def to_sql(

def has_table(self, name: str, schema: str | None = None) -> bool:
wld = "?"
query = f"SELECT name FROM sqlite_master WHERE type='table' AND name={wld};"
query = f"""
Copy link
Member

Choose a reason for hiding this comment

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

is something like this equivalent here opposed to the sql?

self.meta.reflect(bind=self.con)
return name in metadata.tables.keys()

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I believe so https://www.sqlite.org/schematab.html

Since this is the sqlite3 backend unfortunately we can't meta.reflect

@@ -1655,7 +1655,7 @@ def read_table(
SQLDatabase.read_query
"""
self.meta.reflect(bind=self.con, only=[table_name])
self.meta.reflect(bind=self.con, only=[table_name], views=True)
Copy link
Member

Choose a reason for hiding this comment

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

guessing this isnt too expensive performance wise that we will now start reflecting all tables by default (include views/mat views) just thinking if this will impact performance for current users.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah I'm assuming it's negligible relative to the query or IO, especially since the old implementation was able to access views too

@alimcmaster1
Copy link
Member

Hey @mroeschke !

Didn't realise we don't have read_sql_table implemented for sqllite! Looks great to me, some minor thoughts above :)

@mroeschke mroeschke merged commit 743111e into pandas-dev:main Jul 20, 2023
35 of 36 checks passed
@mroeschke mroeschke deleted the bug/sql/view branch July 20, 2023 15:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
IO SQL to_sql, read_sql, read_sql_query
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BUG: read_sql_table fails on any Oracle view and/or materialized view.
2 participants