[
https://issues.apache.org/jira/browse/PHOENIX-4575?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16347793#comment-16347793
]
James Taylor commented on PHOENIX-4575:
---------------------------------------
Thanks for the patch. Here's some feedback:
- Can we define these new properties attributes in QueryServices and their
defaults in QueryServicesOptions instead?
- How about instead of META_DATA_KEEP_DELETED_CELLS we use a name of
DEFAULT_SYSTEM_KEEP_DELETED_CELLS_ATTRIB
(phoenix.system.default.keep.deleted.cells) as the property name and
DEFAULT_SYSTEM_KEEP_DELETED_CELLS = false?
- How about a name of DEFAULT_SYSTEM_MAX_VERSIONS_ATTRIB
(phoenix.system.default.max.versions) as the property name and
DEFAULT_SYSTEM_MAX_VERSIONS = 1?
- I think you want to set the value of KEEP_DELETED_CELLS to the value of the
new DEFAULT_SYSTEM_KEEP_DELETED_CELLS_ATTRIB (i.e. you don't want to set it to
the string of property):
{code}
- HConstants.VERSIONS + "=" +
MetaDataProtocol.DEFAULT_MAX_META_DATA_VERSIONS + ",\n" +
- HColumnDescriptor.KEEP_DELETED_CELLS + "=" +
MetaDataProtocol.DEFAULT_META_DATA_KEEP_DELETED_CELLS + ",\n"+
+ HConstants.VERSIONS + "=" +
MetaDataProtocol.MAX_META_DATA_VERSIONS + ",\n" +
+ HColumnDescriptor.KEEP_DELETED_CELLS + "=" +
MetaDataProtocol.META_DATA_KEEP_DELETED_CELLS + ",\n"+
{code}
- Instead, put %s placeholders in the QueryConstant constant like this:
{code}
public static final String CREATE_TABLE_METADATA =
// Do not use IF NOT EXISTS as we sometimes catch the
TableAlreadyExists
// exception and add columns to the SYSTEM.TABLE dynamically.
"CREATE TABLE " + SYSTEM_CATALOG_SCHEMA + ".\"" +
SYSTEM_CATALOG_TABLE + "\"(\n" +
...
HConstants.VERSIONS + "=" + %s + ",\n" +
HColumnDescriptor.KEEP_DELETED_CELLS + "=" + %s + ",\n" +
...
{code}
and modify this method in ConnectionQueryServiceImpl (and
ConnectionlessQueryServiceImpl):
{code}
// Available for testing
protected String getSystemCatalogDML() {
return String.format(QueryConstants.CREATE_TABLE_METADATA,
props.getInt(DEFAULT_SYSTEM_MAX_VERSIONS_ATTRIB,
DEFAULT_SYSTEM_MAX_VERSIONS),
props.getBoolean(DEFAULT_SYSTEM_KEEP_DELETED_CELLS_ATTRIB,
DEFAULT_SYSTEM_KEEP_DELETED_CELLS));
}
{code}
- Do the same as above for the CREATE_FUNCTION_METADATA constant.
- Remove KEEP_DELETED_CELLS and VERSIONS from CREATE_SEQUENCE_METADATA and
CREATE_STATS_TABLE_METADATA as it doesn't make sense for keep deleted cells to
be on for these tables.
- Remove this code from ConnectionQueryServicesImpl as it'll like cause issues
(and doesn't make sense):
{code}
if(props.get(QueryServices.DEFAULT_KEEP_DELETED_CELLS_ATTRIB) !=
null){
columnDesc.setKeepDeletedCells(props.getBoolean(
QueryServices.DEFAULT_KEEP_DELETED_CELLS_ATTRIB,
QueryServicesOptions.DEFAULT_KEEP_DELETED_CELLS));
}
{code}
- Remove the QueryServices.DEFAULT_KEEP_DELETED_CELLS_ATTRIB and
QueryServicesOptions.DEFAULT_KEEP_DELETED_CELLS as they won't be referenced any
more.
- Remove these properties from MetaDataProtocol:
{code}
public static final int DEFAULT_MAX_META_DATA_VERSIONS = 1000;
public static final boolean DEFAULT_META_DATA_KEEP_DELETED_CELLS = true;
public static final int DEFAULT_MAX_STAT_DATA_VERSIONS = 1;
public static final boolean DEFAULT_STATS_KEEP_DELETED_CELLS = false;
{code}
> Phoenix metadata KEEP_DELETED_CELLS and VERSIONS should be property driven
> --------------------------------------------------------------------------
>
> Key: PHOENIX-4575
> URL: https://issues.apache.org/jira/browse/PHOENIX-4575
> Project: Phoenix
> Issue Type: New Feature
> Reporter: Mujtaba Chohan
> Assignee: Mujtaba Chohan
> Priority: Major
> Attachments: PHOENIX-4575.patch
>
>
> This is to cater for circumstances where we need to alter state of
> KEEP_DELETED_CELLS/VERSION on Phoenix meta tables.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
