This is an automated email from the ASF dual-hosted git repository.
chinmayskulkarni pushed a commit to branch 4.x
in repository https://gitbox.apache.org/repos/asf/phoenix.git
The following commit(s) were added to refs/heads/4.x by this push:
new 1d84495 PHOENIX-5958: Diverged view created via an older client still
sees dropped column data
1d84495 is described below
commit 1d844950bb4ec8221873ecd2b094c20f427cd984
Author: Chinmay Kulkarni <chinmayskulka...@gmail.com>
AuthorDate: Wed Jul 29 23:48:47 2020 -0700
PHOENIX-5958: Diverged view created via an older client still sees dropped
column data
---
.../phoenix/end2end/BackwardCompatibilityIT.java | 120 ++++++++----
.../gold_query_create_diverged_view.txt} | 16 +-
.../{query.sql => create_diverged_view.sql} | 10 +-
.../{query_more.sql => query_add_data.sql} | 0
.../sql_files/{query.sql => query_create_add.sql} | 0
.../{query.sql => query_create_diverged_view.sql} | 10 +-
.../phoenix/coprocessor/AddColumnMutator.java | 3 +-
.../apache/phoenix/coprocessor/ColumnMutator.java | 3 +-
.../phoenix/coprocessor/DropColumnMutator.java | 20 +-
.../phoenix/coprocessor/MetaDataEndpointImpl.java | 4 +-
.../org/apache/phoenix/schema/MetaDataClient.java | 8 +-
.../java/org/apache/phoenix/schema/PTableImpl.java | 4 +-
.../java/org/apache/phoenix/util/ViewUtil.java | 213 ++++++++++++---------
13 files changed, 251 insertions(+), 160 deletions(-)
diff --git
a/phoenix-core/src/it/java/org/apache/phoenix/end2end/BackwardCompatibilityIT.java
b/phoenix-core/src/it/java/org/apache/phoenix/end2end/BackwardCompatibilityIT.java
index fd1adc9..fa614ce 100644
---
a/phoenix-core/src/it/java/org/apache/phoenix/end2end/BackwardCompatibilityIT.java
+++
b/phoenix-core/src/it/java/org/apache/phoenix/end2end/BackwardCompatibilityIT.java
@@ -21,6 +21,7 @@ import static
org.apache.phoenix.query.BaseTest.setUpConfigForMiniCluster;
import static org.apache.phoenix.util.TestUtil.TEST_PROPERTIES;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assume.assumeFalse;
@@ -83,20 +84,24 @@ import com.fasterxml.jackson.databind.ObjectMapper;
public class BackwardCompatibilityIT {
private static final String SQL_DIR = "sql_files/";
- private static final String RESULT_DIR = "gold_files/";
+ private static final String RESULTS_AND_GOLD_FILES_DIR = "gold_files/";
private static final String COMPATIBLE_CLIENTS_JSON =
"compatible_client_versions.json";
private static final String BASH = "/bin/bash";
private static final String EXECUTE_QUERY_SH = "scripts/execute_query.sh";
+ private static final String QUERY_PREFIX = "query_";
private static final String RESULT_PREFIX = "result_";
+ private static final String GOLD_PREFIX = "gold_";
private static final String SQL_EXTENSION = ".sql";
private static final String TEXT_EXTENSION = ".txt";
private static final String CREATE_ADD = "create_add";
+ private static final String CREATE_DIVERGED_VIEW = "create_diverged_view";
private static final String ADD_DATA = "add_data";
private static final String ADD_DELETE = "add_delete";
- private static final String QUERY = "query";
- private static final String QUERY_MORE = "query_more";
- private static final String QUERY_ADD_DELETE = "query_add_delete";
+ private static final String QUERY_CREATE_ADD = QUERY_PREFIX + CREATE_ADD;
+ private static final String QUERY_ADD_DATA = QUERY_PREFIX + ADD_DATA;
+ private static final String QUERY_ADD_DELETE = QUERY_PREFIX + ADD_DELETE;
+ private static final String QUERY_CREATE_DIVERGED_VIEW = QUERY_PREFIX +
CREATE_DIVERGED_VIEW;
private static final String MVN_HOME = "maven.home";
private static final String JAVA_TMP_DIR = "java.io.tmpdir";
@@ -172,15 +177,58 @@ public class BackwardCompatibilityIT {
public void testUpsertWithOldClient() throws Exception {
// Insert data with old client and read with new client
executeQueryWithClientVersion(compatibleClientVersion, CREATE_ADD);
- executeQueriesWithCurrentVersion(QUERY);
- assertExpectedOutput(CREATE_ADD, QUERY);
+ executeQueriesWithCurrentVersion(QUERY_CREATE_ADD);
+ assertExpectedOutput(QUERY_CREATE_ADD);
+ }
+
+ @Test
+ public void testCreateDivergedViewWithOldClientReadFromNewClient() throws
Exception {
+ // Create a base table, view and make it diverge from an old client
+ executeQueryWithClientVersion(compatibleClientVersion,
CREATE_DIVERGED_VIEW);
+ executeQueriesWithCurrentVersion(QUERY_CREATE_DIVERGED_VIEW);
+ assertExpectedOutput(QUERY_CREATE_DIVERGED_VIEW);
+ }
+
+ @Test
+ public void testCreateDivergedViewWithOldClientReadFromOldClient() throws
Exception {
+ // Create a base table, view and make it diverge from an old client
+ executeQueryWithClientVersion(compatibleClientVersion,
CREATE_DIVERGED_VIEW);
+ executeQueryWithClientVersion(compatibleClientVersion,
QUERY_CREATE_DIVERGED_VIEW);
+ assertExpectedOutput(QUERY_CREATE_DIVERGED_VIEW);
+ }
+
+ @Test
+ public void
testCreateDivergedViewWithOldClientReadFromOldClientAfterUpgrade()
+ throws Exception {
+ // Create a base table, view and make it diverge from an old client
+ executeQueryWithClientVersion(compatibleClientVersion,
CREATE_DIVERGED_VIEW);
+ try (Connection conn = DriverManager.getConnection(url)) {
+ // Just connect with a new client to cause a metadata upgrade
+ }
+ // Query with an old client again
+ executeQueryWithClientVersion(compatibleClientVersion,
QUERY_CREATE_DIVERGED_VIEW);
+ assertExpectedOutput(QUERY_CREATE_DIVERGED_VIEW);
+ }
+
+ @Test
+ public void testCreateDivergedViewWithNewClientReadFromOldClient() throws
Exception {
+ executeQueriesWithCurrentVersion(CREATE_DIVERGED_VIEW);
+ executeQueryWithClientVersion(compatibleClientVersion,
QUERY_CREATE_DIVERGED_VIEW);
+ assertExpectedOutput(QUERY_CREATE_DIVERGED_VIEW);
+ }
+
+ @Test
+ public void testCreateDivergedViewWithNewClientReadFromNewClient() throws
Exception {
+ executeQueriesWithCurrentVersion(CREATE_DIVERGED_VIEW);
+ executeQueriesWithCurrentVersion(QUERY_CREATE_DIVERGED_VIEW);
+ assertExpectedOutput(QUERY_CREATE_DIVERGED_VIEW);
}
/**
* Scenario:
* 1. New Client connects to the updated server
* 2. New Client creates tables and inserts data
- * 3. Old Client reads the data inserted by the old client
+ * 3. Old Client reads the data inserted by the new client
*
* @throws Exception thrown if any errors encountered during query
execution or file IO
*/
@@ -188,8 +236,8 @@ public class BackwardCompatibilityIT {
public void testSelectWithOldClient() throws Exception {
// Insert data with new client and read with old client
executeQueriesWithCurrentVersion(CREATE_ADD);
- executeQueryWithClientVersion(compatibleClientVersion, QUERY);
- assertExpectedOutput(CREATE_ADD, QUERY);
+ executeQueryWithClientVersion(compatibleClientVersion,
QUERY_CREATE_ADD);
+ assertExpectedOutput(QUERY_CREATE_ADD);
}
/**
@@ -206,13 +254,13 @@ public class BackwardCompatibilityIT {
public void testSelectUpsertWithNewClient() throws Exception {
// Insert data with old client and read with new client
executeQueryWithClientVersion(compatibleClientVersion, CREATE_ADD);
- executeQueriesWithCurrentVersion(QUERY);
- assertExpectedOutput(CREATE_ADD, QUERY);
+ executeQueriesWithCurrentVersion(QUERY_CREATE_ADD);
+ assertExpectedOutput(QUERY_CREATE_ADD);
// Insert more data with new client and read with old client
executeQueriesWithCurrentVersion(ADD_DATA);
- executeQueryWithClientVersion(compatibleClientVersion, QUERY_MORE);
- assertExpectedOutput(ADD_DATA, QUERY_MORE);
+ executeQueryWithClientVersion(compatibleClientVersion, QUERY_ADD_DATA);
+ assertExpectedOutput(QUERY_ADD_DATA);
}
/**
@@ -229,13 +277,13 @@ public class BackwardCompatibilityIT {
public void testSelectUpsertWithOldClient() throws Exception {
// Insert data with new client and read with old client
executeQueriesWithCurrentVersion(CREATE_ADD);
- executeQueryWithClientVersion(compatibleClientVersion, QUERY);
- assertExpectedOutput(CREATE_ADD, QUERY);
+ executeQueryWithClientVersion(compatibleClientVersion,
QUERY_CREATE_ADD);
+ assertExpectedOutput(QUERY_CREATE_ADD);
// Insert more data with old client and read with new client
executeQueryWithClientVersion(compatibleClientVersion, ADD_DATA);
- executeQueriesWithCurrentVersion(QUERY_MORE);
- assertExpectedOutput(ADD_DATA, QUERY_MORE);
+ executeQueriesWithCurrentVersion(QUERY_ADD_DATA);
+ assertExpectedOutput(QUERY_ADD_DATA);
}
/**
@@ -251,13 +299,13 @@ public class BackwardCompatibilityIT {
public void testUpsertDeleteWithOldClient() throws Exception {
// Insert data with old client and read with new client
executeQueryWithClientVersion(compatibleClientVersion, CREATE_ADD);
- executeQueriesWithCurrentVersion(QUERY);
- assertExpectedOutput(CREATE_ADD, QUERY);
+ executeQueriesWithCurrentVersion(QUERY_CREATE_ADD);
+ assertExpectedOutput(QUERY_CREATE_ADD);
// Deletes with the old client
executeQueryWithClientVersion(compatibleClientVersion, ADD_DELETE);
executeQueryWithClientVersion(compatibleClientVersion,
QUERY_ADD_DELETE);
- assertExpectedOutput(ADD_DELETE, QUERY_ADD_DELETE);
+ assertExpectedOutput(QUERY_ADD_DELETE);
}
/**
@@ -273,13 +321,13 @@ public class BackwardCompatibilityIT {
public void testUpsertDeleteWithNewClient() throws Exception {
// Insert data with old client and read with new client
executeQueriesWithCurrentVersion(CREATE_ADD);
- executeQueryWithClientVersion(compatibleClientVersion, QUERY);
- assertExpectedOutput(CREATE_ADD, QUERY);
+ executeQueryWithClientVersion(compatibleClientVersion,
QUERY_CREATE_ADD);
+ assertExpectedOutput(QUERY_CREATE_ADD);
// Deletes with the new client
executeQueriesWithCurrentVersion(ADD_DELETE);
executeQueriesWithCurrentVersion(QUERY_ADD_DELETE);
- assertExpectedOutput(ADD_DELETE, QUERY_ADD_DELETE);
+ assertExpectedOutput(QUERY_ADD_DELETE);
}
private void checkForPreConditions() throws Exception {
@@ -312,10 +360,12 @@ public class BackwardCompatibilityIT {
.getResource(SQL_DIR + operation + SQL_EXTENSION);
assertNotNull(fileUrl);
cmdParams.add(new File(fileUrl.getFile()).getAbsolutePath());
- fileUrl =
BackwardCompatibilityIT.class.getClassLoader().getResource(RESULT_DIR);
+ fileUrl = BackwardCompatibilityIT.class.getClassLoader().getResource(
+ RESULTS_AND_GOLD_FILES_DIR);
assertNotNull(fileUrl);
- cmdParams.add(new File(fileUrl.getFile()).getAbsolutePath() + "/" +
- RESULT_PREFIX + operation + TEXT_EXTENSION);
+ String resultFilePath = new File(fileUrl.getFile()).getAbsolutePath()
+ "/" +
+ RESULT_PREFIX + operation + TEXT_EXTENSION;
+ cmdParams.add(resultFilePath);
cmdParams.add(System.getProperty(JAVA_TMP_DIR));
if (System.getProperty(MVN_HOME) != null) {
@@ -342,7 +392,9 @@ public class BackwardCompatibilityIT {
};
errorStreamThread.start();
p.waitFor();
- assertEquals(sb.toString(), 0, p.exitValue());
+ assertEquals(String.format("Executing the query failed%s. Check the
result file: %s",
+ sb.length() > 0 ? sb.append(" with : ").toString() : "",
resultFilePath),
+ 0, p.exitValue());
}
// Executes the SQL commands listed in the given operation file from the
sql_files directory
@@ -365,7 +417,8 @@ public class BackwardCompatibilityIT {
ResultSet rs;
String[] sqlCommands = sb.toString().split(";");
- URL fileUrl =
BackwardCompatibilityIT.class.getClassLoader().getResource(RESULT_DIR);
+ URL fileUrl =
BackwardCompatibilityIT.class.getClassLoader().getResource(
+ RESULTS_AND_GOLD_FILES_DIR);
assertNotNull(fileUrl);
final String resultFile = new
File(fileUrl.getFile()).getAbsolutePath() + "/" +
RESULT_PREFIX + operation + TEXT_EXTENSION;
@@ -417,18 +470,18 @@ public class BackwardCompatibilityIT {
// Compares the result file against the gold file to match for the
expected output
// for the given operation
- private void assertExpectedOutput(String gold, String result) throws
Exception {
+ private void assertExpectedOutput(String result) throws Exception {
List<String> resultFile = Lists.newArrayList();
List<String> goldFile = Lists.newArrayList();
String line;
try (BufferedReader resultFileReader = getBufferedReaderForResource(
- RESULT_DIR + RESULT_PREFIX + result + TEXT_EXTENSION)) {
+ RESULTS_AND_GOLD_FILES_DIR + RESULT_PREFIX + result +
TEXT_EXTENSION)) {
while ((line = resultFileReader.readLine()) != null) {
resultFile.add(line.trim());
}
}
try (BufferedReader goldFileReader = getBufferedReaderForResource(
- RESULT_DIR + "gold_query_" + gold + TEXT_EXTENSION)) {
+ RESULTS_AND_GOLD_FILES_DIR + GOLD_PREFIX + result +
TEXT_EXTENSION)) {
while ((line = goldFileReader.readLine()) != null) {
line = line.trim();
if ( !(line.isEmpty() || line.startsWith("*") ||
line.startsWith("/"))) {
@@ -440,7 +493,8 @@ public class BackwardCompatibilityIT {
// We take the first line in gold file and match against the result
file to exclude any
// other WARNING messages that comes as a result of the query execution
int index = resultFile.indexOf(goldFile.get(0));
+ assertNotEquals("Mismatch found between gold file and result file",
-1, index);
resultFile = resultFile.subList(index, resultFile.size());
- assertEquals(resultFile, goldFile);
+ assertEquals(goldFile, resultFile);
}
-}
\ No newline at end of file
+}
diff --git a/phoenix-core/src/it/resources/sql_files/query.sql
b/phoenix-core/src/it/resources/gold_files/gold_query_create_diverged_view.txt
similarity index 79%
copy from phoenix-core/src/it/resources/sql_files/query.sql
copy to
phoenix-core/src/it/resources/gold_files/gold_query_create_diverged_view.txt
index ebf154a..dddaf5e 100644
--- a/phoenix-core/src/it/resources/sql_files/query.sql
+++
b/phoenix-core/src/it/resources/gold_files/gold_query_create_diverged_view.txt
@@ -14,11 +14,13 @@
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
- */
-
-SELECT COUNT(*) from my_schema.my_table;
-SELECT * FROM my_schema.my_table;
-SELECT id from my_table_view;
+ */
-SELECT COUNT(*) from my_schema.my_table_immutable;
-SELECT * FROM my_schema.my_table_immutable;
+'COUNT(1)'
+'1'
+'COUNT(1)'
+'1'
+'A','B','C','D'
+'2','200','def','-20'
+'A','B','D','VA','VB'
+'2','200','-20','91','101'
diff --git a/phoenix-core/src/it/resources/sql_files/query.sql
b/phoenix-core/src/it/resources/sql_files/create_diverged_view.sql
similarity index 73%
copy from phoenix-core/src/it/resources/sql_files/query.sql
copy to phoenix-core/src/it/resources/sql_files/create_diverged_view.sql
index ebf154a..59dee14 100644
--- a/phoenix-core/src/it/resources/sql_files/query.sql
+++ b/phoenix-core/src/it/resources/sql_files/create_diverged_view.sql
@@ -15,10 +15,8 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-
-SELECT COUNT(*) from my_schema.my_table;
-SELECT * FROM my_schema.my_table;
-SELECT id from my_table_view;
-SELECT COUNT(*) from my_schema.my_table_immutable;
-SELECT * FROM my_schema.my_table_immutable;
+CREATE TABLE IF NOT EXISTS S.T (A INTEGER PRIMARY KEY, B INTEGER, C VARCHAR, D
INTEGER);
+CREATE VIEW IF NOT EXISTS S.V (VA INTEGER, VB INTEGER) AS SELECT * FROM S.T
WHERE B=200;
+UPSERT INTO S.V (A, B, C, D, VA, VB) VALUES (2, 200, 'def', -20, 91, 101);
+ALTER VIEW S.V DROP COLUMN C;
diff --git a/phoenix-core/src/it/resources/sql_files/query_more.sql
b/phoenix-core/src/it/resources/sql_files/query_add_data.sql
similarity index 100%
rename from phoenix-core/src/it/resources/sql_files/query_more.sql
rename to phoenix-core/src/it/resources/sql_files/query_add_data.sql
diff --git a/phoenix-core/src/it/resources/sql_files/query.sql
b/phoenix-core/src/it/resources/sql_files/query_create_add.sql
similarity index 100%
copy from phoenix-core/src/it/resources/sql_files/query.sql
copy to phoenix-core/src/it/resources/sql_files/query_create_add.sql
diff --git a/phoenix-core/src/it/resources/sql_files/query.sql
b/phoenix-core/src/it/resources/sql_files/query_create_diverged_view.sql
similarity index 79%
rename from phoenix-core/src/it/resources/sql_files/query.sql
rename to phoenix-core/src/it/resources/sql_files/query_create_diverged_view.sql
index ebf154a..e3d056c 100644
--- a/phoenix-core/src/it/resources/sql_files/query.sql
+++ b/phoenix-core/src/it/resources/sql_files/query_create_diverged_view.sql
@@ -15,10 +15,8 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-
-SELECT COUNT(*) from my_schema.my_table;
-SELECT * FROM my_schema.my_table;
-SELECT id from my_table_view;
-SELECT COUNT(*) from my_schema.my_table_immutable;
-SELECT * FROM my_schema.my_table_immutable;
+SELECT COUNT(*) FROM S.T;
+SELECT COUNT(*) FROM S.V;
+SELECT * FROM S.T;
+SELECT * FROM S.V;
diff --git
a/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/AddColumnMutator.java
b/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/AddColumnMutator.java
index 71b4827..4504666 100644
---
a/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/AddColumnMutator.java
+++
b/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/AddColumnMutator.java
@@ -292,7 +292,8 @@ public class AddColumnMutator implements ColumnMutator {
Region region,
List<ImmutableBytesPtr> invalidateList,
List<Region.RowLock>
locks,
- long clientTimeStamp)
{
+ long clientTimeStamp,
+ long clientVersion) {
byte[] tenantId = rowKeyMetaData[TENANT_ID_INDEX];
byte[] schemaName = rowKeyMetaData[SCHEMA_NAME_INDEX];
byte[] tableName = rowKeyMetaData[TABLE_NAME_INDEX];
diff --git
a/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/ColumnMutator.java
b/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/ColumnMutator.java
index aa090aa..9ef0515 100644
---
a/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/ColumnMutator.java
+++
b/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/ColumnMutator.java
@@ -51,7 +51,8 @@ public interface ColumnMutator {
List<Mutation> tableMetadata, Region region,
List<ImmutableBytesPtr> invalidateList,
List<Region.RowLock> locks,
- long
clientTimeStamp)
+ long
clientTimeStamp,
+ long
clientVersion)
throws IOException, SQLException;
/**
diff --git
a/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/DropColumnMutator.java
b/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/DropColumnMutator.java
index 6422475..3213744 100644
---
a/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/DropColumnMutator.java
+++
b/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/DropColumnMutator.java
@@ -67,6 +67,7 @@ import static
org.apache.phoenix.jdbc.PhoenixDatabaseMetaData.TABLE_NAME_INDEX;
import static org.apache.phoenix.jdbc.PhoenixDatabaseMetaData.TENANT_ID_INDEX;
import static
org.apache.phoenix.query.QueryConstants.DIVERGED_VIEW_BASE_COLUMN_COUNT;
import static org.apache.phoenix.util.SchemaUtil.getVarChars;
+import static org.apache.phoenix.util.ViewUtil.isViewDiverging;
public class DropColumnMutator implements ColumnMutator {
@@ -166,9 +167,8 @@ public class DropColumnMutator implements ColumnMutator {
Region region,
List<ImmutableBytesPtr> invalidateList,
List<Region.RowLock>
locks,
- long clientTimeStamp)
- throws SQLException {
-
+ long clientTimeStamp,
+ long clientVersion)
throws SQLException {
byte[] tenantId = rowKeyMetaData[TENANT_ID_INDEX];
byte[] schemaName = rowKeyMetaData[SCHEMA_NAME_INDEX];
byte[] tableName = rowKeyMetaData[TABLE_NAME_INDEX];
@@ -215,7 +215,8 @@ public class DropColumnMutator implements ColumnMutator {
deletePKColumn = columnToDelete.getFamilyName() ==
null;
if (isView) {
// if we are dropping a derived column add it to
the excluded
- // column list
+ // column list. Note that this is only done for
4.15+ clients
+ // since old clients do not have the isDerived
field
if (columnToDelete.isDerived()) {
mutation =
MetaDataUtil.cloneDeleteToPutAndAddColumn((Delete)
mutation, TABLE_FAMILY_BYTES,
LINK_TYPE_BYTES,
@@ -224,8 +225,7 @@ public class DropColumnMutator implements ColumnMutator {
iterator.set(mutation);
}
- if (table.getBaseColumnCount() !=
DIVERGED_VIEW_BASE_COLUMN_COUNT
- && columnToDelete.isDerived()) {
+ if (isViewDiverging(columnToDelete, table,
clientVersion)) {
// If the column being dropped is inherited
from the base table,
// then the view is about to diverge itself
from the base table.
// The consequence of this divergence is that
that any further
@@ -254,12 +254,8 @@ public class DropColumnMutator implements ColumnMutator {
columnToDelete);
}
// drop any indexes that need the column that is going
to be dropped
- tableAndDroppedColPairs.add(new Pair(table,
columnToDelete));
- } catch (ColumnFamilyNotFoundException e) {
- return new MetaDataProtocol.MetaDataMutationResult(
- MetaDataProtocol.MutationCode.COLUMN_NOT_FOUND,
- EnvironmentEdgeManager.currentTimeMillis(),
table, columnToDelete);
- } catch (ColumnNotFoundException e) {
+ tableAndDroppedColPairs.add(new Pair<>(table,
columnToDelete));
+ } catch (ColumnFamilyNotFoundException |
ColumnNotFoundException e) {
return new MetaDataProtocol.MetaDataMutationResult(
MetaDataProtocol.MutationCode.COLUMN_NOT_FOUND,
EnvironmentEdgeManager.currentTimeMillis(),
table, columnToDelete);
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 d960820..00ce50f 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
@@ -617,7 +617,7 @@ public class MetaDataEndpointImpl extends MetaDataProtocol
implements Coprocesso
&& table.getViewType() != ViewType.MAPPED) {
try (PhoenixConnection connection =
QueryUtil.getConnectionOnServer(env.getConfiguration()).unwrap(PhoenixConnection.class))
{
PTable pTable = PhoenixRuntime.getTableNoCache(connection,
table.getParentName().getString());
- table = ViewUtil.addDerivedColumnsFromParent(connection,
table, pTable);
+ table = ViewUtil.addDerivedColumnsFromParent(table,
pTable);
}
}
builder.setReturnCode(MetaDataProtos.MutationCode.TABLE_ALREADY_EXISTS);
@@ -2717,7 +2717,7 @@ public class MetaDataEndpointImpl extends
MetaDataProtocol implements Coprocesso
getParentPhysicalTableName(table), table.getType());
result = mutator.validateAndAddMetadata(table, rowKeyMetaData,
tableMetadata, region,
- invalidateList, locks, clientTimeStamp);
+ invalidateList, locks, clientTimeStamp, clientVersion);
// if the update mutation caused tables to be deleted, the
mutation code returned
// will be MutationCode.TABLE_ALREADY_EXISTS
if (result != null
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 42ead50..1d6924b 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
@@ -20,6 +20,9 @@ package org.apache.phoenix.schema;
import static com.google.common.collect.Sets.newLinkedHashSet;
import static com.google.common.collect.Sets.newLinkedHashSetWithExpectedSize;
import static
org.apache.phoenix.coprocessor.BaseScannerRegionObserver.RUN_UPDATE_STATS_ASYNC_ATTRIB;
+import static
org.apache.phoenix.coprocessor.MetaDataProtocol.PHOENIX_MAJOR_VERSION;
+import static
org.apache.phoenix.coprocessor.MetaDataProtocol.PHOENIX_MINOR_VERSION;
+import static
org.apache.phoenix.coprocessor.MetaDataProtocol.PHOENIX_PATCH_NUMBER;
import static org.apache.phoenix.coprocessor.tasks.IndexRebuildTask.INDEX_NAME;
import static
org.apache.phoenix.coprocessor.tasks.IndexRebuildTask.REBUILD_ALL;
import static
org.apache.phoenix.exception.SQLExceptionCode.INSUFFICIENT_MULTI_TENANT_COLUMNS;
@@ -168,6 +171,7 @@ import org.apache.phoenix.coprocessor.MetaDataProtocol;
import org.apache.phoenix.coprocessor.MetaDataProtocol.MetaDataMutationResult;
import org.apache.phoenix.coprocessor.MetaDataProtocol.MutationCode;
import org.apache.phoenix.coprocessor.MetaDataProtocol.SharedTableState;
+import org.apache.phoenix.hbase.index.util.VersionUtil;
import org.apache.phoenix.schema.stats.GuidePostsInfo;
import org.apache.phoenix.util.ViewUtil;
import org.apache.phoenix.util.JacksonUtil;
@@ -903,8 +907,8 @@ public class MetaDataClient {
if (!alwaysAddAncestorColumnsAndIndexes && !result.wasUpdated() &&
!parentResult.wasUpdated()) {
return false;
}
- PTable resolvedTable =
ViewUtil.addDerivedColumnsAndIndexesFromParent(connection, table, parentTable);
- result.setTable(resolvedTable);
+ result.setTable(ViewUtil.addDerivedColumnsAndIndexesFromParent(
+ connection, table, parentTable));
return true;
}
return false;
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 e073363..fcd09d3 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
@@ -811,7 +811,9 @@ public class PTableImpl implements PTable {
// When cloning table, ignore the salt column as it will be added back in
the constructor
public static List<PColumn> getColumnsToClone(PTable table) {
- return table.getBucketNum() == null ? table.getColumns() :
table.getColumns().subList(1, table.getColumns().size());
+ return table == null ? Collections.<PColumn> emptyList() :
+ (table.getBucketNum() == null ? table.getColumns() :
+ table.getColumns().subList(1,
table.getColumns().size()));
}
/**
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/util/ViewUtil.java
b/phoenix-core/src/main/java/org/apache/phoenix/util/ViewUtil.java
index bcafd1f..174f6b0 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/util/ViewUtil.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/util/ViewUtil.java
@@ -282,6 +282,15 @@ public class ViewUtil {
return view.getBaseColumnCount() ==
QueryConstants.DIVERGED_VIEW_BASE_COLUMN_COUNT;
}
+ public static boolean isViewDiverging(PColumn columnToDelete, PTable view,
+ long clientVersion) {
+ // If we are dropping a column from a pre-4.15 client, the only way to
know if the
+ // view is diverging is by comparing the base column count
+ return !isDivergedView(view) && (clientVersion <
MIN_SPLITTABLE_SYSTEM_CATALOG ?
+ columnToDelete.getPosition() < view.getBaseColumnCount() :
+ columnToDelete.isDerived());
+ }
+
/**
* Adds indexes of the parent table to inheritedIndexes if the index
contains all required columns
*/
@@ -370,16 +379,16 @@ public class ViewUtil {
*/
public static PTable
addDerivedColumnsAndIndexesFromParent(PhoenixConnection connection,
PTable table, PTable parentTable) throws SQLException {
- PTable pTable = addDerivedColumnsFromParent(connection, table,
parentTable);
+ PTable pTable = addDerivedColumnsFromParent(table, parentTable);
boolean hasIndexId = table.getViewIndexId() != null;
// For views :
if (!hasIndexId) {
// 1. need to resolve the views's own indexes so that any columns
added by ancestors are included
List<PTable> allIndexes = Lists.newArrayList();
- if (!pTable.getIndexes().isEmpty()) {
+ if (pTable !=null && pTable.getIndexes() !=null &&
!pTable.getIndexes().isEmpty()) {
for (PTable viewIndex : pTable.getIndexes()) {
- PTable resolvedViewIndex =
-
ViewUtil.addDerivedColumnsAndIndexesFromParent(connection, viewIndex, pTable);
+ PTable resolvedViewIndex =
ViewUtil.addDerivedColumnsAndIndexesFromParent(
+ connection, viewIndex, pTable);
if (resolvedViewIndex!=null)
allIndexes.add(resolvedViewIndex);
}
@@ -398,25 +407,29 @@ public class ViewUtil {
}
/**
- * Inherit all columns from the parent unless its an excluded column if
the same columns is present in the parent
- * and child (for table metadata created before PHOENIX-3534) we chose the
child column over the parent column
+ * Inherit all columns from the parent unless it's an excluded column.
+ * If the same column is present in the parent and child (for table
metadata created before
+ * PHOENIX-3534) we choose the child column over the parent column
* @return table with inherited columns
*/
- public static PTable addDerivedColumnsFromParent(PhoenixConnection
connection,
- PTable table,
PTable parentTable) throws SQLException {
+ public static PTable addDerivedColumnsFromParent(PTable view, PTable
parentTable)
+ throws SQLException {
// combine columns for view and view indexes
- boolean hasIndexId = table.getViewIndexId() != null;
- boolean isSalted = table.getBucketNum() != null;
- boolean isDiverged = isDivergedView(table);
+ boolean hasIndexId = view.getViewIndexId() != null;
+ boolean isSalted = view.getBucketNum() != null;
+ boolean isDiverged = isDivergedView(view);
+ boolean isDivergedViewCreatedPre4_15 = isDiverged;
List<PColumn> allColumns = Lists.newArrayList();
List<PColumn> excludedColumns = Lists.newArrayList();
// add my own columns first in reverse order
- List<PColumn> myColumns = table.getColumns();
+ List<PColumn> myColumns = view.getColumns();
// skip salted column as it will be created automatically
myColumns = myColumns.subList(isSalted ? 1 : 0, myColumns.size());
for (int i = myColumns.size() - 1; i >= 0; i--) {
PColumn pColumn = myColumns.get(i);
if (pColumn.isExcluded()) {
+ // Diverged views created pre-4.15 will not have
EXCLUDED_COLUMN linking rows
+ isDivergedViewCreatedPre4_15 = false;
excludedColumns.add(pColumn);
}
allColumns.add(pColumn);
@@ -426,13 +439,13 @@ public class ViewUtil {
// then remove the data columns that have not been dropped, so that we
get the columns that
// have been dropped
Map<PColumn, List<String>> indexRequiredDroppedDataColMap =
- Maps.newHashMapWithExpectedSize(table.getColumns().size());
+ Maps.newHashMapWithExpectedSize(view.getColumns().size());
if (hasIndexId) {
- int indexPosOffset = (isSalted ? 1 : 0) + (table.isMultiTenant() ?
1 : 0) + 1;
+ int indexPosOffset = (isSalted ? 1 : 0) + (view.isMultiTenant() ?
1 : 0) + 1;
ColumnNameTrackingExpressionCompiler expressionCompiler =
new ColumnNameTrackingExpressionCompiler();
- for (int i = indexPosOffset; i < table.getPKColumns().size(); i++)
{
- PColumn indexColumn = table.getPKColumns().get(i);
+ for (int i = indexPosOffset; i < view.getPKColumns().size(); i++) {
+ PColumn indexColumn = view.getPKColumns().get(i);
try {
expressionCompiler.reset();
String expressionStr =
IndexUtil.getIndexColumnExpressionStr(indexColumn);
@@ -446,8 +459,8 @@ public class ViewUtil {
}
}
- long maxTableTimestamp = table.getTimeStamp();
- int numPKCols = table.getPKColumns().size();
+ long maxTableTimestamp = view.getTimeStamp();
+ int numPKCols = view.getPKColumns().size();
// set the final table timestamp as the max timestamp of the view/view
index or its ancestors
maxTableTimestamp = Math.max(maxTableTimestamp,
parentTable.getTimeStamp());
if (hasIndexId) {
@@ -480,58 +493,10 @@ public class ViewUtil {
entry.getValue().remove(dataColumnName);
}
}
- } else {
- List<PColumn> currAncestorTableCols =
PTableImpl.getColumnsToClone(parentTable);
- if (currAncestorTableCols != null) {
- // add the ancestor columns in reverse order so that the final
column list
- // contains ancestor columns and then the view columns in the
right order
- for (int j = currAncestorTableCols.size() - 1; j >= 0; j--) {
- PColumn column = currAncestorTableCols.get(j);
- // for diverged views we always include pk columns of the
base table. We
- // have to include these pk columns to be able to support
adding pk
- // columns to the diverged view
- // we only include regular columns that were created
before the view
- // diverged
- if (isDiverged && column.getFamilyName() != null
- && column.getTimestamp() > table.getTimeStamp()) {
- continue;
- }
- // need to check if this column is in the list of excluded
(dropped)
- // columns of the view
- int existingIndex = excludedColumns.indexOf(column);
- if (existingIndex != -1) {
- // if it is, only exclude the column if was created
before the
- // column was dropped in the view in order to handle
the case where
- // a base table column is dropped in a view, then
dropped in the
- // base table and then added back to the base table
- if (column.getTimestamp() <=
excludedColumns.get(existingIndex)
- .getTimestamp()) {
- continue;
- }
- }
- if (column.isExcluded()) {
- excludedColumns.add(column);
- } else {
- int existingColumnIndex = allColumns.indexOf(column);
- if (existingColumnIndex != -1) {
- // for diverged views if the view was created
before
- // PHOENIX-3534 the parent table columns will be
present in the
- // view PTable (since the base column count is
- // QueryConstants.DIVERGED_VIEW_BASE_COLUMN_COUNT
we can't
- // filter them out) so we always pick the parent
column
- // for non diverged views if the same column
exists in a parent
- // and child, we keep the latest column
- PColumn existingColumn =
allColumns.get(existingColumnIndex);
- if (isDiverged || column.getTimestamp() >
existingColumn.getTimestamp()) {
- allColumns.remove(existingColumnIndex);
- allColumns.add(column);
- }
- } else {
- allColumns.add(column);
- }
- }
- }
- }
+ } else if (!isDivergedViewCreatedPre4_15) {
+ // For diverged views created by a pre-4.15 client, we don't need
to inherit columns
+ // from its ancestors
+ inheritColumnsFromParent(view, parentTable, isDiverged,
excludedColumns, allColumns);
}
// at this point indexRequiredDroppedDataColMap only contain the
columns required by a view
// index that have dropped
@@ -548,21 +513,13 @@ public class ViewUtil {
}
}
}
- // remove the excluded columns if the timestamp of the excludedColumn
is newer
- for (PColumn excludedColumn : excludedColumns) {
- int index = allColumns.indexOf(excludedColumn);
- if (index != -1) {
- if (allColumns.get(index).getTimestamp() <=
excludedColumn.getTimestamp()) {
- allColumns.remove(excludedColumn);
- }
- }
- }
+
List<PColumn> columnsToAdd = Lists.newArrayList();
int position = isSalted ? 1 : 0;
// allColumns contains the columns in the reverse order
for (int i = allColumns.size() - 1; i >= 0; i--) {
PColumn column = allColumns.get(i);
- if (table.getColumns().contains(column)) {
+ if (view.getColumns().contains(column)) {
// for views this column is not derived from an ancestor
columnsToAdd.add(new PColumnImpl(column, position++));
} else {
@@ -576,12 +533,12 @@ public class ViewUtil {
: columnsToAdd.size() - myColumns.size() + (isSalted ?
1 : 0);
// Inherit view-modifiable properties from the parent table/view if
the current view has
// not previously modified this property
- Long updateCacheFreq = (table.getType() != PTableType.VIEW ||
- table.hasViewModifiedUpdateCacheFrequency()) ?
- table.getUpdateCacheFrequency() :
parentTable.getUpdateCacheFrequency();
- Boolean useStatsForParallelization = (table.getType() !=
PTableType.VIEW ||
- table.hasViewModifiedUseStatsForParallelization()) ?
- table.useStatsForParallelization() :
parentTable.useStatsForParallelization();
+ long updateCacheFreq = (view.getType() != PTableType.VIEW ||
+ view.hasViewModifiedUpdateCacheFrequency()) ?
+ view.getUpdateCacheFrequency() :
parentTable.getUpdateCacheFrequency();
+ Boolean useStatsForParallelization = (view.getType() !=
PTableType.VIEW ||
+ view.hasViewModifiedUseStatsForParallelization()) ?
+ view.useStatsForParallelization() :
parentTable.useStatsForParallelization();
// When creating a PTable for views or view indexes, use the baseTable
PTable for attributes
// inherited from the physical base table.
@@ -590,7 +547,7 @@ public class ViewUtil {
// if a TableProperty is valid on a view and is mutable on a view, we
use the value set
// on the view if the view had previously modified the property,
otherwise we propagate the
// value from the base table (see PHOENIX-4763)
- PTable pTable = PTableImpl.builderWithColumns(table, columnsToAdd)
+ PTable pTable = PTableImpl.builderWithColumns(view, columnsToAdd)
.setImmutableRows(parentTable.isImmutableRows())
.setDisableWAL(parentTable.isWALDisabled())
.setMultiTenant(parentTable.isMultiTenant())
@@ -604,8 +561,7 @@ public class ViewUtil {
PTable.QualifierEncodingScheme.NON_ENCODED_QUALIFIERS
: parentTable.getEncodingScheme())
.setBaseColumnCount(baseTableColumnCount)
.setTimeStamp(maxTableTimestamp)
- .setExcludedColumns(excludedColumns == null ?
- ImmutableList.<PColumn>of() :
ImmutableList.copyOf(excludedColumns))
+ .setExcludedColumns(ImmutableList.copyOf(excludedColumns))
.setUpdateCacheFrequency(updateCacheFreq)
.setUseStatsForParallelization(useStatsForParallelization)
.build();
@@ -615,6 +571,85 @@ public class ViewUtil {
}
/**
+ * Inherit all columns from the parent unless it's an excluded column.
+ * If the same column is present in the parent and child
+ * (for table metadata created before PHOENIX-3534 or when
+ * {@link
org.apache.phoenix.query.QueryServices#ALLOW_SPLITTABLE_SYSTEM_CATALOG_ROLLBACK}
is
+ * enabled) we choose the child column over the parent column.
+ * Note that we don't need to call this method for views created before
4.15 since they
+ * already contain all the columns from their ancestors.
+ * @param view PTable of the view
+ * @param parentTable PTable of the view's parent
+ * @param isDiverged true if it is a diverged view
+ * @param excludedColumns list of excluded columns
+ * @param allColumns list of all columns. Initially this contains just the
columns in the view.
+ * We will populate inherited columns by adding them to
this list
+ */
+ static void inheritColumnsFromParent(PTable view, PTable parentTable,
+ boolean isDiverged, List<PColumn> excludedColumns, List<PColumn>
allColumns) {
+ List<PColumn> currAncestorTableCols =
PTableImpl.getColumnsToClone(parentTable);
+ if (currAncestorTableCols != null) {
+ // add the ancestor columns in reverse order so that the final
column list
+ // contains ancestor columns and then the view columns in the
right order
+ for (int j = currAncestorTableCols.size() - 1; j >= 0; j--) {
+ PColumn ancestorColumn = currAncestorTableCols.get(j);
+ // for diverged views we always include pk columns of the base
table. We
+ // have to include these pk columns to be able to support
adding pk
+ // columns to the diverged view.
+ // We only include regular columns that were created before
the view
+ // diverged.
+ if (isDiverged && ancestorColumn.getFamilyName() != null
+ && ancestorColumn.getTimestamp() >
view.getTimeStamp()) {
+ // If this is a diverged view, the ancestor column is not
a PK and the ancestor
+ // column was added after the view diverged, ignore this
ancestor column.
+ continue;
+ }
+ // need to check if this ancestor column is in the list of
excluded (dropped)
+ // columns of the view
+ int existingExcludedIndex =
excludedColumns.indexOf(ancestorColumn);
+ if (existingExcludedIndex != -1) {
+ // if it is, only exclude the ancestor column if it was
created before the
+ // column was dropped in the view in order to handle the
case where
+ // a base table column is dropped in a view, then dropped
in the
+ // base table and then added back to the base table
+ if (ancestorColumn.getTimestamp() <=
excludedColumns.get(existingExcludedIndex)
+ .getTimestamp()) {
+ continue;
+ }
+ }
+ // A diverged view from a pre-4.15 client won't ever go in
this case since
+ // isExcluded was introduced in 4.15. If this is a 4.15+
client, excluded columns
+ // will be identifiable via PColumn#isExcluded()
+ if (ancestorColumn.isExcluded()) {
+ excludedColumns.add(ancestorColumn);
+ } else {
+ int existingColumnIndex =
allColumns.indexOf(ancestorColumn);
+ if (existingColumnIndex != -1) {
+ // For non-diverged views, if the same column exists
in a parent and child,
+ // we keep the latest column.
+ PColumn existingColumn =
allColumns.get(existingColumnIndex);
+ if (!isDiverged && ancestorColumn.getTimestamp() >
existingColumn.getTimestamp()) {
+ allColumns.remove(existingColumnIndex);
+ allColumns.add(ancestorColumn);
+ }
+ } else {
+ allColumns.add(ancestorColumn);
+ }
+ }
+ }
+ }
+ // remove the excluded columns if the timestamp of the excludedColumn
is newer
+ for (PColumn excludedColumn : excludedColumns) {
+ int index = allColumns.indexOf(excludedColumn);
+ if (index != -1) {
+ if (allColumns.get(index).getTimestamp() <=
excludedColumn.getTimestamp()) {
+ allColumns.remove(excludedColumn);
+ }
+ }
+ }
+ }
+
+ /**
* See PHOENIX-4763. If we are modifying any table-level properties that
are mutable on a view,
* we mark these cells in SYSTEM.CATALOG with tags to indicate that this
view property should
* not be kept in-sync with the base table and so we shouldn't propagate
the base table's
