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

Aborts the run of the compressor if it will lead to more rows in the database #52

Merged

Conversation

Azrenbeth
Copy link
Contributor

@Azrenbeth Azrenbeth commented Jul 22, 2021

BUILDS ON azren/all_options_readme_and_help

This aborts the run of the compressor if it will lead to MORE rows in the database

This will prevent accidently running the generated SQL on the database and increasing storage requirements.

@Azrenbeth Azrenbeth force-pushed the azren/early_abort_negative_compression branch from d32b222 to 5bed248 Compare August 16, 2021 15:50
@Azrenbeth Azrenbeth changed the title Azren/early abort negative compression Aborts the run of the compressor if it will lead to more rows in the database Aug 16, 2021
@Azrenbeth Azrenbeth marked this pull request as ready for review August 16, 2021 15:53
src/lib.rs Outdated Show resolved Hide resolved
Azrenbeth and others added 2 commits September 6, 2021 10:01
Before this, unless min_saved_rows was specified, it would continue
to generate the SQL and perform validity.
Co-authored-by: Erik Johnston <erikj@jki.re>
@Azrenbeth Azrenbeth force-pushed the azren/early_abort_negative_compression branch from c05b04f to 549a60b Compare September 6, 2021 09:02
@Azrenbeth Azrenbeth merged commit 65861de into matrix-org:main Sep 6, 2021
@heftig
Copy link
Contributor

heftig commented Sep 12, 2021

Wouldn't it be better to just default min_saved_rows to 0 or 1?

@Azrenbeth
Copy link
Contributor Author

Wouldn't it be better to just default min_saved_rows to 0 or 1?

That would work, however a fix to #31 would also be needed in that case too.

I also did it this way in order to draw extra attention to when the compressor is NOT working as opposed to when it's just not working as well as expected, since that might help fix the creating-extra-rows issue in the future

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.

None yet

3 participants