Merge pull request #10410 from Mytherin/exceptionrework

Rework Exception Internals

.github/config/bundled_extensions.cmake

@@ -28,4 +28,3 @@ duckdb_extension_load(autocomplete)
 #
 duckdb_extension_load(sqlsmith DONT_LINK)
 duckdb_extension_load(tpcds DONT_LINK)
-duckdb_extension_load(visualizer DONT_LINK)

.github/config/extensions.csv

@@ -7,7 +7,6 @@ json,,,
 parquet,,,
 tpcds,,,
 tpch,,,
-visualizer,,,
 sqlite_scanner,https://github.com/duckdb/sqlite_scanner,3443b2999ae1e68a108568fd32145705237a5760,
 postgres_scanner,https://github.com/duckdb/postgres_scanner,844f46536b5d5f9e65b57b7ff92f4ce3346e2829,
 substrait,https://github.com/duckdb/substrait,5d621b1d7d16fe86f8b1930870c8e6bf05bcb92a,no-windows

.github/config/in_tree_extensions.cmake

@@ -16,4 +16,3 @@ duckdb_extension_load(parquet)
 duckdb_extension_load(sqlsmith)
 duckdb_extension_load(tpcds)
 duckdb_extension_load(tpch)
-duckdb_extension_load(visualizer)

.github/config/out_of_tree_extensions.cmake

@@ -85,6 +85,7 @@ duckdb_extension_load(sqlite_scanner
         ${STATIC_LINK_SQLITE} LOAD_TESTS
         GIT_URL https://github.com/duckdb/sqlite_scanner
         GIT_TAG 9b558ed2e933817bff96726fec0868e7411cee65
+        APPLY_PATCHES
         )
 
 ################# SUBSTRAIT

.github/patches/extensions/postgres_scanner/combined.patch

@@ -1,5 +1,5 @@
 diff --git a/src/include/storage/postgres_transaction_manager.hpp b/src/include/storage/postgres_transaction_manager.hpp
-index 94427d5..b4ac2da 100644
+index 94427d5..24b82b4 100644
 --- a/src/include/storage/postgres_transaction_manager.hpp
 +++ b/src/include/storage/postgres_transaction_manager.hpp
 @@ -11,6 +11,7 @@
@@ -18,7 +18,7 @@ index 94427d5..b4ac2da 100644
 -	string CommitTransaction(ClientContext &context, Transaction *transaction) override;
 -	void RollbackTransaction(Transaction *transaction) override;
 +	Transaction &StartTransaction(ClientContext &context) override;
-+	string CommitTransaction(ClientContext &context, Transaction &transaction) override;
++	ErrorData CommitTransaction(ClientContext &context, Transaction &transaction) override;
 +	void RollbackTransaction(Transaction &transaction) override;
 
  	void Checkpoint(ClientContext &context, bool force = false) override;
@@ -32,7 +32,7 @@ index 94427d5..b4ac2da 100644
 
  } // namespace duckdb
 diff --git a/src/postgres_filter_pushdown.cpp b/src/postgres_filter_pushdown.cpp
-index 01a034c..6017770 100644
+index 96cedaf..1faf692 100644
 --- a/src/postgres_filter_pushdown.cpp
 +++ b/src/postgres_filter_pushdown.cpp
 @@ -1,5 +1,6 @@
@@ -42,7 +42,7 @@ index 01a034c..6017770 100644
 
  namespace duckdb {
 
-@@ -50,6 +51,12 @@ string PostgresFilterPushdown::TransformFilter(string &column_name, TableFilter
+@@ -51,6 +52,12 @@ string PostgresFilterPushdown::TransformFilter(string &column_name, TableFilter
  		auto operator_string = TransformComparision(constant_filter.comparison_type);
  		return StringUtil::Format("%s %s %s", column_name, operator_string, constant_string);
  	}
@@ -55,7 +55,7 @@ index 01a034c..6017770 100644
  	default:
  		throw InternalException("Unsupported table filter type");
  	}
-@@ -67,6 +74,11 @@ string PostgresFilterPushdown::TransformFilters(const vector<column_t> &column_i
+@@ -69,6 +76,11 @@ string PostgresFilterPushdown::TransformFilters(const vector<column_t> &column_i
  		}
  		auto column_name = KeywordHelper::WriteQuoted(names[column_ids[entry.first]], '"');
  		auto &filter = *entry.second;
@@ -68,10 +68,10 @@ index 01a034c..6017770 100644
  	}
  	return result;
 diff --git a/src/storage/postgres_table_set.cpp b/src/storage/postgres_table_set.cpp
-index 88786cf..bfd37ab 100644
+index e397203..9457c55 100644
 --- a/src/storage/postgres_table_set.cpp
 +++ b/src/storage/postgres_table_set.cpp
-@@ -205,8 +205,8 @@ string PostgresColumnsToSQL(const ColumnList &columns, const vector<unique_ptr<C
+@@ -240,8 +240,8 @@ string PostgresColumnsToSQL(const ColumnList &columns, const vector<unique_ptr<C
  		}
  		if (column.Generated()) {
  			ss << " GENERATED ALWAYS AS(" << column.GeneratedExpression().ToString() << ")";
@@ -83,7 +83,7 @@ index 88786cf..bfd37ab 100644
  	}
  	// print any extra constraints that still need to be printed
 diff --git a/src/storage/postgres_transaction_manager.cpp b/src/storage/postgres_transaction_manager.cpp
-index 6de4b5c..e1b89e6 100644
+index 6de4b5c..d546c29 100644
 --- a/src/storage/postgres_transaction_manager.cpp
 +++ b/src/storage/postgres_transaction_manager.cpp
 @@ -7,26 +7,26 @@ PostgresTransactionManager::PostgresTransactionManager(AttachedDatabase &db_p, P
@@ -104,12 +104,13 @@ index 6de4b5c..e1b89e6 100644
 -string PostgresTransactionManager::CommitTransaction(ClientContext &context, Transaction *transaction) {
 -	auto postgres_transaction = (PostgresTransaction *)transaction;
 -	postgres_transaction->Commit();
-+string PostgresTransactionManager::CommitTransaction(ClientContext &context, Transaction &transaction) {
++ErrorData PostgresTransactionManager::CommitTransaction(ClientContext &context, Transaction &transaction) {
 +	auto &postgres_transaction = transaction.Cast<PostgresTransaction>();
 +	postgres_transaction.Commit();
  	lock_guard<mutex> l(transaction_lock);
  	transactions.erase(transaction);
- 	return string();
+-	return string();
++	return ErrorData();
  }
 
 -void PostgresTransactionManager::RollbackTransaction(Transaction *transaction) {
@@ -122,10 +123,10 @@ index 6de4b5c..e1b89e6 100644
  	transactions.erase(transaction);
  }
 diff --git a/test/sql/storage/attach_detach.test b/test/sql/storage/attach_detach.test
-index 7b7d19f..ba8168f 100644
+index 5def3a1..8b660f9 100644
 --- a/test/sql/storage/attach_detach.test
 +++ b/test/sql/storage/attach_detach.test
-@@ -18,6 +18,7 @@ DETACH s1
+@@ -23,6 +23,7 @@ DETACH s1
 
  statement error
  SELECT * FROM s1.test
@@ -134,29 +135,29 @@ index 7b7d19f..ba8168f 100644
  statement ok
  ATTACH 'dbname=postgresscanner' AS s1 (TYPE POSTGRES)
 diff --git a/test/sql/storage/attach_drop.test b/test/sql/storage/attach_drop.test
-index 8f1090c..90f6952 100644
+index b73d588..518b5aa 100644
 --- a/test/sql/storage/attach_drop.test
 +++ b/test/sql/storage/attach_drop.test
-@@ -27,6 +27,7 @@ DROP TABLE simple.test
+@@ -32,6 +32,7 @@ DROP TABLE simple.test
  # verify the drop was successful
  statement error
  SELECT * FROM simple.test;
 +----
 
  statement error
  DROP TABLE simple.testx
-@@ -34,4 +35,4 @@ DROP TABLE simple.testx
+@@ -39,4 +40,4 @@ DROP TABLE simple.testx
  Table with name testx does not exist
 
  statement ok
 -DROP TABLE IF EXISTS simple.testx
 \ No newline at end of file
 +DROP TABLE IF EXISTS simple.testx
 diff --git a/test/sql/storage/attach_filter_pushdown.test b/test/sql/storage/attach_filter_pushdown.test
-index 17b92e2..25efe1a 100644
+index c80140e..ca0a7d6 100644
 --- a/test/sql/storage/attach_filter_pushdown.test
 +++ b/test/sql/storage/attach_filter_pushdown.test
-@@ -20,3 +20,14 @@ query I
+@@ -25,3 +25,14 @@ query I
  SELECT * FROM s1.filter_pushdown WHERE i=52525
  ----
  52525
@@ -173,10 +174,10 @@ index 17b92e2..25efe1a 100644
 +{'a': {'i': 4, 'j': 5}, 'k': 6}
 \ No newline at end of file
 diff --git a/test/sql/storage/attach_views.test b/test/sql/storage/attach_views.test
-index e57b45d..0e12b54 100644
+index 44bfe4d..b5145f5 100644
 --- a/test/sql/storage/attach_views.test
 +++ b/test/sql/storage/attach_views.test
-@@ -34,9 +34,11 @@ cannot copy to view
+@@ -39,9 +39,11 @@ cannot copy to view
  # FIXME - error message here is not very descriptive
  statement error
  UPDATE v1 SET i=84

.github/patches/extensions/spatial/copy_bind.patch

@@ -1,3 +1,61 @@
+diff --git a/spatial/src/spatial/core/functions/cast/geometry_cast.cpp b/spatial/src/spatial/core/functions/cast/geometry_cast.cpp
+index af7f030..20883c0 100644
+--- a/spatial/src/spatial/core/functions/cast/geometry_cast.cpp
++++ b/spatial/src/spatial/core/functions/cast/geometry_cast.cpp
+@@ -3,7 +3,7 @@
+ #include "spatial/core/functions/cast.hpp"
+ #include "spatial/core/geometry/geometry.hpp"
+ #include "spatial/core/functions/common.hpp"
+-
++#include "duckdb/common/exception/conversion_exception.hpp"
+ #include "duckdb/function/cast/cast_function_set.hpp"
+ #include "duckdb/common/vector_operations/generic_executor.hpp"
+
+@@ -40,11 +40,11 @@ static bool GeometryToPoint2DCast(Vector &source, Vector &result, idx_t count, C
+ 	GenericExecutor::ExecuteUnary<GEOMETRY_TYPE, POINT_TYPE>(source, result, count, [&](GEOMETRY_TYPE &geometry) {
+ 		auto geom = lstate.factory.Deserialize(geometry.val);
+ 		if (geom.Type() != GeometryType::POINT) {
+-			throw CastException("Cannot cast non-point GEOMETRY to POINT_2D");
++			throw ConversionException("Cannot cast non-point GEOMETRY to POINT_2D");
+ 		}
+ 		auto &point = geom.GetPoint();
+ 		if (point.IsEmpty()) {
+-			throw CastException("Cannot cast empty point GEOMETRY to POINT_2D");
++			throw ConversionException("Cannot cast empty point GEOMETRY to POINT_2D");
+ 		}
+ 		auto vertex = point.GetVertex();
+ 		return POINT_TYPE {vertex.x, vertex.y};
+@@ -92,7 +92,7 @@ static bool GeometryToLineString2DCast(Vector &source, Vector &result, idx_t cou
+ 	UnaryExecutor::Execute<string_t, list_entry_t>(source, result, count, [&](string_t &geom) {
+ 		auto geometry = lstate.factory.Deserialize(geom);
+ 		if (geometry.Type() != GeometryType::LINESTRING) {
+-			throw CastException("Cannot cast non-linestring GEOMETRY to LINESTRING_2D");
++			throw ConversionException("Cannot cast non-linestring GEOMETRY to LINESTRING_2D");
+ 		}
+
+ 		auto &line = geometry.GetLineString();
+@@ -157,7 +157,7 @@ static bool GeometryToPolygon2DCast(Vector &source, Vector &result, idx_t count,
+ 	UnaryExecutor::Execute<string_t, list_entry_t>(source, result, count, [&](string_t &geom) {
+ 		auto geometry = lstate.factory.Deserialize(geom);
+ 		if (geometry.Type() != GeometryType::POLYGON) {
+-			throw CastException("Cannot cast non-linestring GEOMETRY to POLYGON_2D");
++			throw ConversionException("Cannot cast non-linestring GEOMETRY to POLYGON_2D");
+ 		}
+
+ 		auto &poly = geometry.GetPolygon();
+diff --git a/spatial/src/spatial/gdal/functions/st_read.cpp b/spatial/src/spatial/gdal/functions/st_read.cpp
+index eecc020..5cf3159 100644
+--- a/spatial/src/spatial/gdal/functions/st_read.cpp
++++ b/spatial/src/spatial/gdal/functions/st_read.cpp
+@@ -39,7 +39,7 @@ struct WKBSpatialFilter : SpatialFilter {
+ 	explicit WKBSpatialFilter(const string &wkb_p) : SpatialFilter(SpatialFilterType::Wkb), geom(nullptr) {
+ 		auto ok = OGR_G_CreateFromWkb(wkb_p.c_str(), nullptr, &geom, (int)wkb_p.size());
+ 		if (ok != OGRERR_NONE) {
+-			throw Exception("WKBSpatialFilter: could not create geometry from WKB");
++			throw InvalidInputException("WKBSpatialFilter: could not create geometry from WKB");
+ 		}
+ 	}
+ 	~WKBSpatialFilter() {
 diff --git a/spatial/src/spatial/gdal/functions/st_write.cpp b/spatial/src/spatial/gdal/functions/st_write.cpp
 index 8ab228d..1a1e821 100644
 --- a/spatial/src/spatial/gdal/functions/st_write.cpp
@@ -20,3 +78,41 @@ index 8ab228d..1a1e821 100644
  		if (StringUtil::Upper(option.first) == "DRIVER") {
  			auto set = option.second.front();
  			if (set.type().id() == LogicalTypeId::VARCHAR) {
+diff --git a/spatial/src/spatial/geos/functions/cast.cpp b/spatial/src/spatial/geos/functions/cast.cpp
+index e6d5cb8..1c87af3 100644
+--- a/spatial/src/spatial/geos/functions/cast.cpp
++++ b/spatial/src/spatial/geos/functions/cast.cpp
+@@ -6,6 +6,7 @@
+
+ #include "duckdb/function/cast/cast_function_set.hpp"
+ #include "duckdb/common/operator/cast_operators.hpp"
++#include "duckdb/common/error_data.hpp"
+
+ namespace spatial {
+
+@@ -54,9 +55,10 @@ static bool TextToGeometryCast(Vector &source, Vector &result, idx_t count, Cast
+ 				    throw InvalidInputException("3D/4D geometries are not supported");
+ 			    }
+ 			    return lstate.ctx.Serialize(result, geos_geom);
+-		    } catch (InvalidInputException &error) {
++		    } catch (InvalidInputException &e) {
+ 			    if (success) {
+ 				    success = false;
++				    ErrorData error(e);
+ 				    HandleCastError::AssignError(error.RawMessage(), parameters.error_message);
+ 			    }
+ 			    mask.SetInvalid(idx);
+diff --git a/spatial/src/spatial/geos/functions/scalar/st_normalize.cpp b/spatial/src/spatial/geos/functions/scalar/st_normalize.cpp
+index f44d990..e90fa2e 100644
+--- a/spatial/src/spatial/geos/functions/scalar/st_normalize.cpp
++++ b/spatial/src/spatial/geos/functions/scalar/st_normalize.cpp
+@@ -21,8 +21,7 @@ static void NormalizeFunction(DataChunk &args, ExpressionState &state, Vector &r
+ 		auto geom = lstate.ctx.Deserialize(input);
+ 		auto res = GEOSNormalize_r(ctx, geom.get());
+ 		if (res == -1) {
+-			throw Exception("Could not normalize geometry");
+-			;
++			throw InvalidInputException("Could not normalize geometry");
+ 		}
+ 		return lstate.ctx.Serialize(result, geom);
+ 	});

.github/patches/extensions/sqlite_scanner/combined.patch

@@ -0,0 +1,44 @@
+diff --git a/src/include/storage/sqlite_transaction_manager.hpp b/src/include/storage/sqlite_transaction_manager.hpp
+index 4982eef..8f9cafc 100644
+--- a/src/include/storage/sqlite_transaction_manager.hpp
++++ b/src/include/storage/sqlite_transaction_manager.hpp
+@@ -20,7 +20,7 @@ public:
+ 	SQLiteTransactionManager(AttachedDatabase &db_p, SQLiteCatalog &sqlite_catalog);
+
+ 	Transaction &StartTransaction(ClientContext &context) override;
+-	string CommitTransaction(ClientContext &context, Transaction &transaction) override;
++	ErrorData CommitTransaction(ClientContext &context, Transaction &transaction) override;
+ 	void RollbackTransaction(Transaction &transaction) override;
+
+ 	void Checkpoint(ClientContext &context, bool force = false) override;
+diff --git a/src/storage/sqlite_catalog.cpp b/src/storage/sqlite_catalog.cpp
+index fad530c..36a5764 100644
+--- a/src/storage/sqlite_catalog.cpp
++++ b/src/storage/sqlite_catalog.cpp
+@@ -3,6 +3,7 @@
+ #include "storage/sqlite_transaction.hpp"
+ #include "sqlite_db.hpp"
+ #include "duckdb/storage/database_size.hpp"
++#include "duckdb/common/exception/transaction_exception.hpp"
+
+ namespace duckdb {
+
+diff --git a/src/storage/sqlite_transaction_manager.cpp b/src/storage/sqlite_transaction_manager.cpp
+index 7b6e132..d800794 100644
+--- a/src/storage/sqlite_transaction_manager.cpp
++++ b/src/storage/sqlite_transaction_manager.cpp
+@@ -16,12 +16,12 @@ Transaction &SQLiteTransactionManager::StartTransaction(ClientContext &context)
+ 	return result;
+ }
+
+-string SQLiteTransactionManager::CommitTransaction(ClientContext &context, Transaction &transaction) {
++ErrorData SQLiteTransactionManager::CommitTransaction(ClientContext &context, Transaction &transaction) {
+ 	auto &sqlite_transaction = transaction.Cast<SQLiteTransaction>();
+ 	sqlite_transaction.Commit();
+ 	lock_guard<mutex> l(transaction_lock);
+ 	transactions.erase(transaction);
+-	return string();
++	return ErrorData();
+ }
+
+ void SQLiteTransactionManager::RollbackTransaction(Transaction &transaction) {

.github/workflows/LinuxRelease.yml

@@ -340,7 +340,6 @@ jobs:
     runs-on: ubuntu-20.04
     needs: linux-release-64
     env:
-      BUILD_VISUALIZER: 1
       BUILD_HTTPFS: 1
       BUILD_TPCH: 1
       BUILD_TPCDS: 1

.github/workflows/Main.yml

@@ -46,7 +46,6 @@ jobs:
       CC: gcc-10
       CXX: g++-10
       TREAT_WARNINGS_AS_ERRORS: 1
-      BUILD_VISUALIZER: 1
       GEN: ninja
 
     steps:
@@ -95,7 +94,6 @@ jobs:
       BUILD_TPCH: 1
       BUILD_TPCDS: 1
       BUILD_FTS: 1
-      BUILD_VISUALIZER: 1
       BUILD_JSON: 1
       BUILD_EXCEL: 1
       BUILD_JEMALLOC: 1
@@ -139,7 +137,6 @@ jobs:
       BUILD_TPCH: 1
       BUILD_TPCDS: 1
       BUILD_FTS: 1
-      BUILD_VISUALIZER: 1
       BUILD_JSON: 1
       BUILD_EXCEL: 1
       BUILD_JEMALLOC: 1

.github/workflows/lcov_exclude

@@ -14,7 +14,6 @@
 */extension/jemalloc/*
 */extension/tpcds/*
 */extension/tpch/*
-*/extension/visualizer/*
 */extension/json/yyjson/*
 */extension_helper.cpp
 */generated_extension_loader.hpp

Makefile

@@ -104,9 +104,6 @@ endif
 ifeq (${BUILD_FTS}, 1)
 	BUILD_EXTENSIONS:=${BUILD_EXTENSIONS};fts
 endif
-ifeq (${BUILD_VISUALIZER}, 1)
-	BUILD_EXTENSIONS:=${BUILD_EXTENSIONS};visualizer
-endif
 ifeq (${BUILD_HTTPFS}, 1)
 	BUILD_EXTENSIONS:=${BUILD_EXTENSIONS};httpfs
 endif

benchmark/interpreted_benchmark.cpp

@@ -302,7 +302,7 @@ void InterpretedBenchmark::LoadBenchmark() {
 	}
 	// set up the queries
 	if (queries.find("run") == queries.end()) {
-		throw Exception("Invalid benchmark file: no \"run\" query specified");
+		throw InvalidInputException("Invalid benchmark file: no \"run\" query specified");
 	}
 	run_query = queries["run"];
 	is_loaded = true;
@@ -325,10 +325,10 @@ unique_ptr<BenchmarkState> InterpretedBenchmark::Initialize(BenchmarkConfigurati
 	for (auto &extension : extensions) {
 		auto result = ExtensionHelper::LoadExtension(state->db, extension);
 		if (result == ExtensionLoadResult::EXTENSION_UNKNOWN) {
-			throw std::runtime_error("Unknown extension " + extension);
+			throw InvalidInputException("Unknown extension " + extension);
 		} else if (result == ExtensionLoadResult::NOT_LOADED) {
-			throw std::runtime_error("Extension " + extension +
-			                         " is not available/was not compiled. Cannot run this benchmark.");
+			throw InvalidInputException("Extension " + extension +
+			                            " is not available/was not compiled. Cannot run this benchmark.");
 		}
 	}