Skip to content

Fix client_prefetch_threads=0 normalization - #2843

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

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

Conversation

@abdulselamadillmohammed

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

@abdulselamadillmohammed
abdulselamadillmohammed requested a review from a team as a code owner April 7, 2026 16:43
@codecov-commenter

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.

@sfc-gh-turbaszek
sfc-gh-turbaszek enabled auto-merge (squash) May 14, 2026 13:27
auto-merge was automatically disabled July 14, 2026 17:36

Head branch was pushed to by a user without write access

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

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

Projects

None yet

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

3 participants