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

Fix clump, add .windows() function for array #3811

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

Dhghomon
Copy link
Contributor

@Dhghomon Dhghomon commented Apr 5, 2024

Thank you for submitting this pull request! We really appreciate you spending the time to work on these changes.

What is the motivation?

For the first: currently .chunks doesn't check for input of 0 but should. I'm guessing that an empty array would be the way to go but could be changed to return a Result as well. Whatever doesn't simply call .chunks() and pass in a 0 is fine.

Two motivations for the second:

  • .windows() in Rust is the counterpart of .chunks() (called clumps in SurrealDB) and we have one but not the other
  • My current use case where I'd like to insert an array of location names and use the output of .windows() to do queries on every graph table in between. e.g. insert array::windows(["Manchester", "Birmingham", "London"]) and get [["Manchester", "Birmingham"], ["Birmingham", "London"]] and use each one to get the route information between each. It's a bit of a stop-gap measure until Feature: Recursive idiom paths #3613 is implemented but would be useful for many other cases as well.

What does this change do?

  • Checks whether a 0 is passed into .chunks() (and .windows() below)
  • Adds an array::windows() function. (Could/probably be renamed since we don't use the chunks name either)

What is your testing strategy?

Duplicating the existing tests for clumps, as the behaviour is almost exactly the same

Is this related to any issues?

No

Does this change need documentation?

If this pull request requires changes, updates, or improvements to the documentation, then add a corresponding issue on the docs.surrealdb.com repository, and link to it here.

Have you read the Contributing Guidelines?

@Dhghomon Dhghomon marked this pull request as ready for review April 8, 2024 01:23
@Dhghomon Dhghomon requested a review from a team April 8, 2024 01:23
@Dhghomon Dhghomon requested review from a team as code owners April 8, 2024 01:23
phughk
phughk previously approved these changes May 1, 2024
Copy link
Contributor

@phughk phughk left a comment

Choose a reason for hiding this comment

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

Lgtm thanks! Though one of the tests is failing

thread 'function_array_windows' panicked at lib/tests/function.rs:19:17:
assertion `left == right` failed: Received response did not match expected.
	Query response #1,
	Desired response: [[0, 1], [1, 2]],
	Actual response: [[0, 1], [1, 2], [2, 3]]
  left: Array(Array([Array(Array([Number(Int(0)), Number(Int(1))])), Array(Array([Number(Int(1)), Number(Int(2))])), Array(Array([Number(Int(2)), Number(Int(3))]))]))
 right: Array(Array([Array(Array([Number(Int(0)), Number(Int(1))])), Array(Array([Number(Int(1)), Number(Int(2))]))]))

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.

None yet

3 participants