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

feat: Add _keys attribute to selectNode simple explain #1546

Conversation

shahzadlone
Copy link
Member

@shahzadlone shahzadlone commented May 30, 2023

Relevant issue(s)

Resolves #1535

Description

  • Delete a dead var that confused me as if we had some dockey on scanNode.
  • Rename commits are unrelated and just rename vars (non-user facing, fairly internal - can be ignored).
  • Adds the missing _keys attribute that represents selectNode keys.
  • Adds testing with type join filter and keys.
  • Adds selectNode targets to assert to some previous tests where it was missed before.
  • Fixes previous selectNode target assertions to adhere to this change.

@shahzadlone shahzadlone added feature New feature or request area/testing Related to any test or testing suite labels May 30, 2023
@shahzadlone shahzadlone added this to the DefraDB v0.6 milestone May 30, 2023
@shahzadlone shahzadlone requested a review from a team May 30, 2023 12:10
@shahzadlone shahzadlone self-assigned this May 30, 2023
@shahzadlone shahzadlone force-pushed the feat/add-keys-attribute-to-select-node branch from bb6e884 to bc6e6ea Compare May 30, 2023 12:57
@shahzadlone shahzadlone force-pushed the feat/add-keys-attribute-to-select-node branch 2 times, most recently from 162ec6c to 09361b6 Compare May 30, 2023 13:12
@codecov
Copy link

codecov bot commented May 30, 2023

Codecov Report

Merging #1546 (09361b6) into develop (43857bb) will decrease coverage by 0.03%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #1546      +/-   ##
===========================================
- Coverage    72.27%   72.25%   -0.03%     
===========================================
  Files          185      185              
  Lines        18374    18379       +5     
===========================================
- Hits         13280    13279       -1     
- Misses        4054     4056       +2     
- Partials      1040     1044       +4     
Impacted Files Coverage Δ
planner/explain.go 56.02% <ø> (ø)
planner/scan.go 88.65% <100.00%> (ø)
planner/select.go 80.62% <100.00%> (-0.72%) ⬇️

... and 6 files with indirect coverage changes

Copy link
Contributor

@AndrewSisley AndrewSisley left a comment

Choose a reason for hiding this comment

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

LGTM

@shahzadlone shahzadlone merged commit 0ed5441 into sourcenetwork:develop May 30, 2023
@shahzadlone shahzadlone deleted the feat/add-keys-attribute-to-select-node branch May 30, 2023 15:15
@islamaliev
Copy link
Contributor

bug bash result:
ran a local defradb node with Books, Authors, Publishers... collections.
Using Altair GraphQL client sent different @Explain queries.
selecteNode always showed _keys fields with either nil or list of keys

shahzadlone added a commit to shahzadlone/defradb that referenced this pull request Feb 23, 2024
…work#1546)

## Relevant issue(s)
Resolves sourcenetwork#1535 

## Description
- Delete a dead var that confused me as if we had some `dockey` on
`scanNode`.
- Rename commits are unrelated and just rename vars (non-user facing,
fairly internal - can be ignored).
- Adds the missing `_keys` attribute that represents `selectNode` keys.
- Adds testing with type join filter and keys.
- Adds `selectNode` targets to assert to some previous tests where it
was missed before.
- Fixes previous `selectNode` target assertions to adhere to this
change.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/testing Related to any test or testing suite feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Simple Explaining selectNode is missing _key attribute.
3 participants