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

Cue positions are stored as doubles, not ints. #10998

Merged
merged 1 commit into from
Nov 3, 2022

Conversation

ywwg
Copy link
Member

@ywwg ywwg commented Oct 24, 2022

This results in cue points being fractionally incorrect when loaded and increases the chance that a cue point will have the "invalid" value of -1.0

@daschuer
Copy link
Member

Do we also need to update the Schema or does it work magically?

@Holzhaus
Copy link
Member

Do we also need to update the Schema or does it work magically?

From schema.xml:

      CREATE TABLE IF NOT EXISTS cues (
        ...
        position INTEGER DEFAULT -1 NOT NULL,
        ...

Note that SQLite is NOT type safe. Maybe we should start enforcing STRICT mode, to prevent storing the wrong type:

SQLite uses a more general dynamic type system. In SQLite, the datatype of a value is associated with the value itself, not with its container. The dynamic type system of SQLite is backwards compatible with the more common static type systems of other database engines in the sense that SQL statements that work on statically typed databases work the same way in SQLite. However, the dynamic typing in SQLite allows it to do things which are not possible in traditional rigidly typed databases. Flexible typing is a feature of SQLite, not a bug.

Update: As of version 3.37.0 (2021-11-27), SQLite provides STRICT tables that do rigid type enforcement, for developers who prefer that kind of thing.

https://www.sqlite.org/datatype3.html

@Holzhaus
Copy link
Member

Holzhaus commented Oct 25, 2022

We should also remove the default value IMHO. A cue should never have "no position", and that way we avoid having to define the "invalid" constant in multiple places (1x in cpp, 1x in SQL).

@ywwg
Copy link
Member Author

ywwg commented Nov 3, 2022

The problem is that "-1" is used in code, including controller configs, to mean "no position". So unfortunately we have to avoid storing cues with that value completely. This sucks but at the end of the day trying to fix this is harder than just working around it.

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.

LGTM.
I think we can merge this here and discuss the schema migration here:
#11000

@daschuer daschuer merged commit 10cd04c into mixxxdj:2.3 Nov 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants