-
Notifications
You must be signed in to change notification settings - Fork 57
Performance: /api/content_node/checklist_nodes #7114
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
Conversation
@@ -62,7 +65,7 @@ abstract class ContentNode extends BaseEntity implements BelongsToContentNodeTre | |||
#[Gedmo\SortableGroup] // this is needed to avoid that all root nodes are in the same sort group (parent:null, slot: '') | |||
#[Groups(['read'])] | |||
#[ORM\ManyToOne(targetEntity: ColumnLayout::class, inversedBy: 'rootDescendants')] | |||
#[ORM\JoinColumn(nullable: true, onDelete: 'CASCADE')] // TODO make not null in the DB using a migration, and get fixtures to run | |||
#[ORM\JoinColumn(nullable: false, onDelete: 'CASCADE')] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, so this finally works now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes - and it improves performance :)
@@ -35,7 +35,7 @@ public function testCreateColumnLayoutValidatesMissingParent() { | |||
$this->assertJsonContains([ | |||
'violations' => [ | |||
[ | |||
'propertyPath' => 'parent', | |||
'propertyPath' => 'root', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test name still says the parent may not be missing, not the root. Should this be a new test instead? Or do we just need to adjust the test's name?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately no.
parent
is the mandatory field. But without parent
root
is not determined.
Unfortunately root
is checked first.
It is correct that a ValidationError occurs. However, the message is unfortunate.
Unfortunately, I have not found a better solution.
177a351
to
be75ad0
Compare
Add Index
ContentNode->strategy
improves joins with strategy-filter
ContentNode->root
not nullableleft join
becomesinner join
Auswirkung
auf /api/content_node/checklist_nodes?camp=/camps/camp_id
Old: (cost=348.72..348.73 rows=1 width=1453)
New: (cost=186.07..186.07 rows=1 width=1453)
Old:
Explain query
New:
Explain query