Postgresql writeChangeSet(Resource, Database) schema <-> catalog confusion

[andnorxor]

Steps to reproduce

1. Alter the test "diffing" in spring-data-examples in a way that it uses a "real" Postgres database.

This can be achieved by adding the necessary dependencies:

<dependency>
    <groupId>org.postgresql</groupId>
    <artifactId>postgresql</artifactId>
    <version>42.5.1</version>
</dependency>
<dependency>
    <groupId>com.zaxxer</groupId>
    <artifactId>HikariCP</artifactId>
    <scope>test</scope>
</dependency>

... and editing the code as follows

	@Test
	void diffing() {

		// the change set will get appended, so we delete any pre existing file.
		new File("cs-diff.yaml").delete();

		context.setInitialEntitySet(Collections.singleton(Minion.class));
		LiquibaseChangeSetWriter writer = new LiquibaseChangeSetWriter(context);

		// drop unused columns
		writer.setDropColumnFilter((table, column) -> !column.equalsIgnoreCase("special"));

		DataSource realPostgresDb = createTestDataSource();
		// for comparison with existing schema
		try (Database db = new PostgresDatabase()) {

			db.setConnection(new JdbcConnection(realPostgresDb.getConnection()));

			writer.writeChangeSet(new FileSystemResource("cs-diff.yaml"), db);

		} catch (IOException | SQLException | LiquibaseException e) {
			throw new RuntimeException("Changeset generation failed", e);
		}
	}

	private DataSource createTestDataSource() {
		HikariConfig config = new HikariConfig();
		config.setJdbcUrl("jdbc:postgresql://localhost:5433/foo");
		config.setUsername("postgres");
		config.setPassword("postgres");

		return new HikariDataSource(config);
	}

2. Run the test

Expected result

Postgres tables should be compared according catalogName and schemaName. The serializer should set the schemaName property correctly.

If other catalog aware databases have the same issue this could probably described in a more generic way as: writeChangeSet should handle catalog aware databases.

Actual Result

Tables get compared kind of "properly", because both datasets (entities and the actual database) use the catalogName as schemaName. So the misconception is cancelled out, because both of the retrieval methods have the same behavior.

Therefore, this seems to be the intended behavior as both parts of the implementation explicitly use the catalogName:

Method: differenceOf uses getDefaultCatalogName()

Tables mappedEntities = Tables.from(mappingContext.getPersistentEntities().stream().filter(schemaFilter),
				sqlTypeMapping, database.getDefaultCatalogName());

Method: getLiquibaseModel uses table.getSchema().getCatalogName()

Table tableModel = new Table(table.getSchema().getCatalogName(), table.getName());

However, this results in incorrect output by the serializer. As evident, the schemaName is mistakenly set to the catalogName, leading to the changelog's failure when applied to the real database, as the schema foo does not exist.

The yml represents the output from my testcase. The schemaName is set to foo which actually is the catalogName. If the yaml property schemaName is omitted or corrected manually (to default public) the generated file would be ok.

databaseChangeLog:
- changeSet:
    id: '1706712353902'
    author: Spring Data Relational
    objectQuotingStrategy: LEGACY
    changes:
    - createTable:
        columns:
        - column:
            autoIncrement: true
            constraints:
              nullable: true
              primaryKey: true
            name: id
            type: BIGINT
        - column:
            constraints:
              nullable: true
            name: name
            type: VARCHAR(255 BYTE)
        schemaName: foo
        tableName: minion

Additional Notes

1. Not all databases have the concept of a catalog. Postgres does.

2. Figuring out a way to solve this issue I also noted that the settings

db.setOutputDefaultCatalog(false);
db.setOutputDefaultSchema(false);

do not seem to influence the serialized changelog. This could be a misunderstanding of the documentation concerning these properties, or perhaps it's a functionality that has not been implemented yet. Anyway... serializers are part of liquibase.serializer.

3. The FormattedSqlChangeLogSerializer provided by liquibase des not work, because the filepath is retrieved from the changeset. And Spring does not set this filepath. Source

protected Database getTargetDatabase(ChangeSet changeSet) {
        String filePath = changeSet.getFilePath();

4. For catalog aware databases the org.springframework.data.relational.core.mapping.Table schema property is not considered.

Deploying the changelog

Maybe the behavior is due how I process the changelog. For completeness I'll also post this snippet.
In order for this to work I've created a resource folder and set the output paths to src/test/resources/cs-diff.yaml

CommandScope updateCommand = new CommandScope(UpdateCommandStep.COMMAND_NAME);
updateCommand.addArgumentValue(DbUrlConnectionCommandStep.DATABASE_ARG, db);
updateCommand.addArgumentValue(UpdateCommandStep.CHANGELOG_FILE_ARG, "cs-diff.yaml");
updateCommand.addArgumentValue(UpdateCommandStep.CONTEXTS_ARG, context.toString());
updateCommand.execute();

This leads to:

Position: 14 [Failed SQL: (0) CREATE TABLE foo.minion (id BIGINT GENERATED BY DEFAULT AS IDENTITY NOT NULL, name VARCHAR(255), CONSTRAINT minion_pkey PRIMARY KEY (id))]

... because schema foo does not exist. foo is the catalog.

Possible fix

Somwhere in the code there has to be a separate handling for catalog aware and NON catalog aware databases. This sounds simple and is obviously an oversimplification.

Liquibase already exposes a method on a database instance called database.supportsCatalogs() in liquibase.database.Database

[andnorxor]

For a quick fix I've updated the Code as follows (changes are commented with "// modification of original source")

The following changes ...

• ... are only superficially tested and not well thought out
• ... enable support for catalog aware databases
• ... consider the @Table annotation schema property
• ... pass the filepath to the changeset to make the liquibase FormattedSqlChangeLogSerializer work.

LiquibaseChangeSetWriter.java

private SchemaDiff differenceOf(Database database) throws LiquibaseException {
		Set<? extends RelationalPersistentEntity<?>> entities = mappingContext.getPersistentEntities().stream()
            .filter(schemaFilter)
            .collect(Collectors.toSet());

        // modification of original source
        String schema = database.supportsCatalogs() ?
            database.getDefaultSchemaName() :
            database.getDefaultCatalogName();

        Tables mappedEntities = Tables.from(entities.stream(), sqlTypeMapping, schema, mappingContext);
        Tables existingTables = getLiquibaseModel(database, getEntitySchemas(database, entities));

		return SchemaDiff.diff(mappedEntities, existingTables, nameComparator);
	}
	
    // modification of original source
private CatalogAndSchema[] getEntitySchemas(Database database, Set<? extends RelationalPersistentEntity<?>> entities) {
        return entities.stream()
            .filter(it -> it.isAnnotationPresent(org.springframework.data.relational.core.mapping.Table.class))
            .map(entity -> entity.getRequiredAnnotation(org.springframework.data.relational.core.mapping.Table.class))
            .map(tableAnnotation -> new CatalogAndSchema(database.getDefaultCatalogName(), tableAnnotation.schema()))
            .distinct()
            .toArray(CatalogAndSchema[]::new);
    }
    
private void writeChangeSet(DatabaseChangeLog databaseChangeLog, ChangeSet changeSet, File changeLogFile)
			throws IOException {

		List<ChangeLogChild> changes = new ArrayList<>(databaseChangeLog.getChangeSets());
		changes.add(changeSet);

		try (FileOutputStream fos = new FileOutputStream(changeLogFile)) {
            // modification of original source
            changeSet.setFilePath(changeLogFile.getAbsolutePath());
			changeLogSerializer.write(changes, fos);
		}
	} 

	private Tables getLiquibaseModel(Database targetDatabase, CatalogAndSchema[] catalogAndSchemas) throws LiquibaseException {

		SnapshotControl snapshotControl = new SnapshotControl(targetDatabase);

		DatabaseSnapshot snapshot = SnapshotGeneratorFactory.getInstance()
            .createSnapshot(catalogAndSchemas, targetDatabase, snapshotControl);

		Set<liquibase.structure.core.Table> tables = snapshot.get(liquibase.structure.core.Table.class);
		List<Table> existingTables = new ArrayList<>(tables.size());

		for (liquibase.structure.core.Table table : tables) {

			// Exclude internal Liquibase tables from comparison
			if (isLiquibaseTable.test(table.getName())) {
				continue;
			}

            // modification of original source
            String schema = targetDatabase.supportsCatalogs() ?
                table.getSchema().getSchema().getName() :
                table.getSchema().getCatalogName();

			Table tableModel = new Table(schema, table.getName());

			List<liquibase.structure.core.Column> columns = table.getColumns();

			for (liquibase.structure.core.Column column : columns) {

				String type = column.getType().toString();
				boolean nullable = column.isNullable();
				Column columnModel = new Column(column.getName(), type, nullable, false);

				tableModel.columns().add(columnModel);
			}

			tableModel.foreignKeys().addAll(extractForeignKeys(table));

			existingTables.add(tableModel);
		}

		return new Tables(existingTables);
	}

Tables.java

	public static Tables from(Stream<? extends RelationalPersistentEntity<?>> persistentEntities,
			SqlTypeMapping sqlTypeMapping, @Nullable String defaultSchema,
			MappingContext<? extends RelationalPersistentEntity<?>, ? extends RelationalPersistentProperty> context) {

		List<ForeignKeyMetadata> foreignKeyMetadataList = new ArrayList<>();
		List<Table> tables = persistentEntities
				.filter(it -> it.isAnnotationPresent(org.springframework.data.relational.core.mapping.Table.class)) //
				.map(entity -> {

                    // modification of original source
                    org.springframework.data.relational.core.mapping.Table tableAnnotation =
                        entity.getRequiredAnnotation(org.springframework.data.relational.core.mapping.Table.class);

                    String tableSchema = !tableAnnotation.schema().isEmpty() ?
                        tableAnnotation.schema() : defaultSchema;

					Table table = new Table(tableSchema, entity.getTableName().getReference());

					Set<RelationalPersistentProperty> identifierColumns = new LinkedHashSet<>();
					entity.getPersistentProperties(Id.class).forEach(identifierColumns::add);

					for (RelationalPersistentProperty property : entity) {

						if (property.isEntity() && !property.isEmbedded()) {
							foreignKeyMetadataList.add(createForeignKeyMetadata(entity, property, context, sqlTypeMapping));
							continue;
						}

						Column column = new Column(property.getColumnName().getReference(), sqlTypeMapping.getColumnType(property),
								sqlTypeMapping.isNullable(property), identifierColumns.contains(property));
						table.columns().add(column);
					}
					return table;
				}).collect(Collectors.toList());

		applyForeignKeyMetadata(tables, foreignKeyMetadataList);

		return new Tables(tables);
	}