[
https://issues.apache.org/jira/browse/PHOENIX-1409?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14270549#comment-14270549
]
James Taylor commented on PHOENIX-1409:
---------------------------------------
Thanks, [~samarthjain]. Here's some feedback:
- Does this test need to be run in it's own cluster, as it's not overriding any
properties? If not, let's keep it as-is.
{code}
-public class AlterTableIT extends BaseHBaseManagedTimeIT {
+public class AlterTableIT extends BaseOwnClusterHBaseManagedTimeIT {
public static final String SCHEMA_NAME = "";
public static final String DATA_TABLE_NAME = "T";
public static final String INDEX_TABLE_NAME = "I";
@@ -61,7 +65,13 @@ public class AlterTableIT extends BaseHBaseManagedTimeIT {
public static final String DATA_TABLE_FULL_NAME =
SchemaUtil.getTableName(SCHEMA_NAME, "T");
public static final String INDEX_TABLE_FULL_NAME =
SchemaUtil.getTableName(SCHEMA_NAME, "I");
public static final String LOCAL_INDEX_TABLE_FULL_NAME =
SchemaUtil.getTableName(SCHEMA_NAME, "LI");
-
+
+ @BeforeClass
+ public static void doSetup() throws Exception {
+ Map<String, String> props = Collections.emptyMap();
+ setUpTestDriver(new ReadOnlyProps(props.entrySet().iterator()));
+ }
+
{code}
- Shouldn't the following old test still pass? If so, let's leave as-is and
create a new one for other stuff we want to test. In general, a change of a
unit test is a potential red flag for a change in behavior IMO.
{code}
+ public void testSetPropertyAndAddColumnForNewColumnFamily() throws
Exception {
Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
Connection conn = DriverManager.getConnection(getUrl(), props);
-
- String ddl = "CREATE TABLE test_table " +
+ String ddl = "CREATE TABLE SETPROPNEWCF " +
" (a_string varchar not null, col1 integer" +
" CONSTRAINT pk PRIMARY KEY (a_string))\n";
try {
- conn.createStatement().execute(ddl);
-
- conn.createStatement().execute("ALTER TABLE TEST_TABLE ADD
col2 integer IN_MEMORY=true");
-
- HTableInterface htable1 =
conn.unwrap(PhoenixConnection.class).getQueryServices().getTable(Bytes.toBytes("TEST_TABLE"));
- HTableDescriptor htableDesciptor1 =
htable1.getTableDescriptor();
- HColumnDescriptor hcolumnDescriptor1 =
htableDesciptor1.getFamily(QueryConstants.DEFAULT_COLUMN_FAMILY_BYTES);
- assertNotNull(hcolumnDescriptor1);
-
- try {
-
- conn.createStatement().execute("ALTER TABLE TEST_TABLE SET
IN_MEMORY=false");
{code}
- Do we have a test for the case where a property is set which isn't a Phoenix
property or a HColumnDescriptor property? The behavior at CREATE time for this
is that it'll end up as an HTableDescriptor property. Can we add a test for
this and make sure that ALTER TABLE matches this behavior?
{code}
+ if (isHTableProperty(propName)) {
+ // Can't have a column family name for a property
that's an HTableProperty
+ if
(!family.equals(QueryConstants.ALL_FAMILY_PROPERTIES_KEY)) {
+ throw new
SQLExceptionInfo.Builder(SQLExceptionCode.COLUMN_FAMILY_NOT_ALLOWED_TABLE_PROPERTY)
+ .setMessage("Column Family: " + family + ",
Property: " + propName).build()
+ .buildException();
+ }
+ tableProps.put(propName, prop.getSecond());
+ } else {
+ if (TableProperty.isPhoenixTableProperty(propName)) {
+ TableProperty.valueOf(propName).validate(true,
!family.equals(QueryConstants.ALL_FAMILY_PROPERTIES_KEY), table.getType());
+ if (propName.equals(TTL)) {
+ // Even though TTL is really a HColumnProperty
we treat it specially.
+ // We enforce that all column families have
the same TTL.
+ commonFamilyProps.put(propName,
prop.getSecond());
+ } else if
(propName.equals(PTable.IS_IMMUTABLE_ROWS_PROP_NAME)) {
+ isImmutableRowsProp =
(Boolean)prop.getSecond();
+ } else if
(propName.equals(PhoenixDatabaseMetaData.MULTI_TENANT)) {
+ multiTenantProp = (Boolean)prop.getSecond();
+ } else if (propName.equals(DISABLE_WAL)) {
+ disableWALProp = (Boolean)prop.getSecond();
+ }
+ } else {
+ if (isHColumnProperty(propName)) {
+ if
(family.equals(QueryConstants.ALL_FAMILY_PROPERTIES_KEY)) {
+ commonFamilyProps.put(propName,
prop.getSecond());
+ } else {
+ colFamilyPropsMap.put(propName,
prop.getSecond());
+ }
+ } else {
+ // invalid property - neither of HTableProp,
HColumnProp or PhoenixTableProp
+ // FIXME: This isn't getting triggered as
currently a property gets evaluated
+ // as HTableProp if its neither HColumnProp or
PhoenixTableProp.
+ throw new
SQLExceptionInfo.Builder(SQLExceptionCode.SET_UNSUPPORTED_PROP_ON_ALTER_TABLE)
+ .setMessage("Column Family: " + family + ",
Property: " + propName).build()
+ .buildException();
+ }
+ }
{code}
> Allow ALTER TABLE <table> SET command to update HTableDescriptor and
> HColumnDescriptor properties
> -------------------------------------------------------------------------------------------------
>
> Key: PHOENIX-1409
> URL: https://issues.apache.org/jira/browse/PHOENIX-1409
> Project: Phoenix
> Issue Type: Improvement
> Affects Versions: 4.2
> Reporter: James Taylor
> Assignee: Alicia Ying Shu
> Attachments: PHOENIX-1409-v3.patch, PHOENIX-1409-v6.patch,
> Phoenix-1409-v1.patch, Phoenix-1409-v4-2.patch, Phoenix-1409-v4.patch,
> Phoenix-1409-v5.patch, Phoenix-1409.patch, WIP.patch, phoenix-1409-v2.patch
>
>
> Once PHOENIX-1408 is fixed, we should allow HTableDescriptor and
> HColumnDescriptor properties through the ALTER TABLE <table> SET command.
> It'd just be a matter of passing these properties through the existing
> methods, as we support this for CREATE TABLE.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
