[robfrank]

Closes #11552

Summary

The issue offered two options; this PR takes the first: _sourceRID is a reserved column of a per-record materialized view, and the link always wins. Measured on main before any change: the link already wins, on both the full pass and the delta pass. Two mechanisms produce that result, and no test pinned either of them:

• Projection.toString renders every * item before the other items, and the derived query (MaterializedViewPerRecordRefresher.buildDerivedQuery) is rendered and parsed again. So @rid AS _sourceRID always comes after *.
• Projection.calculateSingle writes the items in order. The link therefore overwrites the field that * copied from the record.

This PR adds a regression test for that behavior and documents the rule and its mechanism in the javadoc of buildDerivedQuery and MaterializedViewQueryClassifier.declaresSourceRid. The second javadoc used to describe the case as something "no static reading of the schema can see", with no statement of what happens at runtime. The _sourceRID value stored on the source record is not copied into the view. Choosing "fail the pass" instead would have left the whole view in ERROR because of one record, until someone removed the field.

Test plan

• Issue11552PerRecordSourceRidReservedTest (new, 6 cases): every view row is linked to its own source, carries that source's name, and the view stays VALID
  • full pass, with SELECT * FROM P, SELECT FROM P and SELECT *, name AS n FROM P, over a schemaless record whose _sourceRID is a string and one whose _sourceRID is the RID of ANOTHER source record (if that field won, two rows would claim the same source). An explicit REFRESH follows, and each row keeps its RID.
  • delta pass (insert with the field, update that adds the field), followed by a full pass that agrees with it
  • CREATE PROPERTY P._sourceRID STRING after the view exists (the classifier reads the schema at DDL time only)
• The test can fail: when Projection.calculateSingle was temporarily changed to evaluate the items in reverse (the field wins), 5 of the 6 cases failed and the sixth errored. The change was then reverted.
• Related suites green: MaterializedView*Test, Issue11470HandedOffRefreshYieldsTest, Issue11464IncrementalRefreshBoundedOnCommitterTest (154 tests including the new class)

Coverage

Invariant: every row of a per-record view carries in _sourceRID the RID of the source record it was computed from. A field of that name on the source record cannot change this.

Entry point	Status
Full pass (fullPass, view creation, REFRESH MATERIALIZED VIEW, ALTER upgrade, recovery pass): all use derivedQuery(false)	fixed-here (tested)
Delta pass (deltaPass, RID-list variant derivedQuery(true)) on insert and on update	fixed-here (tested)
Undeclared field on a schemaless record	fixed-here (tested)
Property declared after the view was created	fixed-here (tested)
Property declared before creation	already refused per-record by declaresSourceRid (unchanged, MaterializedViewPerRecordClassifierTest)
Query projecting _sourceRID by name or alias	already refused per-record by isPerRecord (unchanged)

Known gaps: none in the engine. Documentation: the per-record section in docs PR ArcadeData/arcadedb-docs#530 does not state the reserved-name rule yet. The suggested wording is left as a comment on that PR.

Notes

• A declared _sourceRID property still keeps the view on full refresh. An undeclared field gets the link instead. The classifier is deliberately conservative here, and this PR does not change it.
• Side observation, not changed: SELECT name, * FROM X is a syntax error in the ANTLR grammar (* must come first). MaterializedViewPerRecordClassifierTest.wholeRecordProjectionOfATypeDeclaringTheReservedNameIsRejected lists it among the rejected queries. That case therefore passes because of the parse error, not because of the guard.

Adversarial pass

No Task/Agent tool was available in this run, so the Phase 1.5 independent subagent could not be spawned. I did the self-check below instead.

• "The rule depends on rendering order, which is implicit." This is real, and the test is what pins it: the mutation above shows that reversing the evaluation order fails the build.
• Field names that differ only in case (_SOURCERID): property names are case-sensitive, so there is no collision in the row. Not a defect.

Review cycles

• cycle 1: 20521a8925. The claude review said LGTM, with one suggestion: add a comment at the Projection ordering that points at the test. Applied in 3d07d074df (comments at Projection.toString and Projection.calculateSingle).
• cycle 2: 3d07d074df. The claude review said LGTM, with no actionable items.

Deferred items

• Skipped (nitpick): "The comment in Projection points at a test by name. That is acceptable, but it will go stale if the test is renamed." I kept it: the reviewer in cycle 1 asked for exactly that pointer.

Final state: clean-approval

🤖 Generated with Claude Code

Summary by CodeRabbit

• Documentation
  • Clarified that per-record materialized views use _sourceRID to link each view row to its source record. If a source record contains a field with that name, the source link takes precedence.
• Tests
  • Added regression coverage for whole-record projections, inserts, updates, and full refreshes involving source records with an _sourceRID field.

[mergify]

This pull request is merged.

[claude]

Review: this is a test and javadoc change only. I did not run the tests locally.

• The new test pins the "link wins" rule across the full pass, the delta pass and a property declared after the view exists. It uses the project's assertThat style.
• The javadoc now states the rendering-order mechanism that the rule depends on.
• One small suggestion: the rule depends on * being rendered before the other items. A one-line comment at Projection.calculateSingle or in the Projection.toString rendering code pointing at this test would help whoever touches that ordering next.
• No security concerns.

LGTM.

[coderabbitai]

No actionable comments were generated in the recent review.

Walkthrough

The change documents that a per-record materialized view’s _sourceRID link takes precedence over a same-named field copied from a source record. Regression tests cover initial population, incremental inserts and updates, full refreshes, and a property declared after view creation.

Changes: Per-record source link behavior

Layer / File(s)	Summary
Reserved field contract and regression coverage engine/src/main/java/com/arcadedb/query/sql/parser/Projection.java, engine/src/main/java/com/arcadedb/schema/MaterializedViewPerRecordRefresher.java, engine/src/main/java/com/arcadedb/schema/MaterializedViewQueryClassifier.java, engine/src/test/java/com/arcadedb/schema/Issue11552PerRecordSourceRidReservedTest.java	Comments document projection ordering and the reserved _sourceRID link behavior. Regression tests cover full and incremental refreshes, including source fields added after view creation.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Other · Severity of issue fixed: Low

Suggested labels: engine, sql

Suggested reviewers: lvca

Merge Risk: Minimal · up to 3d07d

This change documents that the per-record view's source link takes precedence over a same-named field, and adds regression tests. It does not change runtime behavior, so merge risk is minimal.

Pre-merge checks: 4

Check name	Status	Explanation
Title check		The title names the change, the issue, and the rule (_sourceRID is reserved; the link wins), which matches the code, the javadoc updates, and the regression test. It is somewhat long at 100 characters…
Description check		The description follows the template's sections: What the PR does, Motivation (via the Summary and the two-option framing), Related issues (Closes #11552), Additional Notes (Notes section), and a Chec…
Linked Issues check		Issue #11552 offers two options. The PR takes the first: _sourceRID is reserved on per-record views, the link always wins, and the rule is documented. The reviewed head contains a new test, `Issue11…
Out of Scope Changes check		All changed files relate to issue #11552. Projection.java gets comments that document the ordering dependency the test relies on. MaterializedViewPerRecordRefresher.buildDerivedQuery and `Material…

Full details: Title check

Explanation

The title names the change, the issue, and the rule (_sourceRID is reserved; the link wins), which matches the code, the javadoc updates, and the regression test. It is somewhat long at 100 characters and uses the term 'per-record materialized view' without a prefix hint, but it reflects the main change.

Full details: Description check

Explanation

The description follows the template's sections: What the PR does, Motivation (via the Summary and the two-option framing), Related issues (Closes #11552), Additional Notes (Notes section), and a Checklist. The Test plan and Coverage sections add useful detail. The template's 'What does this PR do?' and 'Motivation' headings are not used as separate headings, and the checklist items are not explicitly marked for the mvn clean package build, though the test plan reports related suites passing. These gaps are minor.

Full details: Linked Issues check

Explanation

Issue #11552 offers two options. The PR takes the first: _sourceRID is reserved on per-record views, the link always wins, and the rule is documented. The reviewed head contains a new test, Issue11552PerRecordSourceRidReservedTest, which covers full passes (SELECT * FROM P, SELECT FROM P, SELECT *, name AS n FROM P), delta passes on insert and update, a full REFRESH, and a _sourceRID property declared after view creation. Each case asserts that every row links to its own source, carries that source's name, and that the view is VALID. The behavior is pre-existing. The PR adds no executable logic. The diff to Projection.java only adds comments on the toString ordering of * items and on in-order evaluation in calculateSingle. The javadoc of buildDerivedQuery and MaterializedViewQueryClassifier.isPerRecord documents the rule. The docs repository update (ArcadeData/arcadedb-docs#530) lies outside this repository. It is a documentation follow-up, and this assessment does not treat it as a coding requirement. I did not run the test suite. The PASS rests on reading the test source and the diff. The claimed mutation result (5 failures, 1 error) is not independently verified.

Full details: Out of Scope Changes check

Explanation

All changed files relate to issue #11552. Projection.java gets comments that document the ordering dependency the test relies on. MaterializedViewPerRecordRefresher.buildDerivedQuery and MaterializedViewQueryClassifier.isPerRecord get javadoc that states the reserved-name rule. The new test class pins that rule. The PR description states that the classifier behavior is unchanged. The diff to Projection.java contains no logic change, so no unrelated behavior change appears in the reviewed evidence.

Finishing Touches

Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

Share

• X
• Mastodon
• Reddit
• LinkedIn

Recent review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: f8ff90a8-edab-4e50-87c9-e1b8abcab928

Commits

Reviewing files that changed from the base of the PR and between 95a703e and 3d07d07.

Files selected for processing: 4

• engine/src/main/java/com/arcadedb/query/sql/parser/Projection.java
• engine/src/main/java/com/arcadedb/schema/MaterializedViewPerRecordRefresher.java
• engine/src/main/java/com/arcadedb/schema/MaterializedViewQueryClassifier.java
• engine/src/test/java/com/arcadedb/schema/Issue11552PerRecordSourceRidReservedTest.java

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 2 remain after this review.

[codacy-production]

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 0 complexity

Metric	Results
Complexity	0

View in Codacy

🟢 Coverage ∅ diff coverage · -4.39% coverage variation

Metric	Results
Coverage variation	✅ -4.39% coverage variation
Diff coverage	✅ ∅ diff coverage

View coverage diff in Codacy

Coverage variation details

	Coverable lines	Covered lines	Coverage
Common ancestor commit (95a703e)	282919	249171	88.07%
Head commit (3d07d07)	316518 (+33599)	264869 (+15698)	83.68% (-4.39%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details

	Coverable lines	Covered lines	Diff coverage
Pull request (#11554)	0	0	∅ (not applicable)

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

_{NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.}

[claude]

Review: LGTM. This PR only adds comments and javadoc to main code, plus a regression test that covers the full pass, the delta pass and a property declared after the view was created. The test also reads the stored rows back, which pins the actual behavior. I found no correctness, performance or security concerns, and no new dependencies.

Minor notes:

• The test does not use @Tag, which is fine because it is fast.
• The comment in Projection points at a test by name. That is acceptable, but it will go stale if the test is renamed.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.01%. Comparing base (95a703e) to head (3d07d07).

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #11554      +/-   ##
==========================================
- Coverage   80.02%   80.01%   -0.02%     
==========================================
  Files        2461     2461              
  Lines      282919   282919              
  Branches    62204    62204              
==========================================
- Hits       226400   226368      -32     
- Misses      33790    33813      +23     
- Partials    22729    22738       +9     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.