8000 add initializeCursor back to DistinctCollectExecutor by jsteemann · Pull Request #9386 · arangodb/arangodb · GitHub
[go: up one dir, main page]

Skip to content

add initializeCursor back to DistinctCollectExecutor #9386

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

Conversation

jsteemann
Copy link
Contributor

Scope & Purpose

Add initializeCursor method back to DistinctCollectExecutor. It existed in 3.4 but was missing in devel. It may be handy to free some temporary results and reduce overall memory usage.

  • Bug-Fix for devel-branch (i.e. no need for backports?)

Testing & Verification

This change is already covered by existing tests, such as AQL collect tests.

https://jenkins01.arangodb.biz/view/PR/job/arangodb-matrix-pr/5028/

@jsteemann jsteemann added this to the devel milestone Jul 2, 2019
@jsteemann jsteemann requested a review from goedderz July 2, 2019 14:52
Copy link
Member
@goedderz goedderz left a comment

Choose a reason for hiding this comment

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

LGTM, but I suggest to add an assertion that this will actually be called, see here for an example:
https://github.com/arangodb/arangodb/blob/bug-fix/add-initialize-cursor-to-distinct-collect-executor/arangod/Aql/ExecutionBlockImpl.cpp#L369

@jsteemann
Copy link
Contributor Author

@goedderz : ok, will try to add this, though it feels a bit strange to have to modify a different file (ExecutionBlockImpl.cpp) and add assertions there if I modify a specialization.

@jsteemann
Copy link
Contributor Author

@goedderz
Copy link
Member
goedderz commented Jul 3, 2019

@goedderz : ok, will try to add this, though it feels a bit strange to have to modify a different file (ExecutionBlockImpl.cpp) and add assertions there if I modify a specialization.

Yes, I agree. Perhaps I can find a better way to do that.

Copy link
Member
@goedderz goedderz left a comment

Choose a reason for hiding this comment

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

LGTM

@jsteemann jsteemann merged commit 801c957 into devel Jul 3, 2019
ObiWahn added a commit that referenced this pull request Jul 4, 2019
…ture/one-shard-db

* 'devel' of https://github.com/arangodb/arangodb:
  Pregel additional test & TSan error fix (#9357)
  use a lock when calling unload (#9375)
  @maierlars 😍 (#9394)
  fix typo (#9400)
  add initializeCursor back to DistinctCollectExecutor (#9386)
  Feature/add tcpdump support (#9396)
  apply filters before starting the server, so we can detect whether no test would be executed (#9387)
  [Devel] Queue-Full-Logging (#9388)
  add VelocyPackHelper::equal method (#9389)
  update velocypack version (#9379)
ObiWahn added a commit that referenced this pull request Jul 4, 2019
…ture/mimalloc

* 'devel' of https://github.com/arangodb/arangodb: (37 commits)
  improve handling when procdump detects the process is dead (#9381)
  Bug fix/internal issue #586 (#9401)
  fix tests that didn't properly use env variable to look for test (#9399)
  Pregel additional test & TSan error fix (#9357)
  use a lock when calling unload (#9375)
  @maierlars 😍 (#9394)
  fix typo (#9400)
  add initializeCursor back to DistinctCollectExecutor (#9386)
  Feature/add tcpdump support (#9396)
  apply filters before starting the server, so we can detect whether no test would be executed (#9387)
  [Devel] Queue-Full-Logging (#9388)
  add VelocyPackHelper::equal method (#9389)
  update velocypack version (#9379)
  Consistent formatting of CHANGELOG. (#9392)
  fix JSON statistics (#9385)
  undo removal (#9391)
  make sure all error code names are prefixed with ERROR_ @fceller @KVS85 (#9384)
  remove catch, refresh versions (#9390)
  fix invalid logId
  Fix ArangoSearch documentation examples
  ...
ObiWahn added a commit that referenced this pull request Jul 4, 2019
…-fix/oasis-statistics

* 'devel' of https://github.com/arangodb/arangodb:
  Don't use non-existent variable (#9407)
  relax test condition for windows (#9393)
  improve handling when procdump detects the process is dead (#9381)
  Bug fix/internal issue #586 (#9401)
  fix tests that didn't properly use env variable to look for test (#9399)
  Pregel additional test & TSan error fix (#9357)
  use a lock when calling unload (#9375)
  @maierlars 😍 (#9394)
  fix typo (#9400)
  add initializeCursor back to DistinctCollectExecutor (#9386)
  Feature/add tcpdump support (#9396)
@fceller fceller deleted the bug-fix/add-initialize-cursor-to-distinct-collect-executor branch July 30, 2019 12:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants
0