Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

H-3537: Add interface to read resolved entity type schemas #5594

Merged
merged 6 commits into from
Nov 7, 2024

Conversation

TimDiekmann
Copy link
Member

@TimDiekmann TimDiekmann commented Nov 6, 2024

🌟 What is the purpose of this PR?

When querying entities or entity types it's often desired to have all information combined, which includes the direct link/link destination metadata, all linked property types and the closed data types.
In theory, this is possible with the subgraph, but:

  • The shape of the subgraph does not support other structs than the BP schemas
  • Only using the schemas implies that a lot of information needs to be recalculated every time
  • The subgraph has an unnecessary big footprint
  • It's not possible to query a subgraph which only reads properties from the current entity while returning link entities as well

🔍 What does this change?

  • Changes includeClosed: bool to includeEntityTypes: "closed" | "resolved"
  • Returns a definition table alongside the returned closed entity types
  • Improves handling of external references in OpenAPI

Pre-Merge Checklist 🚀

🚢 Has this modified a publishable library?

This PR:

  • does not modify any publishable blocks or libraries, or modifications do not need publishing

📜 Does this require a change to the docs?

The changes in this PR:

  • are internal and do not require a docs change

🕸️ Does this require a change to the Turbo Graph?

The changes in this PR:

  • do not affect the execution graph

⚠️ Known issues

To read property types currently internally the subgraph is used. This can be optimized in performance (which still needs to be solved regarding permissions) and is limited to "only" 255 deep property types.

🐾 Next steps

🛡 What tests cover this?

  • Two tests were extended to check the defintitions as well.

@github-actions github-actions bot added area/apps > hash* Affects HASH (a `hash-*` app) area/apps > hash-api Affects the HASH API (app) area/libs Relates to first-party libraries/crates/packages (area) type/eng > backend Owned by the @backend team area/tests New or updated tests area/tests > integration New or updated integration tests area/apps area/apps > hash-graph labels Nov 6, 2024
@TimDiekmann TimDiekmann force-pushed the t/h-3537-allow-querying-of-resolved-entity-types branch from 2c0a119 to 9e9fb8b Compare November 6, 2024 12:15
@TimDiekmann TimDiekmann changed the base branch from main to t/h-3543-allow-querying-for-closed-entity-types-alongside-the November 6, 2024 12:15
@TimDiekmann TimDiekmann force-pushed the t/h-3537-allow-querying-of-resolved-entity-types branch from 9e9fb8b to 658f3cd Compare November 6, 2024 12:15
Copy link

codecov bot commented Nov 6, 2024

Codecov Report

Attention: Patch coverage is 0% with 4 lines in your changes missing coverage. Please review.

Project coverage is 20.08%. Comparing base (76bc7bd) to head (d36564a).
Report is 673 commits behind head on main.

Files with missing lines Patch % Lines
...sh-api/src/graph/ontology/primitive/entity-type.ts 0.00% 2 Missing ⚠️
...ocal/hash-isomorphic-utils/src/subgraph-mapping.ts 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5594      +/-   ##
==========================================
- Coverage   20.08%   20.08%   -0.01%     
==========================================
  Files         514      514              
  Lines       17372    17375       +3     
  Branches     2545     2546       +1     
==========================================
  Hits         3490     3490              
- Misses      13844    13847       +3     
  Partials       38       38              
Flag Coverage Δ
apps.hash-ai-worker-ts 1.38% <ø> (ø)
apps.hash-api 1.17% <0.00%> (-0.01%) ⬇️
local.hash-backend-utils 8.80% <ø> (ø)
local.hash-isomorphic-utils 1.05% <0.00%> (-0.01%) ⬇️
local.hash-subgraph 24.54% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

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

CiaranMn
CiaranMn previously approved these changes Nov 7, 2024
Base automatically changed from t/h-3543-allow-querying-for-closed-entity-types-alongside-the to main November 7, 2024 10:01
@TimDiekmann TimDiekmann dismissed CiaranMn’s stale review November 7, 2024 10:01

The base branch was changed.

Copy link
Contributor

github-actions bot commented Nov 7, 2024

Benchmark results

@rust/graph-benches – Integrations

scaling_read_entity_complete_one_depth

Function Value Mean Flame graphs
entity_by_id 25 entities $$77.0 \mathrm{ms} \pm 307 \mathrm{μs}\left({\color{gray}1.52 \mathrm{\%}}\right) $$ Flame Graph
entity_by_id 5 entities $$25.0 \mathrm{ms} \pm 261 \mathrm{μs}\left({\color{gray}0.263 \mathrm{\%}}\right) $$ Flame Graph
entity_by_id 1 entities $$20.0 \mathrm{ms} \pm 86.3 \mathrm{μs}\left({\color{gray}0.722 \mathrm{\%}}\right) $$ Flame Graph
entity_by_id 10 entities $$51.2 \mathrm{ms} \pm 167 \mathrm{μs}\left({\color{red}61.8 \mathrm{\%}}\right) $$ Flame Graph
entity_by_id 50 entities $$563 \mathrm{ms} \pm 4.55 \mathrm{ms}\left({\color{red}107 \mathrm{\%}}\right) $$ Flame Graph

representative_read_entity

Function Value Mean Flame graphs
entity_by_id entity type ID: https://blockprotocol.org/@alice/types/entity-type/organization/v/1 $$17.7 \mathrm{ms} \pm 208 \mathrm{μs}\left({\color{gray}0.566 \mathrm{\%}}\right) $$ Flame Graph
entity_by_id entity type ID: https://blockprotocol.org/@alice/types/entity-type/playlist/v/1 $$17.6 \mathrm{ms} \pm 250 \mathrm{μs}\left({\color{gray}-1.560 \mathrm{\%}}\right) $$ Flame Graph
entity_by_id entity type ID: https://blockprotocol.org/@alice/types/entity-type/song/v/1 $$16.5 \mathrm{ms} \pm 221 \mathrm{μs}\left({\color{lightgreen}-7.371 \mathrm{\%}}\right) $$ Flame Graph
entity_by_id entity type ID: https://blockprotocol.org/@alice/types/entity-type/page/v/2 $$16.5 \mathrm{ms} \pm 204 \mathrm{μs}\left({\color{lightgreen}-6.028 \mathrm{\%}}\right) $$ Flame Graph
entity_by_id entity type ID: https://blockprotocol.org/@alice/types/entity-type/block/v/1 $$17.5 \mathrm{ms} \pm 223 \mathrm{μs}\left({\color{gray}-1.066 \mathrm{\%}}\right) $$ Flame Graph
entity_by_id entity type ID: https://blockprotocol.org/@alice/types/entity-type/building/v/1 $$17.1 \mathrm{ms} \pm 205 \mathrm{μs}\left({\color{gray}-1.656 \mathrm{\%}}\right) $$ Flame Graph
entity_by_id entity type ID: https://blockprotocol.org/@alice/types/entity-type/book/v/1 $$17.1 \mathrm{ms} \pm 233 \mathrm{μs}\left({\color{gray}-3.316 \mathrm{\%}}\right) $$ Flame Graph
entity_by_id entity type ID: https://blockprotocol.org/@alice/types/entity-type/person/v/1 $$16.9 \mathrm{ms} \pm 230 \mathrm{μs}\left({\color{gray}-1.417 \mathrm{\%}}\right) $$ Flame Graph
entity_by_id entity type ID: https://blockprotocol.org/@alice/types/entity-type/uk-address/v/1 $$16.7 \mathrm{ms} \pm 145 \mathrm{μs}\left({\color{lightgreen}-9.213 \mathrm{\%}}\right) $$ Flame Graph

scaling_read_entity_complete_zero_depth

Function Value Mean Flame graphs
entity_by_id 25 entities $$3.35 \mathrm{ms} \pm 17.7 \mathrm{μs}\left({\color{red}22.2 \mathrm{\%}}\right) $$ Flame Graph
entity_by_id 5 entities $$1.94 \mathrm{ms} \pm 9.11 \mathrm{μs}\left({\color{gray}0.466 \mathrm{\%}}\right) $$ Flame Graph
entity_by_id 1 entities $$1.86 \mathrm{ms} \pm 10.2 \mathrm{μs}\left({\color{gray}1.18 \mathrm{\%}}\right) $$ Flame Graph
entity_by_id 10 entities $$2.11 \mathrm{ms} \pm 11.4 \mathrm{μs}\left({\color{gray}1.25 \mathrm{\%}}\right) $$ Flame Graph
entity_by_id 50 entities $$4.23 \mathrm{ms} \pm 15.0 \mathrm{μs}\left({\color{lightgreen}-19.807 \mathrm{\%}}\right) $$ Flame Graph

scaling_read_entity_linkless

Function Value Mean Flame graphs
entity_by_id 1 entities $$1.84 \mathrm{ms} \pm 7.47 \mathrm{μs}\left({\color{gray}-2.628 \mathrm{\%}}\right) $$ Flame Graph
entity_by_id 10000 entities $$14.0 \mathrm{ms} \pm 89.4 \mathrm{μs}\left({\color{gray}1.69 \mathrm{\%}}\right) $$ Flame Graph
entity_by_id 100 entities $$2.06 \mathrm{ms} \pm 10.2 \mathrm{μs}\left({\color{red}6.04 \mathrm{\%}}\right) $$ Flame Graph
entity_by_id 1000 entities $$2.79 \mathrm{ms} \pm 12.2 \mathrm{μs}\left({\color{gray}-3.087 \mathrm{\%}}\right) $$ Flame Graph
entity_by_id 10 entities $$1.88 \mathrm{ms} \pm 5.10 \mathrm{μs}\left({\color{gray}-0.700 \mathrm{\%}}\right) $$ Flame Graph

representative_read_multiple_entities

Function Value Mean Flame graphs
link_by_source_by_property depths: DT=255, PT=255, ET=255, E=255 $$108 \mathrm{ms} \pm 414 \mathrm{μs}\left({\color{gray}-2.210 \mathrm{\%}}\right) $$ Flame Graph
link_by_source_by_property depths: DT=0, PT=0, ET=2, E=2 $$90.8 \mathrm{ms} \pm 442 \mathrm{μs}\left({\color{gray}-0.077 \mathrm{\%}}\right) $$ Flame Graph
link_by_source_by_property depths: DT=2, PT=2, ET=2, E=2 $$99.3 \mathrm{ms} \pm 411 \mathrm{μs}\left({\color{gray}-1.193 \mathrm{\%}}\right) $$ Flame Graph
link_by_source_by_property depths: DT=0, PT=0, ET=0, E=0 $$43.8 \mathrm{ms} \pm 237 \mathrm{μs}\left({\color{gray}-0.521 \mathrm{\%}}\right) $$ Flame Graph
link_by_source_by_property depths: DT=0, PT=2, ET=2, E=2 $$95.8 \mathrm{ms} \pm 345 \mathrm{μs}\left({\color{gray}0.344 \mathrm{\%}}\right) $$ Flame Graph
link_by_source_by_property depths: DT=0, PT=0, ET=0, E=2 $$82.6 \mathrm{ms} \pm 649 \mathrm{μs}\left({\color{gray}-0.063 \mathrm{\%}}\right) $$ Flame Graph
entity_by_property depths: DT=255, PT=255, ET=255, E=255 $$69.4 \mathrm{ms} \pm 313 \mathrm{μs}\left({\color{gray}-0.955 \mathrm{\%}}\right) $$ Flame Graph
entity_by_property depths: DT=0, PT=0, ET=2, E=2 $$50.6 \mathrm{ms} \pm 255 \mathrm{μs}\left({\color{gray}-0.252 \mathrm{\%}}\right) $$ Flame Graph
entity_by_property depths: DT=2, PT=2, ET=2, E=2 $$60.1 \mathrm{ms} \pm 377 \mathrm{μs}\left({\color{gray}0.466 \mathrm{\%}}\right) $$ Flame Graph
entity_by_property depths: DT=0, PT=0, ET=0, E=0 $$42.1 \mathrm{ms} \pm 205 \mathrm{μs}\left({\color{gray}0.424 \mathrm{\%}}\right) $$ Flame Graph
entity_by_property depths: DT=0, PT=2, ET=2, E=2 $$55.6 \mathrm{ms} \pm 282 \mathrm{μs}\left({\color{gray}-0.374 \mathrm{\%}}\right) $$ Flame Graph
entity_by_property depths: DT=0, PT=0, ET=0, E=2 $$46.1 \mathrm{ms} \pm 205 \mathrm{μs}\left({\color{gray}1.26 \mathrm{\%}}\right) $$ Flame Graph

representative_read_entity_type

Function Value Mean Flame graphs
get_entity_type_by_id Account ID: d4e16033-c281-4cde-aa35-9085bf2e7579 $$1.38 \mathrm{ms} \pm 6.11 \mathrm{μs}\left({\color{gray}1.35 \mathrm{\%}}\right) $$ Flame Graph

@TimDiekmann TimDiekmann added this pull request to the merge queue Nov 7, 2024
Merged via the queue into main with commit d95917a Nov 7, 2024
102 checks passed
@TimDiekmann TimDiekmann deleted the t/h-3537-allow-querying-of-resolved-entity-types branch November 7, 2024 11:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/apps > hash* Affects HASH (a `hash-*` app) area/apps > hash-api Affects the HASH API (app) area/apps > hash-graph area/apps area/libs Relates to first-party libraries/crates/packages (area) area/tests > integration New or updated integration tests area/tests New or updated tests type/eng > backend Owned by the @backend team
Development

Successfully merging this pull request may close these issues.

2 participants