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

sqlite: don't count rows written to temporary (ephemeral) tables #1594

Merged

Conversation

smerritt
Copy link
Contributor

SQLite may generate temporary tables while executing a SELECT statement. Rows written to these tables shouldn't count as they're not under control of the user.

This change is essentially
tursodatabase/libsql@5d6c79e, which does this same thing in libsql; libsql is also where the original row-counting logic comes from.

The test was written by glen@cloudflare.com.

@smerritt smerritt requested review from a team as code owners January 30, 2024 19:36
@smerritt smerritt requested review from kentonv and hoodmane January 30, 2024 19:36
Copy link
Contributor

@rozenmd rozenmd left a comment

Choose a reason for hiding this comment

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

LGTM!

@smerritt
Copy link
Contributor Author

Fixup: 3044e11

SQLite may generate temporary tables while executing a SELECT
statement. Rows written to these tables shouldn't count as they're not
under control of the user.

This change is essentially
tursodatabase/libsql@5d6c79e,
which does this same thing in libsql; libsql is also where the
original row-counting logic comes from.

The test was written by glen@cloudflare.com.
@smerritt smerritt force-pushed the smerritt/sqlite-dont-count-ephemeral-writes branch from 3044e11 to 4226541 Compare January 30, 2024 21:38
@smerritt
Copy link
Contributor Author

An existing test was broken; it was updating 2 of 4 rows in an indexed table and asserting rows_written == 4, which becomes 2 with this change.

@smerritt smerritt merged commit 44760c9 into cloudflare:main Jan 30, 2024
11 checks passed
@smerritt smerritt deleted the smerritt/sqlite-dont-count-ephemeral-writes branch January 30, 2024 23:27
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