Skip to content

Commit

Permalink
PHOENIX-3414 Validate DEFAULT when used in ALTER statement
Browse files Browse the repository at this point in the history
  • Loading branch information
@JamesRTaylor
JamesRTaylor committed Oct 27, 2016
1 parent 5744c6f commit 53ca288
Show file tree
Hide file tree
Showing 5 changed files with 210 additions and 58 deletions.
37 changes: 35 additions & 2 deletions phoenix-core/src/it/java/org/apache/phoenix/end2end/DefaultColumnValueIT.java
View file
Expand Up @@ -57,12 +57,12 @@ public void testDefaultColumnValue() throws Exception {
"pk2 INTEGER NOT NULL, " + "pk2 INTEGER NOT NULL, " +
"pk3 INTEGER NOT NULL DEFAULT 10, " + "pk3 INTEGER NOT NULL DEFAULT 10, " +
"test1 INTEGER, " + "test1 INTEGER, " +
"test2 INTEGER DEFAULT 5, " +
"test3 INTEGER, " +
"CONSTRAINT NAME_PK PRIMARY KEY (pk1, pk2, pk3))"; "CONSTRAINT NAME_PK PRIMARY KEY (pk1, pk2, pk3))";


Connection conn = DriverManager.getConnection(getUrl()); Connection conn = DriverManager.getConnection(getUrl());
conn.createStatement().execute(ddl); conn.createStatement().execute(ddl);
conn.createStatement().execute("ALTER TABLE " + sharedTable1 +
" ADD test2 INTEGER DEFAULT 5, est3 INTEGER");


String dml = "UPSERT INTO " + sharedTable1 + " VALUES (1, 2)"; String dml = "UPSERT INTO " + sharedTable1 + " VALUES (1, 2)";
conn.createStatement().execute(dml); conn.createStatement().execute(dml);
Expand Down Expand Up @@ -99,6 +99,39 @@ public void testDefaultColumnValue() throws Exception {
assertFalse(rs.next()); assertFalse(rs.next());
} }


@Test
public void testDefaultColumnValueOnView() throws Exception {
String ddl = "CREATE TABLE IF NOT EXISTS " + sharedTable1 + " (" +
"pk1 INTEGER NOT NULL, " +
"pk2 INTEGER NOT NULL, " +
"pk3 INTEGER NOT NULL DEFAULT 10, " +
"test1 INTEGER, " +
"test2 INTEGER DEFAULT 5, " +
"test3 INTEGER, " +
"CONSTRAINT NAME_PK PRIMARY KEY (pk1, pk2, pk3))";

Connection conn = DriverManager.getConnection(getUrl());
conn.createStatement().execute(ddl);
conn.createStatement().execute("CREATE VIEW " + sharedTable2 +
"(pk4 INTEGER NOT NULL DEFAULT 20 PRIMARY KEY, test4 VARCHAR DEFAULT 'foo') " +
"AS SELECT * FROM " + sharedTable1 + " WHERE pk1 = 1");

String dml = "UPSERT INTO " + sharedTable2 + "(pk2) VALUES (2)";
conn.createStatement().execute(dml);
conn.commit();

ResultSet rs = conn.createStatement()
.executeQuery("SELECT pk1,pk2,pk3,pk4,test2,test4 FROM " + sharedTable2);
assertTrue(rs.next());
assertEquals(1, rs.getInt(1));
assertEquals(2, rs.getInt(2));
assertEquals(10, rs.getInt(3));
assertEquals(20, rs.getInt(4));
assertEquals(5, rs.getInt(5));
assertEquals("foo", rs.getString(6));
assertFalse(rs.next());
}

@Test @Test
public void testDefaultColumnValueProjected() throws Exception { public void testDefaultColumnValueProjected() throws Exception {
String ddl = "CREATE TABLE IF NOT EXISTS " + sharedTable1 + " (" + String ddl = "CREATE TABLE IF NOT EXISTS " + sharedTable1 + " (" +
Expand Down
60 changes: 5 additions & 55 deletions phoenix-core/src/main/java/org/apache/phoenix/compile/CreateTableCompiler.java
View file
Expand Up @@ -54,8 +54,6 @@
import org.apache.phoenix.query.DelegateConnectionQueryServices; import org.apache.phoenix.query.DelegateConnectionQueryServices;
import org.apache.phoenix.query.QueryConstants; import org.apache.phoenix.query.QueryConstants;
import org.apache.phoenix.schema.ColumnRef; import org.apache.phoenix.schema.ColumnRef;
import org.apache.phoenix.schema.ConstraintViolationException;
import org.apache.phoenix.schema.DelegateSQLException;
import org.apache.phoenix.schema.MetaDataClient; import org.apache.phoenix.schema.MetaDataClient;
import org.apache.phoenix.schema.PDatum; import org.apache.phoenix.schema.PDatum;
import org.apache.phoenix.schema.PTable; import org.apache.phoenix.schema.PTable;
Expand All @@ -66,7 +64,6 @@
import org.apache.phoenix.schema.types.PDataType; import org.apache.phoenix.schema.types.PDataType;
import org.apache.phoenix.schema.types.PVarbinary; import org.apache.phoenix.schema.types.PVarbinary;
import org.apache.phoenix.util.ByteUtil; import org.apache.phoenix.util.ByteUtil;
import org.apache.phoenix.util.ExpressionUtil;
import org.apache.phoenix.util.QueryUtil; import org.apache.phoenix.util.QueryUtil;


import com.google.common.collect.Iterators; import com.google.common.collect.Iterators;
Expand Down Expand Up @@ -108,59 +105,12 @@ public MutationPlan compile(CreateTableStatement create) throws SQLException {
throw new SQLExceptionInfo.Builder(SQLExceptionCode.UNALLOWED_COLUMN_FAMILY) throw new SQLExceptionInfo.Builder(SQLExceptionCode.UNALLOWED_COLUMN_FAMILY)
.build().buildException(); .build().buildException();
} }
if (columnDef.getExpression() != null) { // False means we do not need the default (because it evaluated to null)
ExpressionCompiler compiler = new ExpressionCompiler(context); if (!columnDef.validateDefault(context, pkConstraint)) {
ParseNode defaultParseNode = if (overideColumnDefs == null) {
new SQLParser(columnDef.getExpression()).parseExpression(); overideColumnDefs = new ArrayList<>(columnDefs);
Expression defaultExpression = defaultParseNode.accept(compiler);
if (!defaultParseNode.isStateless()
|| defaultExpression.getDeterminism() != Determinism.ALWAYS) {
throw new SQLExceptionInfo.Builder(SQLExceptionCode.CANNOT_CREATE_DEFAULT)
.setColumnName(columnDef.getColumnDefName().getColumnName()).build()
.buildException();
}
if (columnDef.isRowTimestamp() || ( pkConstraint != null && pkConstraint.isColumnRowTimestamp(columnDef.getColumnDefName()))) {
throw new SQLExceptionInfo.Builder(
SQLExceptionCode.CANNOT_CREATE_DEFAULT_ROWTIMESTAMP)
.setColumnName(columnDef.getColumnDefName().getColumnName())
.build().buildException();
}
ImmutableBytesWritable ptr = new ImmutableBytesWritable();
// Evaluate the expression to confirm it's validity
LiteralExpression defaultValue = ExpressionUtil.getConstantExpression(defaultExpression, ptr);
// A DEFAULT that evaluates to null should be ignored as it adds nothing
if (defaultValue.getValue() == null) {
if (overideColumnDefs == null) {
overideColumnDefs = new ArrayList<>(columnDefs);
}
overideColumnDefs.set(i, new ColumnDef(columnDef, null));
continue;
}
PDataType sourceType = defaultExpression.getDataType();
PDataType targetType = columnDef.getDataType();
// Ensure that coercion works (will throw if not)
context.getTempPtr().set(ptr.get(), ptr.getOffset(), ptr.getLength());
try {
targetType.coerceBytes(context.getTempPtr(), defaultValue.getValue(), sourceType,
defaultValue.getMaxLength(), defaultValue.getScale(),
defaultValue.getSortOrder(),
columnDef.getMaxLength(), columnDef.getScale(),
columnDef.getSortOrder());
} catch (ConstraintViolationException e) {
if (e.getCause() instanceof SQLException) {
SQLException sqlE = (SQLException) e.getCause();
throw new DelegateSQLException(sqlE, ". DEFAULT " + SQLExceptionInfo.COLUMN_NAME + "=" + columnDef.getColumnDefName().getColumnName());
}
throw e;
}
if (!targetType.isSizeCompatible(ptr, defaultValue.getValue(), sourceType,
defaultValue.getMaxLength(), defaultValue.getScale(),
columnDef.getMaxLength(), columnDef.getScale())) {
throw new SQLExceptionInfo.Builder(
SQLExceptionCode.DATA_EXCEEDS_MAX_CAPACITY).setColumnName(columnDef.getColumnDefName().getColumnName())
.setMessage("DEFAULT " + columnDef.getExpression()).build()
.buildException();
} }
overideColumnDefs.set(i, new ColumnDef(columnDef, null));
} }
} }
if (overideColumnDefs != null) { if (overideColumnDefs != null) {
Expand Down
65 changes: 65 additions & 0 deletions phoenix-core/src/main/java/org/apache/phoenix/parse/ColumnDef.java
View file
Expand Up @@ -19,12 +19,21 @@


import java.sql.SQLException; import java.sql.SQLException;


import org.apache.hadoop.hbase.io.ImmutableBytesWritable;
import org.apache.phoenix.compile.ExpressionCompiler;
import org.apache.phoenix.compile.StatementContext;
import org.apache.phoenix.exception.SQLExceptionCode; import org.apache.phoenix.exception.SQLExceptionCode;
import org.apache.phoenix.exception.SQLExceptionInfo; import org.apache.phoenix.exception.SQLExceptionInfo;
import org.apache.phoenix.expression.Determinism;
import org.apache.phoenix.expression.Expression;
import org.apache.phoenix.expression.LiteralExpression;
import org.apache.phoenix.schema.ConstraintViolationException;
import org.apache.phoenix.schema.DelegateSQLException;
import org.apache.phoenix.schema.SortOrder; import org.apache.phoenix.schema.SortOrder;
import org.apache.phoenix.schema.types.PDataType; import org.apache.phoenix.schema.types.PDataType;
import org.apache.phoenix.schema.types.PDecimal; import org.apache.phoenix.schema.types.PDecimal;
import org.apache.phoenix.schema.types.PVarbinary; import org.apache.phoenix.schema.types.PVarbinary;
import org.apache.phoenix.util.ExpressionUtil;
import org.apache.phoenix.util.SchemaUtil; import org.apache.phoenix.util.SchemaUtil;


import com.google.common.base.Preconditions; import com.google.common.base.Preconditions;
Expand Down Expand Up @@ -225,4 +234,60 @@ public String toString() {
} }
return buf.toString(); return buf.toString();
} }

public boolean validateDefault(StatementContext context, PrimaryKeyConstraint pkConstraint) throws SQLException {
String defaultStr = this.getExpression();
if (defaultStr == null) {
return true;
}
ExpressionCompiler compiler = new ExpressionCompiler(context);
ParseNode defaultParseNode =
new SQLParser(this.getExpression()).parseExpression();
Expression defaultExpression = defaultParseNode.accept(compiler);
if (!defaultParseNode.isStateless()
|| defaultExpression.getDeterminism() != Determinism.ALWAYS) {
throw new SQLExceptionInfo.Builder(SQLExceptionCode.CANNOT_CREATE_DEFAULT)
.setColumnName(this.getColumnDefName().getColumnName()).build()
.buildException();
}
if (this.isRowTimestamp() || ( pkConstraint != null && pkConstraint.isColumnRowTimestamp(this.getColumnDefName()))) {
throw new SQLExceptionInfo.Builder(
SQLExceptionCode.CANNOT_CREATE_DEFAULT_ROWTIMESTAMP)
.setColumnName(this.getColumnDefName().getColumnName())
.build().buildException();
}
ImmutableBytesWritable ptr = new ImmutableBytesWritable();
// Evaluate the expression to confirm it's validity
LiteralExpression defaultValue = ExpressionUtil.getConstantExpression(defaultExpression, ptr);
// A DEFAULT that evaluates to null should be ignored as it adds nothing
if (defaultValue.getValue() == null) {
return false;
}
PDataType sourceType = defaultExpression.getDataType();
PDataType targetType = this.getDataType();
// Ensure that coercion works (will throw if not)
context.getTempPtr().set(ptr.get(), ptr.getOffset(), ptr.getLength());
try {
targetType.coerceBytes(context.getTempPtr(), defaultValue.getValue(), sourceType,
defaultValue.getMaxLength(), defaultValue.getScale(),
defaultValue.getSortOrder(),
this.getMaxLength(), this.getScale(),
this.getSortOrder());
} catch (ConstraintViolationException e) {
if (e.getCause() instanceof SQLException) {
SQLException sqlE = (SQLException) e.getCause();
throw new DelegateSQLException(sqlE, ". DEFAULT " + SQLExceptionInfo.COLUMN_NAME + "=" + this.getColumnDefName().getColumnName());
}
throw e;
}
if (!targetType.isSizeCompatible(ptr, defaultValue.getValue(), sourceType,
defaultValue.getMaxLength(), defaultValue.getScale(),
this.getMaxLength(), this.getScale())) {
throw new SQLExceptionInfo.Builder(
SQLExceptionCode.DATA_EXCEEDS_MAX_CAPACITY).setColumnName(this.getColumnDefName().getColumnName())
.setMessage("DEFAULT " + this.getExpression()).build()
.buildException();
}
return true;
}
} }
4 changes: 4 additions & 0 deletions phoenix-core/src/main/java/org/apache/phoenix/schema/MetaDataClient.java
View file
Expand Up @@ -2930,6 +2930,7 @@ public MutationState addColumn(PTable table, List<ColumnDef> origColumnDefs,
Set<String> colFamiliesForPColumnsToBeAdded = new LinkedHashSet<>(); Set<String> colFamiliesForPColumnsToBeAdded = new LinkedHashSet<>();
Set<String> families = new LinkedHashSet<>(); Set<String> families = new LinkedHashSet<>();
if (columnDefs.size() > 0 ) { if (columnDefs.size() > 0 ) {
StatementContext context = new StatementContext(new PhoenixStatement(connection), resolver);
try (PreparedStatement colUpsert = connection.prepareStatement(INSERT_COLUMN_ALTER_TABLE)) { try (PreparedStatement colUpsert = connection.prepareStatement(INSERT_COLUMN_ALTER_TABLE)) {
short nextKeySeq = SchemaUtil.getMaxKeySeq(table); short nextKeySeq = SchemaUtil.getMaxKeySeq(table);
for( ColumnDef colDef : columnDefs) { for( ColumnDef colDef : columnDefs) {
Expand All @@ -2949,6 +2950,9 @@ public MutationState addColumn(PTable table, List<ColumnDef> origColumnDefs,
throw new SQLExceptionInfo.Builder(SQLExceptionCode.ROWTIMESTAMP_CREATE_ONLY) throw new SQLExceptionInfo.Builder(SQLExceptionCode.ROWTIMESTAMP_CREATE_ONLY)
.setColumnName(colDef.getColumnDefName().getColumnName()).build().buildException(); .setColumnName(colDef.getColumnDefName().getColumnName()).build().buildException();
} }
if (!colDef.validateDefault(context, null)) {
colDef = new ColumnDef(colDef, null); // Remove DEFAULT as it's not necessary
}
PColumn column = newColumn(position++, colDef, PrimaryKeyConstraint.EMPTY, table.getDefaultFamilyName() == null ? null : table.getDefaultFamilyName().getString(), true); PColumn column = newColumn(position++, colDef, PrimaryKeyConstraint.EMPTY, table.getDefaultFamilyName() == null ? null : table.getDefaultFamilyName().getString(), true);
columns.add(column); columns.add(column);
String pkName = null; String pkName = null;
Expand Down
102 changes: 101 additions & 1 deletion phoenix-core/src/test/java/org/apache/phoenix/compile/QueryCompilerTest.java
View file
Expand Up @@ -2483,6 +2483,23 @@ public void testStatefulDefault() throws Exception {
} }
} }


@Test
public void testAlterTableStatefulDefault() throws Exception {
String ddl = "CREATE TABLE table_with_default (" +
"pk INTEGER PRIMARY KEY)";
String ddl2 = "ALTER TABLE table_with_default " +
"ADD datecol DATE DEFAULT CURRENT_DATE()";

Connection conn = DriverManager.getConnection(getUrl());
conn.createStatement().execute(ddl);
try {
conn.createStatement().execute(ddl2);
fail();
} catch (SQLException e) {
assertEquals(SQLExceptionCode.CANNOT_CREATE_DEFAULT.getErrorCode(), e.getErrorCode());
}
}

@Test @Test
public void testDefaultTypeMismatch() throws Exception { public void testDefaultTypeMismatch() throws Exception {
String ddl = "CREATE TABLE table_with_default (" + String ddl = "CREATE TABLE table_with_default (" +
Expand All @@ -2499,7 +2516,41 @@ public void testDefaultTypeMismatch() throws Exception {
} }


@Test @Test
public void testDefaultRowTimestamp() throws Exception { public void testAlterTableDefaultTypeMismatch() throws Exception {
String ddl = "CREATE TABLE table_with_default (" +
"pk INTEGER PRIMARY KEY)";
String ddl2 = "ALTER TABLE table_with_default " +
"ADD v CHAR(3) DEFAULT 1";

Connection conn = DriverManager.getConnection(getUrl());
conn.createStatement().execute(ddl);
try {
conn.createStatement().execute(ddl2);
fail();
} catch (SQLException e) {
assertEquals(SQLExceptionCode.TYPE_MISMATCH.getErrorCode(), e.getErrorCode());
}
}

@Test
public void testDefaultTypeMismatchInView() throws Exception {
String ddl1 = "CREATE TABLE table_with_default (" +
"pk INTEGER PRIMARY KEY, " +
"v VARCHAR DEFAULT 'foo')";
String ddl2 = "CREATE VIEW my_view(v2 VARCHAR DEFAULT 1) AS SELECT * FROM table_with_default";

Connection conn = DriverManager.getConnection(getUrl());
conn.createStatement().execute(ddl1);
try {
conn.createStatement().execute(ddl2);
fail();
} catch (SQLException e) {
assertEquals(SQLExceptionCode.TYPE_MISMATCH.getErrorCode(), e.getErrorCode());
}
}

@Test
public void testDefaultRowTimestamp1() throws Exception {
String ddl = "CREATE TABLE IF NOT EXISTS table_with_defaults (" String ddl = "CREATE TABLE IF NOT EXISTS table_with_defaults ("
+ "pk1 INTEGER NOT NULL," + "pk1 INTEGER NOT NULL,"
+ "pk2 BIGINT NOT NULL DEFAULT 5," + "pk2 BIGINT NOT NULL DEFAULT 5,"
Expand All @@ -2517,6 +2568,23 @@ public void testDefaultRowTimestamp() throws Exception {
} }
} }


@Test
public void testDefaultRowTimestamp2() throws Exception {
String ddl = "CREATE TABLE table_with_defaults ("
+ "k BIGINT DEFAULT 5 PRIMARY KEY ROW_TIMESTAMP)";

Connection conn = DriverManager.getConnection(getUrl());

try {
conn.createStatement().execute(ddl);
fail();
} catch (SQLException e) {
assertEquals(
SQLExceptionCode.CANNOT_CREATE_DEFAULT_ROWTIMESTAMP.getErrorCode(),
e.getErrorCode());
}
}

@Test @Test
public void testDefaultSizeMismatch() throws Exception { public void testDefaultSizeMismatch() throws Exception {
String ddl = "CREATE TABLE table_with_default (" + String ddl = "CREATE TABLE table_with_default (" +
Expand All @@ -2532,6 +2600,23 @@ public void testDefaultSizeMismatch() throws Exception {
} }
} }


@Test
public void testAlterTableDefaultSizeMismatch() throws Exception {
String ddl = "CREATE TABLE table_with_default (" +
"pk INTEGER PRIMARY KEY)";
String ddl2 = "ALTER TABLE table_with_default " +
"ADD v CHAR(3) DEFAULT 'foobar'";

Connection conn = DriverManager.getConnection(getUrl());
conn.createStatement().execute(ddl);
try {
conn.createStatement().execute(ddl2);
fail();
} catch (SQLException e) {
assertEquals(SQLExceptionCode.DATA_EXCEEDS_MAX_CAPACITY.getErrorCode(), e.getErrorCode());
}
}

@Test @Test
public void testNullDefaultRemoved() throws Exception { public void testNullDefaultRemoved() throws Exception {
String ddl = "CREATE TABLE table_with_default (" + String ddl = "CREATE TABLE table_with_default (" +
Expand All @@ -2545,6 +2630,21 @@ public void testNullDefaultRemoved() throws Exception {
assertNull(table.getColumn("V").getExpressionStr()); assertNull(table.getColumn("V").getExpressionStr());
} }


@Test
public void testNullAlterTableDefaultRemoved() throws Exception {
String ddl = "CREATE TABLE table_with_default (" +
"pk INTEGER PRIMARY KEY)";
String ddl2 = "ALTER TABLE table_with_default " +
"ADD v CHAR(3) DEFAULT null";

Connection conn = DriverManager.getConnection(getUrl());
conn.createStatement().execute(ddl);
conn.createStatement().execute(ddl2);
PTable table = conn.unwrap(PhoenixConnection.class).getMetaDataCache()
.getTableRef(new PTableKey(null,"TABLE_WITH_DEFAULT")).getTable();
assertNull(table.getColumn("V").getExpressionStr());
}

@Test @Test
public void testIndexOnViewWithChildView() throws SQLException { public void testIndexOnViewWithChildView() throws SQLException {
try (Connection conn = DriverManager.getConnection(getUrl())) { try (Connection conn = DriverManager.getConnection(getUrl())) {
Expand Down

0 comments on commit 53ca288

Please sign in to comment.