Return-Path: X-Original-To: apmail-phoenix-commits-archive@minotaur.apache.org Delivered-To: apmail-phoenix-commits-archive@minotaur.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id DC5131724D for ; Mon, 27 Oct 2014 20:45:15 +0000 (UTC) Received: (qmail 37254 invoked by uid 500); 27 Oct 2014 20:45:15 -0000 Delivered-To: apmail-phoenix-commits-archive@phoenix.apache.org Received: (qmail 37210 invoked by uid 500); 27 Oct 2014 20:45:15 -0000 Mailing-List: contact commits-help@phoenix.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@phoenix.apache.org Delivered-To: mailing list commits@phoenix.apache.org Received: (qmail 37197 invoked by uid 99); 27 Oct 2014 20:45:15 -0000 Received: from tyr.zones.apache.org (HELO tyr.zones.apache.org) (140.211.11.114) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 27 Oct 2014 20:45:15 +0000 Received: by tyr.zones.apache.org (Postfix, from userid 65534) id 7C4689843C9; Mon, 27 Oct 2014 20:45:15 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: jamestaylor@apache.org To: commits@phoenix.apache.org Date: Mon, 27 Oct 2014 20:45:15 -0000 Message-Id: X-Mailer: ASF-Git Admin Mailer Subject: [1/2] git commit: PHOENIX-1385 Adding, dropping and adding columns fails with NPE (Samarth Jain, James Taylor) Repository: phoenix Updated Branches: refs/heads/master c0b617f55 -> 4ed9ddb8f PHOENIX-1385 Adding, dropping and adding columns fails with NPE (Samarth Jain, James Taylor) Project: http://git-wip-us.apache.org/repos/asf/phoenix/repo Commit: http://git-wip-us.apache.org/repos/asf/phoenix/commit/692b8cb1 Tree: http://git-wip-us.apache.org/repos/asf/phoenix/tree/692b8cb1 Diff: http://git-wip-us.apache.org/repos/asf/phoenix/diff/692b8cb1 Branch: refs/heads/master Commit: 692b8cb1e653b0b7c9329ee1e2890f79d781ea50 Parents: c0b617f Author: James Taylor Authored: Mon Oct 27 13:35:49 2014 -0700 Committer: James Taylor Committed: Mon Oct 27 13:43:44 2014 -0700 ---------------------------------------------------------------------- .../apache/phoenix/end2end/AlterTableIT.java | 22 +++++++- .../apache/phoenix/jdbc/PhoenixConnection.java | 8 +-- .../query/ConnectionQueryServicesImpl.java | 4 +- .../query/ConnectionlessQueryServicesImpl.java | 6 +-- .../query/DelegateConnectionQueryServices.java | 6 +-- .../apache/phoenix/query/MetaDataMutated.java | 2 +- .../apache/phoenix/schema/MetaDataClient.java | 6 +-- .../apache/phoenix/schema/PMetaDataImpl.java | 53 +++++++++++--------- 8 files changed, 64 insertions(+), 43 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/phoenix/blob/692b8cb1/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 98a98d2..5745bf0 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 @@ -892,4 +892,24 @@ public class AlterTableIT extends BaseHBaseManagedTimeIT { pstmt2.close(); conn1.close(); } -} + + @Test + public void testAddColumnsUsingNewConnection() throws Exception { + Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES); + String ddl = "CREATE TABLE T (\n" + +"ID1 VARCHAR(15) NOT NULL,\n" + +"ID2 VARCHAR(15) NOT NULL,\n" + +"CREATED_DATE DATE,\n" + +"CREATION_TIME BIGINT,\n" + +"LAST_USED DATE,\n" + +"CONSTRAINT PK PRIMARY KEY (ID1, ID2))"; + Connection conn1 = DriverManager.getConnection(getUrl(), props); + conn1.createStatement().execute(ddl); + ddl = "ALTER TABLE T ADD STRING VARCHAR, STRING_DATA_TYPES VARCHAR"; + conn1.createStatement().execute(ddl); + ddl = "ALTER TABLE T DROP COLUMN STRING, STRING_DATA_TYPES"; + conn1.createStatement().execute(ddl); + ddl = "ALTER TABLE T ADD STRING_ARRAY1 VARCHAR[]"; + conn1.createStatement().execute(ddl); + conn1.close(); + }} http://git-wip-us.apache.org/repos/asf/phoenix/blob/692b8cb1/phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixConnection.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixConnection.java b/phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixConnection.java index 9a01018..4c57d09 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixConnection.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixConnection.java @@ -742,11 +742,11 @@ public class PhoenixConnection implements Connection, org.apache.phoenix.jdbc.Jd } @Override - public PMetaData removeColumn(PName tenantId, String tableName, String familyName, String columnName, - long tableTimeStamp, long tableSeqNum) throws SQLException { - metaData = metaData.removeColumn(tenantId, tableName, familyName, columnName, tableTimeStamp, tableSeqNum); + public PMetaData removeColumn(PName tenantId, String tableName, List columnsToRemove, long tableTimeStamp, + long tableSeqNum) throws SQLException { + metaData = metaData.removeColumn(tenantId, tableName, columnsToRemove, tableTimeStamp, tableSeqNum); //Cascade through to connectionQueryServices too - getQueryServices().removeColumn(tenantId, tableName, familyName, columnName, tableTimeStamp, tableSeqNum); + getQueryServices().removeColumn(tenantId, tableName, columnsToRemove, tableTimeStamp, tableSeqNum); return metaData; } http://git-wip-us.apache.org/repos/asf/phoenix/blob/692b8cb1/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 d8ea1b3..dbf786a 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 @@ -512,12 +512,12 @@ public class ConnectionQueryServicesImpl extends DelegateQueryServices implement } @Override - public PMetaData removeColumn(final PName tenantId, final String tableName, final String familyName, final String columnName, final long tableTimeStamp, final long tableSeqNum) throws SQLException { + public PMetaData removeColumn(final PName tenantId, final String tableName, final List columnsToRemove, final long tableTimeStamp, final long tableSeqNum) throws SQLException { return metaDataMutated(tenantId, tableName, tableSeqNum, new Mutator() { @Override public PMetaData mutate(PMetaData metaData) throws SQLException { try { - return metaData.removeColumn(tenantId, tableName, familyName, columnName, tableTimeStamp, tableSeqNum); + return metaData.removeColumn(tenantId, tableName, columnsToRemove, tableTimeStamp, tableSeqNum); } catch (TableNotFoundException e) { // The DROP TABLE may have been processed first, so just ignore. return metaData; http://git-wip-us.apache.org/repos/asf/phoenix/blob/692b8cb1/phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionlessQueryServicesImpl.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionlessQueryServicesImpl.java b/phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionlessQueryServicesImpl.java index 6ecb6d1..386050c 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionlessQueryServicesImpl.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionlessQueryServicesImpl.java @@ -152,9 +152,9 @@ public class ConnectionlessQueryServicesImpl extends DelegateQueryServices imple } @Override - public PMetaData removeColumn(PName tenantId, String tableName, String familyName, String columnName, - long tableTimeStamp, long tableSeqNum) throws SQLException { - return metaData = metaData.removeColumn(tenantId, tableName, familyName, columnName, tableTimeStamp, tableSeqNum); + public PMetaData removeColumn(PName tenantId, String tableName, List columnsToRemove, long tableTimeStamp, + long tableSeqNum) throws SQLException { + return metaData = metaData.removeColumn(tenantId, tableName, columnsToRemove, tableTimeStamp, tableSeqNum); } http://git-wip-us.apache.org/repos/asf/phoenix/blob/692b8cb1/phoenix-core/src/main/java/org/apache/phoenix/query/DelegateConnectionQueryServices.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/query/DelegateConnectionQueryServices.java b/phoenix-core/src/main/java/org/apache/phoenix/query/DelegateConnectionQueryServices.java index bb4bb33..defad5b 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/query/DelegateConnectionQueryServices.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/query/DelegateConnectionQueryServices.java @@ -88,9 +88,9 @@ public class DelegateConnectionQueryServices extends DelegateQueryServices imple } @Override - public PMetaData removeColumn(PName tenantId, String tableName, String familyName, String columnName, - long tableTimeStamp, long tableSeqNum) throws SQLException { - return getDelegate().removeColumn(tenantId, tableName, familyName, columnName, tableTimeStamp, tableSeqNum); + public PMetaData removeColumn(PName tenantId, String tableName, List columnsToRemove, long tableTimeStamp, + long tableSeqNum) throws SQLException { + return getDelegate().removeColumn(tenantId, tableName, columnsToRemove, tableTimeStamp, tableSeqNum); } @Override http://git-wip-us.apache.org/repos/asf/phoenix/blob/692b8cb1/phoenix-core/src/main/java/org/apache/phoenix/query/MetaDataMutated.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/query/MetaDataMutated.java b/phoenix-core/src/main/java/org/apache/phoenix/query/MetaDataMutated.java index 1b8ebda..cd4e2de 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/query/MetaDataMutated.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/query/MetaDataMutated.java @@ -37,5 +37,5 @@ public interface MetaDataMutated { PMetaData addTable(PTable table) throws SQLException; PMetaData removeTable(PName tenantId, String tableName, String parentTableName, long tableTimeStamp) throws SQLException; PMetaData addColumn(PName tenantId, String tableName, List columns, long tableTimeStamp, long tableSeqNum, boolean isImmutableRows) throws SQLException; - PMetaData removeColumn(PName tenantId, String tableName, String familyName, String columnName, long tableTimeStamp, long tableSeqNum) throws SQLException; + PMetaData removeColumn(PName tenantId, String tableName, List columnsToRemove, long tableTimeStamp, long tableSeqNum) throws SQLException; } http://git-wip-us.apache.org/repos/asf/phoenix/blob/692b8cb1/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 1f26274..afe21e8 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 @@ -2308,10 +2308,8 @@ public class MetaDataClient { // If we've done any index metadata updates, don't bother trying to update // client-side cache as it would be too painful. Just let it pull it over from // the server when needed. - if (columnsToDrop.size() > 0 && indexesToDrop.isEmpty()) { - for(PColumn columnToDrop : tableColumnsToDrop) { - connection.removeColumn(tenantId, SchemaUtil.getTableName(schemaName, tableName) , columnToDrop.getFamilyName().getString(), columnToDrop.getName().getString(), result.getMutationTime(), seqNum); - } + if (tableColumnsToDrop.size() > 0 && indexesToDrop.isEmpty()) { + connection.removeColumn(tenantId, SchemaUtil.getTableName(schemaName, tableName) , tableColumnsToDrop, result.getMutationTime(), seqNum); } // If we have a VIEW, then only delete the metadata, and leave the table data alone if (table.getType() != PTableType.VIEW) { http://git-wip-us.apache.org/repos/asf/phoenix/blob/692b8cb1/phoenix-core/src/main/java/org/apache/phoenix/schema/PMetaDataImpl.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/schema/PMetaDataImpl.java b/phoenix-core/src/main/java/org/apache/phoenix/schema/PMetaDataImpl.java index 8b26709..0d75aa2 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/schema/PMetaDataImpl.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/schema/PMetaDataImpl.java @@ -365,38 +365,41 @@ public class PMetaDataImpl implements PMetaData { } @Override - public PMetaData removeColumn(PName tenantId, String tableName, String familyName, String columnName, long tableTimeStamp, long tableSeqNum) throws SQLException { + public PMetaData removeColumn(PName tenantId, String tableName, List columnsToRemove, long tableTimeStamp, long tableSeqNum) throws SQLException { PTableRef tableRef = metaData.get(new PTableKey(tenantId, tableName)); if (tableRef == null) { return this; } PTable table = tableRef.table; PTableCache tables = metaData.clone(); - PColumn column; - if (familyName == null) { - column = table.getPKColumn(columnName); - } else { - column = table.getColumnFamily(familyName).getColumn(columnName); - } - int positionOffset = 0; - int position = column.getPosition(); - List oldColumns = table.getColumns(); - if (table.getBucketNum() != null) { - position--; - positionOffset = 1; - oldColumns = oldColumns.subList(positionOffset, oldColumns.size()); - } - List columns = Lists.newArrayListWithExpectedSize(oldColumns.size() - 1); - columns.addAll(oldColumns.subList(0, position)); - // Update position of columns that follow removed column - for (int i = position+1; i < oldColumns.size(); i++) { - PColumn oldColumn = oldColumns.get(i); - PColumn newColumn = new PColumnImpl(oldColumn.getName(), oldColumn.getFamilyName(), oldColumn.getDataType(), oldColumn.getMaxLength(), oldColumn.getScale(), oldColumn.isNullable(), i-1+positionOffset, oldColumn.getSortOrder(), oldColumn.getArraySize(), oldColumn.getViewConstant(), oldColumn.isViewReferenced()); - columns.add(newColumn); + for (PColumn columnToRemove : columnsToRemove) { + PColumn column; + String familyName = columnToRemove.getFamilyName().getString(); + if (familyName == null) { + column = table.getPKColumn(columnToRemove.getName().getString()); + } else { + column = table.getColumnFamily(familyName).getColumn(columnToRemove.getName().getString()); + } + int positionOffset = 0; + int position = column.getPosition(); + List oldColumns = table.getColumns(); + if (table.getBucketNum() != null) { + position--; + positionOffset = 1; + oldColumns = oldColumns.subList(positionOffset, oldColumns.size()); + } + List columns = Lists.newArrayListWithExpectedSize(oldColumns.size() - 1); + columns.addAll(oldColumns.subList(0, position)); + // Update position of columns that follow removed column + for (int i = position+1; i < oldColumns.size(); i++) { + PColumn oldColumn = oldColumns.get(i); + PColumn newColumn = new PColumnImpl(oldColumn.getName(), oldColumn.getFamilyName(), oldColumn.getDataType(), oldColumn.getMaxLength(), oldColumn.getScale(), oldColumn.isNullable(), i-1+positionOffset, oldColumn.getSortOrder(), oldColumn.getArraySize(), oldColumn.getViewConstant(), oldColumn.isViewReferenced()); + columns.add(newColumn); + } + + table = PTableImpl.makePTable(table, tableTimeStamp, tableSeqNum, columns); } - - PTable newTable = PTableImpl.makePTable(table, tableTimeStamp, tableSeqNum, columns); - tables.put(newTable.getKey(), newTable); + tables.put(table.getKey(), table); return new PMetaDataImpl(tables); }