PHOENIX-973 Lexer skips unexpected characters

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

Branch: refs/heads/4.0
Commit: 19d02fa5f14ae8a9f7a769eaa6d770f521246fb1
Parents: 17eb70d
Author: James Taylor <jtay...@salesforce.com>
Authored: Fri Oct 17 18:12:15 2014 -0700
Committer: James Taylor <jtay...@salesforce.com>
Committed: Fri Oct 17 18:12:15 2014 -0700

----------------------------------------------------------------------
 phoenix-core/src/main/antlr3/PhoenixSQL.g       | 13 ++-
 .../query/ConnectionQueryServicesImpl.java      |  2 +-
 .../org/apache/phoenix/schema/SequenceKey.java  |  4 +-
 .../org/apache/phoenix/util/MetaDataUtil.java   | 14 +--
 .../org/apache/phoenix/util/UpgradeUtil.java    | 95 ++++++++++++--------
 .../apache/phoenix/parse/QueryParserTest.java   | 16 +++-
 .../java/org/apache/phoenix/query/BaseTest.java |  6 +-
 7 files changed, 96 insertions(+), 54 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/phoenix/blob/19d02fa5/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 26d54a2..e2636fb 100644
--- a/phoenix-core/src/main/antlr3/PhoenixSQL.g
+++ b/phoenix-core/src/main/antlr3/PhoenixSQL.g
@@ -949,13 +949,12 @@ SL_COMMENT2: '--';
 
 // Bind names start with a colon and followed by 1 or more 
letter/digit/underscores
 BIND_NAME
-    : COLON (LETTER|DIGIT|'_')+
+    : COLON (DIGIT)+
     ;
 
-// Valid names can have a single underscore, but not multiple
-// Turn back on literal testing, all names are literals.
+
 NAME
-    :    LETTER (FIELDCHAR)* ('\"' (DBL_QUOTE_CHAR)* '\"')?
+    :    LETTER (FIELDCHAR)*
     |    '\"' (DBL_QUOTE_CHAR)* '\"'
     ;
 
@@ -1173,4 +1172,10 @@ SL_COMMENT
 DOT
     : '.'
     ;
+    
+OTHER      
+    : . { if (true) // to prevent compile error
+              throw new RuntimeException("Unexpected char: '" + $text + "'"); 
} 
+    ;
+
 

http://git-wip-us.apache.org/repos/asf/phoenix/blob/19d02fa5/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 fe3ae0a..415fbca 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
@@ -1536,7 +1536,7 @@ public class ConnectionQueryServicesImpl extends 
DelegateQueryServices implement
                             } catch (TableAlreadyExistsException e) {
                                 // This will occur if we have an older 
SYSTEM.SEQUENCE, so we need to update it to include
                                 // any new columns we've added.
-                                if 
(UpgradeUtil.addSaltByteToSequenceTable(metaConnection, nSaltBuckets)) {
+                                if 
(UpgradeUtil.upgradeSequenceTable(metaConnection, nSaltBuckets)) {
                                     metaConnection.removeTable(null,
                                             
PhoenixDatabaseMetaData.SYSTEM_CATALOG_SCHEMA,
                                             
PhoenixDatabaseMetaData.TYPE_SEQUENCE,

http://git-wip-us.apache.org/repos/asf/phoenix/blob/19d02fa5/phoenix-core/src/main/java/org/apache/phoenix/schema/SequenceKey.java
----------------------------------------------------------------------
diff --git 
a/phoenix-core/src/main/java/org/apache/phoenix/schema/SequenceKey.java 
b/phoenix-core/src/main/java/org/apache/phoenix/schema/SequenceKey.java
index 94ca549..6f82630 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/schema/SequenceKey.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/schema/SequenceKey.java
@@ -32,9 +32,9 @@ public class SequenceKey implements Comparable<SequenceKey> {
         this.tenantId = tenantId;
         this.schemaName = schemaName;
         this.sequenceName = sequenceName;
-        this.key = ByteUtil.concat(nBuckets <= 0 ? ByteUtil.EMPTY_BYTE_ARRAY : 
QueryConstants.SEPARATOR_BYTE_ARRAY, tenantId == null  ? 
ByteUtil.EMPTY_BYTE_ARRAY : Bytes.toBytes(tenantId), 
QueryConstants.SEPARATOR_BYTE_ARRAY, schemaName == null ? 
ByteUtil.EMPTY_BYTE_ARRAY : Bytes.toBytes(schemaName), 
QueryConstants.SEPARATOR_BYTE_ARRAY, Bytes.toBytes(sequenceName));
+        this.key = ByteUtil.concat((nBuckets <= 0 ? ByteUtil.EMPTY_BYTE_ARRAY 
: QueryConstants.SEPARATOR_BYTE_ARRAY), tenantId == null  ? 
ByteUtil.EMPTY_BYTE_ARRAY : Bytes.toBytes(tenantId), 
QueryConstants.SEPARATOR_BYTE_ARRAY, schemaName == null ? 
ByteUtil.EMPTY_BYTE_ARRAY : Bytes.toBytes(schemaName), 
QueryConstants.SEPARATOR_BYTE_ARRAY, Bytes.toBytes(sequenceName));
         if (nBuckets > 0) {
-            key[0] = SaltingUtil.getSaltingByte(key, 
SaltingUtil.NUM_SALTING_BYTES, key.length - SaltingUtil.NUM_SALTING_BYTES, 
SaltingUtil.MAX_BUCKET_NUM);
+            key[0] = SaltingUtil.getSaltingByte(key, 
SaltingUtil.NUM_SALTING_BYTES, key.length - SaltingUtil.NUM_SALTING_BYTES, 
nBuckets);
         }
     }
 

http://git-wip-us.apache.org/repos/asf/phoenix/blob/19d02fa5/phoenix-core/src/main/java/org/apache/phoenix/util/MetaDataUtil.java
----------------------------------------------------------------------
diff --git 
a/phoenix-core/src/main/java/org/apache/phoenix/util/MetaDataUtil.java 
b/phoenix-core/src/main/java/org/apache/phoenix/util/MetaDataUtil.java
index 7325161..d5c7a18 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/util/MetaDataUtil.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/util/MetaDataUtil.java
@@ -70,6 +70,7 @@ public class MetaDataUtil {
     public static final String LOCAL_INDEX_TABLE_PREFIX = "_LOCAL_IDX_";
     public static final byte[] LOCAL_INDEX_TABLE_PREFIX_BYTES = 
Bytes.toBytes(LOCAL_INDEX_TABLE_PREFIX);
     public static final String VIEW_INDEX_SEQUENCE_PREFIX = "_SEQ_";
+    public static final String VIEW_INDEX_SEQUENCE_NAME_PREFIX = "_ID_";
     public static final byte[] VIEW_INDEX_SEQUENCE_PREFIX_BYTES = 
Bytes.toBytes(VIEW_INDEX_SEQUENCE_PREFIX);
     public static final String VIEW_INDEX_ID_COLUMN_NAME = "_INDEX_ID";
     public static final String PARENT_TABLE_KEY = "PARENT_TABLE";
@@ -291,13 +292,17 @@ public class MetaDataUtil {
         return SchemaUtil.getTableName(schemaName, tableName);
     }
 
+    public static String getViewIndexSchemaName(PName physicalName) {
+        return VIEW_INDEX_SEQUENCE_PREFIX + physicalName.getString();
+    }
+    
     public static SequenceKey getViewIndexSequenceKey(String tenantId, PName 
physicalName, int nSaltBuckets) {
         // Create global sequence of the form: <prefixed base table 
name><tenant id>
         // rather than tenant-specific sequence, as it makes it much easier
         // to cleanup when the physical table is dropped, as we can delete
         // all global sequences leading with <prefix> + physical name.
-        String schemaName = VIEW_INDEX_SEQUENCE_PREFIX + 
physicalName.getString();
-        String tableName = tenantId == null ? "" : tenantId;
+        String schemaName = getViewIndexSchemaName(physicalName);
+        String tableName = VIEW_INDEX_SEQUENCE_NAME_PREFIX + (tenantId == null 
? "" : tenantId);
         return new SequenceKey(null, schemaName, tableName, nSaltBuckets);
     }
 
@@ -346,11 +351,10 @@ public class MetaDataUtil {
     }
 
     public static void deleteViewIndexSequences(PhoenixConnection connection, 
PName name) throws SQLException {
-        int nSequenceSaltBuckets = 
connection.getQueryServices().getSequenceSaltBuckets();
-        SequenceKey key = getViewIndexSequenceKey(null, name, 
nSequenceSaltBuckets);
+        String schemaName = getViewIndexSchemaName(name);
         connection.createStatement().executeUpdate("DELETE FROM " + 
PhoenixDatabaseMetaData.SEQUENCE_FULLNAME_ESCAPED + 
                 " WHERE " + PhoenixDatabaseMetaData.TENANT_ID + " IS NULL AND 
" + 
-                PhoenixDatabaseMetaData.SEQUENCE_SCHEMA + " = '" + 
key.getSchemaName() + "'");
+                PhoenixDatabaseMetaData.SEQUENCE_SCHEMA + " = '" + schemaName 
+ "'");
         
     }
     

http://git-wip-us.apache.org/repos/asf/phoenix/blob/19d02fa5/phoenix-core/src/main/java/org/apache/phoenix/util/UpgradeUtil.java
----------------------------------------------------------------------
diff --git 
a/phoenix-core/src/main/java/org/apache/phoenix/util/UpgradeUtil.java 
b/phoenix-core/src/main/java/org/apache/phoenix/util/UpgradeUtil.java
index 3054200..b51b455 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/util/UpgradeUtil.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/util/UpgradeUtil.java
@@ -29,10 +29,14 @@ import org.apache.hadoop.hbase.client.Put;
 import org.apache.hadoop.hbase.client.Result;
 import org.apache.hadoop.hbase.client.ResultScanner;
 import org.apache.hadoop.hbase.client.Scan;
+import org.apache.hadoop.hbase.util.Bytes;
 import org.apache.phoenix.coprocessor.MetaDataProtocol;
 import org.apache.phoenix.jdbc.PhoenixConnection;
 import org.apache.phoenix.jdbc.PhoenixDatabaseMetaData;
+import org.apache.phoenix.query.QueryConstants;
 import org.apache.phoenix.schema.PDataType;
+import org.apache.phoenix.schema.PName;
+import org.apache.phoenix.schema.PNameFactory;
 import org.apache.phoenix.schema.SaltingUtil;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -41,15 +45,13 @@ import com.google.common.collect.Lists;
 
 public class UpgradeUtil {
     private static final Logger logger = 
LoggerFactory.getLogger(UpgradeUtil.class);
+    private static final byte[] SEQ_PREFIX_BYTES = 
ByteUtil.concat(QueryConstants.SEPARATOR_BYTE_ARRAY, Bytes.toBytes("_SEQ_"));
 
     private UpgradeUtil() {
     }
 
-    public static boolean addSaltByteToSequenceTable(PhoenixConnection conn, 
int nSaltBuckets) throws SQLException {
-        if (nSaltBuckets <= 0) {
-            logger.info("Not upgrading SYSTEM.SEQUENCE table because 
SALT_BUCKETS is zero");
-            return false;
-        }
+    @SuppressWarnings("deprecation")
+    public static boolean upgradeSequenceTable(PhoenixConnection conn, int 
nSaltBuckets) throws SQLException {
         logger.info("Upgrading SYSTEM.SEQUENCE table");
 
         byte[] seqTableKey = SchemaUtil.getTableKey(null, 
PhoenixDatabaseMetaData.SYSTEM_CATALOG_SCHEMA, 
PhoenixDatabaseMetaData.TYPE_SEQUENCE);
@@ -89,31 +91,33 @@ public class UpgradeUtil {
                     Result result;
                      while ((result = scanner.next()) != null) {
                         for (KeyValue keyValue : result.raw()) {
-                            KeyValue newKeyValue = addSaltByte(keyValue);
-                            sizeBytes += newKeyValue.getLength();
-                            if 
(KeyValue.Type.codeToType(newKeyValue.getType()) == KeyValue.Type.Put) {
-                                // Delete old value
-                                byte[] buf = keyValue.getBuffer();
-                                Delete delete = new Delete(keyValue.getRow());
-                                KeyValue deleteKeyValue = new KeyValue(buf, 
keyValue.getRowOffset(), keyValue.getRowLength(),
-                                        buf, keyValue.getFamilyOffset(), 
keyValue.getFamilyLength(),
-                                        buf, keyValue.getQualifierOffset(), 
keyValue.getQualifierLength(),
-                                        keyValue.getTimestamp(), 
KeyValue.Type.Delete,
-                                        ByteUtil.EMPTY_BYTE_ARRAY,0,0);
-                                delete.addDeleteMarker(deleteKeyValue);
-                                mutations.add(delete);
-                                sizeBytes += deleteKeyValue.getLength();
-                                // Put new value
-                                Put put = new Put(newKeyValue.getRow());
-                                put.add(newKeyValue);
-                                mutations.add(put);
-                            } else if 
(KeyValue.Type.codeToType(newKeyValue.getType()) == KeyValue.Type.Delete){
-                                // Copy delete marker using new key so that it 
continues
-                                // to delete the key value preceding it that 
will be updated
-                                // as well.
-                                Delete delete = new 
Delete(newKeyValue.getRow());
-                                delete.addDeleteMarker(newKeyValue);
-                                mutations.add(delete);
+                            KeyValue newKeyValue = addSaltByte(keyValue, 
nSaltBuckets);
+                            if (newKeyValue != null) {
+                                sizeBytes += newKeyValue.getLength();
+                                if 
(KeyValue.Type.codeToType(newKeyValue.getType()) == KeyValue.Type.Put) {
+                                    // Delete old value
+                                    byte[] buf = keyValue.getBuffer();
+                                    Delete delete = new 
Delete(keyValue.getRow());
+                                    KeyValue deleteKeyValue = new 
KeyValue(buf, keyValue.getRowOffset(), keyValue.getRowLength(),
+                                            buf, keyValue.getFamilyOffset(), 
keyValue.getFamilyLength(),
+                                            buf, 
keyValue.getQualifierOffset(), keyValue.getQualifierLength(),
+                                            keyValue.getTimestamp(), 
KeyValue.Type.Delete,
+                                            ByteUtil.EMPTY_BYTE_ARRAY,0,0);
+                                    delete.addDeleteMarker(deleteKeyValue);
+                                    mutations.add(delete);
+                                    sizeBytes += deleteKeyValue.getLength();
+                                    // Put new value
+                                    Put put = new Put(newKeyValue.getRow());
+                                    put.add(newKeyValue);
+                                    mutations.add(put);
+                                } else if 
(KeyValue.Type.codeToType(newKeyValue.getType()) == KeyValue.Type.Delete){
+                                    // Copy delete marker using new key so 
that it continues
+                                    // to delete the key value preceding it 
that will be updated
+                                    // as well.
+                                    Delete delete = new 
Delete(newKeyValue.getRow());
+                                    delete.addDeleteMarker(newKeyValue);
+                                    mutations.add(delete);
+                                }
                             }
                             if (sizeBytes >= batchSizeBytes) {
                                 logger.info("Committing bactch of 
SYSTEM.SEQUENCE rows");
@@ -179,13 +183,34 @@ public class UpgradeUtil {
         }
     }
     
-    private static KeyValue addSaltByte(KeyValue keyValue) {
+    @SuppressWarnings("deprecation")
+    private static KeyValue addSaltByte(KeyValue keyValue, int nSaltBuckets) {
+        byte[] buf = keyValue.getBuffer();
         int length = keyValue.getRowLength();
         int offset = keyValue.getRowOffset();
-        byte[] buf = keyValue.getBuffer();
-        byte[] newBuf = new byte[length + 1];
-        System.arraycopy(buf, offset, newBuf, SaltingUtil.NUM_SALTING_BYTES, 
length);
-        newBuf[0] = SaltingUtil.getSaltingByte(newBuf, 
SaltingUtil.NUM_SALTING_BYTES, length, SaltingUtil.MAX_BUCKET_NUM);
+        boolean isViewSeq = length > SEQ_PREFIX_BYTES.length && 
Bytes.compareTo(SEQ_PREFIX_BYTES, 0, SEQ_PREFIX_BYTES.length, buf, offset, 
SEQ_PREFIX_BYTES.length) == 0;
+        if (!isViewSeq && nSaltBuckets == 0) {
+            return null;
+        }
+        byte[] newBuf;
+        if (isViewSeq) { // We messed up the name for the sequences for view 
indexes so we'll take this opportunity to fix it
+            if (buf[length-1] == 0) { // Global indexes on views have trailing 
null byte
+                length--;
+            }
+            byte[][] rowKeyMetaData = new byte[3][];
+            SchemaUtil.getVarChars(buf, offset, length, 0, rowKeyMetaData);
+            byte[] schemaName = 
rowKeyMetaData[PhoenixDatabaseMetaData.SCHEMA_NAME_INDEX];
+            byte[] unprefixedSchemaName = new byte[schemaName.length - 
MetaDataUtil.VIEW_INDEX_SEQUENCE_PREFIX_BYTES.length];
+            System.arraycopy(schemaName, 
MetaDataUtil.VIEW_INDEX_SEQUENCE_PREFIX_BYTES.length, unprefixedSchemaName, 0, 
unprefixedSchemaName.length);
+            byte[] tableName = 
rowKeyMetaData[PhoenixDatabaseMetaData.TABLE_NAME_INDEX];
+            PName physicalName = PNameFactory.newName(unprefixedSchemaName);
+            // Reformulate key based on correct data
+            newBuf = MetaDataUtil.getViewIndexSequenceKey(tableName == null ? 
null : Bytes.toString(tableName), physicalName, nSaltBuckets).getKey();
+        } else {
+            newBuf = new byte[length + 1];
+            System.arraycopy(buf, offset, newBuf, 
SaltingUtil.NUM_SALTING_BYTES, length);
+            newBuf[0] = SaltingUtil.getSaltingByte(newBuf, 
SaltingUtil.NUM_SALTING_BYTES, length, nSaltBuckets);
+        }
         return new KeyValue(newBuf, 0, newBuf.length,
                 buf, keyValue.getFamilyOffset(), keyValue.getFamilyLength(),
                 buf, keyValue.getQualifierOffset(), 
keyValue.getQualifierLength(),

http://git-wip-us.apache.org/repos/asf/phoenix/blob/19d02fa5/phoenix-core/src/test/java/org/apache/phoenix/parse/QueryParserTest.java
----------------------------------------------------------------------
diff --git 
a/phoenix-core/src/test/java/org/apache/phoenix/parse/QueryParserTest.java 
b/phoenix-core/src/test/java/org/apache/phoenix/parse/QueryParserTest.java
index 9bf3896..201172b 100644
--- a/phoenix-core/src/test/java/org/apache/phoenix/parse/QueryParserTest.java
+++ b/phoenix-core/src/test/java/org/apache/phoenix/parse/QueryParserTest.java
@@ -28,10 +28,9 @@ import java.sql.SQLFeatureNotSupportedException;
 import java.util.List;
 
 import org.apache.hadoop.hbase.util.Pair;
-import org.junit.Test;
-
 import org.apache.phoenix.exception.SQLExceptionCode;
 import org.apache.phoenix.schema.SortOrder;
+import org.junit.Test;
 
 
 public class QueryParserTest {
@@ -629,6 +628,19 @@ public class QueryParserTest {
     }
 
     @Test
+    public void testTableNameStartsWithUnderscore() throws Exception {
+        SQLParser parser = new SQLParser(
+                new StringReader(
+                        "select* from _t where k in ( 1,2 )"));
+        try {
+            parser.parseStatement();
+            fail();
+        } catch (SQLException e) {
+            assertEquals(SQLExceptionCode.PARSER_ERROR.getErrorCode(), 
e.getErrorCode());
+        }
+    }
+
+    @Test
     public void testValidUpsertSelectHint() throws Exception {
         SQLParser parser = new SQLParser(
                 new StringReader(

http://git-wip-us.apache.org/repos/asf/phoenix/blob/19d02fa5/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 a58bfef..e0b0a96 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
@@ -814,11 +814,7 @@ public abstract class BaseTest {
                 + PhoenixDatabaseMetaData.SEQUENCE_NAME 
                 + " FROM " + 
PhoenixDatabaseMetaData.SEQUENCE_FULLNAME_ESCAPED);
         while (rs.next()) {
-            try {
-                conn.createStatement().execute("DROP SEQUENCE " + 
SchemaUtil.getTableName(rs.getString(1), rs.getString(2)));
-            } catch (Exception e) {
-                //FIXME: see https://issues.apache.org/jira/browse/PHOENIX-973
-            }
+            conn.createStatement().execute("DROP SEQUENCE " + 
SchemaUtil.getEscapedTableName(rs.getString(1), rs.getString(2)));
         }
     }
     

Reply via email to