From ebb9f66675437e310c45ba2d35f4c2a8c17c2750 Mon Sep 17 00:00:00 2001 From: Joe Schafer Date: Mon, 27 Jan 2020 20:32:07 -0800 Subject: [PATCH] Dedupe `CREATE SCHEMA PUBLIC` statements Previously, the CreateSchemaParser blindly added a schema. Instead, only add the schema if it's not `public`. For additional safety, throw an error if a user tries to add a schema that already exists. --- .../pgdiff/parsers/CreateSchemaParser.java | 34 ++++++------------- .../utils/pgdiff/schema/PgDatabase.java | 8 +++++ .../cz/startnet/utils/pgdiff/PgDiffTest.java | 1 + .../create_schema_no_change_table_diff.sql | 0 .../create_schema_no_change_table_new.sql | 2 ++ ...create_schema_no_change_table_original.sql | 2 ++ 6 files changed, 24 insertions(+), 23 deletions(-) create mode 100644 src/test/resources/cz/startnet/utils/pgdiff/create_schema_no_change_table_diff.sql create mode 100644 src/test/resources/cz/startnet/utils/pgdiff/create_schema_no_change_table_new.sql create mode 100644 src/test/resources/cz/startnet/utils/pgdiff/create_schema_no_change_table_original.sql diff --git a/src/main/java/cz/startnet/utils/pgdiff/parsers/CreateSchemaParser.java b/src/main/java/cz/startnet/utils/pgdiff/parsers/CreateSchemaParser.java index ba42f839..dfe6f500 100644 --- a/src/main/java/cz/startnet/utils/pgdiff/parsers/CreateSchemaParser.java +++ b/src/main/java/cz/startnet/utils/pgdiff/parsers/CreateSchemaParser.java @@ -26,32 +26,20 @@ public static void parse(final PgDatabase database, final Parser parser = new Parser(statement); parser.expect("CREATE", "SCHEMA"); - if (parser.expectOptional("AUTHORIZATION")) { - final PgSchema schema = new PgSchema( - ParserUtils.getObjectName(parser.parseIdentifier())); - database.addSchema(schema); - schema.setAuthorization(schema.getName()); - - final String definition = parser.getRest(); - - if (definition != null && !definition.isEmpty()) { - schema.setDefinition(definition); - } - } else { - final PgSchema schema = new PgSchema( - ParserUtils.getObjectName(parser.parseIdentifier())); + String schemaName = ParserUtils.getObjectName(parser.parseIdentifier()); + PgSchema schema = database.getSchema(schemaName); + if (schema == null) { + schema = new PgSchema(schemaName); database.addSchema(schema); + } - if (parser.expectOptional("AUTHORIZATION")) { - schema.setAuthorization( - ParserUtils.getObjectName(parser.parseIdentifier())); - } - - final String definition = parser.getRest(); + if (parser.expectOptional("AUTHORIZATION")) { + schema.setAuthorization(schema.getName()); + } - if (definition != null && !definition.isEmpty()) { - schema.setDefinition(definition); - } + final String definition = parser.getRest(); + if (definition != null && !definition.isEmpty()) { + schema.setDefinition(definition); } } diff --git a/src/main/java/cz/startnet/utils/pgdiff/schema/PgDatabase.java b/src/main/java/cz/startnet/utils/pgdiff/schema/PgDatabase.java index 8231f3c3..37a0f80d 100644 --- a/src/main/java/cz/startnet/utils/pgdiff/schema/PgDatabase.java +++ b/src/main/java/cz/startnet/utils/pgdiff/schema/PgDatabase.java @@ -5,6 +5,7 @@ */ package cz.startnet.utils.pgdiff.schema; +import cz.startnet.utils.pgdiff.parsers.ParserException; import java.util.ArrayList; import java.util.Collections; import java.util.List; @@ -135,8 +136,15 @@ public List getSchemas() { * Adds {@code schema} to the lists of schemas. * * @param schema schema + * @throws ParserException Thrown if schema was already added to the database. */ public void addSchema(final PgSchema schema) { + for (PgSchema existingSchema : schemas) { + if (existingSchema.getName().equals(schema.getName())) { + throw new ParserException( + "Schema '" + schema.getName() + "' already added to database"); + } + } schemas.add(schema); } diff --git a/src/test/java/cz/startnet/utils/pgdiff/PgDiffTest.java b/src/test/java/cz/startnet/utils/pgdiff/PgDiffTest.java index 6187b99e..285c5b4b 100644 --- a/src/test/java/cz/startnet/utils/pgdiff/PgDiffTest.java +++ b/src/test/java/cz/startnet/utils/pgdiff/PgDiffTest.java @@ -272,6 +272,7 @@ public static Collection parameters() { , {"alter_view_owner", false, false, false, false} , {"grant_on_table_cols_mixed", false, false, false, false} , {"grant_on_view_cols_mixed", false, false, false, false} + , {"create_schema_no_change_table", false, false, false, false} }); } /** diff --git a/src/test/resources/cz/startnet/utils/pgdiff/create_schema_no_change_table_diff.sql b/src/test/resources/cz/startnet/utils/pgdiff/create_schema_no_change_table_diff.sql new file mode 100644 index 00000000..e69de29b diff --git a/src/test/resources/cz/startnet/utils/pgdiff/create_schema_no_change_table_new.sql b/src/test/resources/cz/startnet/utils/pgdiff/create_schema_no_change_table_new.sql new file mode 100644 index 00000000..878df121 --- /dev/null +++ b/src/test/resources/cz/startnet/utils/pgdiff/create_schema_no_change_table_new.sql @@ -0,0 +1,2 @@ +CREATE SCHEMA public; +CREATE TABLE public.node (instance_id text); diff --git a/src/test/resources/cz/startnet/utils/pgdiff/create_schema_no_change_table_original.sql b/src/test/resources/cz/startnet/utils/pgdiff/create_schema_no_change_table_original.sql new file mode 100644 index 00000000..878df121 --- /dev/null +++ b/src/test/resources/cz/startnet/utils/pgdiff/create_schema_no_change_table_original.sql @@ -0,0 +1,2 @@ +CREATE SCHEMA public; +CREATE TABLE public.node (instance_id text);