Improve: add NULL handling in DB.lua #6054
Conversation
## Major - introduced tables with `_isNull` key as a representation of NULL values - `db:_migrate`: - simplified fetching columns where `table_info` is unavailable - added checks to prevent columns with non-NULL values from being dropped - added `force` parameter to bypass check if needed ## Minor changes - removed some redundant conditional checks - made `options` local instead of global - condensed schema parsing (`pairs` is unaffected by clearing a key in middle of a loop)
- fixed error in `db:create()` - added force parameter to `db:create()` and `db:_migrate()` to allow replicating previous behaviour concerning dropping columns - changed `DB_spec.lua` to make use of `force` parameter
- fixed error handling NULL values in `db:_sql_convert()` - added two tests to `DB_spec.lua` for column deletion
- added more extensive tests for null handling
|
Hey there! Thanks for helping Mudlet improve. 🌟 Test versionsYou can directly test the changes here:
No need to install anything - just unzip and run. |
src/mudlet-lua/lua/DB.lua
Outdated
| else | ||
| s = "datetime('" .. v._timestamp .. "', 'unixepoch')" | ||
| end | ||
| elseif t == "table" and v._isNull then |
There was a problem hiding this comment.
looks like the indentation got out of whack here somehow
There was a problem hiding this comment.
Was fiddling with that part on Notepad++, where I didn't have tabs to spaces on. Converted to spaces now.
src/mudlet-lua/tests/DB_spec.lua
Outdated
| @@ -105,7 +105,7 @@ describe("Tests DB.lua functions", function() | |||
| local results = db:fetch(mydb.enemies) | |||
| assert.is_true(#results == 1) | |||
| end) | |||
|
|
|||
There was a problem hiding this comment.
Trailing spaces in DB_spec.lua trimmed now.
| @@ -453,7 +457,7 @@ function db:create(db_name, sheets) | |||
| -- field. The db package reserves any key that begins with an underscore to be special and syntax | |||
| -- for its own use. | |||
| for s_name, sht in pairs(sheets) do | |||
| options = {} | |||
| local options = {} | |||
src/mudlet-lua/tests/DB_spec.lua
Outdated
| local res = db:fetch(mydb.sheet) | ||
| test.blubb = nil -- we expect the blubb gets deleted | ||
| assert.are.equal(1, #res) | ||
| res[1]._row_id = nil --we get the row id back, which we don't need | ||
| assert.are.same(test, res[1]) | ||
| end) | ||
|
|
||
|
|
There was a problem hiding this comment.
the indents are also hit or miss in this section, some 2 space some 4. Typically we use a 2 space indent for the Lua stuff.
There was a problem hiding this comment.
Tabs now converted to two-spaces.
- converted tabs to two spaces
- Converted tabs to two spaces
- Trimmed trailing spaces
Co-authored-by: Vadim Peretokin <vperetokin@hey.com>
There was a problem hiding this comment.
Thanks a lot for this @RollanzMushing, and congrats on your first big Mudlet contribution!
vadi2
added
the
needs documentation
|
@RollanzMushing could you update the documentation for your changes? Take the functions affected from https://wiki.mudlet.org/w/Manual:Database_Functions, copy them to https://wiki.mudlet.org/w/Area_51 and apply the necessary changes. At release time, we will move them back out from area51. |
vadi2
removed
the
needs documentation
|
@RollanzMushing thanks a lot! |
Brief overview of PR changes/additions
NULLvalues throughdbmethodsNULLvalues in columns that would be dropped bydb:createanddb:_migratemethods, in accordance with existing comments in DB.luaMotivation for adding to Mudlet
NULLis already partially supported throughdb:is_nilanddb:is_not_nilmethods. This expands on those capabilities""is itself a possibility for a field, it is useful to indicate "unknown" withNULLNULLvalues could cause unexpected loss of data