Skip to content

feat(database): add option to get underlying query for property relation#2102

Open
laylatichy wants to merge 31 commits intotempestphp:3.xfrom
laylatichy:feat-add-option-to-get-underlaying-query-for-property
Open

feat(database): add option to get underlying query for property relation#2102
laylatichy wants to merge 31 commits intotempestphp:3.xfrom
laylatichy:feat-add-option-to-get-underlaying-query-for-property

Conversation

@laylatichy
Copy link
Copy Markdown
Contributor

@laylatichy laylatichy commented Apr 4, 2026

pr to add option to have similar behavior to laravels author->books() to get underlying query for property relation we can chain into

// select with constraints
$author->query('books')->select()->whereField(field: 'title', value: 'Timeline Taxi')->all();
$author->query('books')->select()->limit(limit: 5)->all();

// count
$author->query('books')->count()->execute();

// update scoped to relation
$author->query('books')->update(title: 'Updated')->execute();

// delete scoped to relation
$author->query('books')->delete()->execute();

works with all relation types:

// HasMany / HasOne
$author->query('books')->select()->all();
$book->query('isbn')->select()->first();

// BelongsTo
$book->query('author')->select()->first();

// HasManyThrough / HasOneThrough
$tag->query('reviewers')->select()->all();
$tag->query('topReviewer')->select()->first();

// BelongsToMany
$tag->query('books')->select()->all();

now, I wasnt sure about naming it but opted in for query since it returns querybuilder same as query() helper just scoped to model property

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 4, 2026

Benchmark Results

Comparison of feat-add-option-to-get-underlaying-query-for-property against 3.x (f527f4c1b96acac2a380c50a4daff11518c8dc5d).

Open to see the benchmark results

No benchmark changes above ±5%.

Generated by phpbench against commit 71ee424

@laylatichy laylatichy force-pushed the feat-add-option-to-get-underlaying-query-for-property branch 2 times, most recently from debbc8f to ceec41c Compare April 4, 2026 03:54
@laylatichy laylatichy marked this pull request as ready for review April 4, 2026 04:14
$model = inspect(model: $this);

if (! $model->hasPrimaryKey() || ! $model->getPrimaryKeyProperty()->isInitialized(object: $this)) {
throw new InvalidArgumentException(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you add dedicated exceptions in Tempest\Database\Exceptions?

Our guidelines on exceptions are here: https://tempestphp.com/3.x/extra-topics/contributing#exception-classes

I did notice that we don't have any QueryBuilderException or DatabaseException interface yet, so you don't have to add these marker interfaces.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

added 2 exceptions
packages/database/src/Exceptions/PrimaryKeyWasNotInitialized.php
packages/database/src/Exceptions/PropertyWasNotARelation.php

…cit attr, unsaved model, nonexistent property, empty result)
…e, return QueryBuilder for select/update/delete support
@laylatichy laylatichy force-pushed the feat-add-option-to-get-underlaying-query-for-property branch 2 times, most recently from b3187c9 to 6b0e889 Compare April 9, 2026 13:39
…WasNotInitialized and PropertyWasNotARelation
@laylatichy laylatichy force-pushed the feat-add-option-to-get-underlaying-query-for-property branch from 6b0e889 to 1007539 Compare April 9, 2026 14:02
@laylatichy laylatichy force-pushed the feat-add-option-to-get-underlaying-query-for-property branch from b234fd9 to 71ee424 Compare April 9, 2026 14:59
@laylatichy
Copy link
Copy Markdown
Contributor Author

@brendt had to add #[RunTestsInSeparateProcesses] to rector tests as with my extra tests we went over 1GB mem threshold causing tests to OOM https://github.com/tempestphp/tempest-framework/actions/runs/24194408952/job/70620184698 Im not fully sure if that was the culprit as I was unable to replicate it on my local but mem analysis showed rector tests consuming around 200MB each

If you want i can move that commit into separate pr 71ee424

@innocenzi innocenzi changed the title feat(database): add option to get underlaying query for property relation feat(database): add option to get underlying query for property relation Apr 9, 2026
@brendt
Copy link
Copy Markdown
Member

brendt commented Apr 10, 2026

No it's fine to keep it here. I'll review later today :)

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.

2 participants