Repository: phoenix Updated Branches: refs/heads/3.0 49a5d037f -> 48a2482b4
PHOENIX-1266 Disallow setting NOT NULL constraint on non PK columns Project: http://git-wip-us.apache.org/repos/asf/phoenix/repo Commit: http://git-wip-us.apache.org/repos/asf/phoenix/commit/48a2482b Tree: http://git-wip-us.apache.org/repos/asf/phoenix/tree/48a2482b Diff: http://git-wip-us.apache.org/repos/asf/phoenix/diff/48a2482b Branch: refs/heads/3.0 Commit: 48a2482b46e3b4499b0e16382ea1d6641777d12f Parents: 49a5d03 Author: James Taylor <jtay...@salesforce.com> Authored: Sat Sep 20 21:50:12 2014 -0700 Committer: James Taylor <jtay...@salesforce.com> Committed: Sat Sep 20 21:55:09 2014 -0700 ---------------------------------------------------------------------- .../org/apache/phoenix/end2end/CreateTableIT.java | 14 ++++++++++++++ .../phoenix/end2end/TenantSpecificTablesDDLIT.java | 2 +- .../org/apache/phoenix/schema/MetaDataClient.java | 16 +++++++++------- .../java/org/apache/phoenix/schema/PColumnImpl.java | 8 ++++++-- .../apache/phoenix/compile/QueryCompilerTest.java | 2 +- .../java/org/apache/phoenix/query/BaseTest.java | 16 ++++++++-------- 6 files changed, 39 insertions(+), 19 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/phoenix/blob/48a2482b/phoenix-core/src/it/java/org/apache/phoenix/end2end/CreateTableIT.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/CreateTableIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/CreateTableIT.java index e28273e..1f9afc4 100644 --- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/CreateTableIT.java +++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/CreateTableIT.java @@ -350,4 +350,18 @@ public class CreateTableIT extends BaseClientManagedTimeIT { } } + @Test + public void testNotNullConstraintForWithSinglePKCol() throws Exception { + + String ddl = "create table test.testing(k integer primary key, v bigint not null)"; + + Properties props = new Properties(); + Connection conn = DriverManager.getConnection(getUrl(), props); + try { + conn.createStatement().execute(ddl); + fail(" Non pk column V has a NOT NULL constraint"); + } catch( SQLException sqle) { + assertEquals(SQLExceptionCode.INVALID_NOT_NULL_CONSTRAINT.getErrorCode(),sqle.getErrorCode()); + } + } } http://git-wip-us.apache.org/repos/asf/phoenix/blob/48a2482b/phoenix-core/src/it/java/org/apache/phoenix/end2end/TenantSpecificTablesDDLIT.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/TenantSpecificTablesDDLIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/TenantSpecificTablesDDLIT.java index 591efe1..89a67b5 100644 --- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/TenantSpecificTablesDDLIT.java +++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/TenantSpecificTablesDDLIT.java @@ -179,7 +179,7 @@ public class TenantSpecificTablesDDLIT extends BaseTenantSpecificTablesIT { public void testBaseTableWrongFormatWithTenantTypeId() throws Exception { // only two PK columns for multi_tenant, multi_type try { - createTestTable(getUrl(), "CREATE TABLE BASE_TABLE2 (TENANT_ID VARCHAR NOT NULL PRIMARY KEY, ID VARCHAR NOT NULL, A INTEGER) MULTI_TENANT=true", null, nextTimestamp()); + createTestTable(getUrl(), "CREATE TABLE BASE_TABLE2 (TENANT_ID VARCHAR NOT NULL PRIMARY KEY, ID VARCHAR, A INTEGER) MULTI_TENANT=true", null, nextTimestamp()); fail(); } catch (SQLException expected) { http://git-wip-us.apache.org/repos/asf/phoenix/blob/48a2482b/phoenix-core/src/main/java/org/apache/phoenix/schema/MetaDataClient.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/schema/MetaDataClient.java b/phoenix-core/src/main/java/org/apache/phoenix/schema/MetaDataClient.java index fee329d..9b744e1 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/schema/MetaDataClient.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/schema/MetaDataClient.java @@ -993,15 +993,17 @@ public class MetaDataClient { .setColumnName(colDef.getColumnDefName().getColumnName()).build().buildException(); } isPK = true; + } else { + // do not allow setting NOT-NULL constraint on non-primary columns. + if ( Boolean.FALSE.equals(colDef.isNull()) && + ( isPK || ( pkConstraint != null && !pkConstraint.contains(colDef.getColumnDefName())))) { + throw new SQLExceptionInfo.Builder(SQLExceptionCode.INVALID_NOT_NULL_CONSTRAINT) + .setSchemaName(schemaName) + .setTableName(tableName) + .setColumnName(colDef.getColumnDefName().getColumnName()).build().buildException(); + } } - // do not allow setting NOT-NULL constraint on non-primary columns. - if (!isPK && pkConstraint != null && !pkConstraint.contains(colDef.getColumnDefName())) { - if(Boolean.FALSE.equals(colDef.isNull())) { - throw new SQLExceptionInfo.Builder(SQLExceptionCode.INVALID_NOT_NULL_CONSTRAINT) - .setColumnName(colDef.getColumnDefName().getColumnName()).build().buildException(); - } - } PColumn column = newColumn(position++, colDef, pkConstraint, defaultFamilyName, false); if (SchemaUtil.isPKColumn(column)) { // TODO: remove this constraint? http://git-wip-us.apache.org/repos/asf/phoenix/blob/48a2482b/phoenix-core/src/main/java/org/apache/phoenix/schema/PColumnImpl.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/schema/PColumnImpl.java b/phoenix-core/src/main/java/org/apache/phoenix/schema/PColumnImpl.java index 8490d12..4901882 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/schema/PColumnImpl.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/schema/PColumnImpl.java @@ -132,7 +132,11 @@ public class PColumnImpl implements PColumn { @Override public boolean isNullable() { - return nullable; + // Only PK columns can be NOT NULL. We prevent this in the + // CREATE TABLE statement now (PHOENIX-1266), but this extra + // check for familyName != null will ensure that for existing + // tables we never treat key value columns as NOT NULL. + return nullable || familyName != null; } @Override @@ -231,4 +235,4 @@ public class PColumnImpl implements PColumn { public boolean isViewReferenced() { return isViewReferenced; } -} \ No newline at end of file +} http://git-wip-us.apache.org/repos/asf/phoenix/blob/48a2482b/phoenix-core/src/test/java/org/apache/phoenix/compile/QueryCompilerTest.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/test/java/org/apache/phoenix/compile/QueryCompilerTest.java b/phoenix-core/src/test/java/org/apache/phoenix/compile/QueryCompilerTest.java index 07564d4..cdc1805 100644 --- a/phoenix-core/src/test/java/org/apache/phoenix/compile/QueryCompilerTest.java +++ b/phoenix-core/src/test/java/org/apache/phoenix/compile/QueryCompilerTest.java @@ -208,7 +208,7 @@ public class QueryCompilerTest extends BaseConnectionlessQueryTest { statement.execute(); fail(); } catch (SQLException e) { - assertTrue(e.getMessage(), e.getMessage().contains("ERROR 517 (42895): Invalid not null constraint on non primary key column columnName=PK")); + assertTrue(e.getMessage(), e.getMessage().contains("ERROR 517 (42895): Invalid not null constraint on non primary key column columnName=FOO.PK")); } finally { conn.close(); } http://git-wip-us.apache.org/repos/asf/phoenix/blob/48a2482b/phoenix-core/src/test/java/org/apache/phoenix/query/BaseTest.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/test/java/org/apache/phoenix/query/BaseTest.java b/phoenix-core/src/test/java/org/apache/phoenix/query/BaseTest.java index fc0c154..62d8c47 100644 --- a/phoenix-core/src/test/java/org/apache/phoenix/query/BaseTest.java +++ b/phoenix-core/src/test/java/org/apache/phoenix/query/BaseTest.java @@ -401,21 +401,21 @@ public abstract class BaseTest { " (id varchar not null primary key, d DOUBLE, f FLOAT, ud UNSIGNED_DOUBLE, uf UNSIGNED_FLOAT, i integer, de decimal)"); builder.put(JOIN_ORDER_TABLE_FULL_NAME, "create table " + JOIN_ORDER_TABLE_FULL_NAME + " (\"order_id\" char(15) not null primary key, " + - " \"customer_id\" char(10) not null, " + - " \"item_id\" char(10) not null, " + - " price integer not null, " + - " quantity integer not null, " + - " date timestamp not null)"); + " \"customer_id\" char(10), " + + " \"item_id\" char(10), " + + " price integer, " + + " quantity integer, " + + " date timestamp)"); builder.put(JOIN_CUSTOMER_TABLE_FULL_NAME, "create table " + JOIN_CUSTOMER_TABLE_FULL_NAME + " (\"customer_id\" char(10) not null primary key, " + - " name varchar not null, " + + " name varchar, " + " phone char(12), " + " address varchar, " + " loc_id char(5), " + " date date)"); builder.put(JOIN_ITEM_TABLE_FULL_NAME, "create table " + JOIN_ITEM_TABLE_FULL_NAME + " (\"item_id\" char(10) not null primary key, " + - " name varchar not null, " + + " name varchar, " + " price integer, " + " discount1 integer, " + " discount2 integer, " + @@ -423,7 +423,7 @@ public abstract class BaseTest { " description varchar)"); builder.put(JOIN_SUPPLIER_TABLE_FULL_NAME, "create table " + JOIN_SUPPLIER_TABLE_FULL_NAME + " (\"supplier_id\" char(10) not null primary key, " + - " name varchar not null, " + + " name varchar, " + " phone char(12), " + " address varchar, " + " loc_id char(5))");
