Skip to content

fix(s3select): enforce query and resource limits - #5028

Merged
overtrue merged 7 commits into
rustfs:mainfrom
GatewayJ:fix/s3-select-query-limits
Jul 25, 2026
Merged

fix(s3select): enforce query and resource limits#5028
overtrue merged 7 commits into
rustfs:mainfrom
GatewayJ:fix/s3-select-query-limits

Conversation

@GatewayJ

@GatewayJ GatewayJ commented Jul 19, 2026

Copy link
Copy Markdown
Member

Related Issues

N/A

Summary of Changes

  • Enforce the supported S3 Select SQL shape and bounded query resources while preserving standard GROUP BY, ORDER BY, aliases, and COUNT(*) behavior.
  • Preserve timeout delivery under response backpressure by reserving terminal channel capacity, and prevent a deadline race from yielding a batch or EOF after expiration.
  • Keep JSON DOCUMENT blocking work under query admission for queued, running, cancelled, and dropped parses; reject oversized input as S3 Busy.
  • Fail closed on JSON DOCUMENT, raw object, and scan-range short reads, negative object metadata sizes, while preserving bounded range reads past EOF.
  • Preserve CSV string inference, including leading zeros and header modes; numeric ordering remains available through explicit casts.
  • Add focused regression coverage for every corrected review finding.

Verification

  • cargo fmt --all --check
  • cargo test -p rustfs-s3select-api (62 passed)
  • cargo test -p rustfs-s3select-query (108 passed, 2 ignored)
  • cargo test -p rustfs app::select_object::tests:: (21 passed)
  • make pre-commit
  • make pre-pr (10,047 tests passed, 138 skipped; doctests passed)

Impact

S3 Select now reports deterministic Busy responses for query deadlines and resource exhaustion even when delivery is backpressured. Corrupt or truncated JSON/raw streams fail instead of silently returning partial data, while object-store bounded range semantics remain compatible when the requested end exceeds EOF.

CSV values continue to be inferred as strings, preserving leading zeros and existing header behavior. Queries that need numeric ordering must use an explicit cast. No on-disk or wire format changes are introduced.

Additional Notes

  • Correctness: attacked early EOF, negative sizes, exact limit boundaries, range normalization, timeout races, and terminal delivery; no remaining break found.
  • Simplicity: checked existing helpers and ownership boundaries; no smaller equivalent implementation preserves all cancellation and range contracts.
  • Security: attacked untrusted sizes, parser cancellation, hidden partial reads, and nested error mapping; failures remain fail-closed without leaking internal details.
  • Concurrency/durability: attacked queued-versus-started blocking work, permit lifetime, cancellation, lock scope, and full-channel timeout delivery; no remaining break found. No persistence path changes.
  • Compatibility: attacked ObjectStore range behavior, CSV string semantics from fix(s3select): preserve CSV input as strings #5030, and S3 error codes; no remaining break found.
  • Performance: attacked detached blocking work, hot-path synchronization, allocation, and channel overhead; concurrency remains bounded and no hot-path logging was added.
  • Test coverage: each of review findings 1-7 has a revert-sensitive regression test; no uncovered changed behavior remains.

Thank you for your contribution! Please ensure your PR follows the community standards (CODE_OF_CONDUCT.md). If this is your first contribution, review the CLA document and sign it by commenting I have read and agree to the CLA. on the PR.

@github-actions

Copy link
Copy Markdown
Contributor

CLA requirements are satisfied for this pull request.

@GatewayJ
GatewayJ force-pushed the fix/s3-select-query-limits branch from 68d9451 to 14b2b67 Compare July 22, 2026 08:29
@GatewayJ

Copy link
Copy Markdown
Member Author

@codex

@GatewayJ
GatewayJ requested review from houseme and overtrue and removed request for overtrue July 22, 2026 08:48

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 14b2b67346

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread crates/s3select-query/src/dispatcher/manager.rs
@GatewayJ
GatewayJ force-pushed the fix/s3-select-query-limits branch 3 times, most recently from 2f7d8a5 to 1e0eace Compare July 24, 2026 14:17
@GatewayJ
GatewayJ force-pushed the fix/s3-select-query-limits branch from 1e0eace to 545fbdc Compare July 24, 2026 15:34
@GatewayJ
GatewayJ marked this pull request as ready for review July 25, 2026 03:40
@overtrue
overtrue merged commit 0364523 into rustfs:main Jul 25, 2026
23 checks passed
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.

2 participants