[no sq] Split blockpos into 3 different columns in sqlite3 map database

[sfan5]

This changes the map database schema for sqlite to be (x, y, z, data) instead of (pos, data).

Advantages:

• better range queries enables possible optimizations inside Luanti and for external tools
• moving away from the "block as integer" code that is apparently so cursed

Disadvantages:

• If this SO answer is to be believed, this might actually be minimally slower
  • not true according to @lhofhansl's measurements.

To do

This PR is a Work in Progress

• add simple unit tests
• update docs

How to test

1. create new world, inspect map.sqlite afterwards
2. join old world (it should work)
3. try migrating an old world to e.g. leveldb and back to sqlite (should change to new format)

[appgurueu]

arbitrary range queries enables possible optimizations inside Luanti and for external tools

Note that this index will only allow SQLite to speed up the following (parts of) a query:

• x BETWEEN ? AND ?
• x = ? AND y BETWEEN ? AND ?
• x = ? AND y = ? AND z BETWEEN ? AND ?

That is, a range query can essentially only be done along a single axis, and all preceding axes must be equality comparisons (or SQLite will effectively have to loop over all possible values for each), in the order the keys appear in the index.

[sfan5]

We can add more indexing if needed tho.

Edit: we can however influence the order of the default index. (X, Z, Y) sounds most useful so I'll change that.

[lhofhansl]

While you are at it, we could also split out a block's timestamp into a separate column.
This timestamp is a frequent cause for writing entire blocks back to the db, even when nothing else has changed.

[lhofhansl]

Note that this index will only allow SQLite to speed up the following (parts of) a query

If all our databases support z-order we can do approximate spatial indexing that way. (It's not hard to add it via plpgsql to Postgres for example, and then index on a function.)

On the whole I would expect this change to slow things, but that's just a gut feeling.

[sorcerykid]

Overall I'm in favour of this PR, as it will significantly improve performance of my map analysis scripts, in particularly RocketLib Toolkit which includes many functions for iterating mapblocks in certain areas.

https://bitbucket.org/sorcerykid/rocketlib/src/master/

Currently, I have to resort to generating a "Cache" database that just consists of a cross-reference table for indexing Block Position (X, Y, Z) to Block ID. This process can be extremely time-consuming, particularly on a map.sqlite that is several gigabytes. It would be so nice to finally eliminate that step entirely.

My map viewer application also makes extensive use of position queries for rendering and navigating a density map along the X, Y, or Z axis. Currently, I have to scan the entire database and manually total up the selected mapblocks. But with separate columns for Block Position, this could be streamlined.

[sfan5]

While you are at it, we could also split out a block's timestamp into a separate column.
This timestamp is a frequent cause for writing entire blocks back to the db, even when nothing else has changed.

Would make sense to do at once, but extracting out the timestamp has a whole other bunch of different implications.

If all our databases support z-order we can do approximate spatial indexing that way.

I couldn't find much info on native support in databases for this but also I don't really see any use case.
Letting the database sort through the x,y,z columns should be fast enough if we need it.

[sorcerykid]

This timestamp is a frequent cause for writing entire blocks back to the db, even when nothing else has changed.

Can you point to where this happens? I'm genuinely curious, because I was under the impression mapblocks were only written back to the database if a flag was set for one or more changes (aside from just a new timestamp). After all, if a mapblock is already loaded in memory, then I would think the timestamp is also in memory. So It seems the solution is simply not to write the mapblock to the database if only the timestamp has changed.

[sfan5]

Can you point to where this happens?

luanti/src/serverenvironment.cpp

Lines 1434 to 1448 in dd0070a

	/*
	Handle removed blocks
	*/
	// Convert active objects that are no more in active blocks to static
	deactivateFarObjects(false);
	for (const v3s16 &p: blocks_removed) {
	MapBlock *block = m_map->getBlockNoCreateNoEx(p);
	if (!block)
	continue;
	// Set current time as timestamp (and let it set ChangedFlag)
	block->setTimestamp(m_game_time);
	}

Note that this only marks the block to be written when unloaded, not immediately.
One reason I can think of why we need this is the LBM dtime_s parameter.

Someone could go and test how many byte of disk writes it would save if we could update the timestamp independently, for a typical SP or MP session.

[lhofhansl]

Created two worlds with same seed et al. Seen no measurable degradation in load speed. Good!
Also tried to access old db, works as expected.

[lhofhansl]

Someone could go and test how many byte of disk writes it would save if we could update the timestamp independently, for a typical SP or MP session.

Just rememberd f1349be :), before we marked every loaded block dirty. Now it's just unloaded active blocks.

[lhofhansl]

Now that I am looking a bit more, check this one: https://www.sqlite.org/lang_createtable.html#rowid, especially:

A PRIMARY KEY column only becomes an integer primary key if the declared type name is exactly "INTEGER". Other integer type names like "INT" or "BIGINT" or "SHORT INTEGER" or "UNSIGNED INTEGER" causes the primary key column to behave as an ordinary table column with integer affinity and a unique index, not as an alias for the rowid.

It seems all this time we had declared the sqlite table such that it had two INT keys. (Because we used INT instead of INTEGER to declared the PK).

And when I change that INTEGER and create a new world (with same seed) I see that pure DB load time of a block goes from 6-7us to about 5-6us. In the scheme of things it won't make a visible difference, though.

[sfan5]

It seems all this time we had declared the sqlite table such that it had two INT keys. (Because we used INT instead of INTEGER to declared the PK).

Wow. I can't say I'm a fan of all the subtle behaviors SQLite has (often justified with backwards compatibility).

[lhofhansl]

More of the sqlite quirkiness... According to this https://www.sqlite.org/rowidtable.html

The PRIMARY KEY constraint for a rowid table (as long as it is not the true primary key or INTEGER PRIMARY KEY) is really the same thing as a UNIQUE constraint. Because it is not a true primary key, columns of the PRIMARY KEY are allowed to be NULL, in violation of all SQL standards.

So:

CREATE TABLE `blocks` (
    `x` INT, `y` INT, `z` INT,
    `data` BLOB NOT NULL,
    PRIMARY KEY (`x`, `z`, `y`)
);

Is the same as:

CREATE TABLE `blocks` (
    // the implicit rowid INTEGER PRIMARY KEY,
    `x` INT, `y` INT, `z` INT,
    `data` BLOB NOT NULL,
    UNIQUE  (`x`, `z`, `y`)
);

And if so, we can replace the implicit rowid column with our pos column and maintain as before:

CREATE TABLE `blocks` (
    'pos' INTEGER PRIMARY KEY,
    `x` INT, `y` INT, `z` INT,
    `data` BLOB NOT NULL,
    UNIQUE  (`x`, `z`, `y`)
);

This way we get the benefit of the x, y, z columns and backwards compatibility. Need to be tested of course!
(And well, of course, an old server writing would now mess up our database, since it won't write the x,y, and z columns.)

And according to this: https://www.sqlite.org/lang_createtable.html it's identical to:

CREATE TABLE `blocks` (
    'pos' INTEGER PRIMARY KEY,
    `x` INT, `y` INT, `z` INT,
    `data` BLOB NOT NULL
);
CREATE UNIQUE INDEX xyz ON blocks(x, y, z);

This also makes it clear that by adding the x, y, and z columns we essentially have just added an index.

[sfan5]

I'm sure that could be considered an elegant solution (by some measure), but I really don't want to make the "is this a new format or old format map" decision on a row-basis. It just complicates things for everyone and also literally breaks range queries.

[lhofhansl]

Looked at some DB sizes. Sames seed, and camera position and direction. viewing_range = 1000, mostly an open scene with some ocean. Loads about 8000 blocks.

• Master: 33.2MB
• This PR: 33.2MB - no change in size interestingly and surprisingly
• Master with INT replaced by INTEGER: 26.8MB - this saves the extra index, 20% space. This is actually what I'd prefer.
• This PR (with WITHOUT ROWID) added: 33.2MB - so that doesn't help at all.

sqlite seems to do well with this change.

So while I'd prefer the old format (with the extra useless index removed for 20% space saving), I guess I'll be the only one. At least this one doesn't increase the size, and performance is on par as far as I can tell.

[sfan5]

@appgurueu @SmallJoker @grorp @Desour any more comments?
I'd like to merge but for obvious reasons we can't just revert it if we change our opinion.

[SmallJoker]

any more comments?

Sorry for being late. Backwards compatibility by preserving the pos column might've been nice, but would likely require more hot glue to get it to work well enough. Overall I am surprised by the (apparently non-existent) space increase, and thus in favour of this change. Well done. Thank you.

In the future it might be helpful to have a separate table to store the serialization version. It's not nice to run into incompatible mapblocks during startup (in case of a ser ver bump) or a "hard-crash" upon join. At least that could be wrapped into a nice error message.

[kno10]

When trying to optimize a database, consider the queries you need to perform.
If properly set up as a primary key, then a single integer is largely as good as it gets for a DBMS if your queries are primarily "get" or "set" operations with this key. Typically, the DBMS will either use a hash map, or a B-tree.
The old "cursed" scheme is not at all bad for this purpose.
In fact, the cursed scheme does allow range queries on the X axis, by retrieving IDs encoding(minx,y,z)..encoding(maxx,y,z). I'm pretty sure you can write a (minx,miny,minz)..(maxx,maxy,maxz) query in SQL to offload the selection to the DBMS, as this only involves primitive integer math operations.

Any more complicated encoding (e.g., z curves or similar spatial curves) only ever pay off if you frequently select ranges, and may even harm certain type of selections. Currently, luanti only ever accesses by ID, one block at a time.

In fact, I'd argue that the (x,y,z) scheme comes with next to no advantage (except for ease of use for some tools that need range queries a lot, mappers might be such a case). The main disadvantage is that the old "magic" was a simple unique key, while in x,y,z only the combination is unique (implied by PRIMARY KEY), and will require slightly more effort for the DBMS to optimize and index.

In the "cursed" scheme, by https://www.sqlite.org/rowidtable.html

The exception to this rule is when the rowid table declares an INTEGER PRIMARY KEY. In the exception, the INTEGER PRIMARY KEY becomes an alias for the rowid.
any query by ID would directly go to the primary b-tree storage.

With (x,y,z), sqlite supposedly creates a lookup table (x,y,z) -> rowid, then retrieves the rowid from the b-tree.

Don't try to draw conclusions from 8k blocks. It's too small to make a difference. In many cases, indexes in DBMS will be organized in blocks, the default page size of SQLITE3 is now 4096.
At this database size, the database will likely only index by x in the (x,y,z) scheme, then scan the corresponding page(s) for the desired matches on y,z. Note that this isn't bad by itself. In the old scheme, it would likely primarily split by z, because these are the most significant bits in the encoding.
Some databases may allow you to study their query plan.

[kno10]

Note that the old tables made bad use of SQLITE, unfortunately.

sqlite> .schema
CREATE TABLE `blocks` (
	`pos` INT PRIMARY KEY,
	`data` BLOB
);
sqlite> explain query plan SELECT * from blocks where pos = 0;
QUERY PLAN
`--SEARCH blocks USING INDEX sqlite_autoindex_blocks_1 (pos=?)

supposedly because it is an INT PRIMARY KEY and not an INTEGER PRIMARY KEY:

sqlite> create table blocks2 (pos INTEGER PRIMARY KEY, data BLOB);
sqlite> explain query plan SELECT * from blocks2 where pos = 0;
QUERY PLAN
`--SEARCH blocks2 USING INTEGER PRIMARY KEY (rowid=?)

Note that this now is a direct rowid access, without an index.

Hence I suggest to rather use pos INTEGER PRIMARY KEY for comparison!

For xyz, we get

sqlite> create table blocksxyz (X INTEGER, Y INTEGER, Z INTEGER, data BLOB, PRIMARY KEY (x,y,z));
sqlite> explain query plan SELECT * from blocksxyz where x = 0 AND y = 0 AND z = 0;
QUERY PLAN
`--SEARCH blocksxyz USING INDEX sqlite_autoindex_blocksxyz_1 (X=? AND Y=? AND Z=?)

The file has 136889 blocks in the old format, on average 228 bytes of data + 8 bytes of integer, but the overall file size is 360 bytes per block, so there is a substantial overhead. VACUUM reduced this to 283.

[kno10]

To convert an existing sqlite to use pos INTEGER PRIMARY KEY, use:

sqlite> create table blocks2 (pos INTEGER PRIMARY KEY, data BLOB);
sqlite> insert into blocks2 SELECT * from blocks;
sqlite> drop table blocks;
sqlite> alter table blocks2 rename to blocks;
sqlite> vacuum;

This one currently has 271 bytes per block, as it does not have that unnecessary extra index.

[kno10]

As the pos INTEGER PRIMARY KEY can serve as rowid, but the x,y,z coordinates cannot, it is likely possible to add the pos to the xyz format at no additional cost, as it simply replaces the rowid.

[appgurueu]

I don't think that we need to prematurely optimize the block table. I think this change is/was probably worth it for the code quality improvement alone, both in Luanti and for external tools such as luantimapper.

Because pos was an INT, there should be virtually no significant performance regression (and lhofhansl's measurements, to an extent, confirm this). Both this and the old format require an indexing mapping primary keys to rowids as far as I understand.

And furthermore (see #15768 (comment)) some queries (which may be outside of luanti, e.g. luantimapper) may actually benefit from such an index. So even the overall performance tradeoffs aren't all that obvious.

In conclusion, I think this PR is/was fine as-is (what do you want to happen?).

If there is a significant performance improvement to be made from having a pos INTEGER PRIMARY KEY instead, feel free to open a PR, but it seems very unlikely to me.

[kno10]

@sorcerykid I doubt that your current caching approach is that beneficial. It's very costly to maintain such a table.
You can translate range queries to a SQL query and have SQLite only return those blocks that match your range of interest (but it will likely still scan a substantial slice of the database).

As shown in #15836, the translation of block numbers to x,y,z can be simplified quite a bit (although that branch still needs more tests).

Here is an example query:

SELECT
pos,
((pos + 0x800800800) & 0xFFF) - 0x800 as x,
(((pos + 0x800800800) >> 12) & 0xFFF) - 0x800 as y,
(((pos + 0x800800800) >> 24) & 0xFFF) - 0x800 as z
FROM blocks where
((pos + 0x800800800) & 0xFFF) - 0x800 >= -3  AND -- minx
((pos + 0x800800800) & 0xFFF) - 0x800 <= 3 AND -- maxx
(((pos + 0x800800800) >> 12) & 0xFFF) - 0x800 >= -5 AND -- miny
(((pos + 0x800800800) >> 12) & 0xFFF) - 0x800 <= 1 AND -- maxy
pos >= (-12 << 24) - 0x800800 AND -- minz
pos <= (-4  << 24) + 0x7FF7FF; -- maxz

This supposedly uses the z coordinate on the index (with INTEGER PRIMARY KEY):

QUERY PLAN
`--SEARCH blocks USING INTEGER PRIMARY KEY (rowid>? AND rowid<?)

respectively (with INT PRIMARY KEY)

QUERY PLAN
`--SEARCH blocks USING COVERING INDEX sqlite_autoindex_blocks_1 (pos>? AND pos<?)

If your only want the block numbers, the second may even be faster, as the autoindex is more compact, while in the first case it likely will already read the blobs from disk when scanning.

It returns matching blocks as far as they exist in the database:

-201347075|-3|-5|-12
-201347074|-2|-5|-12
-201347073|-1|-5|-12
...
-67104767|1|1|-4
-67104766|2|1|-4
-67104765|3|1|-4

(feel welcome to very that the math is correct)

[kno10]

@appgurueu:

I don't think that we need to prematurely optimize the block table. I think this change is/was probably worth it for the code quality improvement alone, both in Luanti and for external tools such as luantimapper.

With the old "cursed" code that emulated python modulo operations I understand the motivation to clean this up very well. But with https://github.com/luanti-org/luanti/pull/15836/files it is not too bad actually. The (x,y,z) may be easier to play around with, but then you still need to understand the binary format if you want to do anything. You can put the SQL range query into the documentation.

If there is a significant performance improvement to be made from having a pos INTEGER PRIMARY KEY instead, feel free to open a PR, but it seems very unlikely to me.

As benchmarked by @lhofhansl in #15768 (comment):

And when I change that INTEGER and create a new world (with same seed) I see that pure DB load time of a block goes from 6-7us to about 5-6us.

That is about the magnitude to expect for INT vs. INTEGER, I guess. Using a rowid directly simply avoids around 3-4 page accesses (that is the depth of the b-tree we'll be seeing, with a fanout of ~250) for the index; but these pages will likely be in the LRU cache anyway. x,y,z may need more comparisons per key, and have a smaller fanout, but it will not make a "huge" difference. Tuning the database block size may very well have more impact.

But as you can see from the query I showcased, you can do index-accelerated range queries on the old format, and most likely these will run faster than with the new format. So maybe just keep this in mind in case you update the format again that maybe the old coding wasn't too bad (nor was it as "cursed"); although a scheme that avoided negative numbers would have been more elegant (not needing the 0x800800800 above, simply being 12 bits for each component concatenated).
One benefit from using pos as rowid is that blocks neighboring on the x axis will be stored in order. With the old INT as well as with the (x,y,z) scheme, the row ids will (assuming no deletions) likely be by creation order. So a range query on the index will then lead to unordered access on the primary b-tree. On "naturally grown" worlds as opposed to bulk-emerged worlds, this can make more of a difference, in particular for mapping tools - at least if they can process data in the preferred order of the database.

[sfan5]

Most of your points have been brought up in some way or another, just wanted to note one thing: Luanti currently does not do anything that could benefit from range queries, it always just goes "give me block (x,y,z) if it exists".
If range queries weren't so complicated (hell, nobody dared to touch the encoding code in years) maybe we would already have some optimizations in this area.

in particular for mapping tools - at least if they can process data in the preferred order of the database

FWIW minetestmapper uses z-descending, x-descending, y-descending order.

[farooqkz]

I highly appreciate this change. Cheers!