Add DuckDB::Appender.create_query #981
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
WalkthroughAdds a guarded class method DuckDB::Appender.create_query (with alias from_query), documents uppercase DuckDB::LogicalType constants, and introduces a helper to extract rubyDuckDBLogicalType structs; includes tests for the new appender API. Changes
Sequence Diagram(s)sequenceDiagram
participant Ruby as Ruby code
participant Appender as DuckDB::Appender (Ruby)
participant CExt as C extension (appender.c)
participant DuckDB as DuckDB lib
Ruby->>Appender: create_query(con, query, types, table, columns)
Appender->>CExt: appender_s_create_query(klass, con, query, types, table, columns)
rect rgba(200,220,255,0.25)
Note over CExt: Validate inputs (Connection, Array types)\nConvert Ruby type objects to DuckDB logical types\nPrepare table/column name arrays
end
CExt->>DuckDB: duckdb_appender_create_query(...)
alt success
DuckDB-->>CExt: appender pointer
CExt->>Ruby: instantiate DuckDB::Appender and return
else failure
DuckDB-->>CExt: error
CExt->>Ruby: raise eDuckDBError
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
CHANGELOG.md(1 hunks)ext/duckdb/appender.c(3 hunks)ext/duckdb/logical_type.c(1 hunks)ext/duckdb/logical_type.h(1 hunks)test/duckdb_test/appender_test.rb(1 hunks)
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: suketa
Repo: suketa/ruby-duckdb PR: 871
File: ext/duckdb/appender.c:170-178
Timestamp: 2025-01-26T00:37:44.001Z
Learning: In ruby-duckdb's DuckDB::Appender#append_uint8, values larger than uint8 max (255) are silently truncated to maintain backward compatibility, following C's implicit truncation behavior. Range checking is intentionally omitted to avoid breaking changes.
📚 Learning: 2025-01-24T22:37:32.786Z
Learnt from: suketa
Repo: suketa/ruby-duckdb PR: 0
File: :0-0
Timestamp: 2025-01-24T22:37:32.786Z
Learning: In Ruby-DuckDB C extensions, when initializing resources like prepared statements, store them directly in the context structure instead of using temporary variables to prevent memory leaks, especially when exceptions might be thrown.
Applied to files:
ext/duckdb/logical_type.hext/duckdb/logical_type.cext/duckdb/appender.c
📚 Learning: 2025-01-26T00:37:44.001Z
Learnt from: suketa
Repo: suketa/ruby-duckdb PR: 871
File: ext/duckdb/appender.c:170-178
Timestamp: 2025-01-26T00:37:44.001Z
Learning: In ruby-duckdb's DuckDB::Appender#append_uint8, values larger than uint8 max (255) are silently truncated to maintain backward compatibility, following C's implicit truncation behavior. Range checking is intentionally omitted to avoid breaking changes.
Applied to files:
test/duckdb_test/appender_test.rbCHANGELOG.mdext/duckdb/appender.c
📚 Learning: 2025-01-24T22:18:24.630Z
Learnt from: suketa
Repo: suketa/ruby-duckdb PR: 0
File: :0-0
Timestamp: 2025-01-24T22:18:24.630Z
Learning: When using ruby-asan (AddressSanitizer), avoid letting DuckDB throw C++ exceptions by checking conditions (like table existence) beforehand and raising Ruby exceptions directly to prevent ASAN from intercepting the C++ exceptions.
Applied to files:
ext/duckdb/appender.c
📚 Learning: 2025-01-25T05:20:48.677Z
Learnt from: suketa
Repo: suketa/ruby-duckdb PR: 0
File: :0-0
Timestamp: 2025-01-25T05:20:48.677Z
Learning: In Ruby C extensions, prefer using Ruby's string functions (like rb_str_new2) over C string functions (like strdup) to avoid manual memory management and potential memory leaks.
Applied to files:
ext/duckdb/appender.c
📚 Learning: 2025-01-25T05:11:42.406Z
Learnt from: suketa
Repo: suketa/ruby-duckdb PR: 0
File: :0-0
Timestamp: 2025-01-25T05:11:42.406Z
Learning: In Ruby C extensions, when raising exceptions with dynamically allocated error messages, ensure to free the memory after rb_exc_raise as the code continues execution if the exception is caught with rescue.
Applied to files:
ext/duckdb/appender.c
🧬 Code graph analysis (3)
ext/duckdb/logical_type.h (1)
ext/duckdb/logical_type.c (1)
get_struct_logical_type(44-48)
test/duckdb_test/appender_test.rb (1)
lib/duckdb/connection.rb (1)
appender(129-137)
ext/duckdb/appender.c (2)
ext/duckdb/logical_type.c (2)
get_struct_logical_type(44-48)allocate(50-53)ext/duckdb/connection.c (2)
get_struct_connection(36-40)allocate(27-30)
🪛 Clang (14.0.6)
ext/duckdb/logical_type.h
[error] 12-12: unknown type name 'VALUE'
(clang-diagnostic-error)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (19)
- GitHub Check: test (mswin, 1.3.2)
- GitHub Check: test (3.4.7, 1.4.1)
- GitHub Check: test (mswin, 1.4.1)
- GitHub Check: test (3.3.8, 1.3.2)
- GitHub Check: test (head, 1.3.2)
- GitHub Check: test (head, 1.4.1)
- GitHub Check: test (ucrt, 1.3.2)
- GitHub Check: test (ucrt, 1.4.1)
- GitHub Check: test (mingw, 1.4.1)
- GitHub Check: test (mingw, 1.3.2)
- GitHub Check: test (3.4.1, 1.3.2)
- GitHub Check: test (3.4.1, 1.4.1)
- GitHub Check: test (3.2.6, 1.4.1)
- GitHub Check: test (3.5.0-preview1, 1.3.2)
- GitHub Check: test (3.2.6, 1.3.2)
- GitHub Check: test (3.3.8, 1.4.1)
- GitHub Check: test (3.5.0-preview1, 1.4.1)
- GitHub Check: test (head, 1.3.2)
- GitHub Check: test (3.5.0-preview1, 1.3.2)
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
ext/duckdb/appender.c (1)
106-121: Prevent buffer over-read when column list is shorter than types
column_countis derived from thetypesarray, but we sizecolumn_namesusingcolumns.length. If the caller passes fewer column names than logical types, DuckDB will walk past thecolumn_namesbuffer because the API expects exactlycolumn_countentries, leading to undefined behaviour. Please reject mismatched lengths before callingduckdb_appender_create_query. (duckdb.org)idx_t col_count = RARRAY_LEN(columns); + if (col_count != column_count) { + rb_raise(rb_eArgError, + "column list size (%llu) must match types size (%llu)", + (unsigned long long)col_count, + (unsigned long long)column_count); + } column_names = ALLOCA_N(const char *, (size_t)col_count);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
ext/duckdb/appender.c(3 hunks)lib/duckdb/appender.rb(1 hunks)test/duckdb_test/appender_test.rb(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- test/duckdb_test/appender_test.rb
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: suketa
Repo: suketa/ruby-duckdb PR: 871
File: ext/duckdb/appender.c:170-178
Timestamp: 2025-01-26T00:37:44.001Z
Learning: In ruby-duckdb's DuckDB::Appender#append_uint8, values larger than uint8 max (255) are silently truncated to maintain backward compatibility, following C's implicit truncation behavior. Range checking is intentionally omitted to avoid breaking changes.
Learnt from: suketa
Repo: suketa/ruby-duckdb PR: 0
File: :0-0
Timestamp: 2025-01-24T22:37:32.786Z
Learning: In Ruby-DuckDB C extensions, when initializing resources like prepared statements, store them directly in the context structure instead of using temporary variables to prevent memory leaks, especially when exceptions might be thrown.
📚 Learning: 2025-01-24T22:37:32.786Z
Learnt from: suketa
Repo: suketa/ruby-duckdb PR: 0
File: :0-0
Timestamp: 2025-01-24T22:37:32.786Z
Learning: In Ruby-DuckDB C extensions, when initializing resources like prepared statements, store them directly in the context structure instead of using temporary variables to prevent memory leaks, especially when exceptions might be thrown.
Applied to files:
ext/duckdb/appender.c
📚 Learning: 2025-01-24T22:18:24.630Z
Learnt from: suketa
Repo: suketa/ruby-duckdb PR: 0
File: :0-0
Timestamp: 2025-01-24T22:18:24.630Z
Learning: When using ruby-asan (AddressSanitizer), avoid letting DuckDB throw C++ exceptions by checking conditions (like table existence) beforehand and raising Ruby exceptions directly to prevent ASAN from intercepting the C++ exceptions.
Applied to files:
ext/duckdb/appender.c
📚 Learning: 2025-01-26T00:37:44.001Z
Learnt from: suketa
Repo: suketa/ruby-duckdb PR: 871
File: ext/duckdb/appender.c:170-178
Timestamp: 2025-01-26T00:37:44.001Z
Learning: In ruby-duckdb's DuckDB::Appender#append_uint8, values larger than uint8 max (255) are silently truncated to maintain backward compatibility, following C's implicit truncation behavior. Range checking is intentionally omitted to avoid breaking changes.
Applied to files:
ext/duckdb/appender.c
📚 Learning: 2025-01-25T05:20:48.677Z
Learnt from: suketa
Repo: suketa/ruby-duckdb PR: 0
File: :0-0
Timestamp: 2025-01-25T05:20:48.677Z
Learning: In Ruby C extensions, prefer using Ruby's string functions (like rb_str_new2) over C string functions (like strdup) to avoid manual memory management and potential memory leaks.
Applied to files:
ext/duckdb/appender.c
📚 Learning: 2025-01-25T05:11:42.406Z
Learnt from: suketa
Repo: suketa/ruby-duckdb PR: 0
File: :0-0
Timestamp: 2025-01-25T05:11:42.406Z
Learning: In Ruby C extensions, when raising exceptions with dynamically allocated error messages, ensure to free the memory after rb_exc_raise as the code continues execution if the exception is caught with rescue.
Applied to files:
ext/duckdb/appender.c
🧬 Code graph analysis (1)
ext/duckdb/appender.c (2)
ext/duckdb/logical_type.c (1)
get_struct_logical_type(44-48)ext/duckdb/connection.c (1)
get_struct_connection(36-40)
🪛 GitHub Actions: MacOS
lib/duckdb/appender.rb
[error] 27-27: undefined method 'create_query' for class '#Class:DuckDB::Appender' (NameError)
🪛 GitHub Actions: Ubuntu
lib/duckdb/appender.rb
[error] 27-27: NameError: undefined method `create_query' for class DuckDB::Appender. This occurs during singleton class definition at line 27.
🪛 GitHub Actions: Windows
lib/duckdb/appender.rb
[error] 27-27: undefined method 'create_query' for class '#Class:DuckDB::Appender' (NameError). The rake test command failed with exit code 1.
Comment on lines
26
to
28
| class << self | ||
| alias from_query create_query | ||
| end |
There was a problem hiding this comment.
Guard alias against missing create_query
create_query is only defined when the native extension is built against DuckDB ≥ 1.4.0, but the unconditional alias runs for all builds. On older libraries this raises NameError during class definition (see failing CI jobs). Wrap the alias in a guard or set it up from the C side under the same feature flag so the file loads cleanly when the API is unavailable.
- class << self
- alias from_query create_query
- end
+ class << self
+ if singleton_class.method_defined?(:create_query)
+ alias_method :from_query, :create_query
+ end
+ endCommittable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 GitHub Actions: MacOS
[error] 27-27: undefined method 'create_query' for class '#Class:DuckDB::Appender' (NameError)
🪛 GitHub Actions: Ubuntu
[error] 27-27: NameError: undefined method `create_query' for class DuckDB::Appender. This occurs during singleton class definition at line 27.
🪛 GitHub Actions: Windows
[error] 27-27: undefined method 'create_query' for class '#Class:DuckDB::Appender' (NameError). The rake test command failed with exit code 1.
🤖 Prompt for AI Agents
In lib/duckdb/appender.rb around lines 26–28, the unconditional alias from_query
= create_query fails on builds where create_query isn't defined (DuckDB <
1.4.0); guard the alias so the file loads on older libraries. Replace the bare
alias with a conditional that checks whether the singleton responds
to/create_query is defined (e.g., via respond_to?(:create_query) or
singleton_class.method_defined?) and only performs the alias when present;
alternatively implement the alias on the C side under the same feature flag so
the Ruby file never needs the guard.
This was referenced Nov 1, 2025
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
refs #976
Summary by CodeRabbit
New Features
Tests