Skip to content

Improve CLI describe entity detection - #3934

Merged
kevinjqliu merged 4 commits into
mainfrom
kevinjqliu-cli-describe-entity-detection
Sep 12, 2026
Merged

Improve CLI describe entity detection#3934
kevinjqliu merged 4 commits into
mainfrom
kevinjqliu-cli-describe-entity-detection

Conversation

@kevinjqliu

@kevinjqliu kevinjqliu commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Why

pyiceberg describe currently prints namespace output before it knows whether the same identifier is also a table. This makes ambiguous identifiers look successful and leaves partial output behind.

This splits the generic describe fix from #3926. View support stays in the follow-up PR.

CLI behavior

--entity still accepts only any, namespace, and table.

Fast paths

Explicit entity selection returns directly from the matching catalog loader:

  • pyiceberg describe --entity namespace sales.orders loads the multipart namespace and returns.
  • pyiceberg describe --entity table sales.orders loads the table and returns.

A single-part identifier in the default any mode also keeps its existing fast path:

  • pyiceberg describe sales loads sales as a namespace and returns.

Multipart identifiers in any mode

For pyiceberg describe sales.orders, the CLI must check both entity types to detect a collision:

  1. Try to load sales.orders as a multipart namespace.
  2. Try to load sales.orders as a table, even when the namespace exists.
  3. Print output only after both checks finish.

There is intentionally no early return after the multipart namespace match. Otherwise, a namespace/table collision would be reported as a namespace instead of an ambiguity.

The final behavior is:

  • Namespace only: print the namespace properties.
  • Table only: print the table description.
  • Neither: keep the existing combined table-or-namespace not-found error.
  • Both: fail with:
Identifier sales.orders matches multiple entity types: namespace, table. Use --entity to disambiguate.

Catalog errors other than namespace/table not-found errors are surfaced unchanged.

BigQuery

BigQuery supports dataset.table identifiers but not multipart namespaces. Its namespace loader now reports a multipart namespace candidate as NoSuchNamespaceError, allowing any mode to continue to the table check. Other namespace loader implementations were audited and already follow this contract or support multipart namespaces.

Tests

  • Full CLI test module: 90 passed
  • BigQuery catalog test module: 6 passed
  • Changed-file lint and type hooks passed

kevinjqliu and others added 2 commits September 11, 2026 10:41
Resolve namespace and table candidates before rendering output so ambiguous identifiers require explicit disambiguation without partial descriptions.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI lite review requested due to automatic review settings September 11, 2026 17:42

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

BigQuery multipart table identifiers can fail auto-detection before table loading.

Pull request overview

Improves pyiceberg describe by resolving multipart namespace/table identifiers, supporting explicit entity selection, and requiring disambiguation for collisions.

Changes:

  • Adds automatic namespace/table detection.
  • Adds explicit --entity handling.
  • Expands CLI tests for detection and ambiguity.
File summaries
File Summary
tests/cli/test_console.py Adds coverage for entity detection and ambiguity behavior.
pyiceberg/cli/console.py Implements entity resolution and disambiguation.
Review details

Suppressed comments (1)

pyiceberg/cli/console.py:179

  • In BigQueryMetastoreCatalog, load_namespace_properties calls identifier_to_database without an error override, so a valid multipart table identifier such as dataset.table raises ValueError for the namespace candidate. Because this path only catches NoSuchNamespaceError, auto-detection exits before trying load_table, making describe dataset.table fail for every BigQuery table unless --entity table is supplied. Treat an invalid namespace shape as a non-match during auto-detection (or use a catalog-neutral existence check).
    except NoSuchNamespaceError:
        pass
  • Files reviewed: 2/2 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Report unsupported multipart namespace identifiers as NoSuchNamespaceError so CLI describe can continue to the table candidate without hiding unexpected catalog errors.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@kevinjqliu
kevinjqliu requested review from Fokko and geruh September 11, 2026 18:07
@kevinjqliu

Copy link
Copy Markdown
Contributor Author

@ebyhr i took some inspiration from your original implementation in #3926. What do you think about this new describe implementation? 😄

@rambleraptor rambleraptor left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The logic here looks great. A couple style ideas, but they're all nits.

Comment thread pyiceberg/cli/console.py
namespace_properties: Properties | None = None
catalog_table: Table | None = None

try:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nit: comment here describing that this is a namespace attempt could be good.

Comment thread pyiceberg/cli/console.py
except NoSuchNamespaceError:
pass

try:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

same as above but with table

Comment thread pyiceberg/cli/console.py
raise exc

if is_namespace is False and is_table is False:
if entity == "namespace":

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
if entity == "namespace":
if entity == "namespace" or (entity == "any" and len(identifier_tuple) == 1):

This will clean up the if statement at line 167, but it's a bit harder to parse. Your call.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

i actually did this intentionally. these ifs are for the individual checks namespace / table / and view (in #3926)

starting L167 is the "any" fallback behavior. i can add a inline comment for that to be more explicit

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Yeah that would be great. I saw that's what you were trying to do, but that little bit of duplication was definitely bugging me 😂

Comment thread pyiceberg/cli/console.py
Co-authored-by: Kevin Liu <kevinjqliu@users.noreply.github.com>
@kevinjqliu
kevinjqliu added this pull request to the merge queue Sep 12, 2026
Merged via the queue into main with commit 562d3af Sep 12, 2026
21 checks passed
@kevinjqliu
kevinjqliu deleted the kevinjqliu-cli-describe-entity-detection branch September 12, 2026 02:23
@kevinjqliu
kevinjqliu restored the kevinjqliu-cli-describe-entity-detection branch September 12, 2026 02:27
@kevinjqliu
kevinjqliu deleted the kevinjqliu-cli-describe-entity-detection branch September 12, 2026 02:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants