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

DuckDB Crashes when trying to use r-tree spatial index #405

Closed
DrewScatterday opened this issue Sep 19, 2024 · 17 comments · Fixed by duckdb/duckdb-node#127
Closed

DuckDB Crashes when trying to use r-tree spatial index #405

DrewScatterday opened this issue Sep 19, 2024 · 17 comments · Fixed by duckdb/duckdb-node#127

Comments

@DrewScatterday
Copy link
Contributor

Hi! Thanks for your hardwork on DuckDB absolutely loving it in my day to day spatial data work.

I think this may be be related to #391 but not sure. I think I found a bug with r-tree indexing. I'm using example code from this article https://duckdb.org/docs/extensions/spatial/r-tree_indexes#example

I'm using node.js 20.11.1, Windows 10, and duckdb 1.1.0

If I run this code, it dies with no error thrown:

const duckdb = require("duckdb");

function queryWithCallback(conn, sql) {
	return new Promise((resolve, reject) => {
		conn.all(sql, (err, result) => {
			if (err) {
				reject(err);
			} else {
				resolve(result);
			}
		});
	});
}

async function main() {
	const db = new duckdb.Database(":memory:");
	var sqlQuery = `INSTALL spatial;
					LOAD spatial;
					CREATE TABLE t1 AS SELECT point::GEOMETRY as geom
					FROM st_generatepoints({min_x: 0, min_y: 0, max_x: 100, max_y: 100}::BOX_2D, 10_000, 1337);
					CREATE INDEX my_idx ON t1 USING RTREE (geom);
					SELECT count(*) FROM t1 WHERE ST_Within(geom, ST_MakeEnvelope(45, 45, 65, 65));`
	const rows = await queryWithCallback(db, sqlQuery);
	console.log(rows);
	console.log("🦆 DuckDB initialized 🦆");
}

main();

If I remove the r-tree index, it works with no problem:

const duckdb = require("duckdb");

function queryWithCallback(conn, sql) {
	return new Promise((resolve, reject) => {
		conn.all(sql, (err, result) => {
			if (err) {
				reject(err);
			} else {
				resolve(result);
			}
		});
	});
}

async function main() {
	const db = new duckdb.Database(":memory:");
	var sqlQuery = `INSTALL spatial;
					LOAD spatial;
					CREATE TABLE t1 AS SELECT point::GEOMETRY as geom
					FROM st_generatepoints({min_x: 0, min_y: 0, max_x: 100, max_y: 100}::BOX_2D, 10_000, 1337);
					SELECT count(*) FROM t1 WHERE ST_Within(geom, ST_MakeEnvelope(45, 45, 65, 65));`
	const rows = await queryWithCallback(db, sqlQuery);
	console.log(rows);
	console.log("🦆 DuckDB initialized 🦆");
}

main();

Output:

[ { 'count_star()': 390n } ]
🦆 DuckDB initialized 🦆

I also tried using nightly build:

const duckdb = require("duckdb");

function queryWithCallback(conn, sql) {
	return new Promise((resolve, reject) => {
		conn.all(sql, (err, result) => {
			if (err) {
				reject(err);
			} else {
				resolve(result);
			}
		});
	});
}

async function main() {
	const db = new duckdb.Database(":memory:");
	var sqlQuery = `FORCE INSTALL spatial FROM core_nightly;
					LOAD spatial;
					CREATE TABLE t1 AS SELECT point::GEOMETRY as geom
					FROM st_generatepoints({min_x: 0, min_y: 0, max_x: 100, max_y: 100}::BOX_2D, 10_000, 1337);
					CREATE INDEX my_idx ON t1 USING RTREE (geom);
					SELECT count(*) FROM t1 WHERE ST_Within(geom, ST_MakeEnvelope(45, 45, 65, 65));`
	const rows = await queryWithCallback(db, sqlQuery);
	console.log(rows);
	console.log("🦆 DuckDB initialized 🦆");
}

main();

I get this as the error message:

node:internal/process/promises:289
            triggerUncaughtException(err, true /* fromPromise */);
            ^

�☻] {r: Invalid Error: P☺�
  errno: -1,
  code: 'DUCKDB_NODEJS_ERROR',
  errorType: 'Invalid'
}

Node.js v20.11.1

Any ideas, am I missing something?

@Maxxen
Copy link
Member

Maxxen commented Sep 19, 2024

Hi! Thanks for reporting this issue!

My first hunch is that this is related to duckdb/duckdb#13848 (comment), but the fact that it only crashes when you instantiate the r-tree index is strange... We run the r-tree tests as part of our CI on windows, so maybe it's node related? I'll try to have a look tomorrow.

@DrewScatterday
Copy link
Contributor Author

Thanks @Maxxen !! Let me know what you find out

I can test on a Ubuntu WSL node js env just to help sanity check

@DrewScatterday
Copy link
Contributor Author

Code works with no issues on Ubuntu 22.04.02 and same version of node (20.11.1)

So definitely a windows issue and not a rtree bug hmmm

@DrewScatterday
Copy link
Contributor Author

DrewScatterday commented Sep 19, 2024

I tried installing the nightly version of duckdb on winodws with npm install duckdb@next but the cmd window seems to just hang:

[##################] - reify:duckdb: timing reify:audit Completed in 276ms

edit: uninstalled duckdb and then installed with npm install duckdb@next and then still got the same issue

@DrewScatterday
Copy link
Contributor Author

The duckdb version that gets installed with npm install duckdb@next is "duckdb": "^1.0.1-dev27.0". Was still getting this issue on that version.

I followed the thread in the issue you linked and did the following:

  • updated my MSVC package to latest which is 14.40.33810.0
  • I deleted the 1.1.0 extensions folder in .duckdb/extensions
  • I then fresh installed duckdb in node with npm install duckdb

I then still got same issue

I also tried reinstalling spatial extension with FORCE INSTALL spatial FROM core_nightly; but I still get that weird error message I got above

@carlopi
Copy link
Contributor

carlopi commented Sep 24, 2024

@DrewScatterday: duckdb-node has been updated, now the DuckDB library is at version v1.1.1.
This brings in quite a few changes both DuckDB and spatial extension side, so it's possible that the situation regarding this bug has changed.

Could you possibly try to install https://www.npmjs.com/package/duckdb/v/1.1.1-dev3.0, and re-run the previous test?

Thanks a lot!

@DrewScatterday
Copy link
Contributor Author

Hey @carlopi thanks for your hard work on duckdb!!

Unfortunately I still get the same issue, let me know if you are able to reproduce. I'm wondering if its something fishy with the spatial extension binary for Windows?

  • I installed "duckdb": "^1.1.1-dev3.0" and also deleted the 1.1.0 extensions folder in .duckdb/extensions
  • I'm on the latest MSVC version which is 14.40.33810.0
  • I also tried installing the latest spatial using INSTALL spatial FROM core_nightly; but still no luck there either

@carlopi
Copy link
Contributor

carlopi commented Sep 24, 2024

Thanks for the info, we will dig a bit deeper and get to the bottom of this.

@DrewScatterday
Copy link
Contributor Author

Any updates on this? are you guys able to reproduce this? @Maxxen @carlopi thank you for your hard work!!

@Maxxen
Copy link
Member

Maxxen commented Oct 2, 2024

Hello! I've been able to reproduce it, but yeah it only crashes on the specific combination of windows+node. windows+python seems fine. I've spent about a day looking into this but debugging node on windows is extremely time consuming as almost any change requires a full-recompilation and the windows machines we have access to are either under-powered or impose a completely different DX to what we're used to. This is of course something we want to fix so I am going to investigate this further but I can't provide an exact timeline.

@DrewScatterday
Copy link
Contributor Author

Totally understand Windows is a giant pain haha

Keep me posted and if there is anything I can help test let me know

@Maxxen
Copy link
Member

Maxxen commented Oct 12, 2024

Debugging this almost killed me, but I learned a lot about windows and I think I got a fix for this in duckdb/duckdb-node#127 :). Should hopefully make in for DuckDB v1.1.2

@DrewScatterday
Copy link
Contributor Author

Hahaha thanks @Maxxen you're a legend!!

If it were up to me I wouldn't touch windows with a 10 ft pole but unfortunately the project I'm working on has other dependencies on Windows. Thanks again for looking into this, can I test it on the nightly build?

@carlopi
Copy link
Contributor

carlopi commented Oct 12, 2024

@DrewScatterday: a pre-release version of duckdb-node is being built, it should be up in a couple of hours.

Can you possibly then double check whether this works? This will then next week be released as 1.1.2.

Thanks everyone, and amazing bug hunt from @Maxxen

@DrewScatterday
Copy link
Contributor Author

Yes I'll test as soon as I can on nightly thanks guys!

@carlopi
Copy link
Contributor

carlopi commented Oct 13, 2024

Duckdb-node nightly version is here: https://www.npmjs.com/package/duckdb/v/1.1.2-dev4.0 (on duckdb v1.1.1)

@DrewScatterday
Copy link
Contributor Author

on 1.1.2-dev4.0 the issue is fixed on my side thanks again for your hardwork! 🚀 🥳

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants