fix(native): Hoist plugin loading and de-inline factory map for dynamic connector support - #156
Open
20001020ycx wants to merge 203 commits into
Open
fix(native): Hoist plugin loading and de-inline factory map for dynamic connector support#15620001020ycx wants to merge 203 commits into
20001020ycx wants to merge 203 commits into
Conversation
…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 /> [](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 ```
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
20001020ycx
force-pushed
the
yscope/refactor/deinline-connector-registration
branch
from
April 22, 2026 20:41
d8e4993 to
f7d3730
Compare
20001020ycx
force-pushed
the
yscope/refactor/deinline-connector-registration
branch
3 times, most recently
from
April 23, 2026 15:11
62db030 to
abf1515
Compare
…-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
force-pushed
the
yscope/refactor/deinline-connector-registration
branch
from
April 23, 2026 16:20
abf1515 to
8e60013
Compare
…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.
…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.
…tex to prevent pool name collision (prestodb#27900)
…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
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
registerDynamicFunctions()beforeregisterVeloxConnectors()inPrestoServer.cppRegistration.hintoRegistration.cppMotivation and Context
This PR enables dynamic connector plugins — connectors loaded at runtime via
plugin-dirusingdlopen()— 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 theirConnectorFactoryinstances. BothregisterConnectorFactory(name, factory)andgetConnectorFactory(name)read and write this map.Why
registerDynamicFunctions()must be hoisted beforeregisterVeloxConnectors()registerVeloxConnectors()iterates over every*.propertiesfile in thecatalog/directory. For each one, it readsconnector.nameand callsgetConnectorFactory(connectorName)to look up the factory in the singleton map, then callsfactory->newConnector(...)to instantiate the connector.registerDynamicFunctions()loads plugin.sofiles from theplugin/directory viadlopen(). Each plugin'sregisterExtensions()entry point callsregisterConnectorFactory(...)to insert its factory into the same singleton map.Before this PR,
registerVeloxConnectors()ran beforeregisterDynamicFunctions(). Since plugins had not been loaded yet, the factory map had no entry for dynamically registered connector names. WhenregisterVeloxConnectors()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:
registerDynamicFunctions()— loads plugin.sofiles, plugins callregisterConnectorFactory()to insert into the mapregisterVeloxConnectors()— readscatalog/*.properties, callsgetConnectorFactory()to look up entries in the mapThe 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):
This breaks the singleton property of the factory map: the plugin's
registerConnectorFactory()inserts into its own map (0xBBBB), but the server'sgetConnectorFactory()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
presto_serverwith the changesRelease Notes