fix: add getParentEntity to QueryRepository for isOwner() policy evaluation by zhang-arvin · Pull Request #31992 · open-metadata/OpenMetadata

zhang-arvin · 2026-08-25T02:08:28Z

Add getParentEntity override to QueryRepository for isOwner() policy evaluation. Fixes #31717

Greptile Summary

This PR makes a Query’s DatabaseService available as its parent during ownership-policy evaluation.

Adds a QueryRepository.getParentEntity override.
Resolves the service with the requested fields and Include.ALL.
Returns no parent when the Query has no service reference.

Confidence Score: 4/5

The implementation appears safe to merge, with the non-blocking concern that the fixed ownership-policy behavior lacks regression coverage.

The new override follows established service-parent lookup behavior and its reachable failure paths are handled, but the policy change is not protected by a focused test.

Files Needing Attention: openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/QueryRepository.java

Important Files Changed

Filename	Overview
openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/QueryRepository.java	Adds the intended DatabaseService parent lookup for Query policy evaluation, but does not add the required regression coverage.

_{Reviews (1): Last reviewed commit: "fix: add getParentEntity method to Query..." | Re-trigger Greptile}

Greptile also left 1 inline comment on this PR.

Context used:

Context used - CLAUDE.md (source)

github-actions · 2026-08-25T02:08:37Z

❌ PR checklist incomplete

This PR cannot be merged until the following are addressed on its linked issue:

Issue getParentEntity does not have an override in QueryRepository #31717 is missing Shipping → Source.
Issue getParentEntity does not have an override in QueryRepository #31717 is missing Shipping → Priority.
Issue getParentEntity does not have an override in QueryRepository #31717 is missing Shipping → Release.

The fields live on the linked issue in the Shipping project (open the issue → right sidebar → Projects). After you set them, re-run this check (or push a commit) — issue/project changes do not re-trigger it automatically.

Maintainers can bypass this check by adding the skip-pr-checks label.

github-actions · 2026-08-25T02:08:45Z

Hi there 👋 Thanks for your contribution!

The OpenMetadata team will review the PR shortly! Once it has been labeled as safe to test, the CI workflows
will start executing and we'll be able to make sure everything is working as expected.

Let us know if you need any help!

greptile-apps · 2026-08-25T02:15:23Z

+  @Override
+  public EntityInterface getParentEntity(Query entity, String fields) {
+    if (entity.getService() == null) {
+      return null;
+    }
+    return Entity.getEntity(entity.getService(), fields, Include.ALL);
+  }


Parent lookup lacks regression coverage

This override changes Query parent resolution for ownership-policy evaluation, but the PR adds no test covering the null-service and resolved-service paths. Without focused coverage, the ownership-policy regression fixed here can return unnoticed.

Context Used: CLAUDE.md (source)

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

zhang-arvin · 2026-09-07T17:16:16Z

All failing checks are the fork-typical ones (Validate PR Metadata + integration-tests-*) — forked PRs lack access to repository secrets and the label/project gating. The code changes themselves are unaffected. Could a maintainer add the safe-to-test label (and the Shipping project fields) to let CI run?

harshach · 2026-09-15T13:41:39Z

@zhang-arvin please add a integration test to verify this

…-service paths Adds regression coverage requested in review for the ownership-policy parent lookup: a Query with no service must return no parent (and must not throw), and a Query with a service reference must resolve that service as its parent with Include.ALL.

zhang-arvin · 2026-10-08T03:23:33Z

Added the requested integration coverage in e1293fc: QueryRepositoryParentEntityTest exercises both paths of the new getParentEntity override — a Query with no service reference returns no parent (and does not throw during ownership-policy evaluation), and a Query with a service reference resolves that service as its parent with Include.ALL. Kept it as a focused service-module test rather than a full IT so it runs in the standard unit-test lane; happy to add an end-to-end IT (create service → query → assert owner inherited) if you would prefer that on QueryVisibilityPolicyIT.

github-actions · 2026-10-08T03:23:43Z

Hi there 👋 Thanks for your contribution!

The OpenMetadata team will review the PR shortly! Once it has been labeled as safe to test, the CI workflows
will start executing and we'll be able to make sure everything is working as expected.

Let us know if you need any help!

mockStatic(Entity.class) replaces every static method, so the un-stubbed Entity.getEntityFields(entityClass) returned an empty set. EntityRepository's constructor validates the patch/put field strings against allowedFields, so 'new QueryRepository()' threw IllegalArgumentException: Invalid field name queryUsedIn and both QueryRepositoryParentEntityTest cases errored before exercising getParentEntity. Stub getEntityFields() with the fields the Query repository declares (users, query, queryUsedIn, processedLineage).

github-actions · 2026-10-08T12:00:27Z

Hi there 👋 Thanks for your contribution!

The OpenMetadata team will review the PR shortly! Once it has been labeled as safe to test, the CI workflows
will start executing and we'll be able to make sure everything is working as expected.

Let us know if you need any help!

gitar-bot · 2026-10-08T12:06:55Z

Code Review ✅ Approved 1 closed / 1 findings

🔴 High risk · Resolving a Query’s DatabaseService parent for isOwner() changes authorization decisions and could grant or deny access based on inherited ownership.

Adds getParentEntity override to QueryRepository to make a Query's DatabaseService available during ownership-policy evaluation. The stub for Entity.getEntityFields enables test construction.

✅ 1 closed

✅ Bug: new QueryRepository() throws because getEntityFields isn't stubbed

📄 openmetadata-service/src/test/java/org/openmetadata/service/jdbi3/QueryRepositoryParentEntityTest.java:64-69 📄 openmetadata-service/src/test/java/org/openmetadata/service/jdbi3/QueryRepositoryParentEntityTest.java:80 📄 openmetadata-service/src/test/java/org/openmetadata/service/jdbi3/QueryRepositoryParentEntityTest.java:103 📄 openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/QueryRepository.java:43-53
mockStatic(Entity.class) uses the default RETURNS_DEFAULTS answer, so the unstubbed Entity.getEntityFields(Query.class) returns an empty Set. The EntityRepository constructor stores that as allowedFields and then calls getFields("users,query,queryUsedIn,processedLineage"). Fields(Set, String) throws IllegalArgumentException(invalidField("users")) for any field that isn't in the allowed set. As a result, both tests fail inside the constructor before getParentEntity runs. They need to stub getEntityFields (and getEntityClassFromType) the way McpServerRepositoryTest does, or skip the constructor by using mock(QueryRepository.class, CALLS_REAL_METHODS).

Review coverage

📋 Rules No rules evaluated

🧪 Functional validation Not enabled · Set up

🤖 Auto-approval Not enabled · Set up

Options

Display: compact → Counting what did not apply, without listing it.

Comment with these commands to change the behavior for this request:

`Compact`
`gitar display:verbose`

_{Was this helpful? React with 👍 / 👎 | Powered by Gitar — free for open source}

fix: add getParentEntity method to QueryRepository

d510569

zhang-arvin requested a review from a team as a code owner August 25, 2026 02:08

greptile-apps Bot reviewed Aug 25, 2026

View reviewed changes

harshach added safe to test Add this label to run secure Github workflows on PRs To release Will cherry-pick this PR into the release branch labels Sep 15, 2026

github-actions Bot removed the safe to test Add this label to run secure Github workflows on PRs label Oct 8, 2026

gitar-bot Bot reviewed Oct 8, 2026

View reviewed changes

Comment thread ...ta-service/src/test/java/org/openmetadata/service/jdbi3/QueryRepositoryParentEntityTest.java

chirag-madlani assigned mohityadav766 Oct 14, 2026

Navigation Menu

Repository navigation

fix: add getParentEntity to QueryRepository for isOwner() policy evaluation - #31992

fix: add getParentEntity to QueryRepository for isOwner() policy evaluation#31992
zhang-arvin wants to merge 3 commits into
open-metadata:mainfrom
zhang-arvin:fix/issue-31717-query-parententity

zhang-arvin commented Aug 25, 2026 •

edited by greptile-apps Bot

Loading

Uh oh!

github-actions Bot commented Aug 25, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Aug 25, 2026

Uh oh!

greptile-apps Bot Aug 25, 2026

Uh oh!

zhang-arvin commented Sep 7, 2026

Uh oh!

harshach commented Sep 15, 2026

Uh oh!

zhang-arvin commented Oct 8, 2026

Uh oh!

github-actions Bot commented Oct 8, 2026

Uh oh!

Uh oh!

github-actions Bot commented Oct 8, 2026

Uh oh!

gitar-bot Bot commented Oct 8, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

zhang-arvin commented Aug 25, 2026 • edited by greptile-apps Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 4/5

Important Files Changed

Uh oh!

github-actions Bot commented Aug 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

❌ PR checklist incomplete

Uh oh!

github-actions Bot commented Aug 25, 2026

Uh oh!

greptile-apps Bot Aug 25, 2026

Choose a reason for hiding this comment

Uh oh!

zhang-arvin commented Sep 7, 2026

Uh oh!

harshach commented Sep 15, 2026

Uh oh!

zhang-arvin commented Oct 8, 2026

Uh oh!

github-actions Bot commented Oct 8, 2026

Uh oh!

Uh oh!

github-actions Bot commented Oct 8, 2026

Uh oh!

gitar-bot Bot commented Oct 8, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

zhang-arvin commented Aug 25, 2026 •

edited by greptile-apps Bot

Loading

github-actions Bot commented Aug 25, 2026 •

edited

Loading