phoenix-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jamestay...@apache.org
Subject git commit: PHOENIX-858 Disallow null primary key (JamesTaylor)
Date Mon, 17 Mar 2014 07:22:13 GMT
Repository: incubator-phoenix
Updated Branches:
  refs/heads/master 9c2fc7384 -> 2dee0a86a


PHOENIX-858 Disallow null primary key (JamesTaylor)


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

Branch: refs/heads/master
Commit: 2dee0a86adc872093b40a66ccf4418e44abf84eb
Parents: 9c2fc73
Author: James Taylor <jamestaylor@apache.org>
Authored: Mon Mar 17 00:08:18 2014 -0700
Committer: James Taylor <jamestaylor@apache.org>
Committed: Mon Mar 17 00:19:07 2014 -0700

----------------------------------------------------------------------
 .../org/apache/phoenix/end2end/IsNullIT.java    | 27 ----------------
 phoenix-core/src/main/antlr3/PhoenixSQL.g       |  8 ++---
 .../phoenix/exception/SQLExceptionCode.java     |  1 +
 .../org/apache/phoenix/parse/ColumnDef.java     | 14 ++++++---
 .../apache/phoenix/parse/ParseNodeFactory.java  |  2 +-
 .../query/ConnectionQueryServicesImpl.java      |  2 ++
 .../apache/phoenix/schema/MetaDataClient.java   | 19 ++++++++---
 .../org/apache/phoenix/schema/PTableImpl.java   |  3 ++
 .../phoenix/compile/QueryCompilerTest.java      | 33 ++++++++++++++++++++
 .../java/org/apache/phoenix/query/BaseTest.java |  2 +-
 10 files changed, 69 insertions(+), 42 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-phoenix/blob/2dee0a86/phoenix-core/src/it/java/org/apache/phoenix/end2end/IsNullIT.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/IsNullIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/IsNullIT.java
index 956e6de..18ad27c 100644
--- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/IsNullIT.java
+++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/IsNullIT.java
@@ -68,33 +68,6 @@ public class IsNullIT extends BaseClientManagedTimeIT {
     }
     
     @Test
-    public void testIsNullAsSingleKey() throws Exception {
-        long ts = nextTimestamp();
-        Properties props = new Properties();
-        props.setProperty(PhoenixRuntime.CURRENT_SCN_ATTRIB, Long.toString(ts + 10));
-        Connection conn = DriverManager.getConnection(PHOENIX_JDBC_URL, props);
-        conn.createStatement().execute("CREATE TABLE T(k VARCHAR PRIMARY KEY)");
-        conn.close();
-        
-        props.setProperty(PhoenixRuntime.CURRENT_SCN_ATTRIB, Long.toString(ts + 20));
-        conn = DriverManager.getConnection(PHOENIX_JDBC_URL, props);
-        conn.createStatement().execute("UPSERT INTO T VALUES (null)");
-        conn.createStatement().execute("UPSERT INTO T VALUES ('a')");
-        conn.commit();
-        conn.close();
-        
-        props.setProperty(PhoenixRuntime.CURRENT_SCN_ATTRIB, Long.toString(ts + 30));
-        conn = DriverManager.getConnection(PHOENIX_JDBC_URL, props);
-        ResultSet rs = conn.createStatement().executeQuery("SELECT count(*) FROM T");
-        assertTrue(rs.next());
-        assertEquals(2,rs.getInt(1));
-        rs = conn.createStatement().executeQuery("SELECT count(*) FROM T WHERE k = 'a' or
k is null");
-        assertTrue(rs.next());
-        assertEquals(2,rs.getInt(1));
-        conn.close();
-    }
-    
-    @Test
     public void testIsNullInCompositeKey() throws Exception {
         long ts = nextTimestamp();
         Properties props = new Properties();

http://git-wip-us.apache.org/repos/asf/incubator-phoenix/blob/2dee0a86/phoenix-core/src/main/antlr3/PhoenixSQL.g
----------------------------------------------------------------------
diff --git a/phoenix-core/src/main/antlr3/PhoenixSQL.g b/phoenix-core/src/main/antlr3/PhoenixSQL.g
index 3998ca2..7dc3c5c 100644
--- a/phoenix-core/src/main/antlr3/PhoenixSQL.g
+++ b/phoenix-core/src/main/antlr3/PhoenixSQL.g
@@ -497,8 +497,8 @@ column_defs returns [List<ColumnDef> ret]
 ;
 
 column_def returns [ColumnDef ret]
-    :   c=column_name dt=identifier (LPAREN l=NUMBER (COMMA s=NUMBER)? RPAREN)? ar=ARRAY?
(lsq=LSQUARE (a=NUMBER)? RSQUARE)? (n=NOT? NULL)? (pk=PRIMARY KEY (order=ASC|order=DESC)?)?
-        { $ret = factory.columnDef(c, dt, ar != null || lsq != null, a == null ? null : 
Integer.parseInt( a.getText() ), n==null, 
+    :   c=column_name dt=identifier (LPAREN l=NUMBER (COMMA s=NUMBER)? RPAREN)? ar=ARRAY?
(lsq=LSQUARE (a=NUMBER)? RSQUARE)? (nn=NOT? n=NULL)? (pk=PRIMARY KEY (order=ASC|order=DESC)?)?
+        { $ret = factory.columnDef(c, dt, ar != null || lsq != null, a == null ? null : 
Integer.parseInt( a.getText() ), nn!=null ? Boolean.FALSE : n!=null ? Boolean.TRUE : null,

             l == null ? null : Integer.parseInt( l.getText() ),
             s == null ? null : Integer.parseInt( s.getText() ),
             pk != null, 
@@ -512,7 +512,7 @@ dyn_column_defs returns [List<ColumnDef> ret]
 
 dyn_column_def returns [ColumnDef ret]
     :   c=column_name dt=identifier (LPAREN l=NUMBER (COMMA s=NUMBER)? RPAREN)? ar=ARRAY?
(lsq=LSQUARE (a=NUMBER)? RSQUARE)?
-        {$ret = factory.columnDef(c, dt, ar != null || lsq != null, a == null ? null :  Integer.parseInt(
a.getText() ), true,
+        {$ret = factory.columnDef(c, dt, ar != null || lsq != null, a == null ? null :  Integer.parseInt(
a.getText() ), Boolean.TRUE,
             l == null ? null : Integer.parseInt( l.getText() ),
             s == null ? null : Integer.parseInt( s.getText() ),
             false, 
@@ -521,7 +521,7 @@ dyn_column_def returns [ColumnDef ret]
 
 dyn_column_name_or_def returns [ColumnDef ret]
     :   c=column_name (dt=identifier (LPAREN l=NUMBER (COMMA s=NUMBER)? RPAREN)? ar=ARRAY?
(lsq=LSQUARE (a=NUMBER)? RSQUARE)? )? 
-        {$ret = factory.columnDef(c, dt, ar != null || lsq != null, a == null ? null :  Integer.parseInt(
a.getText() ), true,
+        {$ret = factory.columnDef(c, dt, ar != null || lsq != null, a == null ? null :  Integer.parseInt(
a.getText() ), Boolean.TRUE,
             l == null ? null : Integer.parseInt( l.getText() ),
             s == null ? null : Integer.parseInt( s.getText() ),
             false, 

http://git-wip-us.apache.org/repos/asf/incubator-phoenix/blob/2dee0a86/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 885e3ae..3654fef 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,6 +145,7 @@ public enum SQLExceptionCode {
      * 
      * For the following exceptions, use errorcode 10.
      */
+    SINGLE_PK_MAY_NOT_BE_NULL(1000, "42I00", "Single column primary key may not be NULL."),
     COLUMN_FAMILY_NOT_FOUND(1001, "42I01", "Undefined column family.", new Factory() {
         @Override
         public SQLException newException(SQLExceptionInfo info) {

http://git-wip-us.apache.org/repos/asf/incubator-phoenix/blob/2dee0a86/phoenix-core/src/main/java/org/apache/phoenix/parse/ColumnDef.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/parse/ColumnDef.java b/phoenix-core/src/main/java/org/apache/phoenix/parse/ColumnDef.java
index 4b470d8..3d13ee0 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/parse/ColumnDef.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/parse/ColumnDef.java
@@ -38,7 +38,7 @@ import com.google.common.base.Preconditions;
 public class ColumnDef {
     private final ColumnName columnDefName;
     private PDataType dataType;
-    private final boolean isNull;
+    private final Boolean isNull;
     private final Integer maxLength;
     private final Integer scale;
     private final boolean isPK;
@@ -46,7 +46,7 @@ public class ColumnDef {
     private final boolean isArray;
     private final Integer arrSize;
  
-    ColumnDef(ColumnName columnDefName, String sqlTypeName, boolean isArray, Integer arrSize,
boolean isNull, Integer maxLength,
+    ColumnDef(ColumnName columnDefName, String sqlTypeName, boolean isArray, Integer arrSize,
Boolean isNull, Integer maxLength,
     		            Integer scale, boolean isPK, SortOrder sortOrder) {
    	 try {
          Preconditions.checkNotNull(sortOrder);
@@ -128,7 +128,7 @@ public class ColumnDef {
          throw new ParseException(e);
      }
     }
-    ColumnDef(ColumnName columnDefName, String sqlTypeName, boolean isNull, Integer maxLength,
+    ColumnDef(ColumnName columnDefName, String sqlTypeName, Boolean isNull, Integer maxLength,
             Integer scale, boolean isPK, SortOrder sortOrder) {
     	this(columnDefName, sqlTypeName, false, 0, isNull, maxLength, scale, isPK, sortOrder);
     }
@@ -142,7 +142,13 @@ public class ColumnDef {
     }
 
     public boolean isNull() {
-        return isNull;
+        // null or Boolean.TRUE means NULL
+        // Boolean.FALSE means NOT NULL
+        return !Boolean.FALSE.equals(isNull);
+    }
+
+    public boolean isNullSet() {
+        return isNull != null;
     }
 
     public Integer getMaxLength() {

http://git-wip-us.apache.org/repos/asf/incubator-phoenix/blob/2dee0a86/phoenix-core/src/main/java/org/apache/phoenix/parse/ParseNodeFactory.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/parse/ParseNodeFactory.java b/phoenix-core/src/main/java/org/apache/phoenix/parse/ParseNodeFactory.java
index ce719f4..ed5ac60 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/parse/ParseNodeFactory.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/parse/ParseNodeFactory.java
@@ -251,7 +251,7 @@ public class ParseNodeFactory {
         return new ColumnDef(columnDefName, sqlTypeName, isNull, maxLength, scale, isPK,
sortOrder);
     }
     
-    public ColumnDef columnDef(ColumnName columnDefName, String sqlTypeName, boolean isArray,
Integer arrSize, boolean isNull, Integer maxLength, Integer scale, boolean isPK, 
+    public ColumnDef columnDef(ColumnName columnDefName, String sqlTypeName, boolean isArray,
Integer arrSize, Boolean isNull, Integer maxLength, Integer scale, boolean isPK, 
         	SortOrder sortOrder) {
         return new ColumnDef(columnDefName, sqlTypeName, isArray, arrSize, isNull, maxLength,
scale, isPK, sortOrder);
     }

http://git-wip-us.apache.org/repos/asf/incubator-phoenix/blob/2dee0a86/phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java
b/phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java
index 5790b7a..e7af4a9 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java
@@ -2022,6 +2022,8 @@ public class ConnectionQueryServicesImpl extends DelegateQueryServices
implement
                     Short keySeq = null;
                     Byte linkType = null;
                     if (columnName != null && familyName == null) { // pk column
+                        // TODO: remember columnName when keySeq == 1 so that we
+                        // can update NULLABLE is this is the sole PK column
                         keySeq = nextKeySeq++;
                         pkName = tablePkName;
                     }

http://git-wip-us.apache.org/repos/asf/incubator-phoenix/blob/2dee0a86/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 56d4491..e36cb2e 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
@@ -368,7 +368,7 @@ public class MetaDataClient {
         colUpsert.execute();
     }
 
-    private PColumn newColumn(int position, ColumnDef def, PrimaryKeyConstraint pkConstraint,
String defaultColumnFamily) throws SQLException {
+    private PColumn newColumn(int position, ColumnDef def, PrimaryKeyConstraint pkConstraint,
String defaultColumnFamily, boolean addingToPK) throws SQLException {
         try {
             ColumnName columnDefName = def.getColumnDefName();
             SortOrder sortOrder = def.getSortOrder();
@@ -387,6 +387,7 @@ public class MetaDataClient {
                 throw new SQLExceptionInfo.Builder(SQLExceptionCode.PRIMARY_KEY_ALREADY_EXISTS)
                     .setColumnName(columnName).build().buildException();
             }
+            boolean isNull = def.isNull();
             if (def.getColumnDefName().getFamilyName() != null) {
                 String family = def.getColumnDefName().getFamilyName();
                 if (isPK) {
@@ -401,8 +402,16 @@ public class MetaDataClient {
                 familyName = PNameFactory.newName(defaultColumnFamily == null ? QueryConstants.DEFAULT_COLUMN_FAMILY
: defaultColumnFamily);
             }
             
+            if (isPK && !addingToPK && pkConstraint.getColumnNames().size()
<= 1) {
+                if (def.isNull() && def.isNullSet()) {
+                    throw new SQLExceptionInfo.Builder(SQLExceptionCode.SINGLE_PK_MAY_NOT_BE_NULL)
+                    .setColumnName(columnName).build().buildException();
+                }
+                isNull = false;
+            }
+            
             PColumn column = new PColumnImpl(PNameFactory.newName(columnName), familyName,
def.getDataType(),
-                    def.getMaxLength(), def.getScale(), def.isNull(), position, sortOrder,
def.getArraySize(), null, false);
+                    def.getMaxLength(), def.getScale(), isNull, position, sortOrder, def.getArraySize(),
null, false);
             return column;
         } catch (IllegalArgumentException e) { // Based on precondition check in constructor
             throw new SQLException(e);
@@ -932,7 +941,7 @@ public class MetaDataClient {
                     }
                     isPK = true;
                 }
-                PColumn column = newColumn(position++, colDef, pkConstraint, defaultFamilyName);
+                PColumn column = newColumn(position++, colDef, pkConstraint, defaultFamilyName,
false);
                 if (SchemaUtil.isPKColumn(column)) {
                     // TODO: remove this constraint?
                     if (pkColumnsIterator.hasNext() && !column.getName().getString().equals(pkColumnsIterator.next().getFirst().getColumnName()))
{
@@ -1514,7 +1523,7 @@ public class MetaDataClient {
                             }
                         }                        
                         throwIfAlteringViewPK(colDef, table);
-                        PColumn column = newColumn(position++, colDef, PrimaryKeyConstraint.EMPTY,
table.getDefaultFamilyName() == null ? null : table.getDefaultFamilyName().getString());
+                        PColumn column = newColumn(position++, colDef, PrimaryKeyConstraint.EMPTY,
table.getDefaultFamilyName() == null ? null : table.getDefaultFamilyName().getString(), true);
                         columns.add(column);
                         String pkName = null;
                         Short keySeq = null;
@@ -1539,7 +1548,7 @@ public class MetaDataClient {
                                     PDataType indexColDataType = IndexUtil.getIndexColumnDataType(colDef.isNull(),
colDef.getDataType());
                                     ColumnName indexColName = ColumnName.caseSensitiveColumnName(IndexUtil.getIndexColumnName(null,
colDef.getColumnDefName().getColumnName()));
                                     ColumnDef indexColDef = FACTORY.columnDef(indexColName,
indexColDataType.getSqlTypeName(), colDef.isNull(), colDef.getMaxLength(), colDef.getScale(),
true, colDef.getSortOrder());
-                                    PColumn indexColumn = newColumn(indexPosition++, indexColDef,
PrimaryKeyConstraint.EMPTY, null);
+                                    PColumn indexColumn = newColumn(indexPosition++, indexColDef,
PrimaryKeyConstraint.EMPTY, null, true);
                                     addColumnMutation(schemaName, index.getTableName().getString(),
indexColumn, colUpsert, index.getParentTableName().getString(), index.getPKName() == null
? null : index.getPKName().getString(), ++nextIndexKeySeq);
                                 }
                             }

http://git-wip-us.apache.org/repos/asf/incubator-phoenix/blob/2dee0a86/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 89f8350..07d9781 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
@@ -460,6 +460,9 @@ public class PTableImpl implements PTable {
                     throw new ConstraintViolationException(name.getString() + "." + column.getName().getString()
+ " may not be null");
                 }
             }
+            if (nValues == 0) { 
+                throw new ConstraintViolationException("Primary key may not be null ("+ name.getString()
+ ")");
+            }
             byte[] buf = os.getBuffer();
             int size = os.size();
             if (bucketNum != null) {

http://git-wip-us.apache.org/repos/asf/incubator-phoenix/blob/2dee0a86/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 eacdb5d..bf00159 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
@@ -51,6 +51,7 @@ import org.apache.phoenix.query.QueryConstants;
 import org.apache.phoenix.schema.AmbiguousColumnException;
 import org.apache.phoenix.schema.ColumnAlreadyExistsException;
 import org.apache.phoenix.schema.ColumnNotFoundException;
+import org.apache.phoenix.schema.ConstraintViolationException;
 import org.apache.phoenix.schema.PColumn;
 import org.apache.phoenix.schema.PTableKey;
 import org.apache.phoenix.util.ByteUtil;
@@ -1331,5 +1332,37 @@ public class QueryCompilerTest extends BaseConnectionlessQueryTest
{
         }
         conn.close();
     }
+    
+    @Test
+    public void testInvalidPrimaryKeyDecl() throws Exception {
+        String[] queries = {
+                "CREATE TABLE t (k varchar null primary key)",
+                "CREATE TABLE t (k varchar null, constraint pk primary key (k))",
+        };
+        Connection conn = DriverManager.getConnection(getUrl());
+        for (String query : queries) {
+            try {
+                conn.createStatement().execute(query);
+                fail("Compilation should have failed since this is an invalid PRIMARY KEY
declaration: " + query);
+            } catch (SQLException e) {
+                assertEquals(query, SQLExceptionCode.SINGLE_PK_MAY_NOT_BE_NULL.getErrorCode(),
e.getErrorCode());
+            }
+        }
+    }
+    
+    @Test
+    public void testInvalidNullCompositePrimaryKey() throws Exception {
+        Connection conn = DriverManager.getConnection(getUrl());
+        conn.createStatement().execute("CREATE TABLE t (k1 varchar, k2 varchar, constraint
pk primary key(k1,k2))");
+        PreparedStatement stmt = conn.prepareStatement("UPSERT INTO t values(?,?)");
+        stmt.setString(1, "");
+        stmt.setString(2, "");
+        try {
+            stmt.execute();
+            fail();
+        } catch (ConstraintViolationException e) {
+            assertTrue(e.getMessage().contains("Primary key may not be null"));
+        }
+    }
 
 }

http://git-wip-us.apache.org/repos/asf/incubator-phoenix/blob/2dee0a86/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 0539a61..6a76178 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
@@ -186,7 +186,7 @@ public abstract class BaseTest {
                 "   (i1 integer not null, i2 integer not null\n" +
                 "    CONSTRAINT pk PRIMARY KEY (i1,i2))");
         builder.put(MDTEST_NAME,"create table " + MDTEST_NAME +
-                "   (id char(1) not null primary key,\n" +
+                "   (id char(1) primary key,\n" +
                 "    a.col1 integer,\n" +
                 "    b.col2 bigint,\n" +
                 "    b.col3 decimal,\n" +


Mime
View raw message