Skip to content
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

Expose AO storage format checking functions for external use #954

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

reshke
Copy link
Contributor

@reshke reshke commented Feb 21, 2025

So that external tools such as pg_filedump does not need to copy the code in cdbappendonlystorageformat.c in order to verify checksum, get header info etc. This is done similar to storage/checksum_impl.h.

Fixes #ISSUE_Number

What does this PR do?

Type of Change

  • Bug fix (non-breaking change)
  • New feature (non-breaking change)
  • Breaking change (fix or feature with breaking changes)
  • Documentation update

Breaking Changes

Test Plan

  • Unit tests added/updated
  • Integration tests added/updated
  • Passed make installcheck
  • Passed make -C src/test installcheck-cbdb-parallel

Impact

Performance:

User-facing changes:

Dependencies:

Checklist

Additional Context

CI Skip Instructions


So that external tools such as pg_filedump does not need to copy the code in
cdbappendonlystorageformat.c in order to verify checksum, get header info etc.
This is done similar to storage/checksum_impl.h.
@reshke reshke added the cherry-pick cherry-pick upstream commts label Feb 21, 2025
@reshke
Copy link
Contributor Author

reshke commented Feb 21, 2025

Just cherry-picking single commit. It ain't much, but it's honest work

Copy link
Contributor

@x4m x4m left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mmm, much code, such commit.

static char AoHeaderCheckErrorStr[MAX_AOHEADER_CHECK_ERROR_STR] = "\0";

static pg_crc32
AppendOnlyStorageFormat_ComputeHeaderChecksum(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about we declare functions as public instead of moving codes into header files, which would inline functions?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, okay, lets do it this way. Notice I simply cherry-picking things, leaning toward option to keep upstream changes, which reduces cherry-pick conflicts in future (which are not very likely to occur here)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherry-pick cherry-pick upstream commts
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants