8000 Feature/aql interleave function by maierlars · Pull Request #11352 · arangodb/arangodb · GitHub
[go: up one dir, main page]

Skip to content

Feature/aql interleave function #11352

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

Merged
merged 8 commits into from
Apr 1, 2020
Merged

Feature/aql interleave function #11352

merged 8 commits into from
Apr 1, 2020

Conversation

maierlars
Copy link
Contributor

Scope & Purpose

Add a INTERLEAVE AQL function for arrays.

RETURN INTERLEAVE([1, 4, 7], [2, 5], [3]); // [1, 2, 3, 4, 5, 7]
RETURN INTERLEAVE([1, 1, 1], [2, 2], [3]); // [1, 2, 3, 1, 2, 1]
  • Strictly new functionality (i.e. a new feature / new option, no need for porting)
  • The behavior in this PR can be (and was) manually tested (support / qa / customers can test it)
  • The behavior change can be verified via automatic tests

Testing & Verification

This PR adds tests that were used to verify all changes:

  • Added new integration tests (i.e. in shell_server / shell_server_aql)

Documentation

  • Added a Changelog Entry (referencing the corresponding public or internal issue number)
  • Added entry to Release Notes
  • Added a new section in the Manual

@maierlars maierlars self-assigned this Mar 31, 2020
Copy link
Member
@mchacki mchacki left a comment

Choose a reason for hiding this comment

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

I like this feature.
Especially with graphs this will give benefit:

LET alternatingPath = INTERLEAVE(p.vertices, p.edges)

However I would like some more test-cases to be added.

builder->openArray();

while (!iters.empty()) { // in this loop we only deal with nonempty arrays
for (auto i = iters.begin(); i != iters.end();) {
Copy link
Member

Choose a reason for hiding this comment

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

ok, not that easy to grasp but correct!

{input: [[1, 3, 5], [2, 4, 6]], expected: [1, 2, 3, 4, 5, 6]},
{input: [[1, 3, 5], [2]], expected: [1, 2, 3, 5]},
{input: [[1, 1, 1], [2, 2], [3]], expected: [1, 2, 3, 1, 2, 1]},
{input: [[1, 4, 7], [2, 5], [3]], expected: [1, 2, 3, 4, 5, 7]},
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
{input: [[1, 4, 7], [2, 5], [3]], expected: [1, 2, 3, 4, 5, 7]},
{input: [[1, 4, 7], [2, 5], [3]], expected: [1, 2, 3, 4, 5, 7]},
{input: [[2, 5], [3], [1, 4, 7]], expected: [2, 3, 1, 5, 4, 7]},

I would like a test where the iterators do not "end" in order, e.g. the middle one is shortest, then the first, then the latest

}).join(", ") + ")");
assertEqual(d.expected, actual[0], d);
});
},

Copy link
Member

Choose a reason for hiding this comment

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

Can you add interleaved invalid tests please?
Interleave strings, numbers, objects

{input: [[1, 3, 5], [2]], expected: [1, 2, 3, 5]},
{input: [[1, 1, 1], [2, 2], [3]], expected: [1, 2, 3, 1, 2, 1]},
{input: [[1, 4, 7], [2, 5], [3]], expected: [1, 2, 3, 4, 5, 7]},
{input: [[1],[1],[1],[1],[1],[1],[1]], expected: [1, 1, 1, 1, 1, 1, 1]},
Copy link
Member

Choose a reason for hiding this comment

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

Can you also add a test where you have many arrays, but only one is empty?
The first test can not be distinguished from we return a empty array if we see any empty array in input.

Copy link
Contributor
@jsteemann jsteemann left a comment

Choose a reason for hiding this comment

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

Please also add a CHANGELOG entry for the new function.

@maierlars
Copy link
Contributor Author

@maierlars
Copy link
Contributor Author

@maierlars maierlars marked this pull request as ready for review April 1, 2020 07:27
@jsteemann jsteemann merged commit d044018 into devel Apr 1, 2020
@jsteemann jsteemann deleted the feature/aql-interleave branch April 1, 2020 08:30
ObiWahn added a commit that referenced this pull request Apr 2, 2020
…idation-web-ui

* 'devel' of github.com:arangodb/arangodb:
  Fix an agency supervision bug. (#11356)
  (mostly) restore pre-3.7 API behavior (#11364)
  validation: AQL functions (#11327)
  Check MSVC_VERSION instead of CMAKE_GENERATOR (#11351)
  added sleep time (lousy fix)
  Feature/aql interleave function (#11352)
  cheapify IN lookups on unsorted arrays (#11342)
  Bug fix/fix msvc2019 build (#11052)
ObiWahn added a commit that referenced this pull request Apr 6, 2020
…ture/utf-8-validation

* 'devel' of https://github.com/arangodb/arangodb: (21 commits)
  Bug fix/headers cleanup (#11391)
  Feature/internal issue #672 (#11370)
  GraphNodes now copy the graph when cloned (#11345)
  fix lame compile error
  Feature/aql subquery execution block impl execute implementation harvesting (#11349)
  remove obsolete recoveryData
  Feature/dismantle mmfiles (#11354)
  Fix an agency supervision bug. (#11356)
  (mostly) restore pre-3.7 API behavior (#11364)
  validation: AQL functions (#11327)
  Check MSVC_VERSION instead of CMAKE_GENERATOR (#11351)
  added sleep time (lousy fix)
  Feature/aql interleave function (#11352)
  cheapify IN lookups on unsorted arrays (#11342)
  Bug fix/fix msvc2019 build (#11052)
  Allow easier removal of validation rules. (#11346)
  upgrade RocksDB (#11308)
  Introduce more type-safe identifiers (#11270)
  Bug fix/schema validation return code (#11341)
  Fix explainer output when restricting collections (#11338)
  ...
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
0