Add support for Postgres17 (+ misc linting changes) (stripe#187)

Author: bplunkett-stripe

.github/workflows/run-tests.yaml

@@ -10,11 +10,19 @@ jobs:
     runs-on: ubuntu-latest
     strategy:
       matrix:
-        pg_package: ["postgresql14", "postgresql15", "postgresql16"]
+        include:
+          - base_image: "golang:1.20.14-alpine3.19"
+            pg_package: "postgresql14"
+          - base_image: "golang:1.20.14-alpine3.19"
+            pg_package: "postgresql15"
+          - base_image: "golang:1.20.14-alpine3.19"
+            pg_package: "postgresql16"
+          - base_image: "golang:1.22.10-alpine3.21"
+            pg_package: "postgresql17"
     steps:
       - name: Checkout code
         uses: actions/checkout@v3
       - name: Build Docker image
-        run: docker build -t pg-schema-diff-test-runner -f ./build/Dockerfile.test --build-arg POSTGRES_PACKAGE=${{ matrix.pg_package }} .
+        run: docker build -t pg-schema-diff-test-runner -f ./build/Dockerfile.test --build-arg BASE_IMAGE=${{ matrix.base_image }} --build-arg POSTGRES_PACKAGE=${{ matrix.pg_package }} .
       - name: Run tests
         run: docker run pg-schema-diff-test-runner

.golangci.yaml

@@ -1,6 +1,7 @@
 linters:
   disable-all: true
   enable:
+  - gofmt
   - goimports
   - ineffassign
   - staticcheck

build/Dockerfile.lint

@@ -11,7 +11,7 @@ RUN wget -O- -nv https://raw.githubusercontent.com/golangci/golangci-lint/master
 # Install sqlfluff
 RUN pip install wheel # It complains if we attempt to install this in the same command as Cython
 RUN pip install "Cython<3.0" pyyaml --no-build-isolation # Fix for https://github.com/yaml/pyyaml/issues/601
-RUN pip install "sqlfluff==2.1.2"
+RUN pip install "sqlfluff==3.3.0"
 
 WORKDIR /pg-schema-diff
 COPY . .

build/Dockerfile.test

@@ -1,7 +1,8 @@
-FROM golang:1.20.14-alpine3.19
+ARG BASE_IMAGE=golang:1.20.14-alpine3.19
 
-ARG POSTGRES_PACKAGE=postgresql14
+FROM $BASE_IMAGE
 
+ARG POSTGRES_PACKAGE=postgresql14
 RUN apk update && \
     apk add --no-cache  \
       build-base \

internal/graph/graph.go

@@ -88,7 +88,7 @@ func (g *Graph[V]) HasVertexWithId(id string) bool {
 // Reverse reverses the edges of the map. The sources become the sinks and vice versa.
 func (g *Graph[V]) Reverse() {
 	reversedEdges := make(AdjacencyMatrix)
-	for vertexId, _ := range g.verticesById {
+	for vertexId := range g.verticesById {
 		reversedEdges[vertexId] = make(map[string]bool)
 	}
 	for source, adjacentEdgesMap := range g.edges {

internal/graph/graph_test.go

@@ -136,7 +136,7 @@ func TestCopy(t *testing.T) {
 		"shared_1", "shared_2", "shared_3", "g_1", "g_2", "g_3",
 	})
 
-	// validate overrides weren't copied over and non-overriden shared nodes are the same
+	// validate overrides weren't copied over and non-overridden shared nodes are the same
 	assert.NotEqual(t, g.GetVertex("shared_1"), gC.GetVertex("shared_1"))
 	assert.Equal(t, gC.GetVertex("shared_1"), copyOverrideShared1)
 	assert.NotEqual(t, g.GetVertex("shared_2"), gC.GetVertex("shared_2"))
@@ -425,7 +425,7 @@ func (v vertex) GetId() string {
 
 func getVertexIds(g *Graph[vertex]) []string {
 	var output []string
-	for id, _ := range g.verticesById {
+	for id := range g.verticesById {
 		output = append(output, id)
 	}
 	return output

internal/migration_acceptance_tests/sequence_cases_test.go

@@ -200,7 +200,7 @@ var sequenceAcceptanceTests = []acceptanceTestCase{
 		},
 	},
 	{
-		name: "And and Drop sequences (conflicing schemas)",
+		name: "And and Drop sequences (conflicting schemas)",
 		oldSchemaDDL: []string{
 			`
             CREATE SCHEMA schema_1;

internal/queries/queries.sql

@@ -342,7 +342,7 @@ WHERE
     AND seq_ns.nspname !~ '^pg_temp'
     -- Exclude sequences owned by identity columns.
     --  These manifest as internal dependency on the column
-    AND (depend.deptype IS NULL OR depend.deptype != 'i')
+    AND (depend.deptype IS null OR depend.deptype != 'i')
     -- Exclude sequences belonging to extensions
     AND NOT EXISTS (
         SELECT ext_depend.objid

internal/queries/queries.sql.go

@@ -865,7 +865,8 @@ func (s *schemaFetcher) buildTable(
 		}
 
 		var identity *ColumnIdentity
-		if len(column.IdentityType) > 0 {
+		if len(column.IdentityType) > 0 && table.ParentTableName == "" {
+			// Exclude identity columns from table partitions because they are owned by the parent.
 			identity = &ColumnIdentity{
 				Type:       ColumnIdentityType(column.IdentityType),
 				StartValue: column.StartValue.Int64,

internal/schema/schema.go

@@ -469,7 +469,7 @@ var (
 				content TEXT DEFAULT '',
 				genre VARCHAR(256) NOT NULL,
 				created_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP CHECK (created_at > CURRENT_TIMESTAMP - interval '1 month'),
-				version INT NOT NULL DEFAULT 0,
+				version INT GENERATED ALWAYS AS IDENTITY,
 				PRIMARY KEY (author, id)
 			) PARTITION BY LIST (author);
 			ALTER TABLE foo ADD CONSTRAINT author_check CHECK (author IS NOT NULL AND LENGTH(author) > 0) NOT VALID;
@@ -524,7 +524,7 @@ var (
 			ALTER TABLE foo_fk_1 ADD CONSTRAINT foo_fk_1_fk FOREIGN KEY (author, content) REFERENCES foo_1 (author, content)
 				NOT VALID;
 		`},
-			expectedHash: "cf473d75363e9f77",
+			expectedHash: "38588fed86b25fd",
 			expectedSchema: Schema{
 				NamedSchemas: []NamedSchema{
 					{Name: "public"},
@@ -538,7 +538,9 @@ var (
 							{Name: "content", Type: "text", Default: "''::text", IsNullable: true, Size: -1, Collation: defaultCollation},
 							{Name: "genre", Type: "character varying(256)", Size: -1, Collation: defaultCollation},
 							{Name: "created_at", Type: "timestamp without time zone", Default: "CURRENT_TIMESTAMP", Size: 8},
-							{Name: "version", Type: "integer", Default: "0", Size: 4},
+							{Name: "version", Type: "integer", Size: 4,
+								Identity: &ColumnIdentity{Type: "a", MinValue: 1, MaxValue: 2147483647, StartValue: 1, Increment: 1, CacheSize: 1, Cycle: false},
+							},
 						},
 						CheckConstraints: []CheckConstraint{
 							{Name: "author_check", Expression: "((author IS NOT NULL) AND (length(author) > 0))", IsInheritable: true, KeyColumns: []string{"author"}},
@@ -557,7 +559,7 @@ var (
 							{Name: "content", Type: "text", Default: "''::text", Size: -1, Collation: defaultCollation},
 							{Name: "genre", Type: "character varying(256)", Size: -1, Collation: defaultCollation},
 							{Name: "created_at", Type: "timestamp without time zone", Default: "CURRENT_TIMESTAMP", Size: 8},
-							{Name: "version", Type: "integer", Default: "0", Size: 4},
+							{Name: "version", Type: "integer", IsNullable: false, Size: 4},
 						},
 						CheckConstraints: nil,
 						ReplicaIdentity:  ReplicaIdentityNothing,
@@ -572,7 +574,7 @@ var (
 							{Name: "content", Type: "text", Default: "''::text", IsNullable: true, Size: -1, Collation: defaultCollation},
 							{Name: "genre", Type: "character varying(256)", Size: -1, Collation: defaultCollation},
 							{Name: "created_at", Type: "timestamp without time zone", Default: "CURRENT_TIMESTAMP", Size: 8},
-							{Name: "version", Type: "integer", Default: "0", Size: 4},
+							{Name: "version", Type: "integer", IsNullable: false, Size: 4},
 						},
 						CheckConstraints: nil,
 						ReplicaIdentity:  ReplicaIdentityDefault,
@@ -587,7 +589,7 @@ var (
 							{Name: "content", Type: "text", Default: "''::text", IsNullable: true, Size: -1, Collation: defaultCollation},
 							{Name: "genre", Type: "character varying(256)", Size: -1, Collation: defaultCollation},
 							{Name: "created_at", Type: "timestamp without time zone", Default: "CURRENT_TIMESTAMP", Size: 8},
-							{Name: "version", Type: "integer", Default: "0", Size: 4},
+							{Name: "version", Type: "integer", IsNullable: false, Size: 4},
 						},
 						CheckConstraints: nil,
 						ReplicaIdentity:  ReplicaIdentityDefault,

internal/schema/schema_test.go

@@ -55,7 +55,7 @@ var (
 	}
 	migrationHazardExtensionDroppedCannotTrackDependencies = MigrationHazard{
 		Type:    MigrationHazardTypeHasUntrackableDependencies,
-		Message: "This extension may be in use by tables, indexes, functions, triggers, etc. Tihs statement will be ran last, so this may be OK.",
+		Message: "This extension may be in use by tables, indexes, functions, triggers, etc. This statement will be ran last, so this may be OK.",
 	}
 	migrationHazardExtensionAlteredVersionUpgraded = MigrationHazard{
 		Type:    MigrationHazardTypeExtensionVersionUpgrade,

pkg/diff/sql_generator.go

@@ -14,7 +14,7 @@ import (
 // original SchemaDiff struct
 func dataPackNewTables(s schemaDiff) schemaDiff {
 	copiedNewTables := append([]schema.Table(nil), s.tableDiffs.adds...)
-	for i, _ := range copiedNewTables {
+	for i := range copiedNewTables {
 		copiedColumns := append([]schema.Column(nil), copiedNewTables[i].Columns...)
 		copiedNewTables[i].Columns = copiedColumns
 		sort.Slice(copiedColumns, func(i, j int) bool {
@@ -31,9 +31,9 @@ func dataPackNewTables(s schemaDiff) schemaDiff {
 // generator to ignore changes to column ordering
 func removeChangesToColumnOrdering(s schemaDiff) schemaDiff {
 	copiedTableDiffs := append([]tableDiff(nil), s.tableDiffs.alters...)
-	for i, _ := range copiedTableDiffs {
+	for i := range copiedTableDiffs {
 		copiedColDiffs := append([]columnDiff(nil), copiedTableDiffs[i].columnsDiff.alters...)
-		for i, _ := range copiedColDiffs {
+		for i := range copiedColDiffs {
 			copiedColDiffs[i].oldOrdering = copiedColDiffs[i].newOrdering
 		}
 		copiedTableDiffs[i].columnsDiff.alters = copiedColDiffs