Skip to content

add table_resolved and schemas_resolved fields#9716

Open
Light2Dark wants to merge 5 commits into
sham/hide-empty-schemas-dbfrom
sham/add-resolved-flag
Open

add table_resolved and schemas_resolved fields#9716
Light2Dark wants to merge 5 commits into
sham/hide-empty-schemas-dbfrom
sham/add-resolved-flag

Conversation

@Light2Dark
Copy link
Copy Markdown
Collaborator

@Light2Dark Light2Dark commented May 29, 2026

📝 Summary

These fields are to indicate whether a table's schemas has been loaded vs truly empty.
That allows the frontend to distinguish whether to hide empty vs lazy (which we shouldn't hide).

This change makes sense for future work, since it's important to distinguish between truly empty vs not yet loaded. Another option was considered: list[DataTable] | None, where None indicates not yet loaded. However, this is semantically not clear even if a simpler code change.

This is in support of this PR #9708

📋 Pre-Review Checklist

  • For large changes, or changes that affect the public API: this change was discussed or approved through an issue, on Discord, or the community discussions (Please provide a link if applicable).
  • Any AI generated code has been reviewed line-by-line by the human PR author, who stands by it.
  • Video or media evidence is provided for any visual changes (optional).

✅ Merge Checklist

  • I have read the contributor guidelines.
  • Documentation has been updated where applicable, including docstrings for API changes.
  • Tests have been added for the changes made.

@vercel
Copy link
Copy Markdown

vercel Bot commented May 29, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
marimo-docs Ready Ready Preview, Comment May 29, 2026 5:11am

Request Review

@github-actions github-actions Bot added the bash-focus Area to focus on during release bug bash label May 29, 2026
@Light2Dark Light2Dark requested a review from Copilot May 29, 2026 02:15
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

2 issues found across 15 files

Architecture diagram
sequenceDiagram
    participant FE as Frontend (React)
    participant API as API Route / Kernel
    participant Engine as SQL Engine (adbc/ibis/clickhouse/...)
    participant DB as Database (DuckDB/Snowflake/Redshift/...)
    participant Store as Connection State Store

    Note over FE,DB: DATABASE EXPLORER - Schema/Table Discovery Flow

    FE->>API: GET /datasources (initial load)
    API->>Engine: get_databases(include_schemas, include_tables)
    Engine->>DB: Fetch catalogs/databases
    DB-->>Engine: Catalog list

    alt include_schemas=True
        Engine->>DB: Fetch schemas for each catalog
        DB-->>Engine: Schema list
        Engine->>Engine: Build Schema objects with schemas_resolved=True
        alt include_tables=True
            Engine->>DB: Fetch tables for each schema
            DB-->>Engine: Table list
            Engine->>Engine: Build DataTable objects with tables_resolved=True
        else include_tables=False (deferred)
            Engine->>Engine: Build Schema with tables=[], tables_resolved=False
        end
    else include_schemas=False (deferred)
        Engine->>Engine: Build Database with schemas=[], schemas_resolved=False
    end

    Engine-->>API: List[Database] with schemas_resolved & tables_resolved flags
    API->>Store: Update connection state with resolved flags
    Store-->>API: Confirmed
    API-->>FE: Serialized Database list

    Note over FE,Store: CLIENT-SIDE FILTERING (hideEmptyDatasources)

    FE->>FE: filterEmptyDatabases(databases)
    alt Database.schemas_resolved != false AND schemas empty
        FE->>FE: Hide database (truly empty catalog)
    else Database.schemas_resolved == false
        FE->>FE: Keep database visible (deferred discovery)
    end

    alt Schema.tables_resolved != false AND tables empty
        FE->>FE: Hide schema (truly empty schema)
    else Schema.tables_resolved == false
        FE->>FE: Keep schema visible (deferred table loading)
    end

    Note over FE: LAZY TABLE LOADING

    FE->>FE: User expands schema node
    FE->>API: GET /datasources/schema-tables (lazy fetch)
    API->>Engine: get_schemas(include_tables=True)
    Engine->>DB: Fetch tables for this schema
    DB-->>Engine: Table list
    Engine-->>API: Updated Schema with tables_resolved=True
    API->>Store: Update schema with resolved tables
    Store-->>API: Confirmed
    API-->>FE: DataTable list with resolved flag
    FE->>FE: Re-filter using updated tables_resolved state
Loading

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread packages/openapi/api.yaml
Comment thread packages/openapi/api.yaml
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR introduces two backward-compatible “resolved” flags (schemas_resolved on Database, tables_resolved on Schema) so clients can distinguish “truly empty” schema/table lists from “not yet enumerated / deferred discovery” lists, enabling the frontend to avoid hiding lazy-loaded items when “hide empty” is enabled.

Changes:

  • Extend backend Database/Schema models with schemas_resolved / tables_resolved (defaulting to True) and propagate these flags through SQL engine introspection paths.
  • Update connection update utilities to mark schemas/tables as resolved after a lazy fetch updates the in-memory connection model.
  • Update frontend datasource filtering and connection-state update logic to respect/set the new resolved flags; update OpenAPI artifacts and SQL engine tests accordingly.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
marimo/_data/models.py Adds tables_resolved / schemas_resolved fields (default True) to core data models.
marimo/_sql/connection_utils.py Marks schema/table lists as resolved when updating a connection after lazy fetch.
marimo/_sql/engines/sqlalchemy.py Populates schemas_resolved and tables_resolved during introspection.
marimo/_sql/engines/redshift.py Populates schemas_resolved and tables_resolved during introspection.
marimo/_sql/engines/ibis.py Threads schemas_resolved / tables_resolved through Ibis introspection.
marimo/_sql/engines/adbc.py Threads resolved flags through ADBC metadata enumeration.
marimo/_sql/engines/clickhouse.py Sets tables_resolved depending on whether tables were introspected for a database.
marimo/_sql/engines/pyiceberg.py Sets tables_resolved based on whether table discovery ran.
frontend/src/components/datasources/datasources.tsx Updates “hide empty” filtering to treat unresolved empties as “unknown” (keep visible).
frontend/src/core/datasets/data-source-connections.ts Ensures lazy-loaded schema/table updates set schemas_resolved / tables_resolved to true.
packages/openapi/api.yaml Updates OpenAPI schema/docs for the new optional flags (default true).
packages/openapi/src/api.ts Updates generated TS types/docs for the new optional flags (default true).
tests/_sql/test_sqlalchemy.py Updates expectations to cover the new resolved flags.
tests/_sql/test_ibis.py Adds coverage for empty-vs-deferred semantics using the new flags.

Comment thread marimo/_sql/engines/sqlalchemy.py
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

1 issue found across 15 files

Architecture diagram
sequenceDiagram
    participant UI as Frontend UI
    participant Filter as filterEmptyDatabases()
    participant Model as Data Model (Schema/Database)
    participant Engine as SQL Engine Layer
    participant DB as Database

    Note over UI,DB: Data Source Tree Rendering

    UI->>Engine: getDatabases(include_schemas, include_tables)
    
    alt include_schemas=True, include_tables=True
        Engine->>DB: Full metadata query
        DB-->>Engine: Raw schemas & tables
        Engine->>Engine: Build Database with schemas_resolved=True
        Engine->>Engine: Build Schema with tables_resolved=True
    else include_schemas=True, include_tables=False
        Engine->>DB: Schema names only
        DB-->>Engine: Schema list
        Engine->>Engine: Build Database with schemas_resolved=True
        Engine->>Engine: Build Schema with tables=[], tables_resolved=False
    else include_schemas=False
        Engine-->>UI: Database with schemas=[], schemas_resolved=False
    end

    Engine-->>UI: Database list with resolved flags

    UI->>Filter: filterEmptyDatabases(databases)

    loop For each database
        alt schemas_resolved == false
            Filter-->>UI: Keep database (deferred)
        else schemas_resolved == true AND schemas.length == 0
            Filter-->>UI: Hide database (confirmed empty)
        else schemas.length > 0
            loop For each schema
                alt tables_resolved == false
                    Filter-->>Filter: Keep schema (deferred)
                else tables_resolved == true AND tables.length == 0
                    Filter-->>Filter: Hide schema (confirmed empty)
                else tables.length > 0
                    Filter-->>Filter: Keep schema (has tables)
                end
            end
            alt All schemas hidden
                Filter-->>UI: Hide database
            else Some schemas kept
                Filter-->>UI: Database with visible schemas
            end
        end
    end

    Note over UI: User expands a deferred node
    
    alt User clicks deferred schema
        UI->>Engine: getTablesForSchema(schema)
        Engine->>DB: Query tables
        DB-->>Engine: Table list
        Engine->>Engine: Update schema.tables, set tables_resolved=True
        Engine-->>UI: Updated schema
        UI->>UI: Re-render with table names
    else User clicks deferred database
        UI->>Engine: getSchemasForDatabase(database)
        Engine->>DB: Query schemas
        DB-->>Engine: Schema list
        Engine->>Engine: Update database.schemas, set schemas_resolved=True
        Engine-->>UI: Updated database
        UI->>UI: Re-render with schema list
    end

    Note over UI,Filter: Backward Compatibility
    Note over Model: Old payloads missing resolved flags<br/>default to True (resolved)
Loading

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread marimo/_sql/engines/clickhouse.py Outdated
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@Light2Dark Light2Dark marked this pull request as ready for review May 29, 2026 09:10
@Light2Dark Light2Dark changed the title add table_resolved and schemas_resolved flag add table_resolved and schemas_resolved fields May 29, 2026
@Light2Dark Light2Dark requested a review from dmadisetti May 29, 2026 14:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bash-focus Area to focus on during release bug bash enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants