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

Implement Fetch protocol using Document return type #210

Merged
merged 3 commits into from
Oct 16, 2024

Conversation

flyingsilverfin
Copy link
Member

@flyingsilverfin flyingsilverfin commented Oct 14, 2024

Release notes: usage and product changes

We implement Fetch queries, which return concept Document streams.

@typedb-bot
Copy link
Member

PR Review Checklist

Do not edit the content of this comment. The PR reviewer should simply update this comment by ticking each review item below, as they get completed.


Trivial Change

  • This change is trivial and does not require a code or architecture review.

Code

  • Packages, classes, and methods have a single domain of responsibility.
  • Packages, classes, and methods are grouped into cohesive and consistent domain model.
  • The code is canonical and the minimum required to achieve the goal.
  • Modules, libraries, and APIs are easy to use, robust (foolproof and not errorprone), and tested.
  • Logic and naming has clear narrative that communicates the accurate intent and responsibility of each module (e.g. method, class, etc.).
  • The code is algorithmically efficient and scalable for the whole application.

Architecture

  • Any required refactoring is completed, and the architecture does not introduce technical debt incidentally.
  • Any required build and release automations are updated and/or implemented.
  • Any new components follows a consistent style with respect to the pre-existing codebase.
  • The architecture intuitively reflects the application domain, and is easy to understand.
  • The architecture has a well-defined hierarchy of encapsulated components.
  • The architecture is extensible and scalable.

@flyingsilverfin flyingsilverfin changed the base branch from development to 3.0 October 14, 2024 11:44
AttributeType attribute_type = 3;
message Leaf {
oneof leaf {
Empty empty = 1;
Copy link
Member Author

Choose a reason for hiding this comment

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

New field - nullable entries ( fetch { $x.name }, where name is card(0.1) ).

RelationType relation_type = 11;
AttributeType attribute_type = 12;
RoleType role_type = 13;
ValueType value_type = 15;
Copy link
Member Author

Choose a reason for hiding this comment

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

We will at some point (soon) introduce fetch-specific shorthands to retrieve more than the 'basic' information, such as type, value_type, and kind:

fetch {
  "x-type": type($x),
  "x-kind": kind($x),
  "attr-value-type": value_type($attr),
  "type kind": kind($type_variable)
  }

Encoding these as a general Leaf is consistent. Now our ConceptDocument.Node enum is fully "structural", ie only contains Lists, Maps, or Leaves, where everything that can be a leaf is some kinds of concepts, descriptions (like 'kind' or 'value_type'), or Empty.

@@ -32,20 +32,21 @@ message Query {
message Ok {
oneof ok {
Empty empty = 1;
ReadableConceptTreeStream readable_concept_tree_stream = 3;
ConceptDocumentStream concept_document_stream = 3;
Copy link
Member Author

Choose a reason for hiding this comment

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

ConceptTree becomes Document and readable_concept has disappeared everywhere!

// TODO: Add Type query_type
// TODO: network optimisation: replace keys with IDs, sending keys in the header to rebuild the document on the client side
// TODO: network optimisation: replace types (== mostly constant strings) with a IDs, sending types in the header to rebuild on the client side
Type query_type = 2;
Copy link
Member Author

Choose a reason for hiding this comment

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

Added this already replacing your TODO

Comment on lines +43 to +44
// TODO: network optimisation: replace keys with IDs, sending keys in the header to rebuild the document on the client side
// TODO: network optimisation: replace types (== mostly constant strings) with a IDs, sending types in the header to rebuild on the client side
Copy link
Member Author

Choose a reason for hiding this comment

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

These are future work , not until benchmarking shows the need to squeeze the network.

@flyingsilverfin flyingsilverfin changed the title Implement fetch Implement Fetch protocol using Document return type Oct 16, 2024
@flyingsilverfin flyingsilverfin marked this pull request as ready for review October 16, 2024 19:52
@flyingsilverfin flyingsilverfin merged commit 94b79eb into typedb:3.0 Oct 16, 2024
1 check failed
@flyingsilverfin flyingsilverfin deleted the fetch branch October 16, 2024 19:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants