Repository navigation
gpcheckcat: fix the defects the Behave suite exposes - #2182
Open
MisterRaindrop wants to merge 1 commit into
Open
MisterRaindrop wants to merge 1 commit into
MisterRaindrop wants to merge 1 commit into
Conversation
* gpcheckcat: add the mix_distribution_policy check
Back-port from Greenplum a check Cloudberry was missing.
mix_distribution_policy reports tables whose distribution policy mixes
legacy and non-legacy hash opclasses, and cross-checks the result
against the gp_use_legacy_hashops GUC so the operator is told which of
the two states is inconsistent.
Add the SQL fixtures the corresponding Behave scenarios load.
Greenplum's unit tests for the check mock a psycopg2 cursor, but this
gpcheckcat reaches the database through PyGreSQL (conn.query() and
getresult()) and never opens a cursor, so three of them failed and the
rest passed without exercising the code. They are rewritten against
conn.query(), with tests added for a mixed policy, a clean one, and a
GUC that is read successfully.
Greenplum's companion ao_lastrownums check is deliberately not brought
over: it reads pg_attribute_encoding.lastrownums, a column Cloudberry's
catalog does not have, so the query errors out on every run and
gpcheckcat reports failure on a freshly created database.
* gpcheckcat: give pg_shdescription its TableMainColumn entry
TableMainColumn maps a catalog to the column that identifies the object
an entry belongs to. Greenplum has a row for pg_description and one for
pg_shdescription; Cloudberry has only the first. Without it,
processMissingDuplicateEntryResult() falls through to getOidFromPK(),
which looks the entry up by
SELECT oid FROM pg_shdescription WHERE ...
and pg_shdescription has no oid column, so missing_extraneous died with
Execution error: ERROR: column "oid" does not exist
LINE 4: SELECT oid FROM pg_shdescription
* gpcheckcat: attribute an AO table's visimap to the table in the owner check
checkOwners() compares each relation's owner across segments, and
folds an append-optimized table's auxiliary relations into the table
they belong to, so a mismatch is reported, and repaired, once, on the
table. It knew the segment relation, the block directory and the block
directory's index, but not the visimap relation or its index. Those
were reported as relations of their own, and the repair script tried
ALTER TABLE "pg_aoseg"."pg_aovisimap_17032" OWNER TO "gpadmin";
which the server refuses:
ERROR: permission denied: "pg_aovisimap_17032" is a system catalog
so "gpcheckcat -R owner" on an append-optimized table whose owner
differs on a segment produced a repair that could not run. Match the
visimap and its index, and the block directory's index by the table it
indexes. The repair is then the ALTER TABLE on the table itself, which
carries its auxiliary relations with it.
The feature-file half of this change -- dropping the two ao_lastrownums
scenarios and tagging the -x session GUC scenario, which Cloudberry no
longer supports -- edits the Behave suite as apache#2170 aligns it, so it goes
with that PR.
Split out of apache#1639 (originally 0bfc1d7).
This was referenced Oct 24, 2026
Draft
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Three gpcheckcat fixes that the gpMgmt Behave suite (#2170) exposes. They are in one commit, as they were in #1639. The commit message gives the full reasoning.
mix_distribution_policycheck. This back-ports from Greenplum a check that Cloudberry was missing. It reports tables whose distribution policy mixes legacy and non-legacy hash opclasses, and it cross-checks the result againstgp_use_legacy_hashops. The SQL fixtures its Behave scenarios load are added too. Greenplum's companionao_lastrownumscheck is deliberately not brought over: it readspg_attribute_encoding.lastrownums, which Cloudberry's catalog does not have.checkOwners()folds an AO table's auxiliary relations into the table, so an owner mismatch is reported, and repaired, once, on the table. It did not know the visimap relation or its index. The repair script it generated therefore also ranALTER TABLE "pg_aoseg"."pg_aovisimap_<oid>" OWNER TO ..., and the server refuses that withpermission denied: "pg_aovisimap_<oid>" is a system catalog. Sogpcheckcat -R ownerproduced a repair that could not run.pg_shdescriptionitsTableMainColumnentry. Without the entry,missing_extraneouslooked shared descriptions up byoid, whichpg_shdescriptiondoes not have, and it died withcolumn "oid" does not exist.Split out of #1639 (originally 0bfc1d7, by @tuhaihe).
One change from #1639: Greenplum's unit tests for the new check mock a psycopg2 cursor. This gpcheckcat reaches the database through PyGreSQL (
conn.query()andgetresult()) and never opens a cursor. As a result, three of those tests failed, and the rest passed without running the code they were meant to test. I rewrote them againstconn.query()and added tests for a mixed policy, a consistent one, and a GUC that is read successfully.What stays with #2170: in #1639 this commit also edited the Behave suite. It dropped the two
ao_lastrownumsscenarios, tagged the-xsession GUC scenario (which Cloudberry no longer supports)@not_implemented, and added--tags ~@not_implementedto the gpcheckcat job in the workflow. Those edits apply to the suite as #2170 aligns it, so they are made in #2170.This is one of five PRs that the remaining fixes from #1639 are grouped into, alongside #2178, #2180, #2183 (gprecoverseg) and #2184 (gprecoverseg Greenplum ports).
Test Plan
Rocky 9 build image, built from #2170's tree plus #2177 and #2178 (
767f12685a4andb971d481862, which the demo cluster needs) and #2180, with a demo cluster created the way the Behave workflow creates it. I tookgpcheckcat.featureas #2170 now has it, with the edits described above, and removed the eight@needs_fixtags that point at this change. I then ran it with CI's arguments (--tags ~@extended --tags ~@not_implemented).$GPHOME.gpcheckcat.featureBoth runs skip one scenario, the
-xone.The eight scenarios that fail in A are the eight #2170 tags:
mix_distribution_policyscenarios (fix 1). The check does not exist without this PR.ALTER TABLE. I reproduced this by hand: without the fix, the repair SQL alterst,t_idx,pg_aovisimap_<oid>andpg_aovisimap_<oid>_index, and fails on the third. With it, onlytandt_idxare altered and the script succeeds.Unit tests (
python3 -m unittest, withPYTHONPATHset the waymake -C gpMgmt/bin unitdevelsets it):test_unit_gpcheckcatpasses with this PR, all 41 tests. Without it, the ten new tests error out because the functions they test do not exist. The wholegppylibunit suite, with this PR and #2180 applied together, has no new failures; the tests that fail in it fail the same way without these PRs.#2170 tags these scenarios
@needs_fixpointing at this change. Whichever PR lands second removes those tags.Type of Change
mix_distribution_policycheckChecklist