[DNR] Old commit#12
Closed
tdcmeehan wants to merge 1 commit into
Closed
Conversation
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
7 tasks
tdcmeehan
added a commit
to prestodb/presto
that referenced
this pull request
Nov 6, 2025
## Description Don't use trunk, because if a vulnerability fix has been merged, this requires PRs to rebase. Instead, try to find a merge base where possible and use that as the reference point to ensure no new vulnerabilities are being introduced by a PR. ## Motivation and Context Recent OWASP job failures ## Impact Less false positives from the OWASP job ## Test Plan Old commit without newer security vulnerability fixes doesn't trigger OWASP failure anymore: tdcmeehan#12 Previous vulnerability detection continues to work: tdcmeehan#13 ## Contributor checklist - [ ] Please make sure your submission complies with our [contributing guide](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md), in particular [code style](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#code-style) and [commit standards](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#commit-standards). - [ ] PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced. - [ ] Documented new properties (with its default value), SQL syntax, functions, or other functionality. - [ ] If release notes are required, they follow the [release notes guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines). - [ ] Adequate tests were added if applicable. - [ ] CI passed. - [ ] If adding new dependencies, verified they have an [OpenSSF Scorecard](https://securityscorecards.dev/#the-checks) score of 5.0 or higher (or obtained explicit TSC approval for lower scores). ## Release Notes ``` == NO RELEASE NOTE == ```
tdcmeehan
pushed a commit
that referenced
this pull request
Feb 8, 2026
Velox already provides three layers of mitigation: CachedBufferedInput with 512KB coalesce distance, FileHandleCache (LRU, avoids re-open), and AsyncDataCache (caches footer + data pages). Footer cache is the key win for RPT backward pass. Forward pass cost varies by storage: local NVMe is fine, S3 is expensive but parallelizable. Suggest tunable maxCoalesceDistance for RPT key-column scans on high-latency storage. https://claude.ai/code/session_01SAXk4AS8yQyQkhRVi6RQie
tdcmeehan
pushed a commit
that referenced
this pull request
Feb 8, 2026
Increasing maxCoalesceDistance to 256MB would read through ~122MB gaps between key column pages across row groups, reading ~100% of the file to get 5% of data — defeating column pruning entirely. The default Velox behavior (512KB coalesce within RG, separate I/O across RGs) is correct. S3 forward pass uses 50 parallel 6MB GETs per file (~75ms wall clock), which is a standard S3 access pattern. Concern is moderate, not blocking. https://claude.ai/code/session_01SAXk4AS8yQyQkhRVi6RQie
tdcmeehan
pushed a commit
that referenced
this pull request
Feb 8, 2026
RPT's key-column reads are not extra I/O — the main query would read those same pages as part of full row group reads. Total I/O is identical (6.4GB with or without RPT). RPT front-loads 300MB to build BFs that prune the main query. Even worst case (no pruning), overhead is ~5% from extra S3 requests. With 90% selectivity, total I/O drops to 940MB (6.8x savings). Issue resolved. https://claude.ai/code/session_01SAXk4AS8yQyQkhRVi6RQie
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Motivation and Context
Impact
Test Plan
Contributor checklist
Release Notes
Please follow release notes guidelines and fill in the release notes below.
If release note is NOT required, use: