fix: reject oid columns as unsupported#57
Conversation
📝 WalkthroughWalkthroughThe archiver and ColdFront Iceberg type mappings now reject PostgreSQL Changesoid support removal
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ 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 |
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Duplication | 0 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
cmd/archiver/main_test.go (1)
179-185: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover the archiver remediation text.
Lines 179-185 verify rejection but not the new
oidguidance. Add an expected substring such asoid values as bigintfor this case so the Go error contract remains covered;ci/journey.shonly checks the extension message.🤖 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 `@cmd/archiver/main_test.go` around lines 179 - 185, Extend the oid case in the archiver type-validation tests to assert the remediation text contains the expected substring “oid values as bigint.” Keep the existing rejection assertion for inet and cidr unchanged, and ensure the oid error contract validates both rejection and guidance.
🤖 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.
Nitpick comments:
In `@cmd/archiver/main_test.go`:
- Around line 179-185: Extend the oid case in the archiver type-validation tests
to assert the remediation text contains the expected substring “oid values as
bigint.” Keep the existing rejection assertion for inet and cidr unchanged, and
ensure the oid error contract validates both rejection and guidance.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: dd90cb51-e04d-414c-874c-c07c88e57041
⛔ Files ignored due to path filters (1)
extension/coldfront/test/expected/ddl_alter_column.outis excluded by!**/*.out
📒 Files selected for processing (6)
ci/journey.shcmd/archiver/main.gocmd/archiver/main_test.godocs/architecture_decoupled.mddocs/usage.mdextension/coldfront/coldfront--1.0.sql
|
@coderabbitai char(n) out-of-scope flag is a false positive: this diff only removes oid from that line. On docstring coverage: this commit adds no new exported functions, so nothing is newly undocumented. |
|
Tip For best results, initiate chat on the files or code changes.
|
oidmapped toBIGINT, but pg_duckdb rejects the oid column reference atplan time in any Iceberg-backed query (the same limit as inet/cidr), so an
oid column archived fine yet became unreadable through the tiered view after
cutover. It's now rejected at provisioning in both the archiver and the
extension, with guidance to store oid values as
bigint.Closes #51.