Skip to content

feat(sqllab): Add cache sqllab sidebar table schema (#32796) #32830

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.

Sign up for GitHub

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

Closed
trentlavoie wants to merge 1 commit into from
Closed

feat(sqllab): Add cache sqllab sidebar table schema (#32796) #32830

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions superset/config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -785,6 +785,9 @@ class D3TimeFormat(TypedDict, total=False):
# Cache for datasource metadata and query results
DATA_CACHE_CONFIG: CacheConfig = {"CACHE_TYPE": "NullCache"}

# Schema caching configuration
SCHEMA_CACHE_CONFIG: CacheConfig = {"CACHE_TYPE": "NullCache"}

@korbit-ai korbit-ai Bot Mar 24, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Ineffective Schema Cache Configuration category Performance

Tell me more
What is the issue?

The default schema cache configuration is set to use NullCache, which effectively disables caching.

Why this matters

The NullCache setting will prevent any actual caching from happening, defeating the purpose of implementing the schema caching mechanism for faster table metadata retrieval.

Suggested change ∙ Feature Preview

Set a default cache backend that actually performs caching, for example:

SCHEMA_CACHE_CONFIG: CacheConfig = {
    "CACHE_TYPE": "RedisCache",
    "CACHE_DEFAULT_TIMEOUT": 86400,  # 24 hours
    "CACHE_KEY_PREFIX": "schema_",
    "CACHE_REDIS_URL": "redis://localhost:6379/1"
}
Provide feedback to improve future suggestions

Nice Catch Incorrect Not in Scope Not in coding standard Other

💬 Looking for more details? Reply to this comment to chat with Korbit.


# Cache for dashboard filter state. `CACHE_TYPE` defaults to `SupersetMetastoreCache`
# that stores the values in the key-value table in the Superset metastore, as it's
# required for Superset to operate correctly, but can be replaced by any
Expand Down
12 changes: 11 additions & 1 deletion superset/databases/utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,13 @@
:param table: Table instance
:return: Dict table metadata ready for API response
"""
# Lazy import to prevent circular dependency
from superset.extensions import cache_manager
Comment on lines +74 to +75

@korbit-ai korbit-ai Bot Mar 24, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Incomplete circular dependency comment category Documentation

Tell me more
What is the issue?

The inline comment about lazy import is too brief and doesn't explain which modules are involved in the circular dependency.

Why this matters

Future maintainers may inadvertently create import problems without understanding which modules are affected.

Suggested change ∙ Feature Preview

Lazy import to prevent circular dependency between superset.extensions and superset.databases modules

Provide feedback to improve future suggestions

Nice Catch Incorrect Not in Scope Not in coding standard Other

💬 Looking for more details? Reply to this comment to chat with Korbit.

cache_key = f"table_metadata:{database.id}:{table.catalog or ''}:{table.schema or ''}:{table.table}"
cached_data = cache_manager.schema_cache.get(cache_key)
if cached_data:
return cached_data

Check warning on line 79 in superset/databases/utils.py

View check run for this annotation

Codecov / codecov/patch

superset/databases/utils.py#L79 

Added line #L79 was not covered by tests

keys = []
columns = database.get_columns(table)
primary_key = database.get_pk_constraint(table)
Expand All @@ -94,7 +101,8 @@
"comment": col.get("comment"),
}
)
return {

result = {
"name": table.table,
"columns": payload_columns,
"selectStar": database.select_star(
Expand All @@ -108,6 +116,8 @@
"indexes": keys,
"comment": table_comment,
}
cache_manager.schema_cache.set(cache_key, result)
return result


def make_url_safe(raw_url: str | URL) -> URL:
Expand Down
6 changes: 6 additions & 0 deletions superset/utils/cache_manager.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ def __init__(self) -> None:
self._thumbnail_cache = Cache()
self._filter_state_cache = Cache()
self._explore_form_data_cache = ExploreFormDataCache()
self._schema_cache = Cache()

@staticmethod
def _init_cache(
Expand Down Expand Up @@ -90,6 +91,7 @@ def init_app(self, app: Flask) -> None:
self._init_cache(app, self._cache, "CACHE_CONFIG")
self._init_cache(app, self._data_cache, "DATA_CACHE_CONFIG")
self._init_cache(app, self._thumbnail_cache, "THUMBNAIL_CACHE_CONFIG")
self._init_cache(app, self._schema_cache, "SCHEMA_CACHE_CONFIG")
self._init_cache(
app, self._filter_state_cache, "FILTER_STATE_CACHE_CONFIG", required=True
)
Expand Down Expand Up @@ -119,3 +121,7 @@ def filter_state_cache(self) -> Cache:
@property
def explore_form_data_cache(self) -> Cache:
return self._explore_form_data_cache

@property
def schema_cache(self) -> Cache:
return self._schema_cache
Loading