Repository: phoenix
Updated Branches:
  refs/heads/4.x-HBase-1.3 25ae2568e -> e99b738b6


PHOENIX-2566 Support NOT NULL constraint for any column for immutable table


Project: http://git-wip-us.apache.org/repos/asf/phoenix/repo
Commit: http://git-wip-us.apache.org/repos/asf/phoenix/commit/e99b738b
Tree: http://git-wip-us.apache.org/repos/asf/phoenix/tree/e99b738b
Diff: http://git-wip-us.apache.org/repos/asf/phoenix/diff/e99b738b

Branch: refs/heads/4.x-HBase-1.3
Commit: e99b738b6b6e2d7487dd46e6374d2ce22a164869
Parents: 25ae256
Author: James Taylor <jtay...@salesforce.com>
Authored: Tue Feb 13 23:14:58 2018 -0800
Committer: James Taylor <jtay...@salesforce.com>
Committed: Wed Feb 14 19:26:12 2018 -0800

----------------------------------------------------------------------
 .../apache/phoenix/end2end/AlterTableIT.java    |  2 +-
 .../apache/phoenix/end2end/CreateTableIT.java   |  5 +-
 .../apache/phoenix/compile/UpsertCompiler.java  | 38 ++++----
 .../phoenix/exception/SQLExceptionCode.java     |  5 +-
 .../apache/phoenix/schema/MetaDataClient.java   | 36 +++----
 .../org/apache/phoenix/schema/PColumnImpl.java  |  6 +-
 .../org/apache/phoenix/schema/PTableImpl.java   |  1 -
 .../org/apache/phoenix/util/SchemaUtil.java     |  6 ++
 .../phoenix/compile/QueryCompilerTest.java      | 98 +++++++++++++++++++-
 9 files changed, 149 insertions(+), 48 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/phoenix/blob/e99b738b/phoenix-core/src/it/java/org/apache/phoenix/end2end/AlterTableIT.java
----------------------------------------------------------------------
diff --git 
a/phoenix-core/src/it/java/org/apache/phoenix/end2end/AlterTableIT.java 
b/phoenix-core/src/it/java/org/apache/phoenix/end2end/AlterTableIT.java
index 17f08c4..dd895dc 100644
--- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/AlterTableIT.java
+++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/AlterTableIT.java
@@ -475,7 +475,7 @@ public class AlterTableIT extends ParallelStatsDisabledIT {
                 stmt.execute();
                 fail("Should have failed since altering a table by adding a 
non-nullable column is not allowed.");
             } catch (SQLException e) {
-                
assertEquals(SQLExceptionCode.CANNOT_ADD_NOT_NULLABLE_COLUMN.getErrorCode(), 
e.getErrorCode());
+                
assertEquals(SQLExceptionCode.KEY_VALUE_NOT_NULL.getErrorCode(), 
e.getErrorCode());
             } finally {
                 closeStatement(stmt);
             }

http://git-wip-us.apache.org/repos/asf/phoenix/blob/e99b738b/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 1abc653..491889d 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
@@ -36,7 +36,6 @@ import java.util.Properties;
 
 import org.apache.hadoop.hbase.HColumnDescriptor;
 import org.apache.hadoop.hbase.client.HBaseAdmin;
-import 
org.apache.hadoop.hbase.protobuf.generated.AccessControlProtos.GlobalPermissionOrBuilder;
 import org.apache.hadoop.hbase.regionserver.BloomType;
 import org.apache.hadoop.hbase.util.Bytes;
 import org.apache.phoenix.exception.SQLExceptionCode;
@@ -416,7 +415,7 @@ public class CreateTableIT extends ParallelStatsDisabledIT {
             conn.createStatement().execute(ddl);
             fail(" Non pk column ENTRY_POINT_NAME has a NOT NULL constraint");
         } catch (SQLException sqle) {
-            
assertEquals(SQLExceptionCode.INVALID_NOT_NULL_CONSTRAINT.getErrorCode(),
+            assertEquals(SQLExceptionCode.KEY_VALUE_NOT_NULL.getErrorCode(),
                 sqle.getErrorCode());
         }
     }
@@ -432,7 +431,7 @@ public class CreateTableIT extends ParallelStatsDisabledIT {
             conn.createStatement().execute(ddl);
             fail(" Non pk column V has a NOT NULL constraint");
         } catch (SQLException sqle) {
-            
assertEquals(SQLExceptionCode.INVALID_NOT_NULL_CONSTRAINT.getErrorCode(),
+            assertEquals(SQLExceptionCode.KEY_VALUE_NOT_NULL.getErrorCode(),
                 sqle.getErrorCode());
         }
     }

http://git-wip-us.apache.org/repos/asf/phoenix/blob/e99b738b/phoenix-core/src/main/java/org/apache/phoenix/compile/UpsertCompiler.java
----------------------------------------------------------------------
diff --git 
a/phoenix-core/src/main/java/org/apache/phoenix/compile/UpsertCompiler.java 
b/phoenix-core/src/main/java/org/apache/phoenix/compile/UpsertCompiler.java
index 9a3724e..be7e90c 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/compile/UpsertCompiler.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/compile/UpsertCompiler.java
@@ -431,20 +431,20 @@ public class UpsertCompiler {
             
targetColumns.addAll(Collections.<PColumn>nCopies(columnIndexesToBe.length, 
null));
             Arrays.fill(columnIndexesToBe, -1); // TODO: necessary? So we'll 
get an AIOB exception if it's not replaced
             Arrays.fill(pkSlotIndexesToBe, -1); // TODO: necessary? So we'll 
get an AIOB exception if it's not replaced
-            BitSet pkColumnsSet = new BitSet(table.getPKColumns().size());
+            BitSet columnsBeingSet = new BitSet(table.getColumns().size());
             int i = 0;
             if (isSharedViewIndex) {
                 PColumn indexIdColumn = table.getPKColumns().get(i + 
posOffset);
-                columnIndexesToBe[i] = indexIdColumn.getPosition();
-                pkColumnsSet.set(pkSlotIndexesToBe[i] = i + posOffset);
+                columnsBeingSet.set(columnIndexesToBe[i] = 
indexIdColumn.getPosition());
+                pkSlotIndexesToBe[i] = i + posOffset;
                 targetColumns.set(i, indexIdColumn);
                 i++;
             }
             // Add tenant column directly, as we don't want to resolve it as 
this will fail
             if (isTenantSpecific) {
                 PColumn tenantColumn = table.getPKColumns().get(i + posOffset);
-                columnIndexesToBe[i] = tenantColumn.getPosition();
-                pkColumnsSet.set(pkSlotIndexesToBe[i] = i + posOffset);
+                columnsBeingSet.set(columnIndexesToBe[i] = 
tenantColumn.getPosition());
+                pkSlotIndexesToBe[i] = i + posOffset;
                 targetColumns.set(i, tenantColumn);
                 i++;
             }
@@ -459,18 +459,18 @@ public class UpsertCompiler {
                     overlapViewColumnsToBe.add(column);
                     addViewColumnsToBe.remove(column);
                 }
-                columnIndexesToBe[i] = ref.getColumnPosition();
+                columnsBeingSet.set(columnIndexesToBe[i] = 
ref.getColumnPosition());
                 targetColumns.set(i, column);
                 if (SchemaUtil.isPKColumn(column)) {
-                    pkColumnsSet.set(pkSlotIndexesToBe[i] = 
ref.getPKSlotPosition());
+                    pkSlotIndexesToBe[i] = ref.getPKSlotPosition();
                 }
                 i++;
             }
             for (PColumn column : addViewColumnsToBe) {
-                columnIndexesToBe[i] = column.getPosition();
+                columnsBeingSet.set(columnIndexesToBe[i] = 
column.getPosition());
                 targetColumns.set(i, column);
                 if (SchemaUtil.isPKColumn(column)) {
-                    pkColumnsSet.set(pkSlotIndexesToBe[i] = 
SchemaUtil.getPKPosition(table, column));
+                    pkSlotIndexesToBe[i] = SchemaUtil.getPKPosition(table, 
column);
                 }
                 i++;
             }
@@ -481,20 +481,18 @@ public class UpsertCompiler {
                 // Need to resize columnIndexesToBe and pkSlotIndexesToBe to 
include this extra column.
                 columnIndexesToBe = Arrays.copyOf(columnIndexesToBe, 
columnIndexesToBe.length + 1);
                 pkSlotIndexesToBe = Arrays.copyOf(pkSlotIndexesToBe, 
pkSlotIndexesToBe.length + 1);
-                columnIndexesToBe[i] = rowTimestampCol.getPosition();
-                pkColumnsSet.set(pkSlotIndexesToBe[i] = 
table.getRowTimestampColPos());
+                columnsBeingSet.set(columnIndexesToBe[i] = 
rowTimestampCol.getPosition());
+                pkSlotIndexesToBe[i] = table.getRowTimestampColPos();
                 targetColumns.add(rowTimestampCol);
                 if (valueNodes != null && !valueNodes.isEmpty()) {
                     
valueNodes.add(getNodeForRowTimestampColumn(rowTimestampCol));
                 }
                 nColumnsToSet++;
             }
-            for (i = posOffset; i < table.getPKColumns().size(); i++) {
-                PColumn pkCol = table.getPKColumns().get(i);
-                if (!pkColumnsSet.get(i)) {
-                    if (!pkCol.isNullable() && pkCol.getExpressionStr() == 
null) {
-                        throw new 
ConstraintViolationException(table.getName().getString() + "." + 
pkCol.getName().getString() + " may not be null");
-                    }
+            for (i = posOffset; i < table.getColumns().size(); i++) {
+                PColumn column = table.getColumns().get(i);
+                if (!columnsBeingSet.get(i) && !column.isNullable() && 
column.getExpressionStr() == null) {
+                    throw new 
ConstraintViolationException(SchemaUtil.getColumnDisplayName(column.getFamilyName().getString(),
 column.getName().getString()) + " may not be null");
                 }
             }
         }
@@ -569,6 +567,12 @@ public class UpsertCompiler {
             nColumnsToSet = nValuesToSet;
             columnIndexesToBe = Arrays.copyOf(columnIndexesToBe, nValuesToSet);
             pkSlotIndexesToBe = Arrays.copyOf(pkSlotIndexesToBe, nValuesToSet);
+            for (int i = posOffset + nValuesToSet; i < 
table.getColumns().size(); i++) {
+                PColumn column = table.getColumns().get(i);
+                if (!column.isNullable() && column.getExpressionStr() == null) 
{
+                    throw new 
ConstraintViolationException(SchemaUtil.getColumnDisplayName(column) + " may 
not be null");
+                }
+            }
         }
         
         if (nValuesToSet != nColumnsToSet) {

http://git-wip-us.apache.org/repos/asf/phoenix/blob/e99b738b/phoenix-core/src/main/java/org/apache/phoenix/exception/SQLExceptionCode.java
----------------------------------------------------------------------
diff --git 
a/phoenix-core/src/main/java/org/apache/phoenix/exception/SQLExceptionCode.java 
b/phoenix-core/src/main/java/org/apache/phoenix/exception/SQLExceptionCode.java
index 0f29f3f..dcf761a 100644
--- 
a/phoenix-core/src/main/java/org/apache/phoenix/exception/SQLExceptionCode.java
+++ 
b/phoenix-core/src/main/java/org/apache/phoenix/exception/SQLExceptionCode.java
@@ -145,7 +145,6 @@ public enum SQLExceptionCode {
     }),
     ORDER_BY_ARRAY_NOT_SUPPORTED(515, "42893", "ORDER BY of an array type is 
not allowed."),
     NON_EQUALITY_ARRAY_COMPARISON(516, "42894", "Array types may only be 
compared using = or !=."),
-    INVALID_NOT_NULL_CONSTRAINT(517, "42895", "Invalid not null constraint on 
non primary key column."),
 
     /**
      *  Invalid Transaction State (errorcode 05, sqlstate 25)
@@ -203,14 +202,14 @@ public enum SQLExceptionCode {
     PRIMARY_KEY_WITH_FAMILY_NAME(1003, "42J01", "Primary key columns must not 
have a family name."),
     PRIMARY_KEY_OUT_OF_ORDER(1004, "42J02", "Order of columns in primary key 
constraint must match the order in which they're declared."),
     VARBINARY_IN_ROW_KEY(1005, "42J03", "The VARBINARY/ARRAY type can only be 
used as the last part of a multi-part row key."),
-    NOT_NULLABLE_COLUMN_IN_ROW_KEY(1006, "42J04", "Only nullable columns may 
be added to a multi-part row key."),
+    NOT_NULLABLE_COLUMN_IN_ROW_KEY(1006, "42J04", "Only nullable columns may 
be added to primary key."),
     VARBINARY_LAST_PK(1015, "42J04", "Cannot add column to table when the last 
PK column is of type VARBINARY or ARRAY."),
     NULLABLE_FIXED_WIDTH_LAST_PK(1023, "42J04", "Cannot add column to table 
when the last PK column is nullable and fixed width."),
     CANNOT_MODIFY_VIEW_PK(1036, "42J04", "Cannot modify the primary key of a 
VIEW if last PK column of parent is variable length."),
     BASE_TABLE_COLUMN(1037, "42J04", "Cannot modify columns of base table used 
by tenant-specific tables."),
     UNALLOWED_COLUMN_FAMILY(1090, "42J04", "Column family names should not 
contain local index column prefix: 
"+QueryConstants.LOCAL_INDEX_COLUMN_FAMILY_PREFIX),
     // Key/value column related errors
-    KEY_VALUE_NOT_NULL(1007, "42K01", "A key/value column may not be declared 
as not null."),
+    KEY_VALUE_NOT_NULL(1007, "42K01", "A non primary key column may only be 
declared as not null on tables with immutable rows."),
     // View related errors.
     VIEW_WITH_TABLE_CONFIG(1008, "42L01", "A view may not contain table 
configuration properties."),
     VIEW_WITH_PROPERTIES(1009, "42L02", "Properties may not be defined for a 
view."),

http://git-wip-us.apache.org/repos/asf/phoenix/blob/e99b738b/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 9bf6dc9..10ad199 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
@@ -930,7 +930,8 @@ public class MetaDataClient {
         argUpsert.execute();
     }
 
-    private PColumn newColumn(int position, ColumnDef def, 
PrimaryKeyConstraint pkConstraint, String defaultColumnFamily, boolean 
addingToPK, byte[] columnQualifierBytes) throws SQLException {
+    private PColumn newColumn(int position, ColumnDef def, 
PrimaryKeyConstraint pkConstraint, String defaultColumnFamily,
+            boolean addingToPK, byte[] columnQualifierBytes, boolean 
isImmutableRows) throws SQLException {
         try {
             ColumnName columnDefName = def.getColumnDefName();
             SortOrder sortOrder = def.getSortOrder();
@@ -962,7 +963,7 @@ public class MetaDataClient {
                 if (isPK) {
                     throw new 
SQLExceptionInfo.Builder(SQLExceptionCode.PRIMARY_KEY_WITH_FAMILY_NAME)
                     
.setColumnName(columnName).setFamilyName(family).build().buildException();
-                } else if (!def.isNull()) {
+                } else if (!def.isNull() && !isImmutableRows) {
                     throw new 
SQLExceptionInfo.Builder(SQLExceptionCode.KEY_VALUE_NOT_NULL)
                     
.setColumnName(columnName).setFamilyName(family).build().buildException();
                 }
@@ -2178,7 +2179,6 @@ public class MetaDataClient {
             }
 
             Map<String, PName> familyNames = Maps.newLinkedHashMap();
-            boolean isPK = false;
             boolean rowTimeStampColumnAlreadyFound = false;
             int positionOffset = columns.size();
             if (saltBucketNum != null) {
@@ -2269,19 +2269,20 @@ public class MetaDataClient {
             }
 
             Map<String, Integer> changedCqCounters = new 
HashMap<>(colDefs.size());
+            boolean wasPKDefined = false;
             for (ColumnDef colDef : colDefs) {
                 rowTimeStampColumnAlreadyFound = 
checkAndValidateRowTimestampCol(colDef, pkConstraint, 
rowTimeStampColumnAlreadyFound, tableType);
                 if (colDef.isPK()) { // i.e. the column is declared as CREATE 
TABLE COLNAME DATATYPE PRIMARY KEY...
-                    if (isPK) {
+                    if (wasPKDefined) {
                         throw new 
SQLExceptionInfo.Builder(SQLExceptionCode.PRIMARY_KEY_ALREADY_EXISTS)
                             
.setColumnName(colDef.getColumnDefName().getColumnName()).build().buildException();
                     }
-                    isPK = true;
+                    wasPKDefined = 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)
+                    if (  !colDef.isNull() && !isImmutableRows &&
+                        ( wasPKDefined || !isPkColumn(pkConstraint, colDef))) {
+                            throw new 
SQLExceptionInfo.Builder(SQLExceptionCode.KEY_VALUE_NOT_NULL)
                                 .setSchemaName(schemaName)
                                 .setTableName(tableName)
                                 
.setColumnName(colDef.getColumnDefName().getColumnName()).build().buildException();
@@ -2289,7 +2290,7 @@ public class MetaDataClient {
                 }
                 ColumnName columnDefName = colDef.getColumnDefName();
                 String colDefFamily = columnDefName.getFamilyName();
-                boolean isPkColumn = isPkColumn(pkConstraint, colDef, 
columnDefName);
+                boolean isPkColumn = isPkColumn(pkConstraint, colDef);
                 String cqCounterFamily = null;
                 if (!isPkColumn) {
                     if (immutableStorageScheme == 
SINGLE_CELL_ARRAY_WITH_OFFSETS && encodingScheme != NON_ENCODED_QUALIFIERS) {
@@ -2310,7 +2311,7 @@ public class MetaDataClient {
                     .setSchemaName(schemaName)
                     .setTableName(tableName).build().buildException();
                 }
-                PColumn column = newColumn(position++, colDef, pkConstraint, 
defaultFamilyName, false, columnQualifierBytes);
+                PColumn column = newColumn(position++, colDef, pkConstraint, 
defaultFamilyName, false, columnQualifierBytes, isImmutableRows);
                 if (cqCounter.increment(cqCounterFamily)) {
                     changedCqCounters.put(cqCounterFamily, 
cqCounter.getNextQualifier(cqCounterFamily));
                 }
@@ -2350,7 +2351,7 @@ public class MetaDataClient {
             }
             
             // We need a PK definition for a TABLE or mapped VIEW
-            if (!isPK && pkColumnsNames.isEmpty() && tableType != 
PTableType.VIEW && viewType != ViewType.MAPPED) {
+            if (!wasPKDefined && pkColumnsNames.isEmpty() && tableType != 
PTableType.VIEW && viewType != ViewType.MAPPED) {
                 throw new 
SQLExceptionInfo.Builder(SQLExceptionCode.PRIMARY_KEY_MISSING)
                     .setSchemaName(schemaName)
                     .setTableName(tableName)
@@ -2712,8 +2713,8 @@ public class MetaDataClient {
         }
     }
 
-    private static boolean isPkColumn(PrimaryKeyConstraint pkConstraint, 
ColumnDef colDef, ColumnName columnDefName) {
-        return colDef.isPK() || (pkConstraint != null && 
pkConstraint.getColumnWithSortOrder(columnDefName) != null);
+    private static boolean isPkColumn(PrimaryKeyConstraint pkConstraint, 
ColumnDef colDef) {
+        return colDef.isPK() || (pkConstraint != null && 
pkConstraint.contains(colDef.getColumnDefName()));
     }
     
     /**
@@ -3188,6 +3189,7 @@ public class MetaDataClient {
                 }
 
                 int position = table.getColumns().size();
+                boolean isImmutableRows = table.isImmutableRows();
 
                 List<PColumn> currentPKs = table.getPKColumns();
                 PColumn lastPK = currentPKs.get(currentPKs.size()-1);
@@ -3227,8 +3229,8 @@ public class MetaDataClient {
                                 if(colDef.isPK()) {
                                     throw new 
SQLExceptionInfo.Builder(SQLExceptionCode.NOT_NULLABLE_COLUMN_IN_ROW_KEY)
                                     
.setColumnName(colDef.getColumnDefName().getColumnName()).build().buildException();
-                                } else {
-                                    throw new 
SQLExceptionInfo.Builder(SQLExceptionCode.CANNOT_ADD_NOT_NULLABLE_COLUMN)
+                                } else if (!isImmutableRows) {
+                                    throw new 
SQLExceptionInfo.Builder(SQLExceptionCode.KEY_VALUE_NOT_NULL)
                                     
.setColumnName(colDef.getColumnDefName().getColumnName()).build().buildException();
                                 }
                             }
@@ -3272,7 +3274,7 @@ public class MetaDataClient {
                                 .setSchemaName(schemaName)
                                 
.setTableName(tableName).build().buildException();
                             }
-                            PColumn column = newColumn(position++, colDef, 
PrimaryKeyConstraint.EMPTY, table.getDefaultFamilyName() == null ? null : 
table.getDefaultFamilyName().getString(), true, columnQualifierBytes);
+                            PColumn column = newColumn(position++, colDef, 
PrimaryKeyConstraint.EMPTY, table.getDefaultFamilyName() == null ? null : 
table.getDefaultFamilyName().getString(), true, columnQualifierBytes, 
isImmutableRows);
                             columns.add(column);
                             String pkName = null;
                             Short keySeq = null;
@@ -3310,7 +3312,7 @@ public class MetaDataClient {
                                         ColumnName indexColName = 
ColumnName.caseSensitiveColumnName(IndexUtil.getIndexColumnName(null, 
colDef.getColumnDefName().getColumnName()));
                                         Expression expression = new 
RowKeyColumnExpression(columns.get(i), new RowKeyValueAccessor(pkColumns, 
++pkSlotPosition));
                                         ColumnDef indexColDef = 
FACTORY.columnDef(indexColName, indexColDataType.getSqlTypeName(), 
colDef.isNull(), colDef.getMaxLength(), colDef.getScale(), true, 
colDef.getSortOrder(), expression.toString(), colDef.isRowTimestamp());
-                                        PColumn indexColumn = 
newColumn(indexPosition++, indexColDef, PrimaryKeyConstraint.EMPTY, null, true, 
null);
+                                        PColumn indexColumn = 
newColumn(indexPosition++, indexColDef, PrimaryKeyConstraint.EMPTY, null, true, 
null, index.isImmutableRows());
                                         addColumnMutation(schemaName, 
index.getTableName().getString(), indexColumn, colUpsert, 
index.getParentTableName().getString(), index.getPKName() == null ? null : 
index.getPKName().getString(), ++nextIndexKeySeq, index.getBucketNum() != null);
                                     }
                                 }

http://git-wip-us.apache.org/repos/asf/phoenix/blob/e99b738b/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 78baa4c..45aca98 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
@@ -138,11 +138,7 @@ public class PColumnImpl implements PColumn {
 
     @Override
     public boolean isNullable() {
-        // 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;
+        return nullable;
     }
 
     @Override

http://git-wip-us.apache.org/repos/asf/phoenix/blob/e99b738b/phoenix-core/src/main/java/org/apache/phoenix/schema/PTableImpl.java
----------------------------------------------------------------------
diff --git 
a/phoenix-core/src/main/java/org/apache/phoenix/schema/PTableImpl.java 
b/phoenix-core/src/main/java/org/apache/phoenix/schema/PTableImpl.java
index fb30bc7..a7b31e8 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/schema/PTableImpl.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/schema/PTableImpl.java
@@ -69,7 +69,6 @@ import org.apache.phoenix.schema.types.PDataType;
 import org.apache.phoenix.schema.types.PDouble;
 import org.apache.phoenix.schema.types.PFloat;
 import org.apache.phoenix.schema.types.PVarchar;
-import org.apache.phoenix.transaction.TransactionFactory;
 import org.apache.phoenix.util.ByteUtil;
 import org.apache.phoenix.util.EncodedColumnsUtil;
 import org.apache.phoenix.util.PhoenixRuntime;

http://git-wip-us.apache.org/repos/asf/phoenix/blob/e99b738b/phoenix-core/src/main/java/org/apache/phoenix/util/SchemaUtil.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/util/SchemaUtil.java 
b/phoenix-core/src/main/java/org/apache/phoenix/util/SchemaUtil.java
index 42c2dcb..b43f54e 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/util/SchemaUtil.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/util/SchemaUtil.java
@@ -341,6 +341,12 @@ public class SchemaUtil {
         return getName(cf == null || cf.isEmpty() ? null : cf, cq, false);
     }
     
+    public static String getColumnDisplayName(PColumn column) {
+        PName columnName = column.getFamilyName();
+        String cf = columnName == null ? null : columnName.getString();
+        return getName(cf == null || cf.isEmpty() ? null : cf, 
column.getName().getString(), false);
+    }
+    
     public static String getCaseSensitiveColumnDisplayName(String cf, String 
cq) {
         return getName(cf == null || cf.isEmpty() ? null : cf, cq, true);
     }

http://git-wip-us.apache.org/repos/asf/phoenix/blob/e99b738b/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 1d61003..568fa8a 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
@@ -236,13 +236,30 @@ public class QueryCompilerTest extends 
BaseConnectionlessQueryTest {
             statement.execute();
             fail();
         } catch (SQLException e) {
-            
assertEquals(SQLExceptionCode.INVALID_NOT_NULL_CONSTRAINT.getErrorCode(), 
e.getErrorCode());
+            assertEquals(SQLExceptionCode.KEY_VALUE_NOT_NULL.getErrorCode(), 
e.getErrorCode());
         } finally {
             conn.close();
         }
     }
 
     @Test
+    public void testImmutableRowsPK() throws Exception {
+        Connection conn = DriverManager.getConnection(getUrl());
+        try {
+            String query = "CREATE IMMUTABLE TABLE foo (pk integer not null, 
col1 decimal, col2 decimal)";
+            PreparedStatement statement = conn.prepareStatement(query);
+            statement.execute();
+            fail();
+        } catch (SQLException e) {
+            assertEquals(SQLExceptionCode.PRIMARY_KEY_MISSING.getErrorCode(), 
e.getErrorCode());
+        }
+        String query = "CREATE IMMUTABLE TABLE foo (k1 integer not null, k2 
decimal not null, col1 decimal not null, constraint pk primary key (k1,k2))";
+        PreparedStatement statement = conn.prepareStatement(query);
+        statement.execute();
+        conn.close();
+    }
+
+    @Test
     public void testUnknownFamilyNameInTableOption() throws Exception {
         Connection conn = DriverManager.getConnection(getUrl());
         try {
@@ -1036,6 +1053,25 @@ public class QueryCompilerTest extends 
BaseConnectionlessQueryTest {
     }
 
     @Test
+    public void testAlterNotNull() throws Exception {
+        Connection conn = DriverManager.getConnection(getUrl());
+        try {
+            conn.createStatement().execute("ALTER TABLE atable ADD xyz VARCHAR 
NOT NULL");
+            fail();
+        } catch (SQLException e) { // expected
+            assertEquals(SQLExceptionCode.KEY_VALUE_NOT_NULL.getErrorCode(), 
e.getErrorCode());
+        }
+        conn.createStatement().execute("CREATE IMMUTABLE TABLE foo (K1 VARCHAR 
PRIMARY KEY)");
+        try {
+            conn.createStatement().execute("ALTER TABLE foo ADD xyz VARCHAR 
NOT NULL PRIMARY KEY");
+            fail();
+        } catch (SQLException e) { // expected
+            
assertEquals(SQLExceptionCode.NOT_NULLABLE_COLUMN_IN_ROW_KEY.getErrorCode(), 
e.getErrorCode());
+        }
+        conn.createStatement().execute("ALTER TABLE FOO ADD xyz VARCHAR NOT 
NULL");
+    }
+
+    @Test
     public void testSubstrSetScanKey() throws Exception {
         String query = "SELECT inst FROM ptsdb WHERE substr(inst, 0, 3) = 
'abc'";
         List<Object> binds = Collections.emptyList();
@@ -2669,6 +2705,66 @@ public class QueryCompilerTest extends 
BaseConnectionlessQueryTest {
     }
 
     @Test
+    public void testNotNullKeyValueColumnSalted() throws Exception {
+        testNotNullKeyValueColumn(3);
+    }
+    @Test
+    public void testNotNullKeyValueColumnUnsalted() throws Exception {
+        testNotNullKeyValueColumn(0);
+    }
+    
+    private void testNotNullKeyValueColumn(int saltBuckets) throws Exception {
+        Connection conn = DriverManager.getConnection(getUrl());
+        try {
+            conn.createStatement().execute("CREATE TABLE t1 (k integer not 
null primary key, v bigint not null) IMMUTABLE_ROWS=true" + (saltBuckets == 0 ? 
"" : (",SALT_BUCKETS="+saltBuckets)));
+            conn.createStatement().execute("UPSERT INTO t1 VALUES(0)");
+            fail();
+        } catch (SQLException e) {
+            assertEquals(SQLExceptionCode.CONSTRAINT_VIOLATION.getErrorCode(), 
e.getErrorCode());
+        }
+        try {
+            conn.createStatement().execute("CREATE TABLE t2 (k integer not 
null primary key, v1 bigint not null, v2 varchar, v3 tinyint not null) 
IMMUTABLE_ROWS=true" + (saltBuckets == 0 ? "" : 
(",SALT_BUCKETS="+saltBuckets)));
+            conn.createStatement().execute("UPSERT INTO t2(k, v3) 
VALUES(0,0)");
+            fail();
+        } catch (SQLException e) {
+            assertEquals(SQLExceptionCode.CONSTRAINT_VIOLATION.getErrorCode(), 
e.getErrorCode());
+        }
+        try {
+            conn.createStatement().execute("CREATE TABLE t3 (k integer not 
null primary key, v1 bigint not null, v2 varchar, v3 tinyint not null) 
IMMUTABLE_ROWS=true" + (saltBuckets == 0 ? "" : 
(",SALT_BUCKETS="+saltBuckets)));
+            conn.createStatement().execute("UPSERT INTO t3(k, v1) 
VALUES(0,0)");
+            fail();
+        } catch (SQLException e) {
+            assertEquals(SQLExceptionCode.CONSTRAINT_VIOLATION.getErrorCode(), 
e.getErrorCode());
+        }
+        conn.createStatement().execute("CREATE TABLE t4 (k integer not null 
primary key, v1 bigint not null) IMMUTABLE_ROWS=true" + (saltBuckets == 0 ? "" 
: (",SALT_BUCKETS="+saltBuckets)));
+        conn.createStatement().execute("UPSERT INTO t4 VALUES(0,0)");
+        conn.createStatement().execute("CREATE TABLE t5 (k integer not null 
primary key, v1 bigint not null default 0) IMMUTABLE_ROWS=true" + (saltBuckets 
== 0 ? "" : (",SALT_BUCKETS="+saltBuckets)));
+        conn.createStatement().execute("UPSERT INTO t5 VALUES(0)");
+        conn.close();
+    }
+
+    @Test
+    public void testAlterAddNotNullKeyValueColumn() throws Exception {
+        Connection conn = DriverManager.getConnection(getUrl());
+        conn.createStatement().execute("CREATE TABLE t1 (k integer not null 
primary key, v1 bigint not null) IMMUTABLE_ROWS=true");
+        try {
+            conn.createStatement().execute("ALTER TABLE t1 ADD k2 bigint not 
null primary key");
+            fail();
+        } catch (SQLException e) {
+            
assertEquals(SQLExceptionCode.NOT_NULLABLE_COLUMN_IN_ROW_KEY.getErrorCode(), 
e.getErrorCode());
+        }
+        conn.createStatement().execute("ALTER TABLE t1 ADD v2 bigint not 
null");
+        try {
+            conn.createStatement().execute("UPSERT INTO t1(k, v1) 
VALUES(0,0)");
+            fail();
+        } catch (SQLException e) {
+            assertEquals(SQLExceptionCode.CONSTRAINT_VIOLATION.getErrorCode(), 
e.getErrorCode());
+        }
+        conn.createStatement().execute("UPSERT INTO t1 VALUES(0,0,0)");
+        conn.createStatement().execute("UPSERT INTO t1(v1,k,v2) 
VALUES(0,0,0)");
+    }
+    
+    @Test
     public void testOnDupKeyForImmutableTable() throws Exception {
         Connection conn = DriverManager.getConnection(getUrl());
         try {

Reply via email to