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.

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

fix crash on finalization for logical_type from get_value_type #135

Merged
merged 2 commits into from
Feb 1, 2025
Merged

fix crash on finalization for logical_type from get_value_type #135

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7b85ac1
fix crash on finalization for logical_type from get_value_type
jraymakers Feb 1, 2025
92d2c30
re-enable bindings tests on Windows
jraymakers Feb 1, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions .github/workflows/DuckDBNodeBindingsAndAPI.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -267,10 +267,9 @@ jobs:
working-directory: bindings
run: pnpm run build

# Fails for unknown reasons
# - name: Bindings - Test
# working-directory: bindings
# run: pnpm test
- name: Bindings - Test
working-directory: bindings
run: pnpm test

- name: API - Build
working-directory: api
Expand Down
11 changes: 6 additions & 5 deletions bindings/src/duckdb_node_bindings.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -360,16 +360,17 @@ static const napi_type_tag LogicalTypeTypeTag = {
};

void FinalizeLogicalType(Napi::BasicEnv, duckdb_logical_type logical_type) {
if (logical_type) {
duckdb_destroy_logical_type(&logical_type);
logical_type = nullptr;
}
duckdb_destroy_logical_type(&logical_type);
}

Napi::External<_duckdb_logical_type> CreateExternalForLogicalType(Napi::Env env, duckdb_logical_type logical_type) {
return CreateExternal<_duckdb_logical_type>(env, LogicalTypeTypeTag, logical_type, FinalizeLogicalType);
}

Napi::External<_duckdb_logical_type> CreateExternalForLogicalTypeWithoutFinalizer(Napi::Env env, duckdb_logical_type logical_type) {
return CreateExternalWithoutFinalizer<_duckdb_logical_type>(env, LogicalTypeTypeTag, logical_type);
}

duckdb_logical_type GetLogicalTypeFromExternal(Napi::Env env, Napi::Value value) {
return GetDataFromExternal<_duckdb_logical_type>(env, LogicalTypeTypeTag, value, "Invalid logical type argument");
}
Expand Down Expand Up @@ -2554,7 +2555,7 @@ class DuckDBNodeAddon : public Napi::Addon<DuckDBNodeAddon> {
auto env = info.Env();
auto value = GetValueFromExternal(env, info[0]);
auto logical_type = duckdb_get_value_type(value);
return CreateExternalForLogicalType(env, logical_type);
return CreateExternalForLogicalTypeWithoutFinalizer(env, logical_type);
}

// DUCKDB_API duckdb_blob duckdb_get_blob(duckdb_value val);
Expand Down