Skip to content

[Fix] Disable recursion for invalidated database error #16907

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Apr 1, 2025
Merged

[Fix] Disable recursion for invalidated database error #16907

merged 3 commits into from
Apr 1, 2025

Conversation

taniabogatsch
Copy link
Contributor

With the current design, consecutive calls (e.g., in a concurrent environment) can cause very long error messages before aborting operations due to a fatal error.

This PR ensures that we only prepend the INVALIDATED_DATABASE prefix once.

I am unsure how to include a test for this. Locally, I just added a fatal exception to the CREATE INDEX code and used the test below. I'm not sure if we have some way to 'turn any exception into a fatal exception' or something.

load __TEST_DIR__/fatalerror.db;

statement ok
CREATE TABLE tbl AS SELECT range FROM range(10);

statement error
CREATE INDEX idx ON tbl (range);
----
FATAL Error: oh no

statement error
CREATE INDEX idx ON tbl (range);
----
FATAL Error: Failed: database has been invalidated because of a previous fatal error. The database must be restarted prior to being used again.
Original error: "oh no"

statement error
CREATE INDEX idx ON tbl (range);
----
FATAL Error: Failed: database has been invalidated because of a previous fatal error. The database must be restarted prior to being used again.
Original error: "oh no"

@carlopi
Copy link
Contributor

carlopi commented Mar 31, 2025 •
edited
Loading

We currently have:

SELECT error('I am trapped here');

that will throw a Invalid Input Error with the right message, but I would think for these cases it would be handy to have a way to specify an exception type (error overload with optional second parameter? as a different function?)

@Mytherin?

@duckdb-draftbot duckdb-draftbot marked this pull request as draft March 31, 2025 14:04
@taniabogatsch
Copy link
Contributor Author

I have no idea how to fix that tidy check; it might not be related to this PR.

@Mytherin Mytherin marked this pull request as ready for review March 31, 2025 16:41
@duckdb-draftbot duckdb-draftbot marked this pull request as draft April 1, 2025 07:57
@taniabogatsch taniabogatsch marked this pull request as ready for review April 1, 2025 09:20
@Mytherin Mytherin merged commit c11e855 into duckdb:v1.2-histrionicus Apr 1, 2025
48 of 49 checks passed
@Mytherin
Copy link
Collaborator

Mytherin commented Apr 1, 2025

Thanks!

@taniabogatsch taniabogatsch deleted the truncate-nested-error branch April 1, 2025 12:31
krlmlr added a commit to duckdb/duckdb-r that referenced this pull request Apr 8, 2025
@krlmlr
vendor: Update vendored sources to duckdb/duckdb@c11e855
210428c 
[Fix] Disable recursion for invalidated database error (duckdb/duckdb#16907)
Skip manylinux-extensions-x64 job (duckdb/duckdb#16932)
Unwrap DUCKDB_EXTENSION__LINKED_LIBS via CMake (duckdb/duckdb#16914)
Bump to avro version that now support wasm (duckdb/duckdb#16916)
Skip tests for Python 3.8, already end-of-life (duckdb/duckdb#16923)
Backport "Adding gzip version of shell for linux/osx install script"/2 (duckdb/duckdb#16924)
Backport "Adding gzip version of shell for linux/osx install script" (duckdb/duckdb#16917)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@taniabogatsch @carlopi @Mytherin