Skip to content

Navigation Menu

Sign in
Sign up

Fix client_prefetch_threads=0 normalization - #2843

Open
abdulselamadillmohammed wants to merge 10 commits into
snowflakedb:main from
abdulselamadillmohammed:fix/snow-3324331-client-prefetch-threads-validation
Open

Fix client_prefetch_threads=0 normalization #2843
abdulselamadillmohammed wants to merge 10 commits into
snowflakedb:main from
abdulselamadillmohammed:fix/snow-3324331-client-prefetch-threads-validation

Conversation

@abdulselamadillmohammed

@abdulselamadillmohammed abdulselamadillmohammed commented Apr 7, 2026

Copy link
Copy Markdown
Contributor

Please answer these questions before submitting your pull requests. Thanks!

  1. What GitHub issue is this PR addressing? Make sure that there is an accompanying issue to your PR.

    Fixes SNOW-3324331: client_prefetch_threads validation can bypass the lower bound due to ordering of casting and bounds checks #2841

  2. Fill out the following pre-review checklist:

    • I am adding a new automated test(s) to verify correctness of my new code
    • I am adding new logging messages
    • I am adding a new telemetry message
    • I am modifying authorization mechanisms
    • I am adding new credentials
    • I am modifying OCSP code
    • I am adding a new dependency
  3. Please describe how your code solves the related issue.

This change makes the existing defensive validation path for client_prefetch_threads internally consistent when a non-positive value reaches it.

In the sync path, _validate_client_prefetch_threads() previously read through the defaulting property, which meant a raw value of 0 could resolve to DEFAULT_CLIENT_PREFETCH_THREADS instead of following the existing lower-bound correction. This PR updates the validator to normalize from the raw backing value, preserve None as the default case, and then apply the existing bounds logic before storing the final value.

I also included corresponding async updates so sync and async behavior stay aligned. In particular, async now validates direct assignment through the setter and continues to validate explicitly provided client_prefetch_threads values during connection setup.

The PR adds targeted sync and async unit tests covering:

  • connection construction with client_prefetch_threads=0
  • direct assignment of client_prefetch_threads=0
  1. (Optional) PR for stored-proc connector:
    N/A

Copy link
Copy Markdown

⚠️ JUnit XML file not found

The CLI was unable to find any JUnit XML files to upload.
For more help, visit our troubleshooting guide.

auto-merge was automatically disabled July 14, 2026 17:36

Head branch was pushed to by a user without write access

abdulselamadillmohammed force-pushed the fix/snow-3324331-client-prefetch-threads-validation branch from 890d761 to d2df81a Compare July 14, 2026 17:40

Copy link
Copy Markdown
Contributor Author

@sfc-gh-turbaszek I merged current upstream/main and resolved the DESCRIPTION.md conflict. The affected sync and async unit suites pass locally (116 passed, 1 skipped), along with pre-commit checks. GitHub now shows five workflows awaiting maintainer approval. Could you please approve the workflows and re-enable auto-merge when appropriate? Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sfc-gh-turbaszek sfc-gh-turbaszek sfc-gh-turbaszek approved these changes

Assignees

No one assigned

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

SNOW-3324331: client_prefetch_threads validation can bypass the lower bound due to ordering of casting and bounds checks

AltStyle によって変換されたページ (->オリジナル) /