Skip to content
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

chore(psl): removing parser DB early bailout #4942

Merged
merged 8 commits into from
Jul 9, 2024

Conversation

Druue
Copy link
Contributor

@Druue Druue commented Jul 5, 2024

for #4923

Copy link

codspeed-hq bot commented Jul 5, 2024

CodSpeed Performance Report

Merging #4942 will not alter performance

Comparing check/remove-parserdb-early-bailout (c95cf6c) with feat/hover (b4a66c5)

Summary

✅ 11 untouched benchmarks

Copy link
Contributor

github-actions bot commented Jul 5, 2024

WASM Query Engine file Size

Engine This PR Base branch Diff
Postgres 2.044MiB 2.043MiB 198.000B
Postgres (gzip) 814.534KiB 814.749KiB -220.000B
Mysql 2.014MiB 2.013MiB 206.000B
Mysql (gzip) 800.901KiB 801.003KiB -105.000B
Sqlite 1.915MiB 1.914MiB 202.000B
Sqlite (gzip) 762.854KiB 763.012KiB -162.000B

…on everything that _is_ valid, then this panic still shouldn't occur, so no need to specifically `create_default_attributes`
@Druue Druue changed the base branch from main to feat/hover July 5, 2024 14:33
@Druue Druue changed the title [DO NOT MERGE] - checking effect of removing parser DB early bailout [DO NOT MERGE] - exploring removing parser DB early bailout Jul 5, 2024
Druue and others added 2 commits July 5, 2024 17:58
…ot guarantee a field is either a scalar or relation outside of the QE

Co-authored-by: Sergey Tatarintsev <[email protected]>
@Druue Druue changed the title [DO NOT MERGE] - exploring removing parser DB early bailout chore(psl): removing parser DB early bailout Jul 8, 2024
@Druue Druue marked this pull request as ready for review July 8, 2024 10:00
@Druue Druue requested a review from a team as a code owner July 8, 2024 10:00
@Druue Druue requested review from SevInf and removed request for a team July 8, 2024 10:00
@Druue Druue self-assigned this Jul 8, 2024
@Druue Druue added the PR: Improvement A PR that improves existing functionality label Jul 8, 2024
@Druue Druue added this to the 5.17.0 milestone Jul 8, 2024
Copy link
Contributor

@SevInf SevInf left a comment

Choose a reason for hiding this comment

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

LGTM with suggestion

Co-authored-by: Serhii Tatarintsev <[email protected]>
@Druue Druue merged commit 217e653 into feat/hover Jul 9, 2024
207 checks passed
@Druue Druue deleted the check/remove-parserdb-early-bailout branch July 9, 2024 19:08
Druue added a commit that referenced this pull request Jul 10, 2024
* remove early bailout in parser db
* We can no longer assume that relation fields exist as the opposite side of the relation might not exist yet.
* Added refine_known for usage in QE; refine is now optional as we cannot guarantee a field is either a scalar or relation outside of the QE

---------

Co-authored-by: Sergey Tatarintsev <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: Improvement A PR that improves existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants