chore: Check materialized view metrics emitted in tests

[Sullivan-Patrick]

Description

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

We want to verify rewrites are occurring, this change confirms metrics are emitted during tests in TestHiveMaterializedViewLogicalPlanner verifying rewrites occurred.

Impact

Test Plan

Tests passing with new changes

Contributor checklist

• Please make sure your submission complies with our contributing guide, in particular code style and commit standards.
• PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced.
• Documented new properties (with its default value), SQL syntax, functions, or other functionality.
• If release notes are required, they follow the release notes guidelines.
• Adequate tests were added if applicable.
• CI passed.
• If adding new dependencies, verified they have an OpenSSF Scorecard score of 5.0 or higher (or obtained explicit TSC approval for lower scores).

Release Notes

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

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

[sourcery-ai]

Reviewer's Guide

Adds a reusable test helper that asserts materialized-view rewrites occurred by checking the OPTIMIZED_WITH_MATERIALIZED_VIEW_SUBQUERY_COUNT runtime metric, migrates existing materialized-view planner tests to use it, adds a new metric-focused test, and performs minor test cleanups/renames and formatting adjustments.

File-Level Changes

Change	Details	Files
Introduce a helper that runs a query, checks the materialized-view rewrite runtime metric, and returns the result.	Add private helper assertMaterializedViewRewriteOccurred(Session, String) that executes a query via DistributedQueryRunner.executeWithQueryId Fetch QueryInfo and RuntimeStats for the executed query and read OPTIMIZED_WITH_MATERIALIZED_VIEW_SUBQUERY_COUNT Assert the metric is present and that its sum is greater than zero, then return the query result	presto-hive/src/test/java/com/facebook/presto/hive/TestHiveMaterializedViewLogicalPlanner.java
Refactor existing materialized-view optimization tests to assert rewrites via the new helper instead of bare computeActual calls.	Replace computeActual(session, baseQuery) with assertMaterializedViewRewriteOccurred(session, baseQuery) in multiple tests that expect MV rewrites Keep base computeActual calls for non-optimized or comparison queries to validate result equality Ensure tests that previously only compared results now also validate that a rewrite actually occurred	presto-hive/src/test/java/com/facebook/presto/hive/TestHiveMaterializedViewLogicalPlanner.java
Add a dedicated test to validate that materialized-view rewrites emit the expected runtime metric.	Create testMaterializedViewRewriteRuntimeMetric that creates a partitioned base table and MV, refreshes a partition, and configures MV optimization Execute a base-table query expected to be rewritten and assert via assertMaterializedViewRewriteOccurred that the metric increments Compare the optimized result to an equivalent direct MV query to ensure correctness	presto-hive/src/test/java/com/facebook/presto/hive/TestHiveMaterializedViewLogicalPlanner.java
Adjust or remove specific tests and assertions to align with new semantics and improve clarity.	Rename testMaterializedViewOptimizationWithScalarFunctionSubquery to testMaterializedViewOptimizationWithUnsupportedFunctionSubquery and strengthen its plan assertion to explicitly verify MV-based join shape Remove testMaterializedViewPartitionKeyFilterWithDerivedColumn, which validated derived-column partition filtering behavior outside the new metric focus Tidy up string formatting/indentation in MV join tests to be consistent with code style	presto-hive/src/test/java/com/facebook/presto/hive/TestHiveMaterializedViewLogicalPlanner.java

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[sourcery-ai]

Hey - I've found 1 issue, and left some high level feedback:

• In assertMaterializedViewRewriteOccurred, consider including the executed SQL in the assertion failure messages (in addition to the query id) to make it easier to debug why a materialized view rewrite was not detected.
• The helper assertMaterializedViewRewriteOccurred looks generally useful beyond this class; consider moving it to a shared base test utility so other materialized view tests can assert on the same runtime metric consistently.
• The renamed test testMaterializedViewOptimizationWithUnsupportedFunctionSubquery still contains a comment stating that length() is now supported as a scalar function and both subqueries should be rewritten, which seems inconsistent with the new method name—clarify whether this test is meant to verify support or lack of support and align the name/comment accordingly.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- In `assertMaterializedViewRewriteOccurred`, consider including the executed SQL in the assertion failure messages (in addition to the query id) to make it easier to debug why a materialized view rewrite was not detected.
- The helper `assertMaterializedViewRewriteOccurred` looks generally useful beyond this class; consider moving it to a shared base test utility so other materialized view tests can assert on the same runtime metric consistently.
- The renamed test `testMaterializedViewOptimizationWithUnsupportedFunctionSubquery` still contains a comment stating that `length()` is now supported as a scalar function and both subqueries should be rewritten, which seems inconsistent with the new method name—clarify whether this test is meant to verify support or lack of support and align the name/comment accordingly.

## Individual Comments

### Comment 1
<location path="presto-hive/src/test/java/com/facebook/presto/hive/TestHiveMaterializedViewLogicalPlanner.java" line_range="2130-2131" />
<code_context>

     @Test
-    public void testMaterializedViewOptimizationWithScalarFunctionSubquery()
+    public void testMaterializedViewOptimizationWithUnsupportedFunctionSubquery()
     {
         Session queryOptimizationWithMaterializedView = Session.builder(getSession())
</code_context>
<issue_to_address>
**suggestion (testing):** Clarify test intent regarding "unsupported" vs "supported" function rewrites and consider adding a negative case for truly unsupported functions

The renamed test still asserts that rewrites occur for the `length()` subqueries, so it effectively covers a *supported* function scenario. If you want to keep the new name, please also add a separate test using a truly unsupported function that asserts no rewrite (and no metric increment). Otherwise, consider reverting/adjusting the name to reflect that this test covers supported-function rewrites.

```suggestion
    @Test
    public void testMaterializedViewOptimizationWithSupportedFunctionSubquery()
```
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[sourcery-ai]

suggestion (testing): Clarify test intent regarding "unsupported" vs "supported" function rewrites and consider adding a negative case for truly unsupported functions

The renamed test still asserts that rewrites occur for the length() subqueries, so it effectively covers a supported function scenario. If you want to keep the new name, please also add a separate test using a truly unsupported function that asserts no rewrite (and no metric increment). Otherwise, consider reverting/adjusting the name to reflect that this test covers supported-function rewrites.

Suggested change

	@Test
	public void testMaterializedViewOptimizationWithScalarFunctionSubquery()
	public void testMaterializedViewOptimizationWithUnsupportedFunctionSubquery()
	@Test
	public void testMaterializedViewOptimizationWithSupportedFunctionSubquery()

[github-actions]

Codenotify: Notifying subscribers in CODENOTIFY files for diff 8982f13...1b25a2a.

No notifications.

[ceekay47]

Thanks, lgtm!