Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Fix output string to create IDENTITY sequence#1193

Open
c2main wants to merge 1 commit intoIvorySQL:masterIvorySQL/IvorySQL:masterfrom
Data-Bene:fix-dump-identityData-Bene/IvorySQL:fix-dump-identityCopy head branch name to clipboard
Open

Fix output string to create IDENTITY sequence#1193
c2main wants to merge 1 commit intoIvorySQL:masterIvorySQL/IvorySQL:masterfrom
Data-Bene:fix-dump-identityData-Bene/IvorySQL:fix-dump-identityCopy head branch name to clipboard

Conversation

@c2main
Copy link

@c2main c2main commented Feb 16, 2026

was "NO NULL" insteaf of "ON NULL".

no test added, though it outlines the pg_dump/pg_restore CI is maybe not efficient enough.

Summary by CodeRabbit

  • Bug Fixes
    • Fixed pg_dump output for identity columns to correctly generate BY DEFAULT ON NULL syntax instead of the previously incorrect BY DEFAULT NO NULL.

was "NO NULL" insteaf of "ON NULL"
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 16, 2026

No actionable comments were generated in the recent review. 🎉


📝 Walkthrough

Walkthrough

A single-line text adjustment in pg_dump.c corrects the SQL output for identity columns with DEFAULT identity and ON NULL settings, changing the generated DDL from "BY DEFAULT NO NULL" to "BY DEFAULT ON NULL" for accurate SQL representation.

Changes

Cohort / File(s) Summary
Identity Column DDL Output
src/bin/pg_dump/pg_dump.c
Corrected the emitted SQL text for identity columns with DEFAULT identity and ON NULL from "BY DEFAULT NO NULL" to "BY DEFAULT ON NULL".

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

🐰 A little fix, so small yet grand,
One line of text now better planned,
FROM "NO NULL" to "ON NULL" true,
The SQL sings, now cleaner too! ✨

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: fixing the output string for IDENTITY sequence creation from 'NO NULL' to 'ON NULL' in pg_dump.c.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Merge Conflict Detection ✅ Passed ✅ No merge conflicts detected when merging into master

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Tip

Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@bigplaice
Copy link
Collaborator

bigplaice commented Feb 16, 2026

Thanks for the fix!
I have started the CI workflow and I think it can be merged once the CI is done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments

Close sidebar
Morty Proxy This is a proxified and sanitized view of the page, visit original site.