Support GPDB varlena layout - #47
Conversation
This commit allows cloudbery use GPDB varlena layout, otherwise upgrading from greenplum is blocked. Additionally bump catalog version and write varlena layout metadata (gpdb6 compatible or native Little Endian) in controldata. This allows us to prohibit using wrong binaries with wrong data format, preventing data corruption.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hi, @Smyatkin-Maxim welcome!🎊 Thanks for taking the effort to make our project better! 🙌 Keep making such awesome contributions!
|
@leborchuk we probably need to update CI and builds to use the new macro |
Yep, let's fix it in our workflow |
There was a problem hiding this comment.
Pull request overview
This PR adds a GPDB6-compatible “network byte order” varlena header layout option (via -DFORCE_BIGENDIAN_VARLENA) and records the chosen on-disk varlena layout in pg_control so binaries/data-directory mismatches are detected early (startup and pg_upgrade), preventing silent data corruption.
Changes:
- Add a build-time switch to force GPDB6 varlena 4-byte header encoding, while preserving upstream/native behavior by default.
- Bump
PG_CONTROL_VERSIONand stamp the cluster’s varlena layout intoControlFileData, exposing it viapg_controldata/pg_resetwal. - Teach
pg_upgradeand backend startup to enforce that old/new clusters (and server/data directory) agree on varlena layout.
Reviewed changes
Copilot reviewed 3 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/include/postgres.h | Adds FORCE_BIGENDIAN_VARLENA-aware varlena header read/write macros. |
| src/include/catalog/pg_control.h | Bumps control version and defines/stores varlena layout metadata in ControlFileData. |
| src/bin/pg_upgrade/pg_upgrade.h | Adds a control-version gate and a new ControlData field for varlena layout. |
| src/bin/pg_upgrade/controldata.c | Parses and validates varlena layout from pg_controldata output; enforces compatibility where possible. |
| src/bin/pg_resetwal/pg_resetwal.c | Stamps/prints varlena layout in control output and guessed control values. |
| src/bin/pg_controldata/pg_controldata.c | Prints varlena layout from pg_control. |
| src/backend/access/transam/xlog.c | Writes layout into pg_control and adds a fatal mismatch check at startup. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
1742764 to
59e82d3
Compare
And default it to true for our builds
59e82d3 to
dc3ff3c
Compare
|
@leborchuk please take a look. I've decided to wrap the macro into a configure option --with-varlena-gpdb-layout and set it on in configure_cloudberry.sh for CI. It's easier to resolve conflicts in this one place when we sync with upstream (where this should be set to off always). To prevent problems in future I'll also set it once more in CONFIGURE_EXTRA_OPTS in our production builds, just to be safe if rebase ever goes wrong. |
leborchuk
left a comment
leborchuk
left a comment
There was a problem hiding this comment.
LGTM, only minor notes about fields order
Compared it with GP6, blame shows us what has been changed https://github.com/open-gpdb/gpdb/blame/9f785ada8af44eac5ad5ef05a5a8089f2ca3b701/src/include/postgres.h#L205 - compared it manually with the current code. All looks good
| uint32 large_object; | ||
| bool date_is_int; | ||
| bool float8_pass_by_value; | ||
| bool varlena_bigendian; |
There was a problem hiding this comment.
Maybe field should be added to the end of ControlData, like in postgres/postgres@a8238f8
This commit allows cloudbery use GPDB varlena layout, otherwise upgrading from greenplum is blocked.
Additionally bump catalog version and write varlena layout metadata (gpdb6 compatible or native Little Endian) in controldata. This allows us to prohibit using wrong binaries with wrong data format, preventing data corruption.
What does this PR do?
Type of Change
Breaking Changes
Test Plan
make installcheckmake -C src/test installcheck-cbdb-parallelImpact
Performance:
User-facing changes:
Dependencies:
Checklist
Additional Context
CI Skip Instructions