Add DuckDB::TableDescription and DuckDB::ColumnDescription#1287
Merged
Conversation
- Add DuckDB::TableDescription to retrieve metadata about a table - Add DuckDB::ColumnDescription to describe a column's name, logical type, and whether it has a default value - Add document comments to both classes Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 18 minutes and 52 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughThe pull request introduces Changes
Sequence DiagramsequenceDiagram
actor User
participant Ruby as Ruby Layer
participant C as C Extension
participant DuckDB as DuckDB Library
User->>Ruby: TableDescription.new(connection, "table_name")
Ruby->>Ruby: Validate connection & table name
Ruby->>C: _initialize(conn, catalog, schema, table)
C->>DuckDB: duckdb_create_table_description() or variant
DuckDB-->>C: duckdb_table_description handle
C-->>Ruby: true (success)
Ruby-->>User: TableDescription instance
User->>Ruby: column_descriptions
loop For each column (0 to column_count-1)
Ruby->>C: _column_name(idx)
C->>DuckDB: duckdb_table_description_column_name()
DuckDB-->>C: C string
C-->>Ruby: Ruby UTF-8 string
Ruby->>C: _column_logical_type(idx)
C->>DuckDB: duckdb_table_description_column_logical_type()
DuckDB-->>C: duckdb_logical_type
C-->>Ruby: LogicalType wrapper
Ruby->>C: _column_has_default(idx)
C->>DuckDB: duckdb_table_description_column_has_default()
DuckDB-->>C: boolean result
C-->>Ruby: boolean
Ruby->>Ruby: Create ColumnDescription object
end
Ruby-->>User: Array of ColumnDescription objects
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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 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: 2
🧹 Nitpick comments (3)
test/duckdb_test/table_description_test.rb (1)
13-15: Make teardown nil-safe to avoid masking setup failures.If setup fails early, teardown can raise
NoMethodErroronnil, which can hide the root cause.🧩 Small resiliency tweak
- def teardown - `@con.close` - `@db.close` - end + def teardown + `@con`&.close + `@db`&.close + end🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/duckdb_test/table_description_test.rb` around lines 13 - 15, The teardown method may raise NoMethodError if `@con` or `@db` is nil (masking setup failures); update teardown to be nil-safe by checking/using safe navigation on the instance variables (e.g., replace direct calls to `@con.close` and `@db.close` with conditional calls like `@con`&.close and `@db`&.close or explicit if `@con` then `@con.close` end) so teardown skips closing when those objects weren't created and does not swallow other exceptions.lib/duckdb/column_description.rb (1)
15-24: Use block form in the usage example to model safe resource cleanup.The example currently leaves database/connection lifecycle implicit. Prefer block style in docs to match library best practices.
♻️ Suggested doc example adjustment
- # db = DuckDB::Database.open - # con = db.connect - # con.query("CREATE TABLE t (id INTEGER, name VARCHAR DEFAULT 'anon')") - # - # td = DuckDB::TableDescription.new(con, 't') - # cd = td.column_descriptions.last + # DuckDB::Database.open do |db| + # db.connect do |con| + # con.query("CREATE TABLE t (id INTEGER, name VARCHAR DEFAULT 'anon')") + # td = DuckDB::TableDescription.new(con, 't') + # cd = td.column_descriptions.last + # end + # endAs per coding guidelines: Use block form (e.g.,
db.connect { |con| ... }) for automatic resource cleanup.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/duckdb/column_description.rb` around lines 15 - 24, Update the usage example to use the block form for opening/connecting so resources are cleaned automatically: replace the implicit open/connect sequence with block-style calls (e.g., Database.open { |db| db.connect { |con| ... } } or at minimum db.connect { |con| ... }) around the CREATE TABLE and TableDescription/ColumnDescription inspection (references: DuckDB::Database.open, db.connect, DuckDB::TableDescription.new, ColumnDescription methods like name, logical_type.type, has_default?) so the example demonstrates safe automatic resource cleanup.lib/duckdb/table_description.rb (1)
9-19: Use block-form examples forDatabase.open/#connect.These snippets are likely to be copy-pasted, and they currently demonstrate leaving native
Database/Connectionobjects open. Please switch the examples to block form or show explicit cleanup.As per coding guidelines, "C objects (Database, Connection, PreparedStatement, Appender, Config) must be explicitly destroyed or use block form for automatic cleanup" and "Use block form (e.g.,
db.connect { |con| ... }) for automatic resource cleanup".Also applies to: 29-35
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/duckdb/table_description.rb` around lines 9 - 19, Update the README-style examples to use block form (or explicit destroy) for the C objects so they are not left open: replace usages of Database.open and db.connect that yield persistent Database/Connection objects with block forms (e.g., Database.open { |db| db.connect { |con| ... } } or ensure explicit cleanup) in the examples around the DuckDB::TableDescription demonstration and the similar snippet later (the code creating DuckDB::Database, .connect, and passing con into DuckDB::TableDescription.new). Ensure every Database.open, db.connect, and any temporary Connection/PreparedStatement/Appender/Config in those examples uses block form or shows explicit destruction to guarantee automatic resource cleanup.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ext/duckdb/table_description.c`:
- Around line 89-114: The build CI fails because
duckdb_table_description_get_column_count and
duckdb_table_description_get_column_type require a newer DuckDB than the
extconf.rb minimum; update DUCKDB_REQUIRED_VERSION in ext/duckdb/extconf.rb to
"1.5.0" (or the exact patch where those APIs were introduced) so the native
calls in duckdb_table_description__column_count and
duckdb_table_description__column_logical_type are supported, then re-run CI to
confirm the version bump resolves the missing-symbol errors.
In `@ext/duckdb/table_description.h`:
- Line 5: The unguarded use of duckdb_table_description can break builds on
older DuckDB headers; verify which DuckDB version introduced
duckdb_table_description (per your checks it aligns with minimum 1.3.0) and then
either add a compile-time guard around usages of duckdb_table_description (e.g.,
wrap references in `#if/`#endif tied to a new configure-time macro like
HAVE_DUCKDB_TABLE_DESCRIPTION or version check using detected DuckDB version) or
bump the minimum DuckDB version enforced in extconf.rb to the version that first
provides duckdb_table_description and remove/adjust the unguarded usage
accordingly; update extconf.rb to detect the symbol (or version) and define the
macro used in the source so builds fail cleanly on unsupported DuckDB versions.
---
Nitpick comments:
In `@lib/duckdb/column_description.rb`:
- Around line 15-24: Update the usage example to use the block form for
opening/connecting so resources are cleaned automatically: replace the implicit
open/connect sequence with block-style calls (e.g., Database.open { |db|
db.connect { |con| ... } } or at minimum db.connect { |con| ... }) around the
CREATE TABLE and TableDescription/ColumnDescription inspection (references:
DuckDB::Database.open, db.connect, DuckDB::TableDescription.new,
ColumnDescription methods like name, logical_type.type, has_default?) so the
example demonstrates safe automatic resource cleanup.
In `@lib/duckdb/table_description.rb`:
- Around line 9-19: Update the README-style examples to use block form (or
explicit destroy) for the C objects so they are not left open: replace usages of
Database.open and db.connect that yield persistent Database/Connection objects
with block forms (e.g., Database.open { |db| db.connect { |con| ... } } or
ensure explicit cleanup) in the examples around the DuckDB::TableDescription
demonstration and the similar snippet later (the code creating DuckDB::Database,
.connect, and passing con into DuckDB::TableDescription.new). Ensure every
Database.open, db.connect, and any temporary
Connection/PreparedStatement/Appender/Config in those examples uses block form
or shows explicit destruction to guarantee automatic resource cleanup.
In `@test/duckdb_test/table_description_test.rb`:
- Around line 13-15: The teardown method may raise NoMethodError if `@con` or `@db`
is nil (masking setup failures); update teardown to be nil-safe by
checking/using safe navigation on the instance variables (e.g., replace direct
calls to `@con.close` and `@db.close` with conditional calls like `@con`&.close and
`@db`&.close or explicit if `@con` then `@con.close` end) so teardown skips closing
when those objects weren't created and does not swallow other exceptions.
🪄 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: 36043d03-c549-4eab-b9b1-51cb53b9afeb
📒 Files selected for processing (10)
CHANGELOG.mdext/duckdb/duckdb.cext/duckdb/ruby-duckdb.hext/duckdb/table_description.cext/duckdb/table_description.hlib/duckdb.rblib/duckdb/column_description.rblib/duckdb/table_description.rbtest/duckdb_test/column_description_test.rbtest/duckdb_test/table_description_test.rb
- Move #include "ruby-duckdb.h" before #ifdef in table_description.c so HAVE_DUCKDB_H_GE_V1_5_0 is defined before the guard is evaluated - Wrap struct/typedef/declaration in table_description.h with #ifdef HAVE_DUCKDB_H_GE_V1_5_0 - Wrap Ruby class bodies in `if defined?(DuckDB::TableDescription)` so they are only opened when the C extension registered them Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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
DuckDB::TableDescriptionto retrieve metadata about a table (column count, names, logical types, and default presence)DuckDB::ColumnDescriptionas an immutable value object describing a column's name, logical type, and whether it has a default valueUsage
Summary by CodeRabbit
DuckDB::TableDescription—access column names, logical types, and default-value information for any tableDuckDB::ColumnDescription—an immutable structure containing name, type, and default presence for each column