[ 
https://issues.apache.org/jira/browse/PHOENIX-3534?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16540415#comment-16540415
 ] 

ASF GitHub Bot commented on PHOENIX-3534:
-----------------------------------------

Github user twdsilva commented on a diff in the pull request:

    https://github.com/apache/phoenix/pull/303#discussion_r201776046
  
    --- 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 --
    
    I will change this.


> 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)

Reply via email to