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

lp1937941: Fix update of play counter for different SQLite versions #4152

Merged
merged 4 commits into from
Aug 6, 2021
Merged

lp1937941: Fix update of play counter for different SQLite versions #4152

merged 4 commits into from
Aug 6, 2021

Conversation

uklotzde
Copy link
Contributor

@uklotzde uklotzde commented Jul 27, 2021

https://bugs.launchpad.net/mixxx/+bug/1937941

Temporary mitigation to prevent a debug assertion. No time to fix it with a workaround. Fixed.

And also fixed an uncovered case for SQLite >= 3.33.0 where the counter was not reset for tracks without any history playlist entry. We need to update twice!

Not going to write any tests. I don't want to touch this code ever after.

@uklotzde uklotzde added this to the 2.4.0 milestone Jul 27, 2021
@coveralls
Copy link

coveralls commented Jul 27, 2021

Pull Request Test Coverage Report for Build 1081647478

  • 0 of 58 (0.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.005%) to 28.608%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/library/dao/trackdao.cpp 0 58 0.0%
Totals Coverage Status
Change from base Build 1081645070: -0.005%
Covered Lines: 20109
Relevant Lines: 70291

💛 - Coveralls

@uklotzde uklotzde changed the title lp1937941: Disable update of play counter for SQLite versions before 3.33.0 lp1937941: Fix update of play counter for different SQLite versions Jul 27, 2021
@mr-smidge
Copy link
Contributor

I can no longer replicate the bug using the code in this PR 👍

// https://www.sqlite.org/lang_update.html#upfrom
// UPDATE-FROM is supported beginning in SQLite version 3.33.0 (2020-08-14)
// https://bugs.launchpad.net/mixxx/+bug/1937941
#ifdef __SQLITE3__
Copy link
Member

Choose a reason for hiding this comment

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

So if this is not defined, we use Qt's SQLite version which always supports it?

Copy link
Contributor Author

@uklotzde uklotzde Jul 31, 2021

Choose a reason for hiding this comment

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

🤷 I don't know and don't care. Well it's actually simple. The workaround is only used if all conditions are met. Otherwise the default version takes precedence.

Copy link
Member

@daschuer daschuer left a comment

Choose a reason for hiding this comment

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

I can confirm the original issue and that this is working for Ubuntu Groovy sqlite 3.31.
The code LGTM, Thank you.

@daschuer daschuer merged commit d06bc0e into mixxxdj:main Aug 6, 2021
@uklotzde uklotzde deleted the lp1937941-sqlite branch August 6, 2021 11:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants