[
https://issues.apache.org/jira/browse/PHOENIX-2791?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15277011#comment-15277011
]
James Taylor commented on PHOENIX-2791:
---------------------------------------
Thanks for the patch, [~tdsilva]. A few questions:
- Was the following change intentional? We pass in the table type based on the
statement so that if you do an ALTER VIEW on an entity that's actually a table,
you'll get an error.
{code}
- seqNum = incrementTableSeqNum(table,
statement.getTableType(), columnDefs.size(), isTransactional,
updateCacheFrequency, isImmutableRows, disableWAL, multiTenant, storeNulls);
+ seqNum = incrementTableSeqNum(table, table.getType(),
columnDefs.size(), isTransactional, updateCacheFrequency, isImmutableRows,
disableWAL, multiTenant, storeNulls);
{code}
- This change will affect the handling of a ConcurrentTableMutationException.
In this case, you need to re-resolve the table so that you pick up the new
columns and try your change again. This would happen if client1 adds columns
after client2 has resolved the table. When client2 adds it's columns, it's
working off a different snapshot, but doesn't know it yet. When the table is
re-resolved, client2 will pick up the new table definition and re-generate the
column position and table column count info. I thought we had a unit test like
this. Probably best to have this retry more too, as this scenario may be more
likely given how messages are processed. It currently only retries once - maybe
5 times would be better.
{code}
while (true) {
- ColumnResolver resolver = FromCompiler.getResolver(statement,
connection);
- table = resolver.getTables().get(0).getTable();
int nIndexes = table.getIndexes().size();
{code}
- Was the removal of this code intentional (and if so why)? This code
determines whether a property is an HBase table or column family property.
{code}
@@ -1574,6 +1679,7 @@ public class MetaDataClient {
timestamp = TransactionUtil.getTableTimestamp(connection,
transactional);
storeNulls = parent.getStoreNulls();
isImmutableRows = parent.isImmutableRows();
+ isAppendOnlySchema = parent.isAppendOnlySchema();
// Index on view
// TODO: Can we support a multi-tenant index directly on a
multi-tenant
// table instead of only a view? We don't have anywhere to put
the link
@@ -1630,21 +1736,6 @@ public class MetaDataClient {
pkName = pkConstraint.getName();
}
- Map<String,Object> tableProps =
Maps.newHashMapWithExpectedSize(statement.getProps().size());
- Map<String,Object> commonFamilyProps =
Maps.newHashMapWithExpectedSize(statement.getProps().size() + 1);
- // Somewhat hacky way of determining if property is for
HColumnDescriptor or HTableDescriptor
- HColumnDescriptor defaultDescriptor = new
HColumnDescriptor(QueryConstants.DEFAULT_COLUMN_FAMILY_BYTES);
- if (!statement.getProps().isEmpty()) {
- Collection<Pair<String,Object>> props =
statement.getProps().get(QueryConstants.ALL_FAMILY_PROPERTIES_KEY);
- for (Pair<String,Object> prop : props) {
- if (defaultDescriptor.getValue(prop.getFirst()) == null) {
- tableProps.put(prop.getFirst(), prop.getSecond());
- } else {
- commonFamilyProps.put(prop.getFirst(),
prop.getSecond());
- }
- }
- }
-
{code}
- Why is the new property only being set on a TABLE? For our use case, it'll be
set on the VIEW, no? I see from your tests, you seem to set it on both, so I'm
confused about this check. Is tableType the parent table type and this is
causing a VIEW to default based on it's parent TABLE or VIEW?
{code}
+
+ if (tableType == PTableType.TABLE) {
+ Boolean isAppendOnlySchemaProp = (Boolean)
TableProperty.APPEND_ONLY_SCHEMA.getValue(tableProps);
+ isAppendOnlySchema = isAppendOnlySchemaProp!=null ?
isAppendOnlySchemaProp : false;
+ }
{code}
- Related to the above, does setting APPEND_ONLY_SCHEMA=true on a TABLE impact
the setting when a VIEW over that table is created? It looks like it must be
specified on the TABLE and the VIEW? Or maybe you give an error if it's
specified differently which might make sense. FWIW, we do have properties that
get set on the TABLE and automatically propagated to their VIEWs.
> Support append only schema declaration
> --------------------------------------
>
> Key: PHOENIX-2791
> URL: https://issues.apache.org/jira/browse/PHOENIX-2791
> Project: Phoenix
> Issue Type: Sub-task
> Reporter: James Taylor
> Assignee: Thomas D'Silva
> Labels: argus
> Fix For: 4.8.0
>
> Attachments: PHOENIX-2791.patch
>
>
> If we know in advance that columns will only be added to but never removed
> from a schema, we can prevent the RPC from the client to the server when the
> client already has all columns declared in the CREATE TABLE/VIEW IF NOT
> EXISTS. To enable this, we can add an APPEND_ONLY_SCHEMA boolean flag to
> SYSTEM.CATALOG. Or another potential name would be IMMUTABLE_SCHEMA to match
> IMMUTABLE_ROWS?
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
