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.

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

Fix/ctas oracle column definitions #237

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,11 @@ public T fields(AliasedFieldBuilder... fields) {
return fields(Arrays.asList(fields));
}

public T withFields(Iterable<? extends AliasedFieldBuilder> fields) {
this.fields.clear();
Iterables.addAll(this.fields, Builder.Helper.buildAll(fields));
return castToChild(this);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Rename to replaceFields()


/**
* Selects fields from a specific table:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1105,6 +1105,17 @@ protected List<String> expectedAddTableFromStatements() {
"INSERT INTO TESTSCHEMA.SomeTable SELECT someField, otherField FROM TESTSCHEMA.OtherTable"
);
}
/**
* @see org.alfasoftware.morf.jdbc.AbstractSqlDialectTest#expectedAddTableFromStatementsNullValue()
*/
@Override
protected List<String> expectedAddTableFromStatementsNullValue() {
return ImmutableList.of(
"CREATE TABLE TESTSCHEMA.SomeTable (someField VARCHAR(3) NOT NULL, otherField DECIMAL(3,0) NOT NULL, nullField VARCHAR(3) NOT NULL, CONSTRAINT SomeTable_PK PRIMARY KEY (someField))",
"CREATE INDEX SomeTable_1 ON TESTSCHEMA.SomeTable (otherField)",
"INSERT INTO TESTSCHEMA.SomeTable SELECT someField, otherField, null FROM TESTSCHEMA.OtherTable"
);
}


/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1168,6 +1168,18 @@ protected List<String> expectedAddTableFromStatements() {
);
}

/**
* @see org.alfasoftware.morf.jdbc.AbstractSqlDialectTest#expectedAddTableFromStatementsNullValue()
*/
@Override
protected List<String> expectedAddTableFromStatementsNullValue() {
return ImmutableList.of(
"CREATE TABLE `SomeTable` (`someField` VARCHAR(3) NOT NULL, `otherField` DECIMAL(3,0) NOT NULL, `nullField` VARCHAR(3) NOT NULL, CONSTRAINT `SomeTable_PK` PRIMARY KEY (`someField`)) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_bin",
"ALTER TABLE `SomeTable` ADD INDEX `SomeTable_1` (`otherField`)",
"INSERT INTO SomeTable SELECT someField, otherField, null FROM OtherTable"
);
}


/**
* We only support {@link SelectStatement#useImplicitJoinOrder()}, and only to a limited extent.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1402,6 +1402,19 @@ protected List<String> expectedAddTableFromStatements() {
);
}

/**
* @see org.alfasoftware.morf.jdbc.AbstractSqlDialectTest#expectedAddTableFromStatementsNullValue()
*/
@Override
protected List<String> expectedAddTableFromStatementsNullValue() {
return ImmutableList.of(
"CREATE TABLE SCM.SomeTable (someField VARCHAR(3) NOT NULL, otherField DECIMAL(3,0) NOT NULL, nullField VARCHAR(3) NOT NULL, PRIMARY KEY (someField))",
"DROP INDEX IF EXISTS SCM.SomeTable_1",
"CREATE INDEX SomeTable_1 ON SCM.SomeTable (otherField)",
"INSERT INTO SCM.SomeTable SELECT someField, otherField, null FROM SCM.OtherTable"
);
}


/**
* @see org.alfasoftware.morf.jdbc.AbstractSqlDialectTest#expectedHints1(int)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@
import org.alfasoftware.morf.sql.element.ConcatenatedField;
import org.alfasoftware.morf.sql.element.FieldReference;
import org.alfasoftware.morf.sql.element.Function;
import org.alfasoftware.morf.sql.element.NullFieldLiteral;
import org.alfasoftware.morf.sql.element.SqlParameter;
import org.alfasoftware.morf.sql.element.TableReference;
import org.apache.commons.lang3.StringUtils;
Expand Down Expand Up @@ -96,6 +97,8 @@ class OracleDialect extends SqlDialect {
*/
public static final String NULLS_LAST = "NULLS LAST";

private static final String CANNOT_CONVERT_NULL_STATEMENT_TO_SQL = "Cannot convert a null statement to SQL";

/**
* Database platforms may order nulls first or last. My SQL always orders nulls first, Oracle defaults to ordering nulls last.
* Fortunately on Oracle it is possible to specify that nulls should be ordered first.
Expand Down Expand Up @@ -1105,11 +1108,26 @@ public Collection<String> renameTableStatements(Table fromTable, Table toTable)
*/
@Override
public Collection<String> addTableFromStatements(Table table, SelectStatement selectStatement) {
List<AliasedField> fieldsToInclude = new ArrayList<>();

for(AliasedField field: selectStatement.getFields()) {
if (field instanceof NullFieldLiteral || field instanceof FieldReference) {
Column column = table.columns().get(selectStatement.getFields().indexOf(field));
fieldsToInclude.add(new Cast(
field,
column.getType(),
column.getWidth(),
column.getScale()).as(column.getName()));
} else {
fieldsToInclude.add(field);
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

if (field instanceof NullFieldLiteral || field instanceof FieldReference)

I think we want to handle all fields, including expressions, etc.
So, basically, there should be no else in there.


Builder<String> result = ImmutableList.<String>builder();
result.add(new StringBuilder()
.append(createTableStatement(table, true))
.append(" AS ")
.append(convertStatementToSQL(selectStatement))
.append(convertStatementToSQL(selectStatement.shallowCopy().withFields(fieldsToInclude).build()))
.toString()
);
result.add("ALTER TABLE " + schemaNamePrefix() + table.getName() + " NOPARALLEL LOGGING");
Expand All @@ -1123,7 +1141,6 @@ public Collection<String> addTableFromStatements(Table table, SelectStatement se
return result.build();
}


/**
* Builds the remaining statements (triggers, sequences and comments).
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@
package org.alfasoftware.morf.jdbc.oracle;

import static org.alfasoftware.morf.jdbc.oracle.OracleDialect.NULLS_LAST;
import static org.alfasoftware.morf.sql.SqlUtils.parameter;
import static org.alfasoftware.morf.metadata.SchemaUtils.*;
import static org.alfasoftware.morf.sql.SqlUtils.*;
import static org.alfasoftware.morf.sql.element.Direction.ASCENDING;
import static org.hamcrest.Matchers.contains;
import static org.junit.Assert.assertEquals;
Expand All @@ -43,10 +44,13 @@
import org.alfasoftware.morf.jdbc.SqlScriptExecutor;
import org.alfasoftware.morf.metadata.DataType;
import org.alfasoftware.morf.metadata.SchemaUtils;
import org.alfasoftware.morf.metadata.Table;
import org.alfasoftware.morf.sql.CustomHint;
import org.alfasoftware.morf.sql.OracleCustomHint;
import org.alfasoftware.morf.sql.SelectStatement;
import org.alfasoftware.morf.sql.element.Direction;
import org.alfasoftware.morf.sql.element.SqlParameter;
import org.junit.Test;
import org.mockito.ArgumentCaptor;

import com.google.common.base.Strings;
Expand Down Expand Up @@ -1452,7 +1456,7 @@ protected String expectedSelectLiteralWithWhereClauseString() {
@Override
public List<String> expectedAddTableFromStatements() {
return ImmutableList.of(
"CREATE TABLE TESTSCHEMA.SomeTable (someField NOT NULL, otherField NOT NULL, CONSTRAINT SomeTable_PK PRIMARY KEY (someField) USING INDEX (CREATE UNIQUE INDEX TESTSCHEMA.SomeTable_PK ON TESTSCHEMA.SomeTable (someField))) PARALLEL NOLOGGING AS SELECT someField, otherField FROM TESTSCHEMA.OtherTable",
"CREATE TABLE TESTSCHEMA.SomeTable (someField NOT NULL, otherField NOT NULL, CONSTRAINT SomeTable_PK PRIMARY KEY (someField) USING INDEX (CREATE UNIQUE INDEX TESTSCHEMA.SomeTable_PK ON TESTSCHEMA.SomeTable (someField))) PARALLEL NOLOGGING AS SELECT CAST(someField AS NVARCHAR2(3)) AS someField, CAST(otherField AS DECIMAL(3,0)) AS otherField FROM TESTSCHEMA.OtherTable",
"ALTER TABLE TESTSCHEMA.SomeTable NOPARALLEL LOGGING",
"ALTER INDEX TESTSCHEMA.SomeTable_PK NOPARALLEL LOGGING",
"COMMENT ON TABLE TESTSCHEMA.SomeTable IS '"+OracleDialect.REAL_NAME_COMMENT_LABEL+":[SomeTable]'",
Expand All @@ -1461,6 +1465,22 @@ public List<String> expectedAddTableFromStatements() {
);
}

/**
* @see org.alfasoftware.morf.jdbc.AbstractSqlDialectTest#expectedAddTableFromStatementsNullValue()
*/
@Override
public List<String> expectedAddTableFromStatementsNullValue() {
return ImmutableList.of(
"CREATE TABLE TESTSCHEMA.SomeTable (someField NOT NULL, otherField NOT NULL, nullField NOT NULL, CONSTRAINT SomeTable_PK PRIMARY KEY (someField) USING INDEX (CREATE UNIQUE INDEX TESTSCHEMA.SomeTable_PK ON TESTSCHEMA.SomeTable (someField))) PARALLEL NOLOGGING AS SELECT CAST(someField AS NVARCHAR2(3)) AS someField, CAST(otherField AS DECIMAL(3,0)) AS otherField, CAST(null AS NVARCHAR2(3)) AS nullField FROM TESTSCHEMA.OtherTable",
"ALTER TABLE TESTSCHEMA.SomeTable NOPARALLEL LOGGING",
"ALTER INDEX TESTSCHEMA.SomeTable_PK NOPARALLEL LOGGING",
"COMMENT ON TABLE TESTSCHEMA.SomeTable IS 'REALNAME:[SomeTable]'",
"COMMENT ON COLUMN TESTSCHEMA.SomeTable.someField IS 'REALNAME:[someField]/TYPE:[STRING]'",
"COMMENT ON COLUMN TESTSCHEMA.SomeTable.otherField IS 'REALNAME:[otherField]/TYPE:[DECIMAL]'",
"COMMENT ON COLUMN TESTSCHEMA.SomeTable.nullField IS 'REALNAME:[nullField]/TYPE:[STRING]'"
);
}

/**
* @see org.alfasoftware.morf.jdbc.AbstractSqlDialectTest#expectedHints1(int)
*/
Expand Down
Copy link
Contributor

Choose a reason for hiding this comment

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

Was this not possible to add as tests for all dialects?
Any advantage to have this as a separate test class?

Original file line number Diff line number Diff line change
@@ -0,0 +1,188 @@
package org.alfasoftware.morf.jdbc.oracle;

import org.alfasoftware.morf.metadata.DataType;
import org.alfasoftware.morf.metadata.Table;
import org.alfasoftware.morf.sql.SelectStatement;
import org.junit.Test;

import static org.alfasoftware.morf.metadata.SchemaUtils.*;
import static org.alfasoftware.morf.sql.SqlUtils.*;
import static org.junit.Assert.assertEquals;

public class TestOracleDialectBespokeFunctionality {



@Test
public void testConvertStatementToSQL() {
Table table = table("SomeTable")
.columns(
column("someField", DataType.STRING, 3).primaryKey(),
column("otherField", DataType.DECIMAL, 3),
column("nullField", DataType.STRING, 3)
).indexes(
index("SomeTable_1").columns("otherField")
);
Copy link
Contributor

Choose a reason for hiding this comment

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

Notice that this index is not present in the resulting SQL (which is correct and by design).
Indexes cannot be part of a CTAS, which is documented at SchemaEditor.addTableFrom(Table, SelectStatement).


SelectStatement selectStatement = select(field("someField"), field("otherField"), nullLiteral()).from(tableRef("OtherTable"));

OracleDialect oracleDialect = new OracleDialect("TEST");
String result = oracleDialect.addTableFromStatements(table, selectStatement).toString();

String expectedResult = "[CREATE TABLE TEST.SomeTable (" +
"someField NOT NULL, " +
"otherField NOT NULL, " +
"nullField NOT NULL, " +
"CONSTRAINT SomeTable_PK PRIMARY KEY (someField) " +
"USING INDEX (CREATE UNIQUE INDEX TEST.SomeTable_PK ON TEST.SomeTable (someField))) " +
"PARALLEL NOLOGGING " +
"AS SELECT " +
"CAST(someField AS NVARCHAR2(3)) AS someField, " +
"CAST(otherField AS DECIMAL(3,0)) AS otherField, " +
"CAST(null AS NVARCHAR2(3)) AS nullField " +
"FROM TEST.OtherTable, " +
"ALTER TABLE TEST.SomeTable NOPARALLEL LOGGING, " +
"ALTER INDEX TEST.SomeTable_PK NOPARALLEL LOGGING, " +
"COMMENT ON TABLE TEST.SomeTable IS 'REALNAME:[SomeTable]', " +
"COMMENT ON COLUMN TEST.SomeTable.someField IS 'REALNAME:[someField]/TYPE:[STRING]', " +
"COMMENT ON COLUMN TEST.SomeTable.otherField IS 'REALNAME:[otherField]/TYPE:[DECIMAL]', " +
"COMMENT ON COLUMN TEST.SomeTable.nullField IS 'REALNAME:[nullField]/TYPE:[STRING]']";
assertEquals(expectedResult, result);
}

@Test
public void testDistinctStatement() {
OracleDialect oracleDialect = new OracleDialect("TEST");

Table table = table("SomeTable")
.columns(
column("someField", DataType.STRING, 3).primaryKey(),
column("otherField", DataType.DECIMAL, 3)
).indexes(
index("SomeTable_1").columns("otherField")
);

SelectStatement selectStatement = select(field("someField"), field("otherField")).from(tableRef("OtherTable"));
SelectStatement distinctSelectStatement = selectStatement.shallowCopy().distinct().build();

String result = oracleDialect.addTableFromStatements(table, distinctSelectStatement).toString();

String expectedResult = "[CREATE TABLE TEST.SomeTable (" +
"someField NOT NULL, " +
"otherField NOT NULL, " +
"CONSTRAINT SomeTable_PK PRIMARY KEY (someField) " +
"USING INDEX (CREATE UNIQUE INDEX TEST.SomeTable_PK ON TEST.SomeTable (someField))) " +
"PARALLEL NOLOGGING " +
"AS SELECT DISTINCT " +
"CAST(someField AS NVARCHAR2(3)) AS someField, " +
"CAST(otherField AS DECIMAL(3,0)) AS otherField FROM TEST.OtherTable, " +
"ALTER TABLE TEST.SomeTable NOPARALLEL LOGGING, " +
"ALTER INDEX TEST.SomeTable_PK NOPARALLEL LOGGING, " +
"COMMENT ON TABLE TEST.SomeTable IS 'REALNAME:[SomeTable]', " +
"COMMENT ON COLUMN TEST.SomeTable.someField IS 'REALNAME:[someField]/TYPE:[STRING]', " +
"COMMENT ON COLUMN TEST.SomeTable.otherField IS 'REALNAME:[otherField]/TYPE:[DECIMAL]']";
assertEquals(expectedResult, result);
}


@Test
public void testAllStatement() {
OracleDialect oracleDialect = new OracleDialect("TEST");

Table table = table("SomeTable")
.columns(
column("someField", DataType.STRING, 3).primaryKey(),
column("otherField", DataType.DECIMAL, 3)
).indexes(
index("SomeTable_1").columns("otherField")
);

SelectStatement selectStatement = select().from(tableRef("OtherTable"));
SelectStatement distinctSelectStatement = selectStatement.shallowCopy().distinct().build();

String result = oracleDialect.addTableFromStatements(table, distinctSelectStatement).toString();

String expectedResult = "[CREATE TABLE TEST.SomeTable (" +
"someField NOT NULL, " +
"otherField NOT NULL, " +
"CONSTRAINT SomeTable_PK PRIMARY KEY (someField) " +
"USING INDEX (CREATE UNIQUE INDEX TEST.SomeTable_PK ON TEST.SomeTable (someField))) " +
"PARALLEL NOLOGGING " +
"AS SELECT DISTINCT * FROM TEST.OtherTable, " +
"ALTER TABLE TEST.SomeTable NOPARALLEL LOGGING, " +
"ALTER INDEX TEST.SomeTable_PK NOPARALLEL LOGGING, " +
"COMMENT ON TABLE TEST.SomeTable IS 'REALNAME:[SomeTable]', " +
"COMMENT ON COLUMN TEST.SomeTable.someField IS 'REALNAME:[someField]/TYPE:[STRING]', " +
"COMMENT ON COLUMN TEST.SomeTable.otherField IS 'REALNAME:[otherField]/TYPE:[DECIMAL]']";
assertEquals(expectedResult, result);
}

@Test
public void testUpdateStatement() {
OracleDialect oracleDialect = new OracleDialect("TEST");

Table table = table("SomeTable")
.columns(
column("someField", DataType.STRING, 3).primaryKey(),
column("otherField", DataType.DECIMAL, 3)
).indexes(
index("SomeTable_1").columns("otherField")
);

SelectStatement selectStatement = select().from(tableRef("OtherTable"));
SelectStatement updateSelectStatement = selectStatement.shallowCopy().forUpdate().build();

String result = oracleDialect.addTableFromStatements(table, updateSelectStatement).toString();

String expectedResult = "[CREATE TABLE TEST.SomeTable (" +
"someField NOT NULL, " +
"otherField NOT NULL, " +
"CONSTRAINT SomeTable_PK PRIMARY KEY (someField) " +
"USING INDEX (CREATE UNIQUE INDEX TEST.SomeTable_PK ON TEST.SomeTable (someField))) " +
"PARALLEL NOLOGGING " +
"AS SELECT * FROM TEST.OtherTable " +
"FOR UPDATE, " +
"ALTER TABLE TEST.SomeTable NOPARALLEL LOGGING, " +
"ALTER INDEX TEST.SomeTable_PK NOPARALLEL LOGGING, " +
"COMMENT ON TABLE TEST.SomeTable IS 'REALNAME:[SomeTable]', " +
"COMMENT ON COLUMN TEST.SomeTable.someField IS 'REALNAME:[someField]/TYPE:[STRING]', " +
"COMMENT ON COLUMN TEST.SomeTable.otherField IS 'REALNAME:[otherField]/TYPE:[DECIMAL]']";
assertEquals(expectedResult, result);
}

@Test(expected = IllegalArgumentException.class)
public void testUpdateForDistinctStatement() {
OracleDialect oracleDialect = new OracleDialect("TEST");

Table table = table("SomeTable")
.columns(
column("someField", DataType.STRING, 3).primaryKey(),
column("otherField", DataType.DECIMAL, 3)
).indexes(
index("SomeTable_1").columns("otherField")
);

SelectStatement selectStatement = select().from(tableRef("OtherTable"));
SelectStatement distinctSelectStatement = selectStatement.shallowCopy().forUpdate().distinct().build();

String result = oracleDialect.addTableFromStatements(table, distinctSelectStatement).toString();
}

@Test(expected = IllegalArgumentException.class)
public void testIllegalArgumentExeptionIsThrown() {
OracleDialect oracleDialect = new OracleDialect("TEST");

Table table = table("SomeTable")
.columns(
column("someField", DataType.STRING, 3).primaryKey(),
column("otherField", DataType.DECIMAL, 3),
column("nullField", DataType.STRING, 3)
).indexes(
index("SomeTable_1").columns("otherField")
);

oracleDialect.convertStatementToSQL((SelectStatement) null);
}


}
Original file line number Diff line number Diff line change
Expand Up @@ -1181,6 +1181,23 @@ protected List<String> expectedAddTableFromStatements() {
);
}

/**
* @see org.alfasoftware.morf.jdbc.AbstractSqlDialectTest#expectedAddTableFromStatementsNullValue()
*/
@Override
protected List<String> expectedAddTableFromStatementsNullValue() {
return ImmutableList.of(
"CREATE TABLE testschema.SomeTable (someField VARCHAR(3) COLLATE \"POSIX\" NOT NULL, otherField DECIMAL(3,0) NOT NULL, nullField VARCHAR(3) COLLATE \"POSIX\" NOT NULL, CONSTRAINT SomeTable_PK PRIMARY KEY(someField))",
"COMMENT ON TABLE testschema.SomeTable IS '"+PostgreSQLDialect.REAL_NAME_COMMENT_LABEL+":[SomeTable]'",
"COMMENT ON COLUMN testschema.SomeTable.someField IS '"+PostgreSQLDialect.REAL_NAME_COMMENT_LABEL+":[someField]/TYPE:[STRING]'",
"COMMENT ON COLUMN testschema.SomeTable.otherField IS '"+PostgreSQLDialect.REAL_NAME_COMMENT_LABEL+":[otherField]/TYPE:[DECIMAL]'",
"COMMENT ON COLUMN testschema.SomeTable.nullField IS 'REALNAME:[nullField]/TYPE:[STRING]'",
"CREATE INDEX SomeTable_1 ON testschema.SomeTable (otherField)",
"COMMENT ON INDEX SomeTable_1 IS '"+PostgreSQLDialect.REAL_NAME_COMMENT_LABEL+":[SomeTable_1]'",
"INSERT INTO testschema.SomeTable SELECT someField, otherField, null FROM testschema.OtherTable"
);
}


/**
* @see org.alfasoftware.morf.jdbc.AbstractSqlDialectTest#expectedHints1(int)
Expand Down
Loading