-
Notifications
You must be signed in to change notification settings - Fork 53
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
fix: add meta
to sqlite migration scripts
#2675
Conversation
You can find the image built from this PR at
Built from da062be |
You can find the image built from this PR at
Built from da062be |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks! 💯
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks so much!
I find it curious that it did work with the sqlite test suite. When running it, the meta
field did appear in the table and got filled correctly. Probably the table is created differently in the tests?
Maybe in the
|
Description
While running
./build/waku --store
on latest master, i ran into the following error:When I browsed the db, and executed
.schema message
I saw that the problem was that themeta
column did not exist.Something curious tho, @Ivansete-status and @gabrielmer: when running sqlite, it seems that we use the .sql migration scripts, however we also have this
createTableQuery
proc:nwaku/waku/waku_archive/driver/sqlite_driver/queries.nim
Lines 86 to 92 in 38f8b08
createOldestMessageTimestampIndexQuery
Should we get rid of both of these so it's the migration scripts the ones responsible for updating the db schema for sqlite? based on the behavior before this fix, i'd say that it is highly likely that
createTableQuery
is not being used at all.