TC_INITSCRIPT can't handle PostgreSQL dollar quoted strings

[lukaseder]

A simple program like this:

public static void main(String[] args) throws SQLException {
    Properties properties = new Properties();
    properties.setProperty("username", "postgres");
    properties.setProperty("password", "postgres");

    new ContainerDatabaseDriver().connect(
        "jdbc:tc:postgresql:13:///sakila?TC_TMPFS=/testtmpfs:rw&TC_INITSCRIPT=file:c:/temp/dollar-quoted.sql",
        properties
    );
}

Where the dollar-quoted.sql file contains:

CREATE FUNCTION f ()
RETURNS INT
AS $$
BEGIN
    RETURN 1;
END;
$$ LANGUAGE plpgsql;

Fails with this exception:

Exception in thread "main" org.testcontainers.ext.ScriptUtils$UncategorizedScriptException: Failed to execute database script from resource [CREATE FUNCTION f ()
RETURNS INT
AS $$
BEGIN
    RETURN 1;
END;
$$ LANGUAGE plpgsql;
]
	at org.testcontainers.ext.ScriptUtils.executeDatabaseScript(ScriptUtils.java:375)
	at org.testcontainers.ext.ScriptUtils.executeDatabaseScript(ScriptUtils.java:313)
	at org.testcontainers.jdbc.ContainerDatabaseDriver.runInitScriptIfRequired(ContainerDatabaseDriver.java:196)
	at org.testcontainers.jdbc.ContainerDatabaseDriver.connect(ContainerDatabaseDriver.java:132)
	at org.jooq.example.test.containers.TestContainersTest.main(TestContainersTest.java:34)
Caused by: org.testcontainers.ext.ScriptUtils$ScriptStatementFailedException: Script execution failed (file:c:/temp/dollar-quoted.sql:1): CREATE FUNCTION f () RETURNS INT AS $$ BEGIN
    RETURN 1;
END
	at org.testcontainers.jdbc.JdbcDatabaseDelegate.execute(JdbcDatabaseDelegate.java:49)
	at org.testcontainers.delegate.AbstractDatabaseDelegate.execute(AbstractDatabaseDelegate.java:34)
	at org.testcontainers.ext.ScriptUtils.executeDatabaseScript(ScriptUtils.java:362)
	... 4 more
Caused by: org.postgresql.util.PSQLException: Unterminated dollar quote started at position 36 in SQL CREATE FUNCTION f () RETURNS INT AS $$ BEGIN
    RETURN 1;
END. Expected terminating $$
	at org.postgresql.core.Parser.checkParsePosition(Parser.java:1328)
	at org.postgresql.core.Parser.parseSql(Parser.java:1227)
	at org.postgresql.core.Parser.replaceProcessing(Parser.java:1179)
	at org.postgresql.core.CachedQueryCreateAction.create(CachedQueryCreateAction.java:43)
	at org.postgresql.core.QueryExecutorBase.createQueryByKey(QueryExecutorBase.java:337)
	at org.postgresql.jdbc.PgStatement.executeCachedSql(PgStatement.java:300)
	at org.postgresql.jdbc.PgStatement.executeWithFlags(PgStatement.java:284)
	at org.postgresql.jdbc.PgStatement.execute(PgStatement.java:279)
	at org.testcontainers.jdbc.JdbcDatabaseDelegate.execute(JdbcDatabaseDelegate.java:42)
	... 6 more

It seems that the logic borrowed from Spring's ScriptUtils doesn't handle real world SQL scripts very well? I can think of numerous other issues that aren't currently supported, including e.g. MySQL's DELIMITER statement, etc.

[rnorth]

It seems that the logic borrowed from Spring's ScriptUtils doesn't handle real world SQL scripts very well? I can think of numerous other issues that aren't currently supported, including e.g. MySQL's DELIMITER statement, etc.

You're exactly right. We've patched and added to ScriptUtils beyond a sensible degree, but unfortunately there are still real-world scripts that it will choke on. Our general recommendation when this comes up is that people use a proper DB migration tool to apply schemas, and only use the init script mode for simple scripts.

There's nothing I'd like more than to rip ScriptUtils out and replace it with a more capable generic script splitter library that is dialect aware but not too strict (since I'd hate for this to become a compatibility bottleneck again in the future). I think (but this could be my ignorance talking) that there isn't such a library. You're quite likely to know better, though :)

[lukaseder]

I think (but this could be my ignorance talking) that there isn't such a library. You're quite likely to know better, though :)

Well, I created one ;-), but it depends on being able to parse the entire content, and that's far from feature complete. I'm not sure if a utility that ignores the SQL / procedural language content can get it right for all dialects.

But this request here is merely about dollar quoted strings. ScriptUtils is already capable of detecting ordinary string literals and ignoring their contents... I think that supporting those would already go a long way for PostgreSQL and related dialects (CockroachDB, Yugabyte)

[rnorth]

For me I'm concerned that we would just keep on extending ScriptUtils and have something that is still not capable of splitting every SQL script, and is an ongoing maintenance burden at the fringes of our areas of expertise.

Arguably it was a mistake (my mistake 😬) to extend ScriptUtils at all, but I blame my naive optimism.

I'll invite @bsideup and @kiview to comment on their thoughts about adding dollar-quoted string support - maybe I'm overestimating the downsides and we're missing out on some low-hanging fruit...

[kiview]

It might be a pragmatic thing to add this support and I would be inclined to say, that I'd support adding it if the community would contribute a PR.

However, at the same time, we should raise awareness in the docs, that we support SQL scripts for init only on a best-effort basis of for simple examples.

[lukaseder]

Obviously, like many features, this one is an endless one that's never finished, given how many different SQL script interpreters and preprocessors there are... I get your arguments here. But people would probably love running their pg_dump generated scripts too...

[mjcurciojr]

To me, resolving this issue has enough value where a one-off fix is worthwhile (though maybe I am biased). While it's true that it is probably better to do migrations with the proper tooling, this does break DB set up scripts that are meant for tests only.

For example, if you are inserting sample data into a table with the PK being a generated guid at insert time. If you want to reference this id later via a RETURNING clause, the idiomatic way to do this is inside a DO block, which are typically dollar quoted. I feel this is a fairly common use case.

[kiview]

Whenever the limitations of TC_INITSCRIPT are an issue, you can just execute whatever SQL statements you like against your database from Java, and as part of the test setup, this should not be a blocking issue for anyone.

[membersound]

Also stumbled upon this today.
It would be great it stored procedures/functions would support DELIMITER in testcontainers!