Skip to content

fix(core): reject vertex-property metadata during addV - #3178

Open
1fanwang wants to merge 3 commits into
apache:masterfrom
1fanwang:fix/reject-vertex-meta-properties
Open

fix(core): reject vertex-property metadata during addV#3178
1fanwang wants to merge 3 commits into
apache:masterfrom
1fanwang:fix/reject-vertex-meta-properties

Conversation

@1fanwang

@1fanwang 1fanwang commented Aug 30, 2026

Copy link
Copy Markdown

Purpose of the PR

addV().property(key, value, metaKey, metaValue) accepts unsupported
meta-properties instead of rejecting the traversal.

Main Changes

Fold only T.key and T.value into addV, then leave metadata-bearing
AddPropertyStep instances in the traversal. If the branch executes,
HugeVertex.property() rejects the metadata. Unselected coalesce() and
choose() branches stay lazy, primary-key labels receive their key before
addV executes, and T.id keeps its specific unsupported-ID error.

Verifying these changes

  • Need tests and can be verified as follows:
Scenario Result
Upstream behavior Executed metadata traversals complete without the expected error
Final behavior Executed branches reject at runtime; unreachable branches are unchanged
Memory-backed core suite Passed on JDK 11
Raw logs
$ RED=/tmp/hg-base
$ TEST=hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/core/PrimaryKeyStrategyCoreTest.java
$ git worktree add --detach "$RED" 90035b6e6339572cca2fe1bafd5242b1c300e0ea
$ git archive cf4f1598608b074579a778ca0a5e810ff29d6ebc "$TEST" | tar -x -C "$RED"
$ cd "$RED"
$ JAVA_HOME=$(/usr/libexec/java_home -v 11) mvn test \
    -pl hugegraph-server/hugegraph-test -am -P core-test,memory \
    -Dtest=PrimaryKeyStrategyCoreTest -DfailIfNoTests=false -Drevision=1.7.0
Tests run: 8, Failures: 5, Errors: 0, Skipped: 0
Start, mid-traversal, and primary-key cases:
  No exception was thrown(expected java.lang.UnsupportedOperationException)
Unfolded-step case: assertion failed
T.id: Bad exception type java.lang.IllegalArgumentException
BUILD FAILURE

$ GREEN=/tmp/hg-green
$ git worktree add --detach "$GREEN" cf4f1598608b074579a778ca0a5e810ff29d6ebc
$ cd "$GREEN"
$ JAVA_HOME=$(/usr/libexec/java_home -v 11) mvn test \
    -pl hugegraph-server/hugegraph-test -am -P core-test,memory \
    -Dtest=PrimaryKeyStrategyCoreTest -DfailIfNoTests=false -Drevision=1.7.0
Tests run: 8, Failures: 0, Errors: 0, Skipped: 0
BUILD SUCCESS

Does this PR potentially affect the following parts?

  • Nope

Documentation Status

  • Doc - No Need

@dosubot dosubot Bot added size:L This PR changes 100-499 lines, ignoring generated files. bug Something isn't working tests Add or improve test cases labels Aug 30, 2026
@imbajin
imbajin requested a lite review from Copilot August 30, 2026 05:34

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes an incorrect traversal rewrite in HugePrimaryKeyStrategy where vertex-property metadata supplied via addV().property(key, value, metaKey, metaValue) could be folded into the vertex creation as ordinary vertex properties, despite HugeGraph not supporting meta-properties. It updates the optimization strategy to reject such metadata early and adds core tests to prevent regressions.

Changes:

  • Reject vertex-property meta-properties during HugePrimaryKeyStrategy folding (fail before any write).
  • Add regression tests covering both AddVertexStartStep and mid-traversal AddVertexStep paths.
  • Register the new test class in the core test suite.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/traversal/optimize/HugePrimaryKeyStrategy.java Rejects non-T.key/T.value parameters during property folding to prevent meta-property misinterpretation.
hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/core/PrimaryKeyStrategyCoreTest.java Adds regression tests ensuring meta-properties are rejected (and normal folding still works).
hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/core/CoreTestSuite.java Adds the new test class to the core suite so it runs in CI.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@codecov

codecov Bot commented Aug 30, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 66.66667% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 37.18%. Comparing base (90035b6) to head (cf4f159).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
...aph/traversal/optimize/HugePrimaryKeyStrategy.java 66.66% 2 Missing and 2 partials ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master    #3178      +/-   ##
============================================
+ Coverage     32.66%   37.18%   +4.51%     
- Complexity     5500     6384     +884     
============================================
  Files           789      791       +2     
  Lines         67703    67753      +50     
  Branches       8945     9027      +82     
============================================
+ Hits          22117    25195    +3078     
+ Misses        42984    39551    -3433     
- Partials       2602     3007     +405     

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

@imbajin imbajin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The core fix is directionally correct, but the eager validation changes lazy branch behavior. I also left one readability note about wrapping well before the project's 120-column limit.

Signed-off-by: 1fanwang <1fannnw@gmail.com>
Signed-off-by: 1fanwang <1fannnw@gmail.com>
Throwing from HugePrimaryKeyStrategy validated eagerly, so an unreachable
coalesce()/choose() addV() fallback failed before its branch was picked.

Fold only T.key/T.value into addV so PRIMARY_KEY labels keep their primary
key, leave a metadata-bearing AddPropertyStep in place, and stop folding
across it. HugeVertex.property() then rejects it, T.id case included.

Signed-off-by: 1fanwang <1fannnw@gmail.com>
@1fanwang
1fanwang force-pushed the fix/reject-vertex-meta-properties branch from c673826 to cf4f159 Compare August 30, 2026 09:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working size:L This PR changes 100-499 lines, ignoring generated files. tests Add or improve test cases

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] HugePrimaryKeyStrategy silently converts vertex-property metadata into vertex properties

3 participants