DDL scripts are generated in append-mode

[knutwannheden]

Describe the bug

When using the new Quarkus 2.0 properties quarkus.hibernate-orm.scripts.generation and quarkus.hibernate-orm.scripts.generation.create-target to generate a DDL script to create the database schema, the file identified by the latter will be appended to if it already exists.

Expected behavior

I would expect the DDL script files to be overwritten if they already exist.

Actual behavior

As mentioned in the description the DDL statements are appended to the file, if it already exists.

To Reproduce

Link to a small reproducer (preferably a Maven project if the issue is not Gradle-specific).

Or attach an archive containing the reproducer to the issue.

Steps to reproduce the behavior:

1. Create a Quarkus project with the hibernate-orm and jdbc-h2 extensions
2. Add a simple entity to the project
3. Set configuration properties as seen below
4. Write a simple Quarkus test like the following and run it a few times

@QuarkusTest
class SchemaExportTest {
    @Test
    void exportSchema() {
    }
}

Configuration

quarkus.datasource.db-kind=h2
quarkus.hibernate-orm.scripts.generation=create
quarkus.hibernate-orm.scripts.generation.create-target=create.sql

Environment (please complete the following information):

Output of uname -a or ver

Output of java -version

GraalVM version (if different from Java)

Quarkus version or git rev

2.0.0.CR3 with Hibernate 5.5.2.Final or using latest Quarkus main branch.

Build tool (ie. output of mvnw --version or gradlew --version)

Additional context

(Add any other context about the problem here.)

[quarkus-bot]

/cc @Sanne, @gsmet, @yrodiere

[yrodiere]

This looks intentional... it's possibly a feature of Hibernate ORM.

See org.hibernate.tool.schema.internal.exec.ScriptTargetOutputToFile#toFileWriter:

	static Writer toFileWriter( File file, String charsetName ) {
		try {
			if ( ! file.exists() ) {
				// best effort, since this is very likely not allowed in EE environments
				log.debug( "Attempting to create non-existent script target file : " + file.getAbsolutePath() );
				if ( file.getParentFile() != null ) {
					file.getParentFile().mkdirs();
				}
				file.createNewFile();
			}
		}
		catch (Exception e) {
			log.debug( "Exception calling File#createNewFile : " + e.toString() );
		}
		try {
			return charsetName != null ?
					new OutputStreamWriter(
							new FileOutputStream( file, true ),
							charsetName
					) :
					new OutputStreamWriter( new FileOutputStream(
							file,
							true
					) );
		}
		catch (IOException e) {
			throw new SchemaManagementException( "Unable to open specified script target file for writing : " + file, e );
		}
	}

The change to append mode was done back in 2016, here: hibernate/hibernate-orm@17de173#diff-7fb8c4dca2b0b1a906a80335317a3bffa8ee8907ca3eda7ff6cb70915c5913f9R80

This looks like it was on purpose, but I cannot find the reason in the commit message. It may be related to HHH-10458, but I don't really know how. Maybe @sebersole remembers, but that was 5 years ago...

[knutwannheden]

Yes, that change indeed looks very deliberate. I couldn't find anything regarding that change in the issue or PR comments. I also had a look at the JPA specification, but it doesn't go into detail about this. I am really interested in the use case for this. I can only think of the hbm2ddl update action, where appending to an existing script may make sense?

[Sanne]

I don't know about the resons for that either - and I'm not sure if the "append" boolean was set to true intentionally.

For Quarkus it seems reasonable to not append; it migth be fine to simply flip the boolean but that's almost certain to break other tools or platforms so let's be cautious - at very least let's ask @sebersole .

[yrodiere]

Looks like this has been requested upstream before (thanks to @dreab8 for finding this!): https://hibernate.atlassian.net/browse/HHH-11817

It seems the behavior indeed changed sometime after 4.3.6, and there is indeed nothing in the JPA spec that tells us what to do. So if this gets fixed, it will probably be through a configuration property.

[yrodiere]

Hibernate ORM added a setting to switch between append mode and truncate mode: hibernate/hibernate-orm#4055

It will be available in ORM 5.5.3.

Once we upgrade, not sure if we should just expose the setting, change the default, or both. Append mode has been the default since ORM ~4.2, so it's been a long, long time. At this point I'm not even sure if this issue should be considered a bug report or a feature request.

[knutwannheden]

Excellent! In the linked PR I also found the reason for this behavior (hibernate/hibernate-orm#4055 (comment)):

IIRC the issue is generating these from multiple "sources". JPA allows you to request that these files be populated from the model + some other script (aka, our legacy sql.import). IIRC truncating simply over-wrote whichever source was processed first with the other.

Regarding the question what Quarkus should do:

Once we upgrade, not sure if we should just expose the setting, change the default, or both. Append mode has been the default since ORM ~4.2, so it's been a long, long time. At this point I'm not even sure if this issue should be considered a bug report or a feature request.

For Quarkus this is a new feature with Quarkus 2.0 (IIRC), so it would seem like Quarkus should still be free to decide what its default behavior should be. I am also unsure if the legacy sql.import mentioned above applies to Quarkus or not.

[Sanne]

Right this is new in Quarkus; we should default to truncate; if we get in trouble with multiple sources being applied IMO we should be able to prevent that here from happening.

[yrodiere]

@Sanne We will need a release of ORM and an upgrade in Quarkus before we release Quarkus 2.0.0.Final, then. Is that still possible?

[Sanne]

@yrodiere it is still possible AFAIK yes - we can try but I'm not sure if it's worth our time for a single fix? Let's see.

[knutwannheden]

IMHO, delivering the fix as part of 2.1 (and possibly a potential 2.0.1 release) should also be OK, since there is the workaround of manually deleting the file.

[yrodiere]

@yrodiere it is still possible AFAIK yes - we can try but I'm not sure if it's worth our time for a single fix? Let's see.

I agree it's not worth our time, but the argument was that it's a new feature and thus we can change its behavior. If we roll the behavior change in 2.1, it will no longer be a new feature.

Anyway... Let's say we change the behavior in 2.1 and add something to the migration guide when we do that.