Rename DuckDB::BindInfo to DuckDB::TableFunction::BindInfo - #1178
Merged
Conversation
- Rename files: bind_info.{c,h,rb} -> table_function_bind_info.{c,h,rb}
- Rename test: bind_info_test.rb -> table_function_bind_info_test.rb
- Expose cDuckDBTableFunction (remove static) and add extern to table_function.h
- Define DuckDB::TableFunction::BindInfo as nested class in C and Ruby
- Rename C variable cDuckDBBindInfo -> cDuckDBTableFunctionBindInfo
- Update header guard to RUBY_DUCKDB_TABLE_FUNCTION_BIND_INFO_H
- Update rb_data_type_t name to 'DuckDB/TableFunctionBindInfo'
- Move bind_info init after table_function init in duckdb.c
- Add DuckDB.const_missing for backward-compat: DuckDB::BindInfo emits
deprecation warning on first access, then caches via const_set
- Rename test class to TableFunctionBindInfoTest; add deprecation warning test
- Update CHANGELOG.md with breaking change entry
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- lib/duckdb/table_function_bind_info.rb -> lib/duckdb/table_function/bind_info.rb - test/duckdb_test/table_function_bind_info_test.rb -> test/duckdb_test/table_function/bind_info_test.rb - Update require path in lib/duckdb.rb Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
📝 WalkthroughWalkthroughThe PR restructures the BindInfo class by moving it from the DuckDB namespace into DuckDB::TableFunction namespace. The old DuckDB::BindInfo is maintained as a deprecated alias with a warning. C extension globals, headers, initialization order, and file locations are updated to reflect this reorganization. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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.
🧹 Nitpick comments (1)
test/duckdb_test/table_function/bind_info_test.rb (1)
142-149: Consider restoring the constant in the ensure block.The
ensureblock removesBindInfobut doesn't restore it. While this works becauseconst_missingwill redefine it on next access, it could cause unexpected deprecation warnings in subsequent tests' output.♻️ Suggested improvement
def test_bind_info_alias_deprecation_warning DuckDB.send(:remove_const, :BindInfo) if DuckDB.const_defined?(:BindInfo, false) warning = capture_io { DuckDB::BindInfo }.last assert_match(/deprecated/, warning) ensure - DuckDB.send(:remove_const, :BindInfo) if DuckDB.const_defined?(:BindInfo, false) + # Restore the constant to avoid affecting other tests + unless DuckDB.const_defined?(:BindInfo, false) + DuckDB.const_set(:BindInfo, DuckDB::TableFunction::BindInfo) + end end🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/duckdb_test/table_function/bind_info_test.rb` around lines 142 - 149, Before removing DuckDB::BindInfo, capture whether it originally existed and save its value; then in the ensure block restore the original constant if it existed (using DuckDB.const_set(:BindInfo, original_value)), otherwise remove any redefined BindInfo to leave global state unchanged. Update the test method around the DuckDB.send(:remove_const, :BindInfo) call and the ensure block to reference the saved original_value and restore or clean up accordingly so subsequent tests don't see extra deprecation warnings.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@test/duckdb_test/table_function/bind_info_test.rb`:
- Around line 142-149: Before removing DuckDB::BindInfo, capture whether it
originally existed and save its value; then in the ensure block restore the
original constant if it existed (using DuckDB.const_set(:BindInfo,
original_value)), otherwise remove any redefined BindInfo to leave global state
unchanged. Update the test method around the DuckDB.send(:remove_const,
:BindInfo) call and the ensure block to reference the saved original_value and
restore or clean up accordingly so subsequent tests don't see extra deprecation
warnings.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: de791940-bba9-4b1a-9265-b8dc42545dee
📒 Files selected for processing (11)
CHANGELOG.mdext/duckdb/duckdb.cext/duckdb/ruby-duckdb.hext/duckdb/table_function.cext/duckdb/table_function.hext/duckdb/table_function_bind_info.cext/duckdb/table_function_bind_info.hlib/duckdb.rblib/duckdb/bind_info.rblib/duckdb/table_function/bind_info.rbtest/duckdb_test/table_function/bind_info_test.rb
💤 Files with no reviewable changes (1)
- lib/duckdb/bind_info.rb
This was referenced Mar 25, 2026
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
Rename
DuckDB::BindInfotoDuckDB::TableFunction::BindInfoto clarify it is specific to table functions, and to follow a more idiomatic nested Ruby naming convention.Changes
table_function/bind_infopathsDuckDB::TableFunction::BindInfoas a nested class underDuckDB::TableFunction(both in C and Ruby)cDuckDBTableFunction(removestatic) and addexterntotable_function.hcDuckDBBindInfo→cDuckDBTableFunctionBindInfoDuckDB.const_missingfor backward compatibility: accessingDuckDB::BindInfoemits a deprecation warning once, then caches the constant viaconst_setBreaking changes
DuckDB::BindInfois deprecated. UseDuckDB::TableFunction::BindInfoinstead.Related
Part of #1122
Summary by CodeRabbit
Breaking Changes
DuckDB::BindInfohas been relocated toDuckDB::TableFunction::BindInfoDeprecations
DuckDB::BindInfonow emits a deprecation warning; migrate toDuckDB::TableFunction::BindInfo. The old location remains functional as a backward-compatible alias.