Skip to content

fix(native): Hoist plugin loading and de-inline factory map for dynamic connector support - #156

Open
20001020ycx wants to merge 203 commits into
masterfrom
yscope/refactor/deinline-connector-registration
Open

fix(native): Hoist plugin loading and de-inline factory map for dynamic connector support#156
20001020ycx wants to merge 203 commits into
masterfrom
yscope/refactor/deinline-connector-registration

Conversation

@20001020ycx

@20001020ycx 20001020ycx commented Apr 22, 2026

Copy link
Copy Markdown

Description

  • Hoist registerDynamicFunctions() before registerVeloxConnectors() in PrestoServer.cpp
  • De-inline connector factory registration functions from Registration.h into Registration.cpp

Motivation and Context

This PR enables dynamic connector plugins — connectors loaded at runtime via plugin-dir using dlopen() — as described in RFC-0019: Connector Plugins. Two changes are needed to make this work correctly.

Both changes revolve around a singleton factory map defined in Registration.cpp:

static std::unordered_map<std::string, std::shared_ptr<ConnectorFactory>> factories;

This map lives inside detail::connectorFactories() and maps connector names (e.g. "hive", "tpch") to their ConnectorFactory instances. Both registerConnectorFactory(name, factory) and getConnectorFactory(name) read and write this map.

Why registerDynamicFunctions() must be hoisted before registerVeloxConnectors()

registerVeloxConnectors() iterates over every *.properties file in the catalog/ directory. For each one, it reads connector.name and calls getConnectorFactory(connectorName) to look up the factory in the singleton map, then calls factory->newConnector(...) to instantiate the connector.

registerDynamicFunctions() loads plugin .so files from the plugin/ directory via dlopen(). Each plugin's registerExtensions() entry point calls registerConnectorFactory(...) to insert its factory into the same singleton map.

Before this PR, registerVeloxConnectors() ran before registerDynamicFunctions(). Since plugins had not been loaded yet, the factory map had no entry for dynamically registered connector names. When registerVeloxConnectors() processed a catalog .properties file referencing a dynamic connector, getConnectorFactory() threw because the corresponding factory had not been registered yet.

So the sequence must be:

  1. registerDynamicFunctions() — loads plugin .so files, plugins call registerConnectorFactory() to insert into the map
  2. registerVeloxConnectors() — reads catalog/*.properties, calls getConnectorFactory() to look up entries in the map

The original code had no problem with built-in connectors (Hive, TPC-H, etc.) because their factories are registered statically at compile time.

Why registration functions must be de-inlined

Before this PR, the factory map was a static local variable inside the inline function connectorFactories() in Registration.h. Any connector shared library that #includes this header gets its own copy of the static variable, meaning both presto_server and the plugin library hold separate factory maps (Note, shared library references the header file in the presto-native-execution for compilation, the implementation are resolved at run time):

presto_server           → factory map at 0xAAAA  {"hive": ..., "tpch": ...}
libpresto_plugin.so     → factory map at 0xBBBB  {"my_connector": ...}

This breaks the singleton property of the factory map: the plugin's registerConnectorFactory() inserts into its own map (0xBBBB), but the server's getConnectorFactory() reads from its own map (0xAAAA) — and finds nothing.

De-inlining connectorFactories() ensures the static local variable exists in exactly one place (Registration.cpp, linked into presto_server), allowing dynamically registered connector known to presto_server successfully.

Test Plan

  • Build presto_server with the changes
  • End-to-end tested with a customized native connector shared library (.so) loaded via plugin-dir, verifying successful connector registration and query execution.

Release Notes

== NO RELEASE NOTE ==

zhichenxu-meta and others added 11 commits April 20, 2026 12:14
…on_parallelism session property (prestodb#27603)

Summary:
When RPCNode is in the output stage of a query (e.g., SELECT
fb_llm_inference(...)), force_single_node_output places it in a SINGLE
fragment with 1 task. For queries where RPCNode is not in the output
stage (e.g., inside INSERT or before a JOIN/aggregation), RPCNode may
already run distributed.

This diff adds a session property to explicitly control RPCNode
parallelism using a ROUND_ROBIN exchange, following the same pattern as
remote_function_fixed_parallelism_task_count for Python/Thrift UDFs.

Changes:
- AddExchanges.java: visitRPC() inserts a ROUND_ROBIN exchange below
RPCNode when rpc_function_parallelism > 1
- SystemSessionProperties.java: Add rpc_function_parallelism integer
session property (default 0 = default planning, task count determined by
query structure)

Usage:
  SET SESSION rpc_function_parallelism = 4;
  SELECT fb_llm_inference(...) FROM big_table;

Plan with rpc_function_parallelism=4:
  Fragment 0 [SINGLE]: Output -> GATHER
Fragment 1 [ROUND_ROBIN 4 tasks]: Project -> RPCNode -> ROUND_ROBIN
exchange
  Fragment 2 [SOURCE]: TableScan (distributed reads)

Default (0): No exchange inserted — RPCNode uses default planning. For
output-stage queries this means single task; for other query shapes it
may be distributed.

Differential Revision: D101288404
…prestodb#27614) (prestodb#27614)

Summary:

Upgrades the bundled Apache Iceberg version in presto-trunk from 1.10.0
to 1.10.1.
This is a prerequisite for the Iceberg V3 Java support feature stack
which requires
APIs introduced in 1.10.1 such as PUFFIN deletion vector helpers and the
V3 schema
evolution methods.

Also extracts inline `Request` arguments at 12 sites in
`presto-main/src/test/java/com/facebook/presto/server/TestServer.java`
into local
variables to work around a javac target-type inference ambiguity that
surfaces
after the dependency bump (transitive deps shift causes javac to fail
unifying
`T = QueryResults` vs `T = RuntimeException` when the `Request` is built
inline
inside `client.execute(...)`). This matches the pattern already used
elsewhere in
the same file (e.g. lines 179-182).

Without this fix, `presto-main` test-compile fails on JDK 17 with:
  TestServer.java:[236,42] incompatible types: inference variable T has
  incompatible equality constraints java.lang.RuntimeException,
  com.facebook.presto.client.QueryResults,T

== NO RELEASE NOTES ==

Part of the Iceberg V3 Java support split (was D98749429).

Reviewed By: zzhao0

Differential Revision: D101602649
## Description
Upgrade gcs version to 2.2.28

Upgrade google-oauth-client version to **1.34.1** to address
CVE-2020-7692 and CVE-2021-22573


## Motivation and Context
<!---Why is this change required? What problem does it solve?-->
<!---If it fixes an open issue, please link to the issue here.-->

## Impact
<!---Describe any public API or user-facing feature change or any
performance impact-->

## Test Plan

```
presto> show schemas from gcs;
       Schema       
--------------------
 information_schema 
 ischema            
 ischema1           
 schema01           
 schema02           
 tpch_sf1           
(6 rows)

Query 20260114_063500_00007_6ja2r, FINISHED, 1 node
Splits: 19 total, 19 done (100.00%)
[Latency: client-side: 0:01, server-side: 0:01] [6 rows, 87B] [10 rows/s, 155B/s]

presto> CREATE TABLE gcs.schema01.table1th (id int, name varchar) WITH (external_location = 'gs://gcs_test_nv/schema01/table1th/');
CREATE TABLE

Query 20260114_063735_00009_6ja2r, FINISHED, 0 nodes
Splits: 0 total, 0 done (0.00%)
[Latency: client-side: 0:04, server-side: 0:03] [0 rows, 0B] [0 rows/s, 0B/s]

presto> INSERT INTO gcs.schema01.table1th VALUES (1, 'AVS'), (2, 'VP'), (3, 'PN');
INSERT: 3 rows

Query 20260114_063811_00010_6ja2r, FINISHED, 1 node
Splits: 19 total, 19 done (100.00%)
[Latency: client-side: 0:14, server-side: 0:14] [0 rows, 0B] [0 rows/s, 0B/s]

presto> select * from gcs.schema01.table1th;
 id | name 
----+------
  1 | AVS  
  2 | VP   
  3 | PN   
(3 rows)

Query 20260114_063859_00011_6ja2r, FINISHED, 1 node
Splits: 23 total, 23 done (100.00%)
[Latency: client-side: 0:02, server-side: 0:02] [3 rows, 4.73KB] [10 rows/s, 2.47KB/s]
```


## 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.

## Release Notes
Please follow [release notes
guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines)
and fill in the release notes below.

```
== RELEASE NOTES ==

Security Changes
* Upgrade google-oauth-client version to 1.34.1 to address `CVE-2020-7692 <https://github.com/advisories/GHSA-f263-c949-w85g>`_ and `CVE-2021-22573 <https://github.com/advisories/GHSA-hw42-3568-wj87>`_.
```
…2.25.4 (prestodb#27583)

## Description
<!---Describe your changes in detail-->
Bumps org.apache.logging.log4j:log4j-core from 2.25.3 to 2.25.4.

Additionally, added log4j-slf4j2-impl as a runtime dependency in the
Druid module to ensure consistent Log4j versions across Presto.


Reason:

The transitive dependency from druid-processing was pulling in
log4j-slf4j2-impl:2.22.1, which resulted in a version mismatch with the
updated Log4j APIs (2.25.4).

Dependency tree before:
<img width="792" height="413" alt="Screenshot 2026-04-14 at 12 19 13 PM"
src="https://github.com/user-attachments/assets/8604ccd4-64b8-4f2b-afa6-2462642c59df"
/>

Dependency tree after:

<img width="792" height="413" alt="Screenshot 2026-04-14 at 12 18 30 PM"
src="https://github.com/user-attachments/assets/b413f10a-e7e1-4899-b5fb-afbd41a13bd9"
/>


## Motivation and Context
<!---Why is this change required? What problem does it solve?-->
<!---If it fixes an open issue, please link to the issue here.-->

## Impact
<!---Describe any public API or user-facing feature change or any
performance impact-->

## Test Plan
<!---Please fill in how you tested your change-->
Druid connector is tested locally and the test result is attatched
below:
<img width="1496" height="575" alt="Screenshot 2026-04-14 at 2 51 59 PM"
src="https://github.com/user-attachments/assets/89623dfe-5c64-4923-a541-8a4ebca2f70f"
/>


## 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
Please follow [release notes
guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines)
and fill in the release notes below.

```
== RELEASE NOTES ==

Security Changes
* Upgrade org.apache.logging.log4j:log4j-core from 2.25.3 to 2.25.4 inorder to address `CVE-2026-34480 <https://nvd.nist.gov/vuln/detail/CVE-2026-34480>`_. 

```
…MV optimizer (prestodb#27538) (prestodb#27538)

Summary:

The MV query rewriter only overrode visitSimpleGroupBy, so column
references inside CUBE, ROLLUP, and GROUPING SETS were never rewritten
from base table columns to MV columns. Fix by adding visitCube,
visitRollup, and visitGroupingSets overrides, and extending
removeGroupingElementPrefix to handle all GroupingElement types.

Differential Revision: D99539143

```
== RELEASE NOTES ==

General Changes
* Fix materialized view query rewriting for ``CUBE``, ``ROLLUP``, and
  ``GROUPING SETS`` clauses. Column references inside these grouping
  elements are now correctly rewritten to materialized view columns.
```
…todb#27595)

## Description
Add operational JMX metrics to the sidecar plugin.

## Motivation and Context
Operational metrics are important to understand how the system is
behaving and to troubleshoot issues.

## Impact
JMX metrics exposed. 

## Test Plan
CI


```
== NO RELEASE NOTE ==
```
…st (prestodb#27519)

## Description

Currently, as several major Iceberg test classes
(TestIcebergDistributedQueries, IcebergDistributedSmokeTestBase,
IcebergDistributedTestBase) continue to grow, the number of history
queries tracked within the same `QueryRunner` for each test class has
been increasing. This leads to growing memory usage that often causes
OOM failures in CI testing. For example, after running all tests in
`TestIcebergDistributedQueries`, the total number of history queries
tracked in the queryRunner reaches 2900+, occupying approximately 600MB
of additional memory.

This PR explicitly sets `query.max-history` and `query.max-age` for the
`QueryRunner` built in these test classes to significantly reduce this
memory overhead. For example, after this change, after running all tests
in `TestIcebergDistributedQueries`, the total number of history queries
tracked in the queryRunner is around 200.

## Motivation and Context

Reduce OOM during Iceberg CI tests.

## Impact

N/A

## Test Plan

N/A

## 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 ==
```

## Summary by Sourcery

Tests:
- Configure Iceberg distributed test query runners with low
query.max-age and query.max-history values to cap in-memory query
history during tests.
…n /presto-tests (prestodb#27606)

Bumps
[org.bouncycastle:bcprov-jdk18on](https://github.com/bcgit/bc-java) from
1.81 to 1.84.
<details>
<summary>Changelog</summary>
<p><em>Sourced from <a
href="https://github.com/bcgit/bc-java/blob/main/docs/releasenotes.html">org.bouncycastle:bcprov-jdk18on's
changelog</a>.</em></p>
<blockquote>
<!-- raw HTML omitted -->
<!-- raw HTML omitted -->
<!-- raw HTML omitted -->
</blockquote>
<p>... (truncated)</p>
</details>
<details>
<summary>Commits</summary>
<ul>
<li>See full diff in <a
href="https://github.com/bcgit/bc-java/commits">compare view</a></li>
</ul>
</details>
<br />


[![Dependabot compatibility
score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=org.bouncycastle:bcprov-jdk18on&package-manager=maven&previous-version=1.81&new-version=1.84)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)

Dependabot will resolve any conflicts with this PR as long as you don't
alter it yourself. You can also trigger a rebase manually by commenting
`@dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits
that have been made to it
- `@dependabot show <dependency name> ignore conditions` will show all
of the ignore conditions of the specified dependency
- `@dependabot ignore this major version` will close this PR and stop
Dependabot creating any more for this major version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop
Dependabot creating any more for this minor version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop
Dependabot creating any more for this dependency (unless you reopen the
PR or upgrade to it yourself)
You can disable automated security fix PRs for this repo from the
[Security Alerts
page](https://github.com/prestodb/presto/network/alerts).

</details>

```
== RELEASE NOTES ==

Security Changes
* Upgrade org.bouncycastle:bcprov-jdk18on from 1.81 to 1.84 to resolve `CVE-2026-0636 <https://nvd.nist.gov/vuln/detail/CVE-2026-0636>`_. 

```

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…prestodb#27632)

Summary:

Some queries have really large numbers reported per operator and these
overflow during summation, which leads to the absence of plan in the
query
logging.
To fix that we add functionality to cap the mentrics to still be able to
see
more or less reasonable numbers.

```
== NO RELEASE NOTE ==
```

Differential Revision: D101851654
)

## Description
Add SQL filter pushdown for the Lance connector. Converts Presto's
`TupleDomain` predicates to Lance SQL WHERE clause strings and pushes
them down to the Lance scanner via `ScanOptions.Builder.filter()`. Lance
evaluates filters natively during scan using its DataFusion-based SQL
parser, reducing data read from disk.

### Key components
- **`LanceSqlFilterBuilder`**: Converts `TupleDomain<ColumnHandle>` to
SQL string. Supports Boolean, Integer, Bigint, Real, Double, Varchar,
Date, Timestamp types with equality, IN, range, and IS NULL predicates.
Column names are backtick-quoted for safety.
- **Filter projection columns**: Columns needed for filter evaluation
but not in query output are added to the Lance scan projection, ensuring
Lance can evaluate the filter without scanning all columns.
- **Safety**: The filter is returned as unenforced (following Iceberg's
pattern), so Presto re-evaluates at executor level as a correctness
guard. Unsupported types and complex filters (>100 ranges) are
gracefully skipped — Presto handles them.

### Supported pushed-down predicates

| Predicate | Example |
|---|---|
| Equality | `col = 42` |
| Comparisons | `col > 30`, `col <= 100` |
| IN lists | `col IN (1, 2, 3)` |
| NULL checks | `col IS NULL` |
| Range | `col >= 10 AND col < 20` |
| Multi-column | predicates combined with `AND` |

### Supported types in filter literals

| Presto Type | Lance Filter Literal |
|---|---|
| `BOOLEAN` | `true` / `false` |
| `TINYINT/SMALLINT/INTEGER/BIGINT` | Integer literal |
| `REAL` | Float literal |
| `DOUBLE` | Double literal |
| `VARCHAR` | `'string'` (single quotes escaped) |
| `DATE` | `date '2024-01-15'` |
| `TIMESTAMP` | `timestamp '2024-01-15 10:30:00.000000'` |

No new dependencies — uses Lance's built-in SQL filter parser
(DataFusion-based) via `ScanOptions.Builder.filter(String)`.

## Motivation and Context
Without filter pushdown, Lance reads all rows from disk and Presto
filters them at the executor level. With filter pushdown, Lance
evaluates predicates natively during scan, significantly reducing I/O
for selective queries.

## Impact
New class and minor modifications in `presto-lance` connector only. No
changes to existing Presto code. Documentation updated in `lance.rst`.

## Test Plan
- All 49 presto-lance unit tests pass: `./mvnw test -pl presto-lance`
- `TestLanceSqlFilterBuilder` — 17 new test cases covering:
  - Equality, IN lists, exclusive/inclusive ranges
  - Varchar with single-quote escaping
  - Date and timestamp literals
  - IS NULL, boolean, double
  - Multiple columns with AND
  - All-domain skip, multi-range OR disjunction
  - Nullable with values (OR IS NULL)
- Existing tests updated for new constructor signature

## Contributor checklist

- [x] 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).
- [x] PR description addresses the issue accurately and concisely. If
the change is non-trivial, a GitHub Issue is referenced.
- [x] Documented new properties (with its default value), SQL syntax,
functions, or other functionality.
- [x] If release notes are required, they follow the [release notes
guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines).
- [x] Adequate tests were added if applicable.
- [ ] CI passed.
- [x] 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

```
== RELEASE NOTES ==

Lance Connector Changes
* Add SQL filter pushdown to reduce data read from disk for selective queries. Supports equality, comparisons, IN lists, IS NULL, and range predicates on Boolean, Integer, Bigint, Real, Double, Varchar, Date, and Timestamp types.
```

---------

Co-authored-by: Steve Burnett <burnett@pobox.com>
Co-authored-by: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…restodb#26959)

## Description
Implements incremental refresh for materialized views. Instead of full
recomputation, only stale partitions are refreshed using the IVM delta
algebra established in DifferentialPlanRewriter. Iceberg integration is
added in this PR.

Depends on prestodb#26728

## Motivation and Context
Full MV refresh is expensive when only a subset of partitions changed.
This solution only requires recomputing the updated partitions.

## Impact
REFRESH MATERIALIZED VIEW automatically uses incremental refresh when
possible, falls back to full refresh otherwise.

## Test Plan
Extensive unit tests have been added.

## 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
Please follow [release notes
guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines)
and fill in the release notes below.

```
== RELEASE NOTES ==

General Changes
* Add incremental refresh for materialized views

Iceberg Connector Changes
* Add incremental refresh for materialized views in the Iceberg connector

```
@coderabbitai

coderabbitai Bot commented Apr 22, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 1f689579-b460-43a1-8417-6085711b0972

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch yscope/refactor/deinline-connector-registration

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@20001020ycx
20001020ycx force-pushed the yscope/refactor/deinline-connector-registration branch from d8e4993 to f7d3730 Compare April 22, 2026 20:41
@20001020ycx
20001020ycx changed the base branch from release-0.293-clp-connector to master April 22, 2026 20:47
@20001020ycx
20001020ycx force-pushed the yscope/refactor/deinline-connector-registration branch 3 times, most recently from 62db030 to abf1515 Compare April 23, 2026 15:11
ShahimSharafudeen and others added 2 commits April 23, 2026 21:43
…-40490 (prestodb#27613)

## Description
Upgrade async-http-client to 3.0.9 to address CVE-2026-40490

Dependency tree before fix :

```
[INFO] com.facebook.presto:presto-druid:presto-plugin:0.298-SNAPSHOT
[INFO] +- org.apache.druid:druid-processing:jar:35.0.1:compile
[INFO] |  +- org.apache.commons:commons-compress:jar:1.27.1:compile
[INFO] |  +- commons-codec:commons-codec:jar:1.17.2:compile
[INFO] |  +- org.apache.commons:commons-math3:jar:3.6.1:compile
[INFO] |  +- org.apache.commons:commons-text:jar:1.15.0:compile
[INFO] |  +- net.java.dev.jna:jna:jar:5.18.1:compile
[INFO] |  +- org.asynchttpclient:async-http-client:jar:3.0.2:compile
[INFO] |  |  +- com.sun.activation:jakarta.activation:jar:2.0.1:compile
[INFO] |  |  \- org.jetbrains:annotations:jar:26.0.2:compile
[INFO] |  +- org.hyperic:sigar:jar:1.6.5.132:compile
[INFO] |  \- com.github.oshi:oshi-core:jar:6.4.4:compile
[INFO] |     \- net.java.dev.jna:jna-platform:jar:5.13.0:compile
[INFO] +- at.yawk.lz4:lz4-java:jar:1.10.2:runtime
[INFO] +- com.facebook.airlift:bootstrap:jar:0.227:compile
```

Dependency tree after fix :

```
[INFO] com.facebook.presto:presto-druid:presto-plugin:0.298-SNAPSHOT
[INFO] +- org.apache.druid:druid-processing:jar:35.0.1:compile
[INFO] |  +- org.apache.commons:commons-compress:jar:1.27.1:compile
[INFO] |  +- commons-codec:commons-codec:jar:1.17.2:compile
[INFO] |  +- org.apache.commons:commons-math3:jar:3.6.1:compile
[INFO] |  +- org.apache.commons:commons-text:jar:1.15.0:compile
[INFO] |  +- net.java.dev.jna:jna:jar:5.18.1:compile
[INFO] |  +- org.asynchttpclient:async-http-client:jar:3.0.9:compile
[INFO] |  |  +- com.sun.activation:jakarta.activation:jar:2.0.1:compile
[INFO] |  |  \- org.jetbrains:annotations:jar:26.0.2:compile
[INFO] |  +- org.hyperic:sigar:jar:1.6.5.132:compile
[INFO] |  \- com.github.oshi:oshi-core:jar:6.4.4:compile
[INFO] |     \- net.java.dev.jna:jna-platform:jar:5.13.0:compile
[INFO] +- at.yawk.lz4:lz4-java:jar:1.10.2:runtime
[INFO] +- com.facebook.airlift:bootstrap:jar:0.227:compile
```

## Motivation and Context
<!---Why is this change required? What problem does it solve?-->
<!---If it fixes an open issue, please link to the issue here.-->

## Impact
<!---Describe any public API or user-facing feature change or any
performance impact-->

## Test Plan
Tested in local :

<img width="1692" height="532" alt="image"
src="https://github.com/user-attachments/assets/2c6c0715-4d89-442a-b6ae-5dec51dd5fbc"
/>


## 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
Please follow [release notes
guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines)
and fill in the release notes below.

```
== RELEASE NOTES ==

Security Changes
* Upgrade async-http-client to version 3.0.9 to address `CVE-2026-40490 <https://github.com/advisories/GHSA-cmxv-58fp-fm3g>`_.
```
@20001020ycx 20001020ycx changed the title refactor(native): De-inline connector factory registration for dynamic plugin support fix(native): Hoist plugin loading and de-inline factory map for dynamic connector support Apr 23, 2026
@20001020ycx
20001020ycx force-pushed the yscope/refactor/deinline-connector-registration branch from abf1515 to 8e60013 Compare April 23, 2026 16:20
bibith4 and others added 9 commits April 23, 2026 09:37
…b#27625)

## Description
Upgrade testing-library/react version to 16.3.2

## Motivation and Context
Using a more recent version helps avoid potential vulnerabilities and
ensures we aren't relying on outdated or unsupported code.

## Impact
<!---Describe any public API or user-facing feature change or any
performance impact-->

## Test Plan
<!---Please fill in how you tested your change-->

## 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 ==
```

## Summary by Sourcery

Build:
- Bump @testing-library/react dependency in presto-ui package.json from
^16.1.0 to ^16.3.2 and refresh yarn.lock accordingly.
…restodb#27480) (prestodb#27480)

Summary:

Converts the split_part_reverse implementation from a C++ Velox UDF to a
Presto SQL-invoked scalar function (`SqlInvokedScalarFunction`),
following the pattern in `ArraySqlFunctions.java` per reviewer feedback.

The function body is a single SQL expression:
```sql
RETURN ELEMENT_AT(SPLIT(str, delimiter), idx)
```

This leverages Presto's native `element_at()` negative index support on
arrays:
- Positive indices count from start (1-based), matching split_part
- Negative indices count from end (-1 = last, -2 = second-to-last)
- Returns NULL if |index| exceeds the number of parts
- Index 0 throws an error (Presto native behavior)

**Why SQL inline over C++ UDF:**
- No Velox C++ compilation required
- Globally available in all Presto queries (DaiQuery, pipelines, Bento)
- One-liner SQL expression, trivially maintainable
- Follows established pattern (ArraySqlFunctions, StringSqlFunctions)
- Already registered via `SqlInvokedFunctionsPlugin`
(StringSqlFunctions.class)

**Usage:**
```sql
SELECT split_part_reverse('foo/bar/baz/qux', '/', -1);  -- 'qux'
SELECT split_part_reverse('foo/bar/baz/qux', '/', -2);  -- 'baz'
SELECT split_part_reverse('foo/bar/baz/qux', '/', 1);   -- 'foo'
```

This addresses T248997604 and incorporates feedback from the presto.dev
Workplace thread.

Differential Revision: D89498172
…odb#27624)

## Description
Upgrade testing-library/jest-dom version to 6.9.1

## Motivation and Context
Using a more recent version helps avoid potential vulnerabilities and
ensures we aren't relying on outdated or unsupported code.

## Impact
<!---Describe any public API or user-facing feature change or any
performance impact-->

## Test Plan
<!---Please fill in how you tested your change-->

## 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
Please follow [release notes
guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines)
and fill in the release notes below.

```
== NO RELEASE NOTE ==
```

## Summary by Sourcery

Build:
- Update presto-ui package.json and lockfile to use
@testing-library/jest-dom version 6.9.1.
…stodb#27647)

## Description
Upgrade react-data-table-component version to 7.7.1

## Motivation and Context
Using a more recent version helps avoid potential vulnerabilities and
ensures we aren't relying on outdated or unsupported code.

## Impact
<!---Describe any public API or user-facing feature change or any
performance impact-->

## Test Plan
<!---Please fill in how you tested your change-->

## 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
Please follow [release notes
guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines)
and fill in the release notes below.



```
== NO RELEASE NOTE ==
```

## Summary by Sourcery

Build:
- Bump react-data-table-component in presto-ui package.json from 7.6.2
to 7.7.1 and refresh lockfile accordingly.
…ctor (prestodb#27611)

## Description
This PR resolves issue prestodb#17683 by documenting 5 missing configuration
properties for the Druid connector:

**druid.authentication.type
druid.basic.authentication.username
druid.basic.authentication.password
druid.hadoop.config.resources
druid.ingestion.storage.path**

## Motivation and Context
Added Missing documentation

## Impact
No Performance Impact

## 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 ==
```

## Summary by Sourcery

Document additional authentication and advanced configuration options
for the Druid connector.

Documentation:
- Add documentation for Druid connector authentication properties,
including authentication type and basic auth credentials.
- Describe advanced Druid connector settings such as Hadoop
configuration resources and ingestion storage path.
…n-input-files (prestodb#27633)

## Description
<!---Describe your changes in detail-->
Added rewrite-all option and default value for min-input-files of 5 from
iceberg spec.

Default values for `min-file-size-bytes` and `max-file-size-bytes`
cannot be added because `target-file-size-bytes` is currently not
supported in the iceberg writer.

Modified all test cases that did not use min-input-files or file/group
filters to use rewrite-all option to maintain the old operation.

## Motivation and Context
<!---Why is this change required? What problem does it solve?-->
<!---If it fixes an open issue, please link to the issue here.-->

## Impact
<!---Describe any public API or user-facing feature change or any
performance impact-->

## Test Plan
<!---Please fill in how you tested your change-->
Added test cases for rewrite-all, and modified existing test cases to
support the new default value for min-input-files.

## 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
Please follow [release notes
guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines)
and fill in the release notes below.

```
== NO RELEASE NOTES ==
```

## Summary by Sourcery

Add a rewrite-all option and adjust rewrite_data_files defaults and
behavior for Iceberg to align with spec and support rewriting all files
when requested.

New Features:
- Introduce a rewrite-all option for the Iceberg rewrite_data_files
procedure to rewrite all files regardless of thresholds or grouping.

Enhancements:
- Change the default min-input-files value for Iceberg
rewrite_data_files to 5, matching the Iceberg specification.
- Ensure file-group and file-size filters are skipped when rewrite-all
is enabled in Iceberg utilities.
- Deprecate the legacy max-file-size-bytes parser in favor of a more
flexible variant while keeping backward compatibility.
- Update Iceberg rewrite_data_files tests to cover the new rewrite-all
option and the new min-input-files default.

Tests:
- Extend Iceberg rewrite_data_files procedure tests to validate
rewrite-all semantics, default min-input-files behavior, and
interactions with file size filters.
Requires facebookincubator/velox#17317

## Description
<!---Describe your changes in detail-->

## Motivation and Context
<!---Why is this change required? What problem does it solve?-->
<!---If it fixes an open issue, please link to the issue here.-->

## Impact
<!---Describe any public API or user-facing feature change or any
performance impact-->

## Test Plan
<!---Please fill in how you tested your change-->

## Contributor checklist

- [x] 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).
- [x] PR description addresses the issue accurately and concisely. If
the change is non-trivial, a GitHub Issue is referenced.
- [x] Documented new properties (with its default value), SQL syntax,
functions, or other functionality.
- [x] If release notes are required, they follow the [release notes
guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines).
- [x] Adequate tests were added if applicable.
- [x] 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
Please follow [release notes
guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines)
and fill in the release notes below.

```
== NO RELEASE NOTE ==
```

## Summary by Sourcery

Build:
- Update the presto-native-execution Velox reference to point to a newer
commit, aligning with upstream changes.

## Summary by Sourcery

Build:
- Update the presto-native-execution Velox reference to point to a newer
upstream commit.
…prestodb#27664)

## Summary
- `visitTopN` in `StreamPropertyDerivations` was returning
`StreamProperties.ordered()` without preserving the
`streamPropertiesFromUniqueColumn` from its input
- This causes `AddExchanges` to fail with an `IllegalStateException`
when a downstream node depends on unique column properties originating
from the source table's scan
- Fix: propagate the input's `streamPropertiesFromUniqueColumn` through
the ordered result

## Test plan
- [x] Compile passes (`mvn compile -pl presto-main-base -q`)
- [ ] CI checks pass
- Prerequisite for prestodb#27641 (TopN late materialization)

## Summary by Sourcery

Bug Fixes:
- Ensure StreamPropertyDerivations.visitTopN propagates
streamPropertiesFromUniqueColumn from its input instead of discarding it
for non-partial TopN steps.
…estodb#27623)

## Description
Upgrade testing-library/user-event version to 14.6.1

## Motivation and Context
Using a more recent version helps avoid potential vulnerabilities and
ensures we aren't relying on outdated or unsupported code.


## Impact
<!---Describe any public API or user-facing feature change or any
performance impact-->

## Test Plan
<!---Please fill in how you tested your change-->

## 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
Please follow [release notes
guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines)
and fill in the release notes below.


```
== NO RELEASE NOTE ==
```

## Summary by Sourcery

Build:
- Bump @testing-library/user-event from 14.5.2 to 14.6.1 in presto-ui
package.json and refresh lockfile entries accordingly.
sumi-mathew and others added 30 commits May 28, 2026 21:09
…6-45292 (prestodb#27865)

## Description
Upgrade  opentelemetry-api  to 1.62.0 to address CVE-2026-45292

## Motivation and Context
Using a more recent version helps avoid potential vulnerabilities and
ensures we aren't relying on outdated or unsupported code.

## Impact
<!---Describe any public API or user-facing feature change or any
performance impact-->

## Test Plan
<img width="1340" height="974" alt="Screenshot 2026-05-26 at 6 37 27 PM"
src="https://github.com/user-attachments/assets/ad4fcd86-45ca-481a-9b85-9e3f7f870295"
/>


## 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
Please follow [release notes
guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines)
and fill in the release notes below.

`== RELEASE NOTES ==`

```
Security Changes
* Upgrade opentelemetry-api  to 1.62.0 in response to `CVE-2026-45292  <https://github.com/advisories/GHSA-fmxf-pm6p-7xgm>`_.

```

## Summary by Sourcery

Upgrade OpenTelemetry API dependency and configure automated dependency
updates for the UI package.

Bug Fixes:
- Address a security advisory by bumping the opentelemetry-api
dependency from 1.58.0 to 1.62.0.

CI:
- Add Dependabot configuration to automatically create daily update PRs
for npm dependencies in the presto-ui project, limited to webpack-cli
and labeled as dependencies.
```
== NO RELEASE NOTE ==
```

## Summary by Sourcery

Chores:
- Update the presto-native-execution Velox reference to a newer revision
for alignment with upstream.
…o 2.18.6 (prestodb#27809)

## Description
Updated com.fasterxml.jackson.datatype.jackson-datatype-jdk8
dependencies from 2.16.2 to 2.18.6

## Motivation and Context
Using a more recent version helps avoid potential vulnerabilities and
ensures we aren't relying on outdated or unsupported code.

## Impact
<!---Describe any public API or user-facing feature change or any
performance impact-->

## Test Plan
<!---Please fill in how you tested your change-->

## 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
Please follow [release notes
guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines)
and fill in the release notes below.

```
== NO RELEASE NOTE ==
```
…in (prestodb#27884)

## Summary

When the outer grouping keys are a strict subset of the inner grouping
keys, insert a local `ROUND_ROBIN` exchange above the inner aggregation
so the outer `PARTIAL` fans out across all local drivers instead of
inheriting the inner aggregation's parallelism.

```sql
SELECT approx_percentile(s, 0.5)
FROM (SELECT sum(x) AS s FROM t GROUP BY k1, k2)
GROUP BY k2
```

Gated by session property `parallelize_chained_aggregation` (default:
`false`).

## Test plan

- [x] Unit + plan tests (`TestParallelizeChainedAggregation`,
`TestParallelizeChainedAggregationPlan`)
- [x] End-to-end correctness test in `AbstractTestQueries` (compares
enabled vs disabled across sum/sum, max/sum, min/sum, count/sum,
sum/avg, plus negative cases)
- [x] Benchmarks (`BenchmarkParallelizeChainedAggregation`,
`BenchmarkParallelizeChainedAggregationDistributed`)
- [x] Docs in `properties-session.rst`

```
== RELEASE NOTES ==

General Changes
* Added optimizer rule ``parallelize_chained_aggregation`` (default: false) that
  inserts a local round-robin exchange to parallelize the outer PARTIAL in
  chained aggregations.
```

## Summary by Sourcery

Introduce an optimizer rule to parallelize certain chained aggregations
by inserting a local round-robin exchange between outer partial and
inner final aggregations, controlled by a new session property.

New Features:
- Add ParallelizeChainedAggregation optimizer rule to parallelize outer
partial aggregations when grouping keys are a strict subset of an inner
aggregation's keys.
- Expose a parallelize_chained_aggregation session/system property and
corresponding config flag to enable or disable the optimization.

Enhancements:
- Wire the new ParallelizeChainedAggregation rule into the main planner
optimizer pipeline.
- Document the new parallelize_chained_aggregation session property in
the admin session properties reference.

Documentation:
- Update session properties documentation to describe the
parallelize_chained_aggregation optimizer setting.

Tests:
- Add planner rule unit tests for ParallelizeChainedAggregation covering
firing conditions and non-firing cases.
- Add distributed plan tests to verify insertion of a local round-robin
exchange and unchanged aggregation steps when the rule is enabled.
- Add end-to-end query tests comparing results with the optimization
enabled versus disabled across multiple aggregation patterns.
- Add local and distributed JMH benchmarks to measure performance impact
of the chained aggregation parallelization rule.
…6-45300 (prestodb#27863)

## Description
Upgrade async-http-client to 3.0.10 to address CVE-2026-45300

Dependnecy tree after fix : 

<img width="834" height="380" alt="image"
src="https://github.com/user-attachments/assets/4cab5567-3c1a-4be8-b0fc-98f4f865ad81"
/>


## Motivation and Context
<!---Why is this change required? What problem does it solve?-->
<!---If it fixes an open issue, please link to the issue here.-->

## Impact
<!---Describe any public API or user-facing feature change or any
performance impact-->

## Test Plan
<!---Please fill in how you tested your change-->

<img width="1728" height="1117" alt="image"
src="https://github.com/user-attachments/assets/9d94da13-b1cf-4123-8721-cd9e221adc8b"
/>


## 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
Please follow [release notes
guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines)
and fill in the release notes below.

`== RELEASE NOTES ==`

```
Security Changes
* Upgrade async-http-client to 3.0.10 in response to `CVE-2026-45300  <https://github.com/advisories/GHSA-fmxf-pm6p-7xgm>`_.

```
…7879)

Summary:
C++-only diffs under presto-native-execution/presto_cpp/ were triggering
all Java CI workflows (OWASP, Hive tests, JDBC, Spark integration,
product tests, etc.) unnecessarily, wasting CI resources.

Added `!presto-native-execution/presto_cpp/**` path exclusion to the
codechange filter in 12 workflow files.

Differential Revision: D106568848

```
== NO RELEASE NOTE ==
```
…er (prestodb#27875) (prestodb#27875)

Summary:

## Overview

Adds two-phase memory reclaim for MaterializedOutputBuffer, integrated
with the Velox memory arbitrator. When the arbitrator needs to free
memory, the Reclaimer flushes partition buffers to the ShuffleWriter and
waits for network drain.

## Two-Phase Reclaim

```
Velox Memory Arbitrator
  |
  v
Reclaimer::reclaim(pool, targetBytes, maxWaitMs)
  |
  |-- canReclaim(pool, targetBytes)?
  |     - false if targetBytes == 0, pool empty, or state is kClosed/kAborted
  |     - true for kActive (both phases) and kDraining (Phase 2 only)
  |
  |-- Phase 1: FLUSH partition buffers (kActive only)
  |     canReclaimFromPartitionBuffers()?
  |       - true only if kActive AND bufferedBytes > 0
  |     tryDrainPartitions():
  |       1. Snapshot partition sizes via atomic bufferedBytes (lock-free)
  |       2. Sort partitions largest-first
  |       3. Skip partitions below reclaimDrainThresholdBytes
  |       4. tryDrainPartition(): try_lock -> swap buffers -> unlock
  |          -> coalesce + flush outside lock
  |     Early return if poolUsedBytes <= poolTargetBytes
  |
  |-- Phase 2: WAIT for writer network drain
  |     waitForWriterDrain(pool, poolTargetBytes, deadline):
  |       Poll pool->usedBytes() every 10ms until target or deadline
  |
  |-- totalFreedBytes = poolStartBytes - pool->usedBytes()
  |-- recordStats(totalFreedBytes)
```

## State-Aware Dispatch

| State | canReclaim | canReclaimFromPartitionBuffers | Behavior |
|-------|-----------|-------------------------------|----------|
| kActive | true | true (if bufferedBytes > 0) | Phase 1 + Phase 2 |
| kDraining | true | false | Phase 2 only (noMoreData is flushing,
reclaim waits for network) |
| kClosed | false | N/A | return 0 (buffer fully drained) |
| kAborted | false | N/A | return 0 (error teardown) |

The kDraining scenario occurs when `noMoreData()` calls `close()` which
calls `coalesceRowGroups() -> allocateTrackedIOBuf()`. That allocation
can trigger arbitration -> reclaim on the same pool. Since the partition
buffers are already being drained by `noMoreData()`, Phase 1 is skipped
and reclaim only waits for the writer to drain packages to network.

## Deadlock Prevention

The reclaimer uses `try_lock` on partition mutexes instead of blocking
locks. If a partition lock is held by `enqueue()` (which may call
`writer->collect()` -> allocation -> arbitration -> reclaim),
`tryDrainPartition` skips it and moves on. The contested partition is
self-draining via its own enqueue threshold.

Coalesce and flush happen outside the try_lock to avoid holding the
partition mutex across pool allocations and writer calls. This matches
the pattern used by `drainPartition()`. Safe because reclaim is
single-threaded (the arbitrator pauses the triggering driver) and Velox
prevents re-entrant arbitration on the same thread.

## Bytes Accounting

Single-baseline approach: `poolStartBytes` captured once at entry,
`poolTargetBytes` computed once as `poolStartBytes - targetBytes`. Phase
1 and Phase 2 just do work (void functions). `totalFreedBytes` measured
once at the end: `poolStartBytes - pool->usedBytes()`. No bytes passed
between functions.

## Configurable Reclaim Drain Threshold

New config `exchange.materialization.reclaim-drain-threshold-ratio`
(default 0.67). The reclaim drain threshold is `partitionDrainThreshold
* ratio` -- generally lower than the regular drain threshold, but high
enough that draining actually reduces memory. Without this lower bound,
reclaim would flush small partition buffers that produce data too small
for the writer to compress efficiently -- low ROI flushes.

## Key Components

- `Reclaimer` class: nested inside MaterializedOutputBuffer, registered
with `kHighReclaimPriority = -1` (reclaimed before operator pools).
Requires non-null `partitionBuffer_` (enforced by
`VELOX_CHECK_NOT_NULL`).
- `canReclaim(pool, targetBytes)`: gates the entire reclaim call
- `canReclaimFromPartitionBuffers()`: gates Phase 1 (kActive +
bufferedBytes > 0)
- `tryDrainPartitions()` / `tryDrainPartition()`: Phase 1 implementation
- `waitForWriterDrain(pool, poolTargetBytes, deadline)`: Phase 2
implementation
- `updateDrainStats(drainedBytes)`: shared helper for drain count,
drained bytes, and buffered bytes accounting
- `MATERIALIZED_BUFFER_LOG` macro: structured logging with pool name and
`velox::succinctBytes`
- `reclaimDrainThresholdBytes_`: precomputed in constructor from config
- Per-partition `bufferedBytes_` is `std::atomic<int64_t>` for lock-free
size snapshots during reclaim sorting
- All config values (`maxBufferedBytes`, `partitionDrainThreshold`,
`reclaimDrainThresholdRatio`) read from SystemConfig inside the
constructor

Reviewed By: xiaoxmeng

Differential Revision: D106394830
…y functions (prestodb#27698)

Summary:
## Description

Introduces infrastructure for the presto-on-spark driver to launch a
metadata-only Velox sidecar process at bootstrap, fetch the registered
native function definitions over HTTP from its `/v1/functions` endpoint,
and register them into `FunctionAndTypeManager` as built-in functions
before query planning begins.

New classes (all in `presto-spark-base`, package
`com.facebook.presto.spark.execution.nativeprocess`):

| Class | Purpose |
|---|---|
| `MetadataSidecarConfig` | Airlift `Config` for
`metadata-sidecar.enabled` and `metadata-sidecar.executable-path` |
| `MetadataSidecarProcess` | Concrete `AbstractNativeProcess` subclass
that writes a sidecar-only etc/ tree (no shuffle, no task discovery) |
| `MetadataSidecarProcessFactory` | Lifecycle owner; lazily starts the
sidecar, exposes the `SidecarBinaryLocator` interface for binary
discovery |
| `DriverSidecarFunctionRegistryTool` | `WorkerFunctionRegistryTool`
implementation that boots the sidecar, GETs `/v1/functions`,
deserializes `JsonBasedUdfFunctionMetadata`, converts to
`SqlInvokedFunction` via `WorkerFunctionUtil`, then shuts the sidecar
down |
| `DriverSidecarModule` | Guice wiring; binds the above + an
`ForMetadataSidecar`-qualified OkHttp client and executors |
| `ForMetadataSidecar` | Binding annotation |

The feature is **disabled by default**. Two configs gate activation:
`metadata-sidecar.enabled` (off by default) and the upstream
`built-in-sidecar-functions-enabled` (off by default in
`FeaturesConfig`). The bootstrap call site is added in a follow-up diff
in this stack.

## Motivation and Context

When presto-on-spark uses Velox for execution, the Velox workers know
about a superset of functions compared to the Java engine — many native
UDFs have no Java implementation at all (e.g. native-only Velox
built-ins). Today such functions fail at the driver during query
analysis with `FUNCTION_NOT_FOUND` because the Java planner only knows
about Java-registered functions, even though the Velox executors could
happily evaluate them.

Mirroring the design used by Prestissimo's
`NativeSidecarFunctionRegistryTool` (which fetches function metadata
from a sidecar at coordinator boot), this diff adds the analogous
machinery for the presto-on-spark driver. After registration, the
planner can resolve native-only function references and the query
proceeds normally.

`SidecarBinaryLocator` is an interface (with a default Optional-empty
binding) so deployments can plug in their own way of discovering the
sidecar binary on disk — the Java module deliberately doesn't depend on
any deployment-specific launcher.

## Impact

Disabled by default; no behavior change unless the operator opts in.
When enabled:

- A short-lived sidecar process spawns at driver bootstrap (~1-2s; shut
down before planning begins).
- One additional bootstrap step issues an HTTP GET against the local
sidecar.
- Native function metadata becomes available in `FunctionAndTypeManager`
as built-in `SqlInvokedFunction`s.
- No effect on executors; module bindings stay lazy when not on the
driver.

## Test Plan

- Unit tests: `TestMetadataSidecarConfig`,
`TestMetadataSidecarProcessSmoke`,
`TestDriverSidecarFunctionRegistryTool` — verify config binding, sidecar
lifecycle, and the JSON → `SqlInvokedFunction` conversion path.


## Contributor checklist

- [x] My PR adheres to the code style of this project.
- [x] My code builds clean without any errors or warnings.
- [x] I have included tests in my PR.
- [x] I am willing to help maintain this change if there are any issues
in the future.
Differential Revision: D103025711

## Release Notes

```
== RELEASE NOTES ==

General Changes
* Add a driver-side metadata sidecar that registers native-only Velox functions into the Java planner at driver bootstrap
```
…ion (prestodb#27836)

Currently, when mapping `Session` to `SessionRepresentation`,
`principal.toString()` is passed as the value for the principal
argument. But, inside `SessionRepresentation`, a new `BasicPrincipal` is
reconstructed from that string:
https://github.com/prestodb/presto/blob/81ea0f14c068d961a12a7114f8291e4178868746/presto-main-base/src/main/java/com/facebook/presto/SessionRepresentation.java#L334

Since `BasicPrincipal` expects the principal name in its constructor, we
should use `getName()` instead of `toString()` when creating the
`SessionRepresentation` object. This is also consistent with the rest of
the Presto codebase, where `getName()` is used whenever a string
representation of a `Principal` is required.

Using `toString()` may cause issues for custom access control plugins
that return an overloaded implementation of `BasicPrincipal` with
different implementations of `getName()` and `toString()`.
## Description
<!---Describe your changes in detail-->
Adds a test helper `assertMaterializedViewRewriteOccurred` that verifies
the `optimizedWithMaterializedViewSubqueryCount` runtime metric was
incremented during query execution, and migrates existing tests to use
it.

## Motivation and Context
<!---Why is this change required? What problem does it solve?-->
<!---If it fixes an open issue, please link to the issue here.-->
We want to verify rewrites are occurring, this change confirms metrics
are emitted during tests in `TestHiveMaterializedViewLogicalPlanner`
verifying rewrites occurred.

## Impact
<!---Describe any public API or user-facing feature change or any
performance impact-->

## Test Plan
<!---Please fill in how you tested your change-->
Tests passing with new changes

## 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
Please follow [release notes
guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines)
and fill in the release notes below.

```
== NO RELEASE NOTE ==
```

## Summary by Sourcery

Add a test helper to assert that materialized view rewrites occurred and
update materialized view planner tests to validate runtime metrics for
rewrites.

Tests:
- Add a dedicated test validating the materialized view rewrite runtime
metric is emitted when queries are optimized to use materialized views.
- Refactor existing Hive materialized view planner tests to use a shared
helper that asserts a materialized view rewrite occurred instead of
directly executing optimized queries.
…E-2026-45205 (prestodb#27862)

## Description
Upgrade commons-configuration2 to 2.15.1 to address CVE-2026-45205

## Motivation and Context
<!---Why is this change required? What problem does it solve?-->
<!---If it fixes an open issue, please link to the issue here.-->
Using a more recent version helps avoid potential vulnerabilities and
ensures we aren't relying on outdated or unsupported code.

## Impact
<!---Describe any public API or user-facing feature change or any
performance impact-->

## Test Plan
<!---Please fill in how you tested your change-->
<img width="1048" height="809" alt="Screenshot 2026-05-25 at 9 32 23 PM"
src="https://github.com/user-attachments/assets/2038013c-5011-47fc-91f1-17eddde3d061"
/>
<img width="974" height="742" alt="Screenshot 2026-05-25 at 9 32 37 PM"
src="https://github.com/user-attachments/assets/a5c518af-79ca-459d-8687-5ffa2d21982f"
/>

<img width="1631" height="576" alt="Screenshot 2026-05-25 at 9 32 49 PM"
src="https://github.com/user-attachments/assets/0b056070-24ab-4db5-9080-f26319e8e255"
/>

## 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
Please follow [release notes
guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines)
and fill in the release notes below.

`== RELEASE NOTES ==`

```
Security Changes
* Upgrade commons-configuration2 in response to `CVE-2026-45205  <https://github.com/advisories/GHSA-337m-mw94-2v6g>`_.

```
```
== NO RELEASE NOTE ==
```

## Summary by Sourcery

Chores:
- Update the presto-native-execution Velox dependency to the latest
upstream commit.
… property to control Velox materialized exchange (prestodb#27881)

Summary:
Adds a new session property `native_exchange_materialization_enabled`
(default: false) that controls whether the Velox native worker uses
MaterializedOutput/MaterializedExchange operators in the Presto on Spark
native codepath. When set to false, the native process falls back to
PartitionAndSerialize + ShuffleWrite.

The session property is read in
NativeExecutionProcess.updateWorkerProperties() and unconditionally
propagated to the C++ system config `exchange.materialization.enabled`
before the native process starts. The session value always overrides the
static config in both directions.

Changes:
- SystemSessionProperties: Add NATIVE_EXCHANGE_MATERIALIZATION_ENABLED
constant, boolean property registration (default false), and static
getter
- NativeExecutionSystemConfig: Add EXCHANGE_MATERIALIZATION_ENABLED
constant and default
- NativeExecutionProcess.updateWorkerProperties(): Set
exchange.materialization.enabled from session property value
- TestNativeExecutionSystemConfig: Add new property to all test maps


Reviewed By: xiaoxmeng

Differential Revision: D106582232



```
== RELEASE NOTE ==

General Changes
* Add `native_exchange_materialization_enabled` session property (Presto on Spark native codepath only) to control whether Velox native workers use MaterializedOutput/MaterializedExchange operators. When set to `true`, enables materialized exchange; when `false` (default), falls back to PartitionAndSerialize + ShuffleWrite.
```
Summary:
Extends the AST-level MV query rewriter to substitute a base table
inside a JOIN with a materialized view when the MV's columns fully cover
that table's usage in the query. Uses the shared
MaterializedViewExpressionRewriter (from the parent diff) for all
expression-level rewriting.

Components:
- MaterializedViewJoinQueryRewriter: top-level driver for JOIN MV
rewriting
- JoinRewriteContext: handles join tree transformation via AstVisitor,
safety validation, MV freshness checks
- Recursive tryRewrite: after one leaf is swapped, recurses to try
remaining leaves
- collectJoinLeaves via DefaultTraversalVisitor: walks the join tree to
find leaf tables

Safety guards when MV has GROUP BY:
- Reject if query has no GROUP BY (MV would collapse rows)
- Reject aggregates over non-swapped table columns (fan-out changes
results)
- Reject LEFT/RIGHT JOIN on the preserved side, and FULL OUTER JOIN
- COUNT(*) rewritten to SUM(mv.cnt) when MV has a matching column
- Upfront isEligibleForRewrite validation (no exception-based rejection)

```
== RELEASE NOTES ==

General Changes
* Add JOIN support to the materialized view query optimizer. Queries
  that join a base table covered by a materialized view with another
  table can now be rewritten to scan the materialized view in place
  of the base table, subject to safety guards (matching GROUP BY,
  no aggregates over non-swapped tables, supported join types).
```

Differential Revision: D103812393
Address modernizer plugin issues reported by our modernizer tool.

This was debt from the JDK upgrade because we disabled the modernizer on all modules
…restodb#27699)

Summary: Differential Revision: D103025710
## Description

Wires the driver-side metadata sidecar into the presto-on-spark
bootstrap. After Airlift initializes, when both
`built-in-sidecar-functions-enabled=true` and `sparkProcessType ==
DRIVER`, `PrestoSparkInjectorFactory` invokes
`WorkerFunctionRegistryTool.getWorkerFunctions()` to fetch native
function metadata from the sidecar and registers the result into
`FunctionAndTypeManager` as built-in `SqlInvokedFunction`s.

Also adds the default `OptionalBinder<SidecarBinaryLocator>` binding
(returns `Optional.empty`) to `DriverSidecarModule` so deployments can
override with their own concrete locator without requiring a binding
from this module.

## Motivation and Context

D103025711 introduced the metadata-sidecar machinery but did not call
it. This diff adds the bootstrap call site, mirroring the equivalent
code path in `PrestoServer` for Prestissimo's coordinator-side sidecar
registration.

The companion change in `PrestoFacebookSparkServiceFactory`
(Meta-internal) installs `DriverSidecarModule` on driver and executor
JVMs so Airlift's strict-config check accepts the `metadata-sidecar.*`
properties on both tiers.

## Impact

Disabled by default — `built-in-sidecar-functions-enabled` defaults to
`false` in `FeaturesConfig`. When enabled by an operator, the only
behavior change is a one-time bootstrap step that registers additional
built-in functions; no effect on already-resolvable function references
or query execution paths.

## Test Plan

End-to-end validation in D103025708 (paste P2298835496) — boots the
sidecar from a real native worker binary on the driver, registers
`koski_cosine_similarity` (a native-only Velox function with no Java
implementation), plans an INSERT that uses it, and confirms native
execution writes the expected rows.

## Contributor checklist

- [x] My PR adheres to the code style of this project.
- [x] My code builds clean without any errors or warnings.
- [x] I am willing to help maintain this change if there are any issues
in the future.


## Release Notes

```
== RELEASE NOTES ==
General Presto-on-Spark Changes
* Update the driver-side metadata sidecar registration of worker functions into the Airlift bootstrap
```
…spatch (prestodb#27908)

## Description
- Widen the try-catch in `ExpressionOptimizer.cpp` to cover
`velox::expression::optimize()` in addition to `tryEvaluateToConstant()`
- Previously, exceptions thrown during expression optimization (e.g.
Velox type dispatch failures on UNKNOWN type) escaped uncaught and
crashed the sidecar process
- Now any `VeloxException` or `std::exception` during optimization is
caught and returned as a structured `NativeSidecarFailureInfo` error
response

## Motivation and Context
Resolves prestodb#27907.
Uncovered by prestodb#27011.

Velox's type dispatch macros (`VELOX_DYNAMIC_TEMPLATE_TYPE_DISPATCH`
etc.) do not handle `TypeKind::UNKNOWN` and throw `VeloxRuntimeError`
via `VELOX_FAIL`. This affects ~20 Presto functions (array_except,
array_intersect, contains, array_distinct, etc.) when given empty array
literals or NULL arrays.

The existing try-catch only wrapped `tryEvaluateToConstant` but the
crash occurs earlier in `optimize()` during function resolution. Moving
the try-catch to cover the entire optimization pipeline prevents the
sidecar from crashing.

## Impact
This fix is narrow and impacts only the correctness of expressions
optimized by the native expression optimizer. Other possible fixes were
considered and dropped in favor of this:
1. Catch all `VeloxException`s including `VeloxRuntimeError`s in Velox
expression optimizer:
pramodsatya/velox@8e9f188
**Pros:** Fix in Velox itself, `makeFailExpr` produces proper failure
expression that flows through normally, the sidecar's `toVeloxExpr` →
`optimize` → `veloxToPresto` path works end-to-end, returns a structured
failure with the error message
**Cons:** Catches broader than intended in Velox (RuntimErrors during
constant folding are usually bugs), requires Velox PR

2. Fix each Presto function in Velox to guard against `UNKNOWN` type

**Where:** Each of the ~20 affected functions in
`velox/functions/prestosql/`

**What:** Either:
- Switch to `_ALL` macros (requires `UnknownValue` to be hashable — not
currently possible for set-based functions)
- Add explicit `if (elementType->isUnknown()) { return special_impl; }`
guards before dispatch (like `approx_distinct` and `merge` already do)

**Pros:** Each function handles UNKNOWN correctly at its own level;
could even produce correct results for trivial cases (empty arrays)
**Cons:** 20+ function changes, each needs its own logic, large Velox
PR, doesn't protect against future functions that forget the guard

## Test Plan
e2e testcase added.

## Release Notes

```
== NO RELEASE NOTE ==
```
```
== NO RELEASE NOTE ==
```
prestodb#27668)

Co-authored-by: Reetika Agrawal <reetika.agrawal@ibm.com>
…restodb#27498)

## Description
Minor code refactoring of Sphinx config
`presto-docs/src/main/sphinx/conf.py`:
- The PR replaces all single quotes with double quotes for strings to
ensure consistency.
- The PR replaces the modulo operator (%) and replace method call with
f-strings for text formatting.

## Motivation and Context
Align Python code with the current standards.

## Impact
No

## Test Plan

Build the documentation and check that the pages are correct:
```shell
presto-docs/build
open target/html/router/deployment.html
```
<img width="1323" height="816" alt="image"
src="https://github.com/user-attachments/assets/f274196d-fdd6-4dd7-804f-b6a1449f0783"
/>



## Contributor checklist

- [x] 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).
- [x] 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 ==
```

## Summary by Sourcery

Refactor the Sphinx documentation configuration for Presto to
standardize string formatting and modernize text interpolation.

Enhancements:
- Standardize all string literals in the Sphinx config to use double
quotes for consistency with code style guidelines.
- Replace legacy percent and replace-based string formatting with
f-strings in the Sphinx configuration to align with modern Python
practices.

Signed-off-by: Denis Krivenko <dnskrv88@gmail.com>
prestodb#27903)

## Summary

`protocol::AggregationNode::aggregations` is `std::map<VRE,
Aggregation>` on the native side (sorted by variable name); the Java
side iterates `LinkedHashMap` insertion order. When the two orders
diverge — e.g. `approx_distinct_*` mixed with `sum_*`, alphabetical sort
`approx_distinct < sum` but Java inserts `sum`s first — the native
`AggregationNode` output schema differs from the Java planner's. Channel
positions shift and type mismatches surface at exchange operators:

```
type_->kindEquals(vector.type()) Type mismatch: BIGINT vs. DOUBLE
Operator: LocalPartition(...)
```

prestodb#27493 partially addressed this by switching three Java optimizer rules
from `HashMap` to `LinkedHashMap` (deterministic insertion order), but
did not align Java with the native side's sort. Queries whose variable
names happened to sort consistently with their insertion order passed;
others still crashed.

This adds an explicit `aggregationOutputs:
List<VariableReferenceExpression>` field, populated on the Java side
from `aggregations.keySet()` in `LinkedHashMap` order. The native
converter uses this list when present and falls back to `std::map`
iteration only for older coordinators that don't send the field (so
rolling upgrades are safe).

Fixes prestodb#27902.

## Changes

- `presto-spi/.../AggregationNode.java`: `@JsonProperty(READ_ONLY)
getAggregationOutputs()` — derived from `aggregations`, no constructor
change.
- `presto_protocol_core.h`/`.cpp`: `List<VRE> aggregationOutputs` on
`protocol::AggregationNode`, with backward-compatible `from_json`.
- `PrestoToVeloxQueryPlan.cpp`: converter uses `aggregationOutputs` when
non-empty; falls back to map iteration otherwise.
- Tests: protocol round-trip + backward-compat
(`AggregationNodeTest.cpp`); Java getter ordering
(`TestPreAggregateBeforeGroupId`).

## Test plan

- [ ] `mvn test -pl presto-main-base
-Dtest=TestPreAggregateBeforeGroupId#testAggregationOutputsPreservesInsertionOrder`
- [ ] `presto-native-execution` build + run `presto_protocol_test
--gtest_filter="AggregationNodeTest.*"`
- [ ] Existing `TestPreAggregateBeforeGroupId` suite stays green
- [ ] Manually verify a reproducer query (`approx_distinct(...)` +
`sum(...)` under `GROUPING SETS` with
`optimizer.pre_aggregate_before_grouping_sets=true`) no longer crashes
on a Prestissimo cluster

## Release notes

\`\`\`
== RELEASE NOTES ==

Native Execution Changes
* Fix runtime type-mismatch crashes at exchange operators in Prestissimo
when aggregation variable names sort differently from their Java
allocation order. The protocol now carries an explicit aggregation
output ordering so native workers build the AggregationNode output
schema in the order the Java planner intended.
\`\`\`

## Summary by Sourcery

Ensure native AggregationNode output schemas follow Java planner
aggregation ordering by explicitly propagating aggregation output
variables through the protocol and converter.

Bug Fixes:
- Prevent type-mismatch failures in native execution caused by differing
aggregation iteration orders between Java and C++.

Enhancements:
- Expose an explicit aggregationOutputs list on AggregationNode in the
Java SPI and carry it through the native protocol for deterministic
aggregation output ordering.

Tests:
- Add Java and C++ regression tests to verify aggregation output
ordering is preserved and that the protocol remains backward compatible
when aggregationOutputs is absent.
…pe/refactor/deinline-connector-registration
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.