fix: correct get_view_names for older sqlalchemy #393
Conversation
|
🤖 I detect that the PR title and the commit message differ and there's only one commit. To use the PR title for the commit history, you can use Github's automerge feature with squashing, or use -- conventional-commit-lint bot |
for more information, see https://pre-commit.ci
duckdb_engine/__init__.py
Outdated
| s = "SELECT name FROM sqlite_master WHERE type='view' ORDER BY name" | ||
| rs = connection.exec_driver_sql(s) | ||
| s = f"SELECT table_name FROM information_schema.tables WHERE table_type='VIEW' and table_schema=?" | ||
| rs = connection.execute(s, schema if schema is not None else "main") |
There was a problem hiding this comment.
It needs to use fetchall() to get the query result
rs = connection.execute(s, schema if schema is not None else "main").fetchall()
duckdb_engine/__init__.py
Outdated
| @@ -207,7 +207,7 @@ def get_view_names( | |||
| include: Any = ..., | |||
| **kw: Any, | |||
| ) -> Any: | |||
| s = "SELECT name FROM sqlite_master WHERE type='view' ORDER BY name" | |||
| rs = connection.exec_driver_sql(s) | |||
| s = f"SELECT table_name FROM information_schema.tables WHERE table_type='VIEW' and table_schema=?" | |||
There was a problem hiding this comment.
Is filter condition: table_type='VIEW' required? since it is also called from superset - list table function, in such case, no need this filter or specify table_type= 'Base Table'
Codecov Report
@@ Coverage Diff @@
## master #393 +/- ##
==========================================
- Coverage 93.46% 93.11% -0.35%
==========================================
Files 8 8
Lines 459 465 +6
==========================================
+ Hits 429 433 +4
- Misses 30 32 +2
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Mause
changed the title
|
just notice, so for new version sqlalchemy, s = f"SELECT table_name FROM information_schema.tables WHERE table_type='VIEW' and table_schema=%s "
schema = schema if schema is not None else "main"
rs = connection.exec_driver_sql(s % schema)and for old version, should s = f"SELECT table_name FROM information_schema.tables WHERE table_type='VIEW' and table_schema=? "
schema = schema if schema is not None else "main"
rs = connection.execute(s ,schema)
``
or view will not show. |
supports both old and new version sqlalchemy and supports schema
for more information, see https://pre-commit.ci
|
Which old versions of sqlalchemy are you trying to support? |
|
It looks like the current superset version only supports >=1.4 https://github.com/apache/superset/blob/master/setup.py#L118 |
|
the last release version apache superset 2.0 still needs <1.4 |
|
I've merged most of these changes in another pr, #404 so thanks for those. The remaining changes don't seem to be required and in fact introduce a potential security issue The fixes will be released shortly |
BEGIN_COMMIT_OVERRIDE
fix: add schema support to get_view_names
END_COMMIT_OVERRIDE