GG-596: Implement checks for ggcheckmigrate by bandetto · Pull Request #25 · GreengageDB/gpbackup

bandetto · 2026-08-04T14:27:01Z

Implement checks for ggcheckmigrate

Implement GG6-to-GG7 migration validation in ggcheckmigrate.

The utility now:

Checks all connectable, non-template source databases by default.
Supports limiting checks to one database with --source-database.
Validates source and target versions and connection flags.
Reports migration findings with exit code 1 and operational failures with exit code 5.
Cleans up temporary database objects after success or failure.

The implemented checks cover:

Multi-column LIST partitions.
Python 2 functions.
Views using removed operators, functions, or types.
Columns using removed data types.
Missing append-optimized partition options.
Restricted EXECUTE ON functions.
Incomplete unique partition indexes.
Incompatible range partition boundaries.
Statement triggers.
Libraries missing from the target cluster.

The PR has been tested against live GG6 6.31.0 and GG7 7.5.0 clusters.

dkovalev1

The patch is solid and comprehensive. Special thanks for embedding SQL using separated file. However, some questions arose.

dkovalev1 · 2026-08-13T04:18:27Z

+		return false, returnError
+	}
+
+	hasCheckFindings, returnError := checkMultiColumnListPartitions(sourceConnection)


This is repetitive code, have you considered to make an array of check* functions and call them in a loop?

dkovalev1 · 2026-08-13T04:20:16Z

+	LibraryName string `db:"library_name"`
+}
+
+func runMigrationChecks(sourceConnection *dbconn.DBConn, targetConnection *dbconn.DBConn) (hasFindings bool, returnError error) {


Could it be more informative to return a number of successful checks and a number of unsuccessful checks instead of a single bool?

dkovalev1 · 2026-08-13T04:53:10Z

 func DoCheckMigrate() {
+	if sourceConnectionPool == nil {
+		gplog.SetErrorCode(5)
+		panic(fmt.Errorf("The source connection is not initialized"))


The Go convention is: "Error strings should not be capitalized (unless beginning with proper nouns or acronyms) or end with punctuation, since they are usually printed following other context. "
See https://go.dev/wiki/CodeReviewComments#error-strings

dkovalev1 · 2026-08-13T05:02:38Z

@@ -0,0 +1,67 @@
+SET LOCAL track_counts TO off;
+CREATE SCHEMA __ggcheckmigrate_tmp;


Is the schema expected to be left after the run? Is the second run allowed? What if someone runs 2 instances in parallel?

dkovalev1 · 2026-08-13T05:41:47Z

+	}
+
+	var output strings.Builder
+	output.WriteString("Your cluster contains partitioned tables with child partitions, which do not have the parent table's settings defined.\nIn version 7, they will be inherited from the parent table instead of being taken by default.\nYou can recreate following tables with defined setting.\nList of partitioned tables, partitions, and settings with the specified problem:\n")


why not use multiline string here?

dkovalev1 · 2026-08-13T08:00:19Z

+SELECT datname::text AS database_name
+FROM pg_catalog.pg_database
+WHERE datallowconn
+  AND NOT datistemplate


Template database can still be modified by the user and can be worst migrating and checking.

dkovalev1 · 2026-08-13T08:45:10Z

+JOIN unnest(parent_relation.reloptions) po ON true
+LEFT JOIN unnest(child_relation.reloptions) co ON split_part(po, '=', 1) = split_part(co, '=', 1)
+WHERE co IS NULL
+ORDER BY p.schemaname, p.tablename, p.partitionschemaname, p.partitiontablename, po;


The query in spec is simpler, with no child_relation and no WHERE co IS NULL

dkovalev1 · 2026-08-13T09:54:57Z

+FROM pg_catalog.pg_class c
+JOIN pg_catalog.pg_namespace n ON n.oid = c.relnamespace
+WHERE c.relkind IN ('v', 'm')
+  AND c.oid >= 16384


Why this checks restrict selection by user objects while neighbor removed_operator_views.sql does not?

dkovalev1 · 2026-08-13T10:35:01Z

+JOIN pg_catalog.pg_namespace n ON n.oid = c.relnamespace
+JOIN pg_catalog.pg_attribute a ON a.attrelid = c.oid AND a.attnum = ANY(p.paratts)
+JOIN pg_catalog.pg_type t ON t.oid = a.atttypid
+JOIN pg_catalog.pg_class child_relation ON child_relation.oid = partition_rule.parchildrelid


what is child_relation join for?

dkovalev1 · 2026-08-13T10:40:04Z

A language created explicitly (CREATE LANGUAGE mypy HANDLER plpython_call_handler) has no template row and is invisible to the check. Resolving through the handler is robust: pg_language.lanplcallfoid → pg_proc.probin = '$libdir/plpython2'.

dkovalev1 · 2026-08-13T11:12:12Z

Can we be sure that this is a complete set of potential problems in the migration?

dkovalev1 · 2026-08-13T11:12:20Z

Are the tests against live 6.x schema considered?

dkovalev1 · 2026-08-13T11:12:26Z

Shall we add a single summary line (databases checked / skipped / total findings)? Right now a large run's outcome has to be reconstructed from scattered stderr blocks plus the exit code.

riogor · 2026-08-13T13:32:14Z

20260813:16:19:16 ggcheckmigrate:rgorchakov:rgorchakov-CREFG-XX:2174600-[ERROR]:-Your cluster contains partitioned tables with child partitions, which do not have the parent table's settings defined.
In version 7, they will be inherited from the parent table instead of being taken by default.
You can recreate following tables with defined setting.
List of partitioned tables, partitions, and settings with the specified problem:
Database "postgres" contains object "part_table_1_prt_p42" of type "partition" in schema "public". The detail is "Parent table \"part_table\" in schema \"public\" defines option \"compresstype=zstd\"".

I think \" symbols look ugly in printout, can you remove them?

Also, i think The detail is " part is obsolete and you can print details string right after main string, also removing double quotes around details string. Should look like this:

Database "postgres" contains object "part_table_1_prt_p42" of type "partition" in schema "public". Parent table "part_table" in schema "public" defines option "compresstype=zstd".

I guess this is also relevant to other options as they all use %q to format strings.

bandetto added 3 commits August 4, 2026 17:21

Implement Greengage migration checks in ggcheckmigrate

5853e5e

Align ggcheckmigrate checks with source database scope

3bf8f1a

Filter incompatible range partition checks

e14b467

dkovalev1 reviewed Aug 13, 2026

View reviewed changes

ci rawrw

110a701

		@@ -0,0 +1,67 @@
		SET LOCAL track_counts TO off;
		CREATE SCHEMA __ggcheckmigrate_tmp;

Uh oh!

Conversation

bandetto commented Aug 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dkovalev1 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dkovalev1 commented Aug 13, 2026

Uh oh!

dkovalev1 commented Aug 13, 2026

Uh oh!

dkovalev1 commented Aug 13, 2026

Uh oh!

riogor commented Aug 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

bandetto commented Aug 4, 2026 •

edited

Loading

riogor commented Aug 13, 2026 •

edited

Loading