Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support table creation DDL translation #194

Closed
wants to merge 9 commits into from
Closed

Support table creation DDL translation #194

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
bccac22
ref PDB-191: Create table translation refactor
Jan 4, 2021
838517f
ref PDB-191: Private keys constraints refactor
Jan 5, 2021
d629a16
ref PDB-191: Foreign keys refactor
Jan 5, 2021
31ab485
ref PDB-191: Add index refactor
Jan 5, 2021
1c0e801
ref PDB-191: Add sequence refactor
Jan 5, 2021
2bc24a3
ref PDB-191: Documentation
Jan 5, 2021
7d9bb8d
ref PDB-191: Documentation
Jan 5, 2021
81fd866
ref PDB-191: Fix imports
Jan 6, 2021
80ad018
Apply suggestions from code review
Jan 12, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions src/main/java/com/feedzai/commons/sql/abstraction/ddl/DbEntity.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
package com.feedzai.commons.sql.abstraction.ddl;

import com.feedzai.commons.sql.abstraction.dml.K;
import com.feedzai.commons.sql.abstraction.engine.AbstractTranslator;

import com.google.common.collect.ImmutableList;

import java.io.Serializable;
Expand Down Expand Up @@ -141,6 +143,21 @@ public Builder newBuilder() {
.addIndexes(indexes);
}

/**
* Translates the given entity table creation to the translator's dialect.
*
* @param translator The translator.
* @return The create table translation result.
*/
public String translateEntityCreation(final AbstractTranslator translator) {
return translator.translateCreateTable(this)
+ translator.translatePrimaryKeysNotNull(this)
+ translator.translatePrimaryKeysConstraints(this)
+ translator.translateForeignKey(this)
+ translator.translateCreateIndexes(this)
+ translator.translateCreateSequences(this);
Comment on lines +153 to +158
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just checked this with H2 and PostgreSQL on the following entity:

                dbEntity()
                        .name("TEST")
                        .addColumn("PK_COL", INT, true)
                        .addColumn("COL1", INT, new K(1))
                        .addColumn("COL2", BOOLEAN, new K(false), NOT_NULL)
                        .addColumn("COL3", DOUBLE, new K(2.2d))
                        .addColumn("COL4", LONG, new K(3L))
                        .pkFields("PK_COL")
                        .addIndex(true, "COL1")
                .build();

With H2 it doesn't work, throws UnsupportedOperationException (see my comment in H2Translator.java);
with PostgreSQL, the result is

CREATE TABLE "TEST" ("PK_COL" SERIAL, "COL1" INT DEFAULT 1, "COL2" BOOLEAN NOT NULL DEFAULT FALSE, "COL3" DOUBLE PRECISION DEFAULT 2.2, "COL4" BIGINT DEFAULT 3)ALTER TABLE "TEST" ADD CONSTRAINT "a8175238f007307f7bdfb531ee9943" PRIMARY KEY ("PK_COL")[][CREATE UNIQUE INDEX "bb260359a58a48fbfcb93859e498b7" ON "TEST" ("COL1")][]

This is kind of messy to look at in a log, and it is almost useless as input for another tool.

  • each translated statement (CREATE TABLE, CREATE INDEX, etc) should end with ; and start in a new line.
  • some of the translations are strings, some are lists, which results in those []; the translations should all result in strings.
    Maybe you could use something like
Stream.of(translator.translateCreateTable(this), ......)
        .filter(stmt -> !stmt.isEmpty())
        .collect(Collectors.joining(";\n"));

(each translate* that returns a list should first be converted into a string with String.join(";\n", list))
By following that, we get

CREATE TABLE "TEST" ("PK_COL" SERIAL, "COL1" INT DEFAULT 1, "COL2" BOOLEAN NOT NULL DEFAULT FALSE, "COL3" DOUBLE PRECISION DEFAULT 2.2, "COL4" BIGINT DEFAULT 3);
ALTER TABLE "TEST" ADD CONSTRAINT "a8175238f007307f7bdfb531ee9943" PRIMARY KEY ("PK_COL");
CREATE UNIQUE INDEX "bb260359a58a48fbfcb93859e498b7" ON "TEST" ("COL1")

}

/**
* Builder to create immutable {@link DbEntity} objects.
*/
Expand Down
11 changes: 11 additions & 0 deletions src/main/java/com/feedzai/commons/sql/abstraction/engine/AbstractDatabaseEngine.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -984,6 +984,17 @@ public String translate(final Expression query) {
return query.translate();
}

/**
* Translates the given entity creation to the current dialect.
*
* @param entity The entity to translate.
* @return The translation result.
*/
@Override
public String translateTableCreation(final DbEntity entity) {
return entity.translateEntityCreation(translator);
}

/**
* @return The dialect being in use.
*/
Expand Down
56 changes: 56 additions & 0 deletions src/main/java/com/feedzai/commons/sql/abstraction/engine/AbstractTranslator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

import com.feedzai.commons.sql.abstraction.ddl.AlterColumn;
import com.feedzai.commons.sql.abstraction.ddl.DbColumn;
import com.feedzai.commons.sql.abstraction.ddl.DbEntity;
import com.feedzai.commons.sql.abstraction.ddl.DropPrimaryKey;
import com.feedzai.commons.sql.abstraction.ddl.Rename;
import com.feedzai.commons.sql.abstraction.dml.Between;
Expand Down Expand Up @@ -44,6 +45,7 @@
import com.feedzai.commons.sql.abstraction.dml.dialect.SqlBuilder;
import com.feedzai.commons.sql.abstraction.engine.configuration.PdbProperties;
import com.google.common.base.Joiner;
import com.google.common.collect.ImmutableList;
francisco-polaco marked this conversation as resolved.
Show resolved Hide resolved
import com.google.inject.Inject;
import com.google.inject.Injector;
import org.apache.commons.lang3.StringUtils;
Expand Down Expand Up @@ -631,4 +633,58 @@ protected Union rowsToUnion(final List<Expression> rows) {
* @return The string representation of the given object.
*/
public abstract String translate(StringAgg stringAgg);

/**
* Translates the given entity table creation to the current dialect.
*
* @param entity The entity to translate.
* @return The create table translation result.
*/
public abstract String translateCreateTable(DbEntity entity);

/**
* Translates the primary key not null constraints of the given entity table to the current dialect.
*
* @param entity The entity to translate.
* @return The primary key not null constraints translation result.
*/
public String translatePrimaryKeysNotNull(DbEntity entity) {
// usually engines don't need to specify columns as not nulls to be PK.
return "";
}
Comment on lines +645 to +654
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When creating a table, the PRIMARY KEY columns should already be specified as NOT NULL if really needed.
H2 is doing it, and so should do DB2 and SQL Server; then this method won't be needed.

I suppose that the other engines that don't need this already assume that the PRIMARY KEY column is NOT NULL implicitly.


/**
* Translates the primary key constraints of the given entity table to the current dialect.
*
* @param entity The entity to translate.
* @return The primary key constraints translation result.
*/
public abstract String translatePrimaryKeysConstraints(DbEntity entity);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if the method translatePrimaryKeysNotNullis removed as suggested,
this one can be just translatePrimaryKeys


/**
* Translates the foreign key constraints of the given entity table to the current dialect.
*
* @param entity The entity to translate.
* @return The foreign key constraints translation result.
*/
public abstract List<String> translateForeignKey(DbEntity entity);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

translateForeignKeys
(plural)


/**
* Translates the index creation of the given entity table to the current dialect.
*
* @param entity The entity to translate.
* @return The index creation translation result.
*/
public abstract List<String> translateCreateIndexes(DbEntity entity);

/**
* Translates the sequence creation of the given entity table to the current dialect.
*
* @param entity The entity to translate.
* @return The sequence creation translation result.
*/
public List<String> translateCreateSequences(DbEntity entity) {
// the majority of engines don't need additional SQL to create sequences.
return ImmutableList.of();
francisco-polaco marked this conversation as resolved.
Show resolved Hide resolved
}
}
8 changes: 8 additions & 0 deletions src/main/java/com/feedzai/commons/sql/abstraction/engine/DatabaseEngine.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,14 @@ public interface DatabaseEngine extends AutoCloseable {
*/
String translate(final Expression query);

/**
* Translates the given entity creation to the current dialect.
*
* @param entity The entity to translate.
* @return The translation result.
*/
String translateTableCreation(final DbEntity entity);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a breaking change that will require classes that implement this interface
(e.g. wrappers for instrumentation) to implement the new method, otherwise compilation fails.

Let's avoid this, by turning this method into a default method that throws an UnsupportedOperationException or something like that.


/**
* Gets the dialect being used.
*
Expand Down
87 changes: 4 additions & 83 deletions src/main/java/com/feedzai/commons/sql/abstraction/engine/impl/CockroachDBEngine.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
package com.feedzai.commons.sql.abstraction.engine.impl;

import com.feedzai.commons.sql.abstraction.ddl.DbColumn;
import com.feedzai.commons.sql.abstraction.ddl.DbColumnConstraint;
import com.feedzai.commons.sql.abstraction.ddl.DbColumnType;
import com.feedzai.commons.sql.abstraction.ddl.DbEntity;
import com.feedzai.commons.sql.abstraction.engine.AbstractTranslator;
Expand All @@ -31,9 +30,7 @@
import java.sql.ResultSet;
import java.sql.SQLException;
import java.sql.Statement;
import java.util.ArrayList;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;

import static com.feedzai.commons.sql.abstraction.util.StringUtils.md5;
Expand Down Expand Up @@ -80,54 +77,7 @@ public CockroachDBEngine(final PdbProperties properties) throws DatabaseEngineEx
@Override
protected void createTable(final DbEntity entity) throws DatabaseEngineException {

List<String> createTable = new ArrayList<>();

createTable.add("CREATE TABLE");
createTable.add(quotize(entity.getName()));

// COLUMNS
List<String> columns = new ArrayList<>();
for (DbColumn c : entity.getColumns()) {
List<String> column = new ArrayList<>();
column.add(quotize(c.getName()));
column.add(translateType(c));

for (DbColumnConstraint cc : c.getColumnConstraints()) {
column.add(cc.translate());
}

if (c.isDefaultValueSet()) {
column.add("DEFAULT");
column.add(translate(c.getDefaultValue()));
}

columns.add(join(column, " "));
}
createTable.add("(" + join(columns, ", "));
// COLUMNS end


// PRIMARY KEY
List<String> pks = new ArrayList<>();
for (String pk : entity.getPkFields()) {
pks.add(quotize(pk));
}

if (!pks.isEmpty()) {
createTable.add(",");

final String pkName = md5(format("PK_%s", entity.getName()), properties.getMaxIdentifierSize());

createTable.add("CONSTRAINT");
createTable.add(quotize(pkName));
createTable.add("PRIMARY KEY");
createTable.add("(" + join(pks, ", ") + ")");
}
// PK end

createTable.add(")");

final String createTableStatement = join(createTable, " ");
final String createTableStatement = translator.translateCreateTable(entity);

logger.trace(createTableStatement);

Expand Down Expand Up @@ -189,37 +139,7 @@ protected void addFks(final DbEntity entity) throws DatabaseEngineException {

@Override
protected void addSequences(final DbEntity entity) throws DatabaseEngineException {
for (final DbColumn column : entity.getColumns()) {
if (!column.isAutoInc()) {
continue;
}

final String sequenceName = getQuotizedSequenceName(entity, column.getName());

final StringBuilder createSequence = new StringBuilder()
.append("CREATE SEQUENCE ")
.append(sequenceName)
.append(" MINVALUE 0 MAXVALUE ");
switch (column.getDbColumnType()) {
case INT:
createSequence.append(Integer.MAX_VALUE);
break;
case LONG:
createSequence.append(Long.MAX_VALUE);
break;
default:
throw new DatabaseEngineException("Auto incrementation is only supported on INT and LONG");
}
createSequence.append(" START 1 INCREMENT 1;");

createSequence.append("ALTER TABLE ")
.append(quotize(entity.getName()))
.append(" ALTER COLUMN ")
.append(quotize(column.getName()))
.append(" SET DEFAULT nextval('").append(sequenceName).append("')");

final String statement = createSequence.toString();

for (final String statement : translator.translateCreateSequences(entity)) {
logger.trace(statement);

Statement s = null;
Expand All @@ -228,7 +148,8 @@ protected void addSequences(final DbEntity entity) throws DatabaseEngineExceptio
s.executeUpdate(statement);
} catch (final SQLException ex) {
if (ex.getSQLState().equals(NAME_ALREADY_EXISTS)) {
logger.debug(dev, "Sequence {} is already defined", sequenceName);
logger.debug(dev, "Sequence is already defined");
logger.debug(dev, ex.getMessage());
handleOperation(
new OperationFault(entity.getName(), OperationFault.Type.SEQUENCE_ALREADY_EXISTS), ex
);
Expand Down
105 changes: 105 additions & 0 deletions src/main/java/com/feedzai/commons/sql/abstraction/engine/impl/CockroachDBTranslator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,23 @@
package com.feedzai.commons.sql.abstraction.engine.impl;

import com.feedzai.commons.sql.abstraction.ddl.DbColumn;
import com.feedzai.commons.sql.abstraction.ddl.DbColumnConstraint;
import com.feedzai.commons.sql.abstraction.ddl.DbEntity;
import com.feedzai.commons.sql.abstraction.dml.Cast;
import com.feedzai.commons.sql.abstraction.dml.Expression;
import com.feedzai.commons.sql.abstraction.dml.RepeatDelimiter;
import com.feedzai.commons.sql.abstraction.engine.DatabaseEngineException;
import com.feedzai.commons.sql.abstraction.engine.DatabaseEngineRuntimeException;
import com.feedzai.commons.sql.abstraction.engine.OperationNotSupportedRuntimeException;

import java.util.ArrayList;
import java.util.List;

import static com.feedzai.commons.sql.abstraction.engine.configuration.PdbProperties.VARCHAR_SIZE;
import static com.feedzai.commons.sql.abstraction.util.StringUtils.md5;
import static com.feedzai.commons.sql.abstraction.util.StringUtils.quotize;
import static java.lang.String.format;
import static org.apache.commons.lang3.StringUtils.join;

/**
* Provides SQL translation for CockroachDB.
Expand Down Expand Up @@ -126,4 +132,103 @@ public String translate(final RepeatDelimiter rd) {
return join(all, delimiter);
}
}

@Override
public String translateCreateTable(final DbEntity entity) {
final List<String> createTable = new ArrayList<>();

createTable.add("CREATE TABLE");
createTable.add(quotize(entity.getName()));

// COLUMNS
final List<String> columns = new ArrayList<>();
for (DbColumn c : entity.getColumns()) {
final List<String> column = new ArrayList<>();
column.add(quotize(c.getName()));
column.add(translate(c));

for (DbColumnConstraint cc : c.getColumnConstraints()) {
column.add(cc.translate());
}

if (c.isDefaultValueSet()) {
column.add("DEFAULT");
column.add(translate(c.getDefaultValue()));
}

columns.add(join(column, " "));
}
createTable.add("(" + join(columns, ", "));
// COLUMNS end


// PRIMARY KEY
final List<String> pks = new ArrayList<>();
for (String pk : entity.getPkFields()) {
pks.add(quotize(pk));
}

if (!pks.isEmpty()) {
createTable.add(",");

final String pkName = md5(format("PK_%s", entity.getName()), properties.getMaxIdentifierSize());

createTable.add("CONSTRAINT");
createTable.add(quotize(pkName));
createTable.add("PRIMARY KEY");
createTable.add("(" + join(pks, ", ") + ")");
}
// PK end

createTable.add(")");

return join(createTable, " ");
}

@Override
public String translatePrimaryKeysConstraints(final DbEntity entity) {
// primary keys are created on table creation.
return "";
}

@Override
public List<String> translateCreateIndexes(final DbEntity entity) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should take a look at the tests, they didn't fail with this change and they should...
You correctly copied the code from the CockroachDBEngine, but from addSequences:
this method should be called translateCreateSequences;
the addIndexes is inherited from PostgreSQL engine, and thus the translateCreateIndexes should also be inherited from PostgreSQL translator.

final List<String> createSeqs = new ArrayList<>();

for (final DbColumn column : entity.getColumns()) {
if (!column.isAutoInc()) {
continue;
}

final String sequenceName = quotize(md5(format("%s_%s_SEQ", entity.getName(), column.getName()),
properties.getMaxIdentifierSize()));

final StringBuilder createSequence = new StringBuilder()
.append("CREATE SEQUENCE ")
.append(sequenceName)
.append(" MINVALUE 0 MAXVALUE ");
switch (column.getDbColumnType()) {
case INT:
createSequence.append(Integer.MAX_VALUE);
break;
case LONG:
createSequence.append(Long.MAX_VALUE);
break;
default:
throw new DatabaseEngineRuntimeException("Auto incrementation is only supported on INT and LONG");
}
createSequence.append(" START 1 INCREMENT 1;");

createSequence.append("ALTER TABLE ")
.append(quotize(entity.getName()))
.append(" ALTER COLUMN ")
.append(quotize(column.getName()))
.append(" SET DEFAULT nextval('").append(sequenceName).append("')");

final String statement = createSequence.toString();

createSeqs.add(statement);
}
return createSeqs;
}
}
Loading