Add granular JMX metrics and runtime stats tracing for SystemAccessControl operations with automatic integration #3

Copilot · 2025-09-19T03:29:51Z

This PR implements granular timing and failure metrics for SystemAccessControl operations to help debug latency issues in security access control plugins, addressing issue prestodb#25574. The implementation provides automatic integration with AccessControlManager for seamless E2E functionality and includes both JMX metrics and runtime stats tracing.

Overview

The implementation provides a decorator-based approach that wraps existing SystemAccessControl implementations without requiring any changes to them. The design uses an inner Stats class with @Managed getters for automatic JMX export and direct field access for efficient metrics recording. All SystemAccessControl instances are automatically wrapped with stats recording during initialization via AccessControlManager.

Key Components

SystemAccessControlStats

A JMX-exportable statistics class that tracks:

Timing distributions (min, max, mean, percentiles) for method execution using TimeDistribution
Failure counts when methods throw exceptions using CounterStat

StatsRecordingSystemAccessControl

A wrapper implementation that:

Delegates all calls to an underlying SystemAccessControl
Records timing metrics using System.nanoTime()
Tracks failures with CounterStat
Records runtime stats for query-level performance tracing
Provides minimal performance overhead
Contains an inner Stats class with direct field access for metrics recording

AccessControlManager Integration

NEW: The AccessControlManager automatically wraps all SystemAccessControl instances with stats recording:

Modified setSystemAccessControl() method to automatically wrap SystemAccessControl instances
Added @Managed getter getSystemAccessControlStats() for JMX export
Zero configuration required - works out of the box

Usage

Automatic (Recommended): Statistics are automatically available via JMX for any configured SystemAccessControl without any code changes. The AccessControlManager handles all setup automatically.

Manual (Advanced): For custom implementations:

// Wrap your existing SystemAccessControl implementation
SystemAccessControl delegate = new YourSystemAccessControlImplementation();
StatsRecordingSystemAccessControl instrumentedSAC = 
    new StatsRecordingSystemAccessControl(delegate);

// Statistics are automatically available via JMX through @Managed getters on the inner Stats class

Method Coverage

All 37 SystemAccessControl methods are instrumented with both JMX metrics and runtime stats tracing, including:

Access control checks (checkCanAccessCatalog, checkCanCreateTable, etc.)
Filtering operations (filterCatalogs, filterSchemas, filterTables)
Security policy methods (getRowFilters, getColumnMasks)
Identity management (checkCanSetUser, selectAuthorizedIdentity)

Performance Monitoring

The implementation provides dual monitoring capabilities:

JMX Metrics

Uses CounterStat for efficient failure tracking
Timing measurements with nanosecond precision
Automatic JMX export through @Managed annotations
Direct field access for metrics recording (e.g., stats.checkCanSetUser.record(nanos))
Zero overhead when no SystemAccessControl is configured
Minimal overhead during normal operations

Runtime Stats Tracing

NEW: Each method call adds runtime stats via context.getRuntimeStats().addMetricValue()
Enables query-level performance analysis and debugging
Provides detailed timing information for individual query execution
Uses consistent naming pattern: systemAccessControl.methodName

Testing

Focused test coverage includes:

Interface compliance validation ensuring all SystemAccessControl methods are overridden
Functionality tests validating metrics are recorded correctly
Integration tests for the wrapper functionality

This implementation enables users to identify performance bottlenecks in their SystemAccessControl plugins by providing detailed timing and failure metrics through both JMX monitoring tools and runtime stats tracing, with automatic integration and zero configuration required.

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

aaneja · 2025-09-19T09:02:11Z

...rc/main/java/com/facebook/presto/plugin/base/security/StatsRecordingSystemAccessControl.java

+        stats.register(delegate, "checkCanSetUser");
+        stats.register(delegate, "selectAuthorizedIdentity");
+        stats.register(delegate, "checkQueryIntegrity");
+        stats.register(delegate, "checkCanSetSystemSessionProperty");
+        stats.register(delegate, "checkCanAccessCatalog");
+        stats.register(delegate, "filterCatalogs");
+        stats.register(delegate, "checkCanCreateSchema");
+        stats.register(delegate, "checkCanDropSchema");
+        stats.register(delegate, "checkCanRenameSchema");
+        stats.register(delegate, "checkCanShowSchemas");
+        stats.register(delegate, "filterSchemas");
+        stats.register(delegate, "checkCanShowCreateTable");
+        stats.register(delegate, "checkCanCreateTable");
+        stats.register(delegate, "checkCanSetTableProperties");
+        stats.register(delegate, "checkCanDropTable");
+        stats.register(delegate, "checkCanRenameTable");
+        stats.register(delegate, "checkCanShowTablesMetadata");
+        stats.register(delegate, "filterTables");
+        stats.register(delegate, "checkCanShowColumnsMetadata");
+        stats.register(delegate, "filterColumns");
+        stats.register(delegate, "checkCanAddColumn");
+        stats.register(delegate, "checkCanDropColumn");
+        stats.register(delegate, "checkCanRenameColumn");
+        stats.register(delegate, "checkCanSelectFromColumns");
+        stats.register(delegate, "checkCanInsertIntoTable");
+        stats.register(delegate, "checkCanDeleteFromTable");
+        stats.register(delegate, "checkCanTruncateTable");
+        stats.register(delegate, "checkCanUpdateTableColumns");
+        stats.register(delegate, "checkCanCreateView");
+        stats.register(delegate, "checkCanRenameView");
+        stats.register(delegate, "checkCanDropView");
+        stats.register(delegate, "checkCanCreateViewWithSelectFromColumns");
+        stats.register(delegate, "checkCanSetCatalogSessionProperty");
+        stats.register(delegate, "checkCanGrantTablePrivilege");
+        stats.register(delegate, "checkCanRevokeTablePrivilege");
+        stats.register(delegate, "checkCanDropConstraint");
+        stats.register(delegate, "checkCanAddConstraint");
+        stats.register(delegate, "getRowFilters");
+        stats.register(delegate, "getColumnMasks");


@copilot I changed my mind. Instead of using this pattern to register method names, can you instead create real getters in SystemAccessControlStatsRecorder for each method ? Annotating them with @Managed should be enough to achieve their JMX export

Refactored the SystemAccessControlStatsRecorder to use individual @Managed getters for each method instead of the string-based registration pattern. Each SystemAccessControl method now has its own dedicated stats field and @Managed @nested getter (e.g., getCheckCanSetUser(), getFilterCatalogs()), which enables automatic JMX export without manual MBean management. This simplifies the API and eliminates the need for export/unexport methods. Commit: 34dde1d

aaneja

Looks good for the most part. See individual comment for the suggested refactoring

aaneja · 2025-09-19T11:09:12Z

@copilot Please take a look at the comments and address them

aaneja

@copilot I have some feedback. Please fix

aaneja · 2025-09-19T14:18:26Z

...src/main/java/com/facebook/presto/plugin/base/security/SystemAccessControlStatsRecorder.java

+import org.weakref.jmx.Managed;
+import org.weakref.jmx.Nested;
+
+public class SystemAccessControlStatsRecorder


Define this class instead as an inner class of StatsRecordingSystemAccessControl

Moved SystemAccessControlStatsRecorder as an inner "Stats" class within StatsRecordingSystemAccessControl. This simplifies the API and eliminates the need for a separate class. Commit: 90664ca

aaneja · 2025-09-19T14:19:39Z

...src/main/java/com/facebook/presto/plugin/base/security/SystemAccessControlStatsRecorder.java

+    public void recordCheckCanSetUser(long nanos)
+    {
+        checkCanSetUser.record(nanos);
+    }
+
+    public void recordCheckCanSetUserFailure()
+    {
+        checkCanSetUser.recordFailure();
+    }
+
+    public void recordSelectAuthorizedIdentity(long nanos)
+    {
+        selectAuthorizedIdentity.record(nanos);
+    }


recordXXX methods like these are increasing code verbosity. As an inner class of StatsRecordingSystemAccessControl, keep the visibility of the fields such that you can call checkCanSetUser.record(nanos); at the call site

Removed verbose recordXXX methods. Fields in the inner Stats class now have package visibility allowing direct access like stats.checkCanSetUser.record(nanos) and stats.checkCanSetUser.recordFailure(). Commit: 90664ca

aaneja · 2025-09-19T14:20:11Z

presto-plugin-toolkit/pom.xml

            <scope>test</scope>
        </dependency>
+
+        <dependency>


I don't want to take a dependency on mockito. In Presto we hand roll our own mocks

Removed mockito dependency. The test now uses ForwardingSystemAccessControl.of(() -> null) for creating test delegates without external mocking libraries. Commit: 90664ca

aaneja · 2025-09-19T14:21:37Z

...test/java/com/facebook/presto/plugin/base/security/TestSystemAccessControlStatsRecorder.java

+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertNotNull;
+
+public class TestSystemAccessControlStatsRecorder


I don't want this test, this class will end up being mostly a POJO + getters for reporting JMX metrics

Removed TestSystemAccessControlStatsRecorder test file. Since the stats recorder is now an inner class with mainly @Managed getters, this test is no longer needed. Commit: 90664ca

aaneja · 2025-09-19T14:22:02Z

...kit/src/test/java/com/facebook/presto/plugin/base/security/TestSystemAccessControlStats.java

+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertNotNull;
+
+public class TestSystemAccessControlStats


This is also a test I do not want

Removed TestSystemAccessControlStats test file as requested. This class is now primarily a simple POJO with JMX annotations. Commit: 90664ca

aaneja · 2025-09-19T14:24:15Z

...est/java/com/facebook/presto/plugin/base/security/TestStatsRecordingSystemAccessControl.java

+    }
+
+    @Test
+    public void testNullInputs()


Remove this test

Removed the null input validation test as requested. The remaining tests focus on interface compliance and functionality. Commit: 90664ca

aaneja · 2025-09-19T14:24:21Z

...est/java/com/facebook/presto/plugin/base/security/TestStatsRecordingSystemAccessControl.java

+    }
+
+    @Test
+    public void testCreation()


Remove this test

Removed the creation test as requested. Kept only the essential interface compliance and functionality tests. Commit: 90664ca

...n-base/src/test/java/com/facebook/presto/security/TestStatsRecordingSystemAccessControl.java

aaneja · 2025-09-19T14:26:12Z

...rc/main/java/com/facebook/presto/plugin/base/security/StatsRecordingSystemAccessControl.java

+public final class StatsRecordingSystemAccessControl
+        implements SystemAccessControl
+{
+    private final SystemAccessControlStatsRecorder stats;


Add a getter for this, so that the TestStatsRecordingSystemAccessControl can assert on - Recorded stats are non-zero on a successful run

Added getStats() getter method that returns the inner Stats class, allowing tests to access and verify recorded metrics. The test now validates that stats are non-zero after successful runs. Commit: 90664ca

aaneja · 2025-09-19T14:26:57Z

...toolkit/src/main/java/com/facebook/presto/plugin/base/security/SystemAccessControlStats.java

+
+public class SystemAccessControlStats
+{
+    private final AtomicLong failures = new AtomicLong();


Make failures a CounterStat instead

Changed failures field from AtomicLong to CounterStat for better integration with the existing metrics infrastructure. Updated getFailures() to return the CounterStat object with @Managed @nested annotations. Commit: 90664ca

aaneja

@copilot
Move the StatsRecordingSystemAccessControl and related classes and tests to presto-main-base

Take a look at the AccessControlManager.setSystemAccessControl method. I want to wrap the systemAccessControl instantiated there. I also want to export the mbean for it at this point. Use the simple name of the delegated class as a prefix for the exported name

This should complete the wireup E2E

aaneja · 2025-09-20T07:05:51Z

...est/java/com/facebook/presto/plugin/base/security/TestStatsRecordingSystemAccessControl.java

+    }
+
+    @Test
+    public void testGetDelegate()


Actually, lets remove this. Not much here in terms of an assertion

Copilot · 2025-09-20T07:34:40Z

@copilot
Move the StatsRecordingSystemAccessControl and related classes and tests to presto-main-base

...

Moved StatsRecordingSystemAccessControl and related classes to presto-main-base and integrated with AccessControlManager.setSystemAccessControl(). The method now automatically wraps all SystemAccessControl instances with stats recording and exports the metrics via the @Managed getter getSystemAccessControlStats(). This provides complete E2E wireup - stats are automatically available via JMX for any configured SystemAccessControl without additional setup. Commit: 8954fb7

aaneja · 2025-09-22T03:21:39Z

...-main-base/src/main/java/com/facebook/presto/security/StatsRecordingSystemAccessControl.java

+            long duration = System.nanoTime() - start;
+            context.getRuntimeStats().addMetricValue("systemAccessControl.checkCanSetUser", RuntimeUnit.NANO, duration);
+            stats.checkCanSetUser.record(duration);


@copilot I've added code to also trace runtime stats. Do the same for the other methods

Added runtime stats tracing to all 37 SystemAccessControl methods following the same pattern you established. Each method now includes context.getRuntimeStats().addMetricValue("systemAccessControl.methodName", RuntimeUnit.NANO, duration) for comprehensive runtime performance tracking. Commit: 9abe4ba

…racle connector

…b#26122) ## Description We decided in the TSC meeting to use conventional commits to enforce consistent commit messages in this project. This adds an advisory Github Action to validate the PR title, and documentation for developers on the new expectation. When this PR is merged, I will begin to socialize the new requirements over Slack. After a time, we'll make the Github Action mandatory. ## Motivation and Context Consistent commit messages, alignment with Velox and other open source projects ## Impact Presto will begin to use conventional commits. ## 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. ## Release Notes ``` == NO RELEASE NOTE == ```

## Description Refactor the ClickHouse connector and remove unused code ## Motivation and Context   ## Impact  ## Test Plan  ## 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. ``` == NO RELEASE NOTE == ```

…stodb#26063) ## Description  Upgraded com.google.api-client version from 1.27.0 to 2.8.0 As part this change upgraded com.google.apis:google-api-services-sheets to 4-rev20250616-2.0.0 com.google.http-client to 2.0.0 ## Motivation and Context Using a more recent version helps avoid potential vulnerabilities and ensures we aren't relying on outdated or unsupported code. ## Impact  ## Test Plan  ## 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 com.google.api:google-api-client version to 2.8.0 in response to the use of an outdated version.

## Description Enable case sensitive support for Redis connector ## Motivation and Context ## Impact ## Test Plan ## 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 == Redis Connector Changes * Add support for case-sensitive identifiers in Redis. It can be enabled by setting ``case-sensitive-name-matching=true`` configuration in the catalog configuration ```

…prestodb#26178) ## Description Currently in Presto, the logic for column matching on ORC files which complies with Iceberg specification (meaning they record column IDs) have some issues. Based on Iceberg specification [here](https://iceberg.apache.org/spec/#column-projection), when columns IDs are present in the data files, we should match the columns strictly based on IDs. There is no need to do any name-based fallback matching logic when a column fails to match by ID, since it may introduce extra unnecessary problems. Refer to the following example: ``` CREATE TABLE test_table(a int, b varchar) with ("write.format.default" = 'ORC'); INSERT INTO test_table VALUES(1, '1001'), (2, '1002'); ALTER TABLE test_table RENAME COLUMN a to a2; SELECT * FROM test_table; ALTER TABLE test_table ADD COLUMN a varchar; SELECT * FROM test_table; ALTER TABLE test_table DROP COLUMN a; ALTER TABLE test_table ADD COLUMN a int; SELECT * FROM test_table; ``` ## Motivation and Context - Correct column mapping behavior for Iceberg with ORC files ## Impact N/A ## Test Plan - Newly added test case to show the column mapping behaviors on certain schema evolution, which would fail with ORC file before this 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. - [ ] 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). - [x] Adequate tests were added if applicable. - [x] CI passed. ## Release Notes ``` == NO RELEASE NOTE == ```

…estodb#26206)

## Description  ## Motivation and Context   ## Impact  ## Test Plan  ## 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. ``` == NO RELEASE NOTE == ```

…prestodb#26087) Summary: Add the relevant session properties that can help control and mitigate specific regressions. Notably `preferred_output_batch_bytes`, `preferred_output_batch_rows` and `max_output_batch_rows` are existing velox query configs that were just not exposed. `row_size_tracking_enabled` is to expose the newly added fallback to missing/failed average row size estimates. Differential Revision: D81291261 ``` == NO RELEASE NOTE == ``` Co-authored-by: Huameng (Michael) Jiang <[email protected]>

…db#26220) This reverts commit 993944a. ## Description Revert unexpected edge release ## Motivation and Context ## Impact The current version ## 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. ## 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 == ```

…estodb#26096) ## Description Currently, only SqlQueryExecution runs access permissions checks (authorization of the query). The other types of executions, like explain and DDL types do not run this check. ## Motivation and Context   Ensure that query authorization is checked for all types of queries. ## Impact  Authorization to run a query will be checked on DDL, explain, and session queries, not just SQL queries. ## Test Plan  Explain type validate queries check permissions in the tests. There is some complexity with EXPLAIN type queries. There are 3 major types of explain: regular explain, validate, and analyze. Before this change: regular explain calls check query integrity twice: once during analysis while generating query plan. and once during initialization of SqlQueryExecution instance. validate: called once during accessControlExecution's execute task. analyze: called once during initialization of SqlQueryExecution instance In order to ensure that regular explain only calls it once, in this PR, the check is skipped for regular explain in Analyzer.java while it is generating query plan during Explain rewrite. ## 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. ``` == NO RELEASE NOTE == ```

…5715) ## Description  Bug fix for the SQL Client's query results component, now handles null values for bigint columns without throwing errors. ## Motivation and Context   Fixes prestodb#25714 ## Impact  UI query results in SQL Client handles null bigint values ## Test Plan  1. Create table will a bigint column 2. Insert row with null value for bigint column 3. Open SQL Client in UI 4. Query table, to include the null bigint row 5. Confirm that `NULL` appears in query results table, no UI error ## 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. ## Release Notes ``` == NO RELEASE NOTE == ```

…6211) Differential Revision: D83664384 ### Release Notes ``` == NO RELEASE NOTE == ```

## Description Adds e2e tests for Presto C++ custom functions. Testcases borrowed from [AbstractTestQueries](https://github.com/prestodb/presto/blob/master/presto-tests/src/main/java/com/facebook/presto/tests/AbstractTestQueries.java) in `presto-tests`. ``` == NO RELEASE NOTE == ``` Co-authored-by: mohsaka <[email protected]> Co-authored-by: soumiiow <[email protected]>

Add to get connector factory from the deprecated velox factory pool. Will cleanup this later. ``` == NO RELEASE NOTE == ```

…odb#26232) ``` == NO RELEASE NOTE == ```

This will help users with custom SystemAccessControl (SAC) implementations get detailed metrics about latency and failures of the various SAC calls Co-authored-by: copilot-swe-agent[bot] <[email protected]> Fixes prestodb#25574

This will help users with custom SystemAccessControl (SAC) implementations get detailed metrics about latency and failures of the various SAC calls ## Motivation and Context Fixes #25574 ## Impact - Detailed stats captured as JMX metrics and per-query runtime stats - JMX metrics are under the existing `com.facebook.presto.security:name=accesscontrolmanager` bean - ```sql presto> describe jmx.current."com.facebook.presto.security:name=accesscontrolmanager"; Column | Type | Extra | Comment | Precision | Scale | Length ---------------------------------------------------------------------------------------+---------+-------+---------+-----------+-------+------------ authenticationfail.fifteenminute.count | double | | | 53 | NULL | NULL authenticationfail.fifteenminute.rate | double | | | 53 | NULL | NULL ... detailedstats.checkcanaccesscatalog.failures.fifteenminute.count | double | | | 53 | NULL | NULL detailedstats.checkcanaccesscatalog.failures.fifteenminute.rate | double | | | 53 | NULL | NULL detailedstats.checkcanaccesscatalog.failures.fiveminute.count | double | | | 53 | NULL | NULL detailedstats.checkcanaccesscatalog.failures.fiveminute.rate | double | | | 53 | NULL | NULL detailedstats.checkcanaccesscatalog.failures.oneminute.count | double | | | 53 | NULL | NULL ... ``` querying - ```sql presto> select "detailedstats.checkcanaccesscatalog.time.alltime.count" , "detailedstats.checkcanaccesscatalog.time.alltime.max" from jmx.current."com.facebook.presto.security:name=accesscontrolmanager"; detailedstats.checkcanaccesscatalog.time.alltime.count | detailedstats.checkcanaccesscatalog.time.alltime.max --------------------------------------------------------+------------------------------------------------------ 2.0 | 1.362 (1 row) ``` ## Test Plan - New unit test - Manual testing to confirm JMX and runtime stats wireup ## Other details I used the [Github co-pilot agent](aaneja#3) to help bootstrap and iterate on the branch. I wanted to test its effectiveness in helping address well-defined issues like this one. It helped avoid some repetitive work .I have attributed it as co-author on the commit ## 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. ## Release Notes ``` == RELEASE NOTES == General Changes * Add detailed latency and failure count metrics for the system access control plugin ```

…b#26529) Summary: 1. Fix an error of refresh MV after base table insertion 2. Use ExpressionUtil in MV refresh query composing 3. Fix an issue in MV caused by copying catalog properties during session creation For #3, we need a follow up to understand why the check of setting transaction id and catalog properties during session constructor is needed. I don't see a clear reason by checking the previous relavant commits. I'll check with some additional people. If no clear objection, I'll remove that check at least from the Session constructor in following PRs, and restoring the copy of catalog properties in buildOwnerSession(). Differential Revision: D86223888 ## Release Notes ``` == NO RELEASE NOTE == ```

Copilot AI assigned Copilot and aaneja Sep 19, 2025

Copilot started work on behalf of aaneja September 19, 2025 03:29 View session

Copilot AI changed the title ~~[WIP] Create a draft PR to address @prestodb/presto/issues/25574 in my fork (aaneja/presto2)~~ Add granular JMX metrics for SystemAccessControl operations Sep 19, 2025

Copilot AI requested a review from aaneja September 19, 2025 03:48

Copilot finished work on behalf of aaneja September 19, 2025 03:48

aaneja reviewed Sep 19, 2025

View reviewed changes

aaneja requested changes Sep 19, 2025

View reviewed changes

Copilot started work on behalf of aaneja September 19, 2025 11:09 View session

Copilot AI requested a review from aaneja September 19, 2025 11:20

Copilot finished work on behalf of aaneja September 19, 2025 11:20

aaneja reviewed Sep 19, 2025

View reviewed changes

Copilot started work on behalf of aaneja September 19, 2025 14:27 View session

Copilot AI requested a review from aaneja September 19, 2025 14:41

Copilot finished work on behalf of aaneja September 19, 2025 14:41

aaneja force-pushed the copilot/fix-57600240-92fd-4c02-a7a6-46e6a15daf28 branch from 90664ca to 8536990 Compare September 20, 2025 06:57

aaneja changed the base branch from master to moreQueryBuilder September 20, 2025 07:00

aaneja changed the base branch from moreQueryBuilder to master September 20, 2025 07:01

aaneja requested changes Sep 20, 2025

View reviewed changes

Copilot started work on behalf of aaneja September 20, 2025 07:20 View session

Copilot AI changed the title ~~Add granular JMX metrics for SystemAccessControl operations~~ Add granular JMX metrics for SystemAccessControl operations with automatic integration Sep 20, 2025

Copilot AI requested a review from aaneja September 20, 2025 07:35

Copilot finished work on behalf of aaneja September 20, 2025 07:35

aditi-pandit and others added 2 commits September 20, 2025 07:35

[Native] Advance Velox

4f8bff8

CI: Fix presto-on-spark native tests for PRs that only modify docs

f30a72a

aaneja reviewed Sep 22, 2025

View reviewed changes

Copilot started work on behalf of aaneja September 22, 2025 03:21 View session

Copilot AI changed the title ~~Add granular JMX metrics for SystemAccessControl operations with automatic integration~~ Add granular JMX metrics and runtime stats tracing for SystemAccessControl operations with automatic integration Sep 22, 2025

agrawalreetika and others added 23 commits October 1, 2025 09:05

Disable UpperCase conversion when case-sensitve flag is enabled for O…

dd1cf90

…racle connector

Fix resource groups docs

48f758b

[sv] Fix file based json udf tests

0ba1059

Upgrade org.jetbrains:annotations version 26.0.2

fb866da

[maven-release-plugin] prepare release 0.295

54e1578

[maven-release-plugin] prepare for next development iteration

0502695

[pos] Do not load function namespace managers on native executors (pr…

37f83a5

…estodb#26206)

fix(ci): Clear more space for Maven checks (prestodb#26216)

d297df5

Prepare for next development iteration - 0.295-edge23

993944a

feat(httpclient): Log stats for reactor netty http client (prestodb#2…

2c63235

…6211) Differential Revision: D83664384 ### Release Notes ``` == NO RELEASE NOTE == ```

fix: Fix presto connector registration failure (prestodb#26226)

5e22b9a

Add to get connector factory from the deprecated velox factory pool. Will cleanup this later. ``` == NO RELEASE NOTE == ```

refactor: Advance velox and update task creation to use disk … (prest…

3837f97

…odb#26232) ``` == NO RELEASE NOTE == ```

aaneja force-pushed the copilot/fix-57600240-92fd-4c02-a7a6-46e6a15daf28 branch 3 times, most recently from 008cc0d to 2013cb4 Compare October 13, 2025 07:25

feat(plugin): Add detailed stats for SAC calls

d3ef52e

This will help users with custom SystemAccessControl (SAC) implementations get detailed metrics about latency and failures of the various SAC calls Co-authored-by: copilot-swe-agent[bot] <[email protected]> Fixes prestodb#25574

aaneja force-pushed the copilot/fix-57600240-92fd-4c02-a7a6-46e6a15daf28 branch from 2013cb4 to d3ef52e Compare October 13, 2025 07:29

Add granular JMX metrics and runtime stats tracing for SystemAccessControl operations with automatic integration #3

Are you sure you want to change the base?

Add granular JMX metrics and runtime stats tracing for SystemAccessControl operations with automatic integration #3

Uh oh!

Conversation

Copilot AI commented Sep 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Overview

Key Components

SystemAccessControlStats

StatsRecordingSystemAccessControl

AccessControlManager Integration

Usage

Method Coverage

Performance Monitoring

JMX Metrics

Runtime Stats Tracing

Testing

Uh oh!

aaneja Sep 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 19, 2025

Choose a reason for hiding this comment

Uh oh!

aaneja left a comment

Choose a reason for hiding this comment

Uh oh!

aaneja commented Sep 19, 2025

Uh oh!

aaneja left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 19, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 19, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 19, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 19, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 19, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 19, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 19, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 19, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI commented Sep 19, 2025 •

edited

Loading

aaneja Sep 19, 2025 •

edited

Loading