[
https://issues.apache.org/jira/browse/PHOENIX-3534?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16533110#comment-16533110
]
ASF GitHub Bot commented on PHOENIX-3534:
-----------------------------------------
Github user JamesRTaylor commented on a diff in the pull request:
https://github.com/apache/phoenix/pull/303#discussion_r200206109
--- Diff:
phoenix-core/src/main/java/org/apache/phoenix/coprocessor/MetaDataEndpointImpl.java
---
@@ -586,48 +590,359 @@ public void getTable(RpcController controller,
GetTableRequest request,
builder.setMutationTime(minNonZerodisableIndexTimestamp - 1);
}
}
-
- if (table.getTimeStamp() != tableTimeStamp) {
+ // the PTable of views and indexes on views might get updated
because a column is added to one of
+ // their parents (this won't change the timestamp)
+ if (table.getType()!=PTableType.TABLE || table.getTimeStamp()
!= tableTimeStamp) {
builder.setTable(PTableImpl.toProto(table));
}
done.run(builder.build());
- return;
} catch (Throwable t) {
logger.error("getTable failed", t);
ProtobufUtil.setControllerException(controller,
ServerUtil.createIOException(SchemaUtil.getTableName(schemaName, tableName),
t));
}
}
+ /**
+ * Used to add the columns present the ancestor hierarchy to the
PTable of the given view or
+ * view index
+ * @param table PTable of the view or view index
+ * @param skipAddingIndexes if true the returned PTable won't include
indexes
+ * @param skipAddingParentColumns if true the returned PTable won't
include columns derived from
+ * ancestor tables
+ * @param lockedAncestorTable ancestor table table that is being
mutated (as we won't be able to
+ * resolve this table as its locked)
+ */
+ private Pair<PTable, MetaDataProtos.MutationCode>
combineColumns(PTable table, long timestamp,
+ int clientVersion, boolean skipAddingIndexes, boolean
skipAddingParentColumns,
+ PTable lockedAncestorTable) throws SQLException, IOException {
+ boolean hasIndexId = table.getViewIndexId() != null;
+ if (table.getType() != PTableType.VIEW && !hasIndexId) {
+ return new Pair<PTable, MetaDataProtos.MutationCode>(table,
+ MetaDataProtos.MutationCode.TABLE_ALREADY_EXISTS);
+ }
+ if (!skipAddingParentColumns) {
+ table =
+ addDerivedColumnsFromAncestors(table, timestamp,
clientVersion,
+ lockedAncestorTable);
+ if (table==null) {
+ return new Pair<PTable, MetaDataProtos.MutationCode>(table,
+ MetaDataProtos.MutationCode.TABLE_NOT_FOUND);
+ }
+ // we need to resolve the indexes of views (to get ensure they
also have all the columns
+ // derived from their ancestors)
+ if (!skipAddingIndexes && !table.getIndexes().isEmpty()) {
+ List<PTable> indexes =
Lists.newArrayListWithExpectedSize(table.getIndexes().size());
+ for (PTable index : table.getIndexes()) {
+ byte[] tenantIdBytes =
+ index.getTenantId() == null ?
ByteUtil.EMPTY_BYTE_ARRAY
+ : index.getTenantId().getBytes();
+ PTable latestIndex =
+ doGetTable(tenantIdBytes,
index.getSchemaName().getBytes(),
+ index.getTableName().getBytes(),
timestamp, null, clientVersion, true,
+ false, lockedAncestorTable);
+ if (latestIndex == null) {
+ throw new TableNotFoundException(
+ "Could not find index table while
combining columns "
+ + index.getTableName().getString()
+ " with tenant id "
+ + index.getTenantId());
+ }
+ indexes.add(latestIndex);
+ }
+ table = PTableImpl.makePTable(table, table.getTimeStamp(),
indexes);
+ }
+ }
+
+ MetaDataProtos.MutationCode mutationCode =
+ table != null ?
MetaDataProtos.MutationCode.TABLE_ALREADY_EXISTS
+ : MetaDataProtos.MutationCode.TABLE_NOT_FOUND;
+ return new Pair<PTable, MetaDataProtos.MutationCode>(table,
mutationCode);
+ }
+
+
+ private PTable addDerivedColumnsFromAncestors(PTable table, long
timestamp,
+ int clientVersion, PTable lockedAncestorTable) throws
IOException, SQLException, TableNotFoundException {
+ // combine columns for view and view indexes
+ byte[] tenantId =
+ table.getTenantId() != null ?
table.getTenantId().getBytes()
+ : ByteUtil.EMPTY_BYTE_ARRAY;
+ byte[] schemaName = table.getSchemaName().getBytes();
+ byte[] tableName = table.getTableName().getBytes();
+ String fullTableName =
SchemaUtil.getTableName(table.getSchemaName().getString(),
+ table.getTableName().getString());
+ boolean hasIndexId = table.getViewIndexId() != null;
+ boolean isSalted = table.getBucketNum() != null;
+ if (table.getType() != PTableType.VIEW && !hasIndexId) {
+ return table;
+ }
+ boolean isDiverged = isDivergedView(table);
+ // here you combine columns from the parent tables the logic is as
follows, if the PColumn
+ // is in the EXCLUDED_COLUMNS remove it, otherwise priority of
keeping duplicate columns is
+ // child -> parent
+ List<TableInfo> ancestorList = Lists.newArrayList();
+ TableViewFinderResult viewFinderResult = new
TableViewFinderResult();
+ if (PTableType.VIEW == table.getType()) {
+ findAncestorViews(tenantId, schemaName, tableName,
viewFinderResult,
+ table.isNamespaceMapped());
+ } else { // is a view index
+ findAncestorViewsOfIndex(tenantId, schemaName, tableName,
viewFinderResult,
+ table.isNamespaceMapped());
+ }
+ if (viewFinderResult.getResults().isEmpty()) {
+ // no need to combine columns for local indexes on regular
tables
+ return table;
+ }
+ for (TableInfo viewInfo : viewFinderResult.getResults()) {
+ ancestorList.add(viewInfo);
+ }
+ List<PColumn> allColumns = Lists.newArrayList();
+ List<PColumn> excludedColumns = Lists.newArrayList();
+ // add my own columns first in reverse order
+ List<PColumn> myColumns = table.getColumns();
+ for (int i = myColumns.size() - 1; i >= 0; i--) {
+ PColumn pColumn = myColumns.get(i);
+ if (pColumn.isExcluded()) {
+ excludedColumns.add(pColumn);
+ } else if (!pColumn.equals(SaltingUtil.SALTING_COLUMN)) {
+ // skip salted column as it will be added from the base
table columns
+ allColumns.add(pColumn);
+ }
+ }
+
+ // initialize map from with indexed expression to list of required
data columns
+ // 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());
+ if (hasIndexId) {
+ int indexPosOffset = (isSalted ? 1 : 0) +
(table.isMultiTenant() ? 1 : 0) + 1;
+ ColumnNameTrackingExpressionCompiler expressionCompiler =
+ new ColumnNameTrackingExpressionCompiler();
+ for (int i = indexPosOffset; i < table.getPKColumns().size();
i++) {
+ PColumn indexColumn = table.getPKColumns().get(i);
+ try {
+ expressionCompiler.reset();
+ String expressionStr =
IndexUtil.getIndexColumnExpressionStr(indexColumn);
+ ParseNode parseNode =
SQLParser.parseCondition(expressionStr);
+ parseNode.accept(expressionCompiler);
+ indexRequiredDroppedDataColMap.put(indexColumn,
+
Lists.newArrayList(expressionCompiler.getDataColumnNames()));
+ } catch (SQLException e) {
+ throw new RuntimeException(e); // Impossible
+ }
+ }
+ }
+
+ // now go up from child to parent all the way to the base table:
+ PTable baseTable = null;
+ long maxTableTimestamp = -1;
+ int numPKCols = table.getPKColumns().size();
+ for (int i = 0; i < ancestorList.size(); i++) {
+ TableInfo parentTableInfo = ancestorList.get(i);
+ PTable pTable = null;
+ String fullParentTableName =
SchemaUtil.getTableName(parentTableInfo.getSchemaName(),
+ parentTableInfo.getTableName());
+ PName parentTenantId =
+ (parentTableInfo.getTenantId() != null &&
parentTableInfo.getTenantId().length!=0)
+ ?
PNameFactory.newName(parentTableInfo.getTenantId()) : null;
+ PTableKey pTableKey = new PTableKey(parentTenantId,
fullParentTableName);
+ // if we already have the PTable of an ancestor that has been
locked, no need to look up
+ // the table
+ if (lockedAncestorTable != null &&
lockedAncestorTable.getKey().equals(pTableKey)) {
+ pTable = lockedAncestorTable;
+ } else {
+ // if we are currently combining columns for a view index
and are looking up its
+ // ancestors we do not add the indexes to the ancestor
PTable (or else we end up in
+ // a circular loop)
+ // we also don't need to add parent columns of the
ancestors as we combine columns
+ // from all ancestors
+ pTable =
+ doGetTable(parentTableInfo.getTenantId(),
parentTableInfo.getSchemaName(),
+ parentTableInfo.getTableName(), timestamp,
null, clientVersion, hasIndexId,
+ true, null);
+ }
+ if (pTable == null) {
+ throw new ParentTableNotFoundException(parentTableInfo,
fullTableName);
+ } else {
+ // only combine columns for view indexes (and not local
indexes on regular tables
+ // which also have a viewIndexId)
+ if (i == 0 && hasIndexId && pTable.getType() !=
PTableType.VIEW) {
+ return table;
+ }
+ if (TABLE.equals(pTable.getType())) {
+ baseTable = pTable;
+ }
+ // set the final table timestamp as the max timestamp of
the view/view index or its
+ // ancestors
+ maxTableTimestamp = Math.max(maxTableTimestamp,
pTable.getTimeStamp());
+ if (hasIndexId) {
+ // add all pk columns of parent tables to indexes
+ for (PColumn column : pTable.getPKColumns()) {
+ // don't add the salt column of ancestor tables
for view indexes
+ if (column.equals(SaltingUtil.SALTING_COLUMN) ||
column.isExcluded()) {
--- End diff --
Same comment as before - we should be able to match based on the column
being the first column. Note that the salt column would only be in the base
physical table.
> Support multi region SYSTEM.CATALOG table
> -----------------------------------------
>
> Key: PHOENIX-3534
> URL: https://issues.apache.org/jira/browse/PHOENIX-3534
> Project: Phoenix
> Issue Type: Bug
> Reporter: James Taylor
> Assignee: Thomas D'Silva
> Priority: Major
> Fix For: 5.0.0, 4.15.0
>
> Attachments: PHOENIX-3534.patch
>
>
> Currently Phoenix requires that the SYSTEM.CATALOG table is single region
> based on the server-side row locks being held for operations that impact a
> table and all of it's views. For example, adding/removing a column from a
> base table pushes this change to all views.
> As an alternative to making the SYSTEM.CATALOG transactional (PHOENIX-2431),
> when a new table is created we can do a lazy cleanup of any rows that may be
> left over from a failed DDL call (kudos to [~lhofhansl] for coming up with
> this idea). To implement this efficiently, we'd need to also do PHOENIX-2051
> so that we can efficiently find derived views.
> The implementation would rely on an optimistic concurrency model based on
> checking our sequence numbers for each table/view before/after updating. Each
> table/view row would be individually locked for their change (metadata for a
> view or table cannot span regions due to our split policy), with the sequence
> number being incremented under lock and then returned to the client.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
