-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
BUG: read_sql_table unable to read views #54185
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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) | ||
table = SQLTable(table_name, self, index=index_col, schema=schema) | ||
if chunksize is not None: | ||
self.returns_generator = True | ||
|
@@ -1989,7 +1989,9 @@ def get_table(self, table_name: str, schema: str | None = None) -> Table: | |
def drop_table(self, table_name: str, schema: str | None = None) -> None: | ||
schema = schema or self.meta.schema | ||
if self.has_table(table_name, schema): | ||
self.meta.reflect(bind=self.con, only=[table_name], schema=schema) | ||
self.meta.reflect( | ||
bind=self.con, only=[table_name], schema=schema, views=True | ||
) | ||
with self.run_transaction(): | ||
self.get_table(table_name, schema).drop(bind=self.con) | ||
self.meta.clear() | ||
|
@@ -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""" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is something like this equivalent here opposed to the sql?
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
SELECT | ||
name | ||
FROM | ||
sqlite_master | ||
WHERE | ||
type IN ('table', 'view') | ||
AND name={wld}; | ||
""" | ||
|
||
return len(self.execute(query, [name]).fetchall()) > 0 | ||
|
||
|
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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