Skip to content

Fix unbounded enumeration in postgres list#2920

Merged
vcolin7 merged 5 commits into
mainfrom
vcolin7-postgres-list-row-limits
Jun 23, 2026
Merged

Fix unbounded enumeration in postgres list#2920
vcolin7 merged 5 commits into
mainfrom
vcolin7-postgres-list-row-limits

Conversation

@vcolin7

@vcolin7 vcolin7 commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

postgres list databases and postgres list tables previously returned every row from pg_database / information_schema.tables with no upper bound, so a server with hundreds of databases or thousands of tables would stream everything into memory with no cap and no truncation signal to the caller. This PR closes that gap by mirroring the existing mysql list row-cap pattern.

Approach (parity with MySQL):

  • Added a MaxRowCount = 10_000 constant in PostgresService.
  • ListDatabasesAsync: query now uses ORDER BY datname LIMIT @maxResults. When the result hits the cap, a human-readable sentinel row is appended to the list (identical wording and behavior to MySqlService.ListDatabasesAsync).
  • ListTablesAsync: now returns a new TableListResult(List<string> Tables, bool IsTruncated) sealed record (mirrors MySql/.../TableListResult.cs). Query uses ORDER BY table_name LIMIT @maxResults + 1; truncation is detected via tables.Count > MaxRowCount, then the extra row is trimmed.
  • PostgresListCommand surfaces the flag via bool? TablesTruncated on PostgresListCommandResult (4th positional param, default null), matching MySqlListCommandResult exactly. bool? + JsonIgnoreCondition.WhenWritingNull keeps the field absent from non-truncated responses for backward compatibility.

Notes:

  • The two methods deliberately use different SQL caps (LIMIT cap vs LIMIT cap+1) because they use different truncation signals (sentinel row vs structured flag). Inline comments on both methods explain the differences.
  • I considered using MySQL's read-then-probe pattern for tables (no SQL LIMIT, post-loop Read to detect overflow) but it silently fails to detect truncation once a SQL LIMIT is added: the loop's exit iteration consumes the probe row. Switched to LIMIT cap+1 + Count > cap, which keeps the public surface identical to MySQL.
  • No --max-results CLI option was added. MySQL does not expose one, so Postgres does not either (parity, simpler, backward compatible).

GitHub issue number?

Fixes #472

Pre-merge Checklist

  • Required for All PRs
    • Read contribution guidelines
    • PR title clearly describes the change
    • Commit history is clean with descriptive messages
    • Added comprehensive tests for new/modified functionality
    • Created a changelog entry
  • For MCP tool changes:
    • One tool per PR: only azmcp postgres list is modified
    • Updated servers/Azure.Mcp.Server/README.md and/or servers/Fabric.Mcp.Server/README.md documentation - N/A (tool description unchanged)
    • Validate README.md changes running the script ./eng/scripts/Process-PackageReadMe.ps1 - N/A
    • For new or modified tool descriptions, ran ToolDescriptionEvaluator - N/A (no tool description changes)
    • For tools with new names, including new tools or renamed tools, update consolidated-tools.json - N/A (no new/renamed tools)
    • For renamed tools, follow the Tool Rename Checklist - N/A
    • For new tools associated with Azure services or publicly available tools/APIs/products, add URL to documentation - N/A
  • Extra steps for Azure MCP Server tool changes:
    • Updated command list in servers/Azure.Mcp.Server/docs/azmcp-commands.md
    • Ran ./eng/scripts/Update-AzCommandsMetadata.ps1 to update tool metadata - N/A (no command surface or metadata change; output shape is backward compatible)
    • Updated test prompts in servers/Azure.Mcp.Server/docs/e2eTestPrompts.md - existing prompts still apply
    • 👉 For Community (non-Microsoft team member) PRs - N/A

Invoking Livetests

Copilot submitted PRs are not trustworthy by default. Users with write access to the repo need to validate the contents of this PR before leaving a comment with the text /azp run mcp - pullrequest - live. This will trigger the necessary livetest workflows to complete required validation.

Cap both 'azmcp postgres list' database and table results at 10,000
entries to prevent OOM/perf issues on servers with very large catalogs,
mirroring the existing MySQL behavior:

- Add MaxRowCount = 10_000 constant in PostgresService (parity with
  MySQL's MaxRowCount).
- ListDatabasesAsync: ORDER BY datname LIMIT @MaxResults; append the
  same '... (output limited to 10,000 databases ...)' sentinel row that
  MySQL appends when the cap is reached.
- ListTablesAsync: ORDER BY table_name LIMIT @MaxResults (cap+1) and
  return new TableListResult(Tables, IsTruncated) so the command can
  surface truncation via an explicit flag. Truncation is detected via
  Count > cap rather than the MySQL read-then-probe pattern, which
  silently swallows the truncation signal when combined with a SQL
  LIMIT cap+1.
- IPostgresService updated to return TableListResult from
  ListTablesAsync.
- PostgresListCommandResult gains an optional 'tablesTruncated' bool.
- Add unit tests for the new cap/truncation behavior and update
  existing PostgresListCommand tests for the new return type.
- Document the cap in azmcp-commands.md and add a Bugs Fixed
  changelog entry.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@vcolin7 vcolin7 requested a review from kk-src as a code owner June 22, 2026 05:48
Copilot AI review requested due to automatic review settings June 22, 2026 05:48
@vcolin7 vcolin7 requested review from a team and xiangyan99 as code owners June 22, 2026 05:48
@github-actions github-actions Bot added the tools-Postgres PostgreSQL label Jun 22, 2026
@vcolin7 vcolin7 changed the title Fix unbounded enumeration in azmcp postgres list (#472) Fix unbounded enumeration in azmcp postgres list Jun 22, 2026
@vcolin7 vcolin7 changed the title Fix unbounded enumeration in azmcp postgres list Fix unbounded enumeration in postgres list Jun 22, 2026

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.

Pull request overview

This PR addresses unbounded enumeration in azmcp postgres list by introducing a hard cap (10,000) for database/table listing to avoid streaming extremely large result sets into memory, aligning behavior with the existing MySQL tooling patterns.

Changes:

  • Add a 10,000 row cap for PostgreSQL database/table listing, with truncation signaling (sentinel row for databases; tablesTruncated flag for tables).
  • Introduce TableListResult to return both table names and a truncation indicator from the service layer.
  • Add/extend tests plus doc/changelog updates to reflect the new bounded behavior.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
tools/Azure.Mcp.Tools.Postgres/src/Services/PostgresService.cs Adds MaxRowCount and applies SQL LIMIT + truncation signaling for database/table listing.
tools/Azure.Mcp.Tools.Postgres/src/Services/IPostgresService.cs Updates ListTablesAsync contract to return TableListResult.
tools/Azure.Mcp.Tools.Postgres/src/Services/TableListResult.cs New result type for tables list + truncation flag.
tools/Azure.Mcp.Tools.Postgres/src/Commands/PostgresListCommand.cs Surfaces table truncation via TablesTruncated in the command result.
tools/Azure.Mcp.Tools.Postgres/tests/Azure.Mcp.Tools.Postgres.Tests/PostgresListCommandTests.cs Updates tests for new service return type and validates TablesTruncated behavior.
tools/Azure.Mcp.Tools.Postgres/tests/Azure.Mcp.Tools.Postgres.Tests/Services/PostgresServiceRowLimitTests.cs New tests covering under-cap/at-cap/over-cap behaviors.
servers/Azure.Mcp.Server/docs/azmcp-commands.md Documents the new caps and truncation signaling for azmcp postgres list.
servers/Azure.Mcp.Server/changelog-entries/1782105637872.yaml Adds a changelog entry for the bug fix and output-shape details.

Comment thread servers/Azure.Mcp.Server/docs/azmcp-commands.md Outdated
Comment thread servers/Azure.Mcp.Server/changelog-entries/1782105637872.yaml Outdated
Comment thread tools/Azure.Mcp.Tools.Postgres/src/Services/PostgresService.cs Outdated
Comment thread tools/Azure.Mcp.Tools.Postgres/src/Services/PostgresService.cs
vcolin7 and others added 4 commits June 22, 2026 16:30
Address alzimmermsft's PR review:
- ListDatabasesAsync now returns DatabaseListResult with an IsTruncated
  flag instead of appending a sentinel row, and uses LIMIT cap+1 so
  truncation is actually detectable.
- Surface DatabasesTruncated on PostgresListCommandResult.
- Make PostgresService.MaxRowCount internal so tests reference it
  instead of duplicating the constant.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
A postgres list call returns servers, databases, or tables - never more
than one - so a single ResultsTruncated flag is sufficient instead of
separate DatabasesTruncated/TablesTruncated fields.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Reconcile #472 row-limit/truncation work with main's Postgres refactor:
- Adopt main's removal of subscriptionId/resourceGroup from service methods
- Adopt main's parameterized @Schema filter on ListTablesAsync
- Keep DatabaseListResult/TableListResult return types and cap+1 truncation
- Combine @Schema filter with LIMIT @MaxResults in the tables query
- Update tests for new signatures (drop sub/rg, add schema) and the
  parameterized-schema test to expect the added maxResults parameter

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-project-automation github-project-automation Bot moved this from Untriaged to In Progress in Azure MCP Server Jun 23, 2026
@vcolin7 vcolin7 merged commit 6410829 into main Jun 23, 2026
15 checks passed
@github-project-automation github-project-automation Bot moved this from In Progress to Done in Azure MCP Server Jun 23, 2026
@vcolin7 vcolin7 deleted the vcolin7-postgres-list-row-limits branch June 23, 2026 18:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

tools-Postgres PostgreSQL

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants