Skip to content

[DNR] Old commit#12

Closed
tdcmeehan wants to merge 1 commit into
masterfrom
owasp-test-old-branch
Closed

[DNR] Old commit#12
tdcmeehan wants to merge 1 commit into
masterfrom
owasp-test-old-branch

Conversation

@tdcmeehan

Copy link
Copy Markdown
Owner

Description

Motivation and Context

Impact

Test Plan

Contributor checklist

  • Please make sure your submission complies with our contributing guide, in particular code style and 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.
  • Adequate tests were added if applicable.
  • CI passed.
  • If adding new dependencies, verified they have an OpenSSF Scorecard score of 5.0 or higher (or obtained explicit TSC approval for lower scores).

Release Notes

Please follow release notes guidelines and fill in the release notes below.

== RELEASE NOTES ==

General Changes
* ... 
* ... 

Hive Connector Changes
* ... 
* ... 

If release note is NOT required, use:

== NO RELEASE NOTE ==

@github-actions

github-actions Bot commented Nov 4, 2025

Copy link
Copy Markdown

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

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 tdcmeehan closed this Nov 6, 2025
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
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.

1 participant