Add DuckDB::Error#error_type#1382
Conversation
Expose the DuckDB error category as a Symbol (e.g. :constraint, :catalog, :parser) on DuckDB::Error, or nil for errors not originating from a query result. A shared C helper captures duckdb_result_error_type at the two result-error sites (raw query + prepared statement execution) and hands the message + raw id to DuckDB::Error.new; the int -> Symbol mapping lives in Ruby. This lets the ActiveRecord adapter map failures to RecordNotUnique / NotNullViolation etc. without parsing error messages. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds ChangesDuckDB::Error#error_type feature
Sequence Diagram(s)sequenceDiagram
participant Client as Ruby caller
participant conn_c as connection.c
participant err_c as rbduckdb_raise_result_error
participant DuckDBError as DuckDB::Error (Ruby)
participant IntToSym as Converter::IntToSym
Client->>conn_c: db.query(sql)
conn_c->>conn_c: duckdb_query → DuckDBError result
conn_c->>err_c: rbduckdb_raise_result_error(&result)
err_c->>err_c: duckdb_result_error → message
err_c->>err_c: duckdb_result_error_type → type_id
err_c->>DuckDBError: new(message, type_id)
err_c-->>Client: rb_exc_raise (NORETURN)
Client->>DuckDBError: rescue → e.error_type
DuckDBError->>IntToSym: error_type_to_sym(`@error_type_id`)
IntToSym-->>DuckDBError: :constraint / :catalog / …
DuckDBError-->>Client: Symbol
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/duckdb/converter/int_to_sym.rb`:
- Around line 127-131: The error_type_to_sym method only validates the upper
bound but does not check for negative values. In Ruby, negative indices access
the array from the end, so passing a negative value would silently return an
incorrect error type instead of raising an error. Add an additional bounds check
in the error_type_to_sym method to ensure the value is not negative (val < 0)
and raise a DuckDB::Error for any negative indices, treating them as invalid
error type IDs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f46376cb-4c2e-4868-a397-bcd0de3fe41b
📒 Files selected for processing (10)
.rubocop.ymlCHANGELOG.mdext/duckdb/connection.cext/duckdb/error.cext/duckdb/error.hext/duckdb/prepared_statement.clib/duckdb.rblib/duckdb/converter/int_to_sym.rblib/duckdb/error.rbtest/duckdb_test/error_type_test.rb
| def error_type_to_sym(val) # :nodoc: | ||
| raise DuckDB::Error, "Unknown error type: #{val}" if val >= ERROR_TYPES.size | ||
|
|
||
| ERROR_TYPES[val] | ||
| end |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify no lower-bound guard currently exists and inspect neighboring converter guards.
rg -n -C2 'def error_type_to_sym|Unknown error type|ERROR_TYPES\[val\]' lib/duckdb/converter/int_to_sym.rbRepository: suketa/ruby-duckdb
Length of output: 282
🏁 Script executed:
# Check ERROR_TYPES constant and search for calls to error_type_to_sym
rg -n 'ERROR_TYPES\s*=' lib/duckdb/converter/int_to_sym.rb | head -5
rg -n 'error_type_to_sym' lib/duckdb/ --type rbRepository: suketa/ruby-duckdb
Length of output: 134
🏁 Script executed:
# Check the full ERROR_TYPES definition to understand its scope
sed -n '79,123p' lib/duckdb/converter/int_to_sym.rbRepository: suketa/ruby-duckdb
Length of output: 920
🏁 Script executed:
# Search for calls to error_type_to_sym throughout the codebase
rg 'error_type_to_sym' lib/ --type-list | head -20
rg -n 'error_type_to_sym' lib/Repository: suketa/ruby-duckdb
Length of output: 653
🏁 Script executed:
# Check if there are any direct uses of ERROR_TYPES or if they come from C bindings
rg -n 'ERROR_TYPES' lib/duckdb/converter/int_to_sym.rb
rg -n 'DuckDB::Error' lib/duckdb/ | grep -E '\(.*error_type|error.*code'Repository: suketa/ruby-duckdb
Length of output: 207
🏁 Script executed:
# Check error.rb to see where `@error_type_id` comes from
cat -n lib/duckdb/error.rb | head -50Repository: suketa/ruby-duckdb
Length of output: 1141
🏁 Script executed:
# Search for C extension code that sets error_type_id
find lib/duckdb -name '*.rb' -exec grep -l 'error_type_id' {} \;
find ext -name '*.c' -o -name '*.h' 2>/dev/null | head -20
rg 'error_type_id|duckdb_error_type' ext/ 2>/dev/null || echo "Checking ext directory..."Repository: suketa/ruby-duckdb
Length of output: 674
🏁 Script executed:
# Check if there's any documentation about valid error type range
rg -n 'error.*type.*id|negative|negative?' lib/duckdb/ --type rubyRepository: suketa/ruby-duckdb
Length of output: 1075
🏁 Script executed:
# Check the C extension error handling code
cat -n ext/duckdb/error.c | grep -A 20 -B 5 'error_type_id'Repository: suketa/ruby-duckdb
Length of output: 942
🏁 Script executed:
# Check DuckDB C API documentation or how error codes are generated
rg -n 'duckdb_error_type\|error_type' ext/duckdb/error.c | head -20Repository: suketa/ruby-duckdb
Length of output: 44
🏁 Script executed:
# Search for DuckDB error type definitions or enums in comments/code
rg -n 'duckdb_error_type\|ErrorType\|error_type.*enum' ext/duckdb/ -A 3Repository: suketa/ruby-duckdb
Length of output: 44
🏁 Script executed:
# Check if there's any validation or bounds information in existing code
rg -n 'ERROR_TYPES.size\|ERROR_TYPES.length' lib/duckdb/converter/int_to_sym.rbRepository: suketa/ruby-duckdb
Length of output: 44
🏁 Script executed:
# Look for any tests or examples that might show valid error type ranges
find . -name '*test*' -o -name '*spec*' | xargs grep -l 'error_type' 2>/dev/null | head -5Repository: suketa/ruby-duckdb
Length of output: 100
🏁 Script executed:
# Check the error type test file
cat -n ./test/duckdb_test/error_type_test.rbRepository: suketa/ruby-duckdb
Length of output: 1504
🏁 Script executed:
# Check the DuckDB C header files for error type definitions
rg -n 'typedef.*error.*type|enum.*error' ext/duckdb/*.h | head -20Repository: suketa/ruby-duckdb
Length of output: 44
🏁 Script executed:
# Check if duckdb_result_error_type can return negative values
rg -n 'duckdb_result_error_type\|error_type.*result' ext/duckdb/error.cRepository: suketa/ruby-duckdb
Length of output: 44
🏁 Script executed:
# Check if there are any other bounds checks in the codebase similar to error_type_to_sym
rg -n 'if.*negative\?' lib/duckdb/ | grep -E 'guard|raise|return'Repository: suketa/ruby-duckdb
Length of output: 379
Add bounds check for negative error type IDs.
Line 128 checks the upper bound but allows negative indices, which are valid in Ruby. This could cause error_type_to_sym(-1) to return :invalid_configuration instead of raising, silently misclassifying invalid IDs.
Suggested fix
def error_type_to_sym(val) # :nodoc:
- raise DuckDB::Error, "Unknown error type: #{val}" if val >= ERROR_TYPES.size
+ raise DuckDB::Error, "Unknown error type: #{val}" if val.negative? || val >= ERROR_TYPES.size
ERROR_TYPES[val]
end📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def error_type_to_sym(val) # :nodoc: | |
| raise DuckDB::Error, "Unknown error type: #{val}" if val >= ERROR_TYPES.size | |
| ERROR_TYPES[val] | |
| end | |
| def error_type_to_sym(val) # :nodoc: | |
| raise DuckDB::Error, "Unknown error type: #{val}" if val.negative? || val >= ERROR_TYPES.size | |
| ERROR_TYPES[val] | |
| end |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/duckdb/converter/int_to_sym.rb` around lines 127 - 131, The
error_type_to_sym method only validates the upper bound but does not check for
negative values. In Ruby, negative indices access the array from the end, so
passing a negative value would silently return an incorrect error type instead
of raising an error. Add an additional bounds check in the error_type_to_sym
method to ensure the value is not negative (val < 0) and raise a DuckDB::Error
for any negative indices, treating them as invalid error type IDs.
1 participant
Expose the DuckDB error category as a Symbol on
DuckDB::Error, so callers (notably the ActiveRecord adapter) can map failures toRecordNotUnique/NotNullViolationetc. without parsing error messages.Design
rbduckdb_raise_result_errorcapturesduckdb_result_error_typeat the two result-error sites — rawConnection#queryand prepared-statement execution — because the failed result is destroyed before any Ruby object exists.DuckDB::Error.new(message, error_type_id). The int→Symbol mapping, thenilrule, and the public#error_typeall live in Ruby (Converter::IntToSym::ERROR_TYPES).error_typeisnilfor errors not originating from a query result (internal failures, async path).Known limitation
DuckDB groups unique / not-null / FK violations under a single
:constrainttype, so consumers still need the message for the sub-kind —error_typeprovides the first-level dispatch.Tests
error_type_test.rb::constraintvia both raw-query and bind paths,:catalogfor unknown table,nilfor a plainDuckDB::Error. Full suite green.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
error_typemethod that returns the DuckDB error category as a Symbol (such as:constraintfor constraint violations or:catalogfor catalog-related errors), ornilfor non-query-result-originating errors.Tests