Detect materialized views with non-deterministic functions as stale by hashhar · Pull Request #28682 · trinodb/trino

hashhar · 2026-03-17T05:37:16Z

Description

Materialized views using non-deterministic scalar functions like current_timestamp, current_date, or random() were not detected as stale because the refresh only tracks table function dependencies.

Add a hasNonDeterministicFunctions flag that propagates through the refresh. When set the MV freshness is reported as UNKNOWN, which causes re-execution after the grace period expires to match the existing behavior for table functions.

It covers both current-time AST nodes (CurrentDate, CurrentTime, CurrentTimestamp, LocalTime, LocalTimestamp) and non-deterministic functions via analysis.getResolvedFunctions().

Additional context and related issues

Release notes

( ) This is not user-visible or is docs only, and no release notes are required.
(x) Release notes are required. Please propose a release note for me.
( ) Release notes are required, with the following suggested text:

## Section
* Fix some things. ({issue}`issuenumber`)

Submitted from repo to ensure all tests are run.

findepi · 2026-03-17T07:09:57Z

Materialized views using non-deterministic scalar functions like current_timestamp, current_date, or random() were not detected as stale because the refresh only tracks table function dependencies.

This is not necessarily a bug.

There is some usefulness to having e.g. current_timestamp AS refresh_time column in a MV or using random() for sampling the source.

If we consider every MV with such functions to be stale, there is no point in producing materialization. It should never be used¹. So we should rather forbid creation of such MV.

cc @martint

except during grace period, but that seems rather odd to me. ↩

hashhar · 2026-03-17T07:15:57Z

that might make sense, I think at a minimum the 2nd commit needs to be there to avoid incorrectly computing an incremental refresh.

findepi · 2026-03-17T07:27:54Z

that might make sense, I think at a minimum the 2nd commit needs to be there to avoid incorrectly computing an incremental refresh.

That matches my intuition too, but I'd rather clarify desired semantics first, before making the final call.

hashhar · 2026-03-17T07:37:31Z

Sure, I'll discuss with @martint

findepi · 2026-03-17T17:16:39Z

Fixes #22533, #28696

this will close only the first of two
use - to make it a list. GH renders link previews within list items.

piotrrzysko · 2026-03-18T07:26:28Z

Fixes #22533, #28696

this will close only the first of two

I think this PR addresses both. We should create a third issue to track optimization for incremental refreshes -- detecting whether materialized data is valid despite non-deterministic expressions.

hashhar · 2026-03-18T07:30:13Z

Fixes #22533, #28696

this will close only the first of two

I think this PR addresses both. We should create a third issue to track optimization for incremental refreshes -- detecting whether materialized data is valid despite non-deterministic expressions.

see #28731

hashhar · 2026-03-18T08:18:18Z

@martint Looks like a pre-existing bug too about session-scoped things.

I see that current_user (and other session-scoped expressions) are stored as literal SQL text in the MV definition (

trino/core/trino-main/src/main/java/io/trino/execution/CreateMaterializedViewTask.java

Line 118 in 6b4690e

String sql = getFormattedSql(statement.getQuery(), sqlParser);

), not
replaced with constants at creation time. This creates an inconsistency in what they resolve to depending on the code path:

FRESH query path reads directly from the storage table so current_user returns
whatever value was materialized during the last refresh without any re-evaluation.
STALE inline path goes through createScopeForMaterializedView, passes view.getRunAsIdentity() (the MV owner)
through to analyzeView which creates a
viewSession with the owner's identity. So
current_user resolves to the MV owner.
REFRESH path uses visitRefreshMaterializedView, re-parses the original SQL but calls process(query, scope) directly and no viewSession with the owner's identity is created, so current_user resolves to the refreshing user.

The refresh path is the wrong one. It probably should also create a view session with the owner's identity (like the inline path does).

findepi · 2026-03-18T08:20:13Z

I think this PR addresses both.

but still doesn't close both, per #28682 (comment)

hashhar · 2026-03-18T08:57:39Z

I extracted the REFRESH and session-scoped issues into #28738, WILL NOT ADDRESS them as part of this PR. This PR is focused on the correctness issue + staleness logic.

martint · 2026-03-26T04:16:47Z

The planner changes look good. I haven't reviewed the tests.

Materialized views using non-deterministic scalar functions like current_timestamp, current_date, or random() were not detected as stale because the refresh only tracks table function dependencies. Add a hasNonDeterministicFunctions flag that propagates through the refresh. When set the MV freshness is reported as UNKNOWN, which causes re-execution after the grace period expires to match the existing behavior for table functions. It covers both current-time AST nodes (CurrentDate, CurrentTime, CurrentTimestamp, LocalTime, LocalTimestamp) and non-deterministic functions via analysis.getResolvedFunctions().

Incremental refresh only scans newly appended rows from the source table. For MVs with non-deterministic functions like current_timestamp in time-based predicates this means old rows that fall outside a shifted time window are never removed. We now force full refresh when non-deterministic functions are detected.

hashhar · 2026-03-30T13:45:02Z

Rebased to resolved conflicts. @findepi / @martint can you PTAL at the tests so that we can merge this and move on to the DEFINER semantics bug.

findepi

Skimmed the tests, lgtm.
Solid test coverage, good job! I have not verified every single assertion though.

findepi · 2026-03-30T15:26:43Z

+        // TODO https://github.com/trinodb/trino/issues/28738 session-scoped expressions should resolve using
+        // the MV owner's identity during refresh (like the stale inline path does via analyzeView), but currently
+        // the refresh path resolves them from the refreshing user's session.
+        // When fixed, this should be: VALUES ('user'), ('user')
+        assertQuery("SELECT created_by FROM " + mvName, "VALUES ('user'), ('other_user')");


( MV REFRESH resolves session-scoped expressions from refreshing user instead of MV owner #28738 )

hashhar · 2026-03-30T16:41:11Z

hit #23537 before retries

cla-bot bot added the cla-signed label Mar 17, 2026

github-actions bot added iceberg Iceberg connector lakehouse labels Mar 17, 2026

hashhar requested review from alexjo2144, findepi and piotrrzysko March 17, 2026 05:37

hashhar commented Mar 17, 2026

View reviewed changes

Comment thread core/trino-main/src/main/java/io/trino/sql/planner/LogicalPlanner.java Outdated

hashhar force-pushed the user/hashhar/fix-mv-nondeterminism branch from d7249e7 to fd3e300 Compare March 17, 2026 07:05

hashhar mentioned this pull request Mar 17, 2026

Materialized view incremental refresh produces incorrect results with non-deterministic functions #28696

Closed

findepi reviewed Mar 17, 2026

View reviewed changes

Comment thread plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMetadata.java

findepi reviewed Mar 17, 2026

View reviewed changes

Comment thread core/trino-main/src/main/java/io/trino/sql/planner/LogicalPlanner.java

findepi reviewed Mar 17, 2026

View reviewed changes

Comment thread core/trino-main/src/main/java/io/trino/sql/planner/LogicalPlanner.java

hashhar force-pushed the user/hashhar/fix-mv-nondeterminism branch 2 times, most recently from d5ca28c to 5404a6e Compare March 18, 2026 09:27

hashhar requested a review from martint March 18, 2026 19:02

hashhar added 3 commits March 30, 2026 19:13

Add test showcasing session-scoped expression issues in MV refresh

081e637

hashhar force-pushed the user/hashhar/fix-mv-nondeterminism branch from 5404a6e to 081e637 Compare March 30, 2026 13:44

findepi approved these changes Mar 30, 2026

View reviewed changes

hashhar merged commit 04e1836 into master Mar 30, 2026
212 of 215 checks passed

hashhar deleted the user/hashhar/fix-mv-nondeterminism branch March 30, 2026 17:22

github-actions bot added this to the 481 milestone Mar 30, 2026

hashhar mentioned this pull request Mar 30, 2026

Remove incorrect revapi exclusions #28934

Merged

ebyhr mentioned this pull request Mar 31, 2026

Add 481 release notes #28894

Open

Conversation

hashhar commented Mar 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Additional context and related issues

Release notes

Uh oh!

Uh oh!

findepi commented Mar 17, 2026

Footnotes

Uh oh!

hashhar commented Mar 17, 2026

Uh oh!

findepi commented Mar 17, 2026

Uh oh!

hashhar commented Mar 17, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

findepi commented Mar 17, 2026

Uh oh!

piotrrzysko commented Mar 18, 2026

Uh oh!

hashhar commented Mar 18, 2026

Uh oh!

hashhar commented Mar 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

findepi commented Mar 18, 2026

Uh oh!

hashhar commented Mar 18, 2026

Uh oh!

martint commented Mar 26, 2026

Uh oh!

hashhar commented Mar 30, 2026

Uh oh!

findepi left a comment

Choose a reason for hiding this comment

Uh oh!

findepi Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

hashhar commented Mar 30, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

hashhar commented Mar 17, 2026 •

edited

Loading

hashhar commented Mar 18, 2026 •

edited

Loading