PHOENIX-2087 Ensure predictable column position during alter table
Project: http://git-wip-us.apache.org/repos/asf/phoenix/repo Commit: http://git-wip-us.apache.org/repos/asf/phoenix/commit/72a7356b Tree: http://git-wip-us.apache.org/repos/asf/phoenix/tree/72a7356b Diff: http://git-wip-us.apache.org/repos/asf/phoenix/diff/72a7356b Branch: refs/heads/calcite Commit: 72a7356bcade01990a59cfd5d72161f18ae909f3 Parents: a8a9d01 Author: James Taylor <jtay...@salesforce.com> Authored: Tue Jun 30 08:44:37 2015 -0700 Committer: James Taylor <jtay...@salesforce.com> Committed: Tue Jun 30 17:31:24 2015 -0700 ---------------------------------------------------------------------- .../apache/phoenix/end2end/AlterTableIT.java | 51 +++++++++++++++----- .../coprocessor/MetaDataEndpointImpl.java | 5 +- .../apache/phoenix/schema/MetaDataClient.java | 9 +++- 3 files changed, 46 insertions(+), 19 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/phoenix/blob/72a7356b/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 cd46927..56bba9b 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 @@ -448,7 +448,7 @@ public class AlterTableIT extends BaseOwnClusterHBaseManagedTimeIT { conn.commit(); assertIndexExists(conn,true); - conn.createStatement().execute("ALTER TABLE " + DATA_TABLE_FULL_NAME + " ADD v3 VARCHAR, k2 DECIMAL PRIMARY KEY"); + conn.createStatement().execute("ALTER TABLE " + DATA_TABLE_FULL_NAME + " ADD v3 VARCHAR, k2 DECIMAL PRIMARY KEY, k3 DECIMAL PRIMARY KEY"); rs = conn.getMetaData().getPrimaryKeys("", SCHEMA_NAME, DATA_TABLE_NAME); assertTrue(rs.next()); assertEquals("K",rs.getString("COLUMN_NAME")); @@ -456,6 +456,10 @@ public class AlterTableIT extends BaseOwnClusterHBaseManagedTimeIT { assertTrue(rs.next()); assertEquals("K2",rs.getString("COLUMN_NAME")); assertEquals(2, rs.getShort("KEY_SEQ")); + assertTrue(rs.next()); + assertEquals("K3",rs.getString("COLUMN_NAME")); + assertEquals(3, rs.getShort("KEY_SEQ")); + assertFalse(rs.next()); rs = conn.getMetaData().getPrimaryKeys("", SCHEMA_NAME, INDEX_TABLE_NAME); assertTrue(rs.next()); @@ -467,6 +471,10 @@ public class AlterTableIT extends BaseOwnClusterHBaseManagedTimeIT { assertTrue(rs.next()); assertEquals(IndexUtil.INDEX_COLUMN_NAME_SEP + "K2",rs.getString("COLUMN_NAME")); assertEquals(3, rs.getShort("KEY_SEQ")); + assertTrue(rs.next()); + assertEquals(IndexUtil.INDEX_COLUMN_NAME_SEP + "K3",rs.getString("COLUMN_NAME")); + assertEquals(4, rs.getShort("KEY_SEQ")); + assertFalse(rs.next()); query = "SELECT * FROM " + DATA_TABLE_FULL_NAME; rs = conn.createStatement().executeQuery(query); @@ -478,19 +486,21 @@ public class AlterTableIT extends BaseOwnClusterHBaseManagedTimeIT { assertFalse(rs.next()); // load some data into the table - stmt = conn.prepareStatement("UPSERT INTO " + DATA_TABLE_FULL_NAME + "(K,K2,V1,V2) VALUES(?,?,?,?)"); + stmt = conn.prepareStatement("UPSERT INTO " + DATA_TABLE_FULL_NAME + "(K,K2,V1,V2,K3) VALUES(?,?,?,?,?)"); stmt.setString(1, "b"); stmt.setBigDecimal(2, BigDecimal.valueOf(2)); stmt.setString(3, "y"); stmt.setString(4, "2"); + stmt.setBigDecimal(5, BigDecimal.valueOf(3)); stmt.execute(); conn.commit(); - query = "SELECT k,k2 FROM " + DATA_TABLE_FULL_NAME + " WHERE v1='y'"; + query = "SELECT k,k2,k3 FROM " + DATA_TABLE_FULL_NAME + " WHERE v1='y'"; rs = conn.createStatement().executeQuery(query); assertTrue(rs.next()); assertEquals("b",rs.getString(1)); assertEquals(BigDecimal.valueOf(2),rs.getBigDecimal(2)); + assertEquals(BigDecimal.valueOf(3),rs.getBigDecimal(3)); assertFalse(rs.next()); } @@ -2345,6 +2355,21 @@ public class AlterTableIT extends BaseOwnClusterHBaseManagedTimeIT { return false; } + private int getIndexOfPkColumn(PhoenixConnection conn, String columnName, String tableName) throws SQLException { + String normalizedTableName = SchemaUtil.normalizeIdentifier(tableName); + PTable table = conn.getMetaDataCache().getTable(new PTableKey(conn.getTenantId(), normalizedTableName)); + List<PColumn> pkCols = table.getPKColumns(); + String normalizedColumnName = SchemaUtil.normalizeIdentifier(columnName); + int i = 0; + for (PColumn pkCol : pkCols) { + if (pkCol.getName().getString().equals(normalizedColumnName)) { + return i; + } + i++; + } + return -1; + } + private Connection getTenantConnection(String tenantId) throws Exception { Properties tenantProps = new Properties(); tenantProps.setProperty(PhoenixRuntime.TENANT_ID_ATTRIB, tenantId); @@ -2444,35 +2469,35 @@ public class AlterTableIT extends BaseOwnClusterHBaseManagedTimeIT { ResultSet rs = tenantConn.createStatement().executeQuery("SELECT K2, K3, V3 FROM " + view1); PhoenixConnection phxConn = tenantConn.unwrap(PhoenixConnection.class); - assertTrue(checkColumnPartOfPk(phxConn, "k2", view1)); - assertTrue(checkColumnPartOfPk(phxConn, "k3", view1)); + assertEquals(2, getIndexOfPkColumn(phxConn, "k2", view1)); + assertEquals(3, getIndexOfPkColumn(phxConn, "k3", view1)); assertEquals(1, getTableSequenceNumber(phxConn, view1)); assertEquals(4, getMaxKeySequenceNumber(phxConn, view1)); verifyNewColumns(rs, "K2", "K3", "V3"); rs = tenantConn.createStatement().executeQuery("SELECT K2, K3, V3 FROM " + view2); - assertTrue(checkColumnPartOfPk(phxConn, "k2", view2)); - assertTrue(checkColumnPartOfPk(phxConn, "k3", view2)); + assertEquals(2, getIndexOfPkColumn(phxConn, "k2", view2)); + assertEquals(3, getIndexOfPkColumn(phxConn, "k3", view2)); assertEquals(1, getTableSequenceNumber(phxConn, view2)); assertEquals(4, getMaxKeySequenceNumber(phxConn, view2)); verifyNewColumns(rs, "K2", "K3", "V3"); - assertTrue(checkColumnPartOfPk(phxConn, IndexUtil.getIndexColumnName(null, "k2"), view2Index)); - assertTrue(checkColumnPartOfPk(phxConn, IndexUtil.getIndexColumnName(null, "k3"), view2Index)); + assertEquals(4, getIndexOfPkColumn(phxConn, IndexUtil.getIndexColumnName(null, "k2"), view2Index)); + assertEquals(5, getIndexOfPkColumn(phxConn, IndexUtil.getIndexColumnName(null, "k3"), view2Index)); assertEquals(1, getTableSequenceNumber(phxConn, view2Index)); assertEquals(6, getMaxKeySequenceNumber(phxConn, view2Index)); } try (Connection tenantConn = getTenantConnection(tenant2)) { ResultSet rs = tenantConn.createStatement().executeQuery("SELECT K2, K3, V3 FROM " + view3); PhoenixConnection phxConn = tenantConn.unwrap(PhoenixConnection.class); - assertTrue(checkColumnPartOfPk(phxConn, "k2", view3)); - assertTrue(checkColumnPartOfPk(phxConn, "k3", view3)); + assertEquals(2, getIndexOfPkColumn(phxConn, "k2", view3)); + assertEquals(3, getIndexOfPkColumn(phxConn, "k3", view3)); assertEquals(1, getTableSequenceNumber(phxConn, view3)); verifyNewColumns(rs, "K22", "K33", "V33"); - assertTrue(checkColumnPartOfPk(phxConn, IndexUtil.getIndexColumnName(null, "k2"), view3Index)); - assertTrue(checkColumnPartOfPk(phxConn, IndexUtil.getIndexColumnName(null, "k3"), view3Index)); + assertEquals(4, getIndexOfPkColumn(phxConn, IndexUtil.getIndexColumnName(null, "k2"), view3Index)); + assertEquals(5, getIndexOfPkColumn(phxConn, IndexUtil.getIndexColumnName(null, "k3"), view3Index)); assertEquals(1, getTableSequenceNumber(phxConn, view3Index)); assertEquals(6, getMaxKeySequenceNumber(phxConn, view3Index)); } http://git-wip-us.apache.org/repos/asf/phoenix/blob/72a7356b/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/MetaDataEndpointImpl.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/MetaDataEndpointImpl.java b/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/MetaDataEndpointImpl.java index cc486d5..dc1a3b4 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/MetaDataEndpointImpl.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/MetaDataEndpointImpl.java @@ -207,6 +207,7 @@ import com.google.protobuf.Service; * * @since 0.1 */ +@SuppressWarnings("deprecation") public class MetaDataEndpointImpl extends MetaDataProtocol implements CoprocessorService, Coprocessor { private static final Logger logger = LoggerFactory.getLogger(MetaDataEndpointImpl.class); @@ -526,7 +527,6 @@ public class MetaDataEndpointImpl extends MetaDataProtocol implements Coprocesso indexes.add(indexTable); } - @SuppressWarnings("deprecation") private void addColumnToTable(List<Cell> results, PName colName, PName famName, Cell[] colKeyValues, List<PColumn> columns, boolean isSalted) { int i = 0; @@ -1176,14 +1176,12 @@ public class MetaDataEndpointImpl extends MetaDataProtocol implements Coprocesso } private static final byte[] PHYSICAL_TABLE_BYTES = new byte[] {PTable.LinkType.PHYSICAL_TABLE.getSerializedValue()}; - private static final byte[] PARENT_TABLE_BYTES = new byte[] {PTable.LinkType.PARENT_TABLE.getSerializedValue()}; /** * @param tableName parent table's name * Looks for whether child views exist for the table specified by table. * TODO: should we pass a timestamp here? */ - @SuppressWarnings("deprecation") private TableViewFinderResult findChildViews(Region region, byte[] tenantId, PTable table, byte[] linkTypeBytes) throws IOException { byte[] schemaName = table.getSchemaName().getBytes(); byte[] tableName = table.getTableName().getBytes(); @@ -2181,7 +2179,6 @@ public class MetaDataEndpointImpl extends MetaDataProtocol implements Coprocesso done.run(builder.build()); } - @SuppressWarnings("deprecation") @Override public void updateIndexState(RpcController controller, UpdateIndexStateRequest request, RpcCallback<MetaDataResponse> done) { http://git-wip-us.apache.org/repos/asf/phoenix/blob/72a7356b/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 d77ded8..0ad9b56 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 @@ -2341,7 +2341,10 @@ public class MetaDataClient { while (true) { ColumnResolver resolver = FromCompiler.getResolver(statement, connection); table = resolver.getTables().get(0).getTable(); - List<Mutation> tableMetaData = Lists.newArrayListWithExpectedSize(2); + int nIndexes = table.getIndexes().size(); + int nNewColumns = columnDefs.size(); + List<Mutation> tableMetaData = Lists.newArrayListWithExpectedSize((1 + nNewColumns) * (nIndexes + 1)); + List<Mutation> columnMetaData = Lists.newArrayListWithExpectedSize(nNewColumns * (nIndexes + 1)); if (logger.isDebugEnabled()) { logger.debug(LogUtil.addCustomAnnotations("Resolved table to " + table.getName().getString() + " with seqNum " + table.getSequenceNumber() + " at timestamp " + table.getTimeStamp() + " with " + table.getColumns().size() + " columns: " + table.getColumns(), connection)); } @@ -2453,7 +2456,7 @@ public class MetaDataClient { } } - tableMetaData.addAll(connection.getMutationState().toMutations().next().getSecond()); + columnMetaData.addAll(connection.getMutationState().toMutations().next().getSecond()); connection.rollback(); } else { // Check that HBase configured properly for mutable secondary indexing @@ -2489,6 +2492,8 @@ public class MetaDataClient { // Force the table header row to be first Collections.reverse(tableMetaData); + // Add column metadata afterwards, maintaining the order so columns have more predictable ordinal position + tableMetaData.addAll(columnMetaData); byte[] family = families.size() > 0 ? families.iterator().next().getBytes() : null;