-
-
Notifications
You must be signed in to change notification settings - Fork 26
fix error handling of DuckDB::Append#append_date #904
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.
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
Conversation
WalkthroughThis pull request updates the error handling and documentation for the Changes
Sequence Diagram(s)sequenceDiagram
participant R as Ruby Appender
participant C as C appender__append_date
participant DB as DuckDB System
R->>C: Call append_date(date)
C->>DB: Execute duckdb_append_date(dt)
DB-->>C: Return execution state
C-->>R: Return boolean success value
alt Success is true
R->>R: Return self
else Failure detected
R->>R: Raise error "failed to append_date"
end
Possibly related PRs
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
CHANGELOG.md
(1 hunks)ext/duckdb/appender.c
(1 hunks)lib/duckdb/appender.rb
(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (19)
- GitHub Check: test (head, 1.2.0)
- GitHub Check: test (mswin, 1.1.1)
- GitHub Check: test (mswin, 1.1.3)
- GitHub Check: test (mswin, 1.2.0)
- GitHub Check: test (mingw, 1.1.3)
- GitHub Check: test (mingw, 1.2.0)
- GitHub Check: test (ucrt, 1.1.1)
- GitHub Check: test (3.4.1, 1.1.1)
- GitHub Check: test (3.4.1, 1.1.3)
- GitHub Check: test (3.4.1, 1.2.0)
- GitHub Check: test (3.3.6, 1.1.1)
- GitHub Check: test (3.3.6, 1.1.3)
- GitHub Check: test (3.3.6, 1.2.0)
- GitHub Check: test (3.2.6, 1.1.1)
- GitHub Check: test (3.2.6, 1.1.3)
- GitHub Check: test (3.2.6, 1.2.0)
- GitHub Check: test (3.1.6, 1.1.1)
- GitHub Check: test (3.1.6, 1.1.3)
- GitHub Check: test (3.1.6, 1.2.0)
🔇 Additional comments (3)
ext/duckdb/appender.c (1)
284-284
: Changed error handling approach to align with other methodsThe function now returns a boolean value using
duckdb_state_to_bool_value()
instead of raising an exception directly. This makes the error handling consistent with other similar appender methods.CHANGELOG.md (1)
12-13
: Documentation updated correctlyThe CHANGELOG now includes
DuckDB::Appender#append_date
in the list of methods that can fail, maintaining consistency with the implementation changes.lib/duckdb/appender.rb (1)
461-465
: Documentation improved with clear method signatureThe added call-seq comment clarifies the method signature and return value, improving the API documentation.
return self if _append_date(date.year, date.month, date.day) | ||
|
||
raise_appender_error('failed to append_date') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed error handling by checking return value
The implementation now properly checks the return value from _append_date
and raises an appropriate error when the operation fails, matching the pattern used by other appender methods.
This change fixes a previously inconsistent error handling approach that could potentially hide errors when appending date values.
Summary by CodeRabbit
Documentation
Refactor