Skip to content

Remove apoc.doIt from custom cypher mutations #2805

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Jan 30, 2023
Merged

Remove apoc.doIt from custom cypher mutations #2805

merged 5 commits into from
Jan 30, 2023

Conversation

angrykoala
Copy link
Member

Description

Removes apoc.cypher.doIt

This has an increased performance on mutations with custom @cypher

@changeset-bot
Copy link

changeset-bot bot commented Jan 30, 2023 •
edited
Loading

⚠️ No Changeset found

Latest commit: 33d389b

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@neo4j-team-graphql
Copy link
Collaborator

neo4j-team-graphql commented Jan 30, 2023 •
edited
Loading

Performance Report

name dbHits old dbHits time (ms) old time (ms) maxRows
🟦 cypher-directive.TopLevelMutationDirective 1135 N/A 57 N/A 1134

🟥 - Performance worsened (dbHits)
🟩 - Performance improved (dbHits)
🟦 - New test

Show Full Table
name dbHits old dbHits time (ms) old time (ms) maxRows
aggregations.TopLevelAggregate 3403 3403 42 53 1134
aggregations.NestedAggregation 13514 13514 101 144 2174
aggregations.AggregationWithWhere 10942 10942 70 89 2174
aggregations.NestedCountFromMovieToActors 8694 8694 81 89 2174
aggregations.NestedCountFromActorsToMovie 8900 8900 63 114 2174
aggregations.DeeplyNestedCount 10054408 10054408 4167 4542 2008534
batch-create.BatchCreate 3600 3600 111 104 600
connect.createAndConnect 12419 12419 173 221 3003
connections.Connection 13042 13042 101 98 2174
connections.NestedConnection 33883 33883 112 127 4516
connections.ConnectionWithSort 2277 2277 56 110 1040
connections.ConnectionWithSortAndCypher 14278 14278 84 104 2174
create.SimpleMutation 6 6 28 35 1
🟦 cypher-directive.TopLevelMutationDirective 1135 N/A 57 N/A 1134
delete.SimpleDelete 18361 18361 256 275 1040
delete.NestedDeleteInUpdate 16779 16779 178 166 2040
query.SimpleQuery 14120 14120 106 81 2174
query.QueryWhere 8662 8677 32 51 2162
query.SimpleQueryWithNestedWhere 8833 8848 47 61 2162
query.Nested 10084290 10084290 10235 9199 2008534
query.NestedWithFilter 37172 37172 108 172 4511
query.OrFilterOnRelationships 36488 36353 174 196 1855
query.OrFilterOnRelationshipsAndNested 30968 30024 221 277 1855
query.QueryWithNestedIn 12815 12901 58 67 1374
query.NestedConnectionWhere 8794 8794 55 60 2174
query.DeeplyNestedConnectionWhere 8989 8856 69 86 2174
query.DeeplyNestedWithRelationshipFilters 6251 6251 116 143 1134
query.DeeplyNestedWithRelationshipSingleFilters 15438 15438 82 131 2174
query.Fulltext 96 96 25 29 16
query.FulltextWithNestedQuery 571 571 39 46 84
sorting.SortMultipleTypes 2493 2493 59 69 1040
sorting.SortMultipleTypesWithCypherWithCypher 13430 13426 103 112 2174
sorting.SortOnNestedFields 13042 13042 115 97 2174
sorting.SortDeeplyNestedFields 38965 38965 153 206 4516
sorting.SortWithTopLevelCypher 14120 14120 48 59 2174
unions.SimpleUnionQuery 321 321 54 73 35
unions.SimpleUnionQueryWithMissingFields 293 293 60 113 35
unions.NestedUnion 398553 398553 271 356 33033
unions.NestedUnionWithMissingFields 372527 372527 271 353 33033
update.NestedUpdate 2119 2119 58 45 1040

Old Schema Generation: 27.908s
Schema Generation: 27.886s

darrellwarde
darrellwarde approved these changes Jan 30, 2023
View reviewed changes
Copy link
Contributor

@darrellwarde darrellwarde left a comment

Choose a reason for hiding this comment

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

Looks good! Possibly address the change I pointed out, and also add a changeset?

packages/graphql/src/translate/translate-top-level-cypher.ts Outdated
WITH [k in keys(value) | value[k]][0] AS this
`);
CALL {
WITH *
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a bit of a weirdness, we shouldn't really need WITH * (or anything for that matter) because it is a top-level Mutation?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good spot

@angrykoala
Copy link
Member Author

Not really need for changeset, this basically has the same changes as runFirstColumn

@angrykoala angrykoala force-pushed the remove-apoc-doit branch 2 times, most recently from 8beebe9 to d327984 Compare January 30, 2023 17:29
@angrykoala angrykoala merged commit 0035556 into neo4j:4.0.0 Jan 30, 2023
@angrykoala angrykoala deleted the remove-apoc-doit branch January 30, 2023 18:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@darrellwarde darrellwarde darrellwarde approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@angrykoala @neo4j-team-graphql @darrellwarde