EXTERNAL property overrides tableType #115
Conversation
A table is external in the following two cases: - It is declared as EXTERNAL_TABLE and EXTERNAL is not false - EXTERNAL is true This will allow to alter tables through DDL statements, which was not possible with previous behaviors.
| else { | ||
| String externalParam = params.get(EXTERNAL_TABLE_PROP); | ||
| if (externalParam != null) { | ||
| return Boolean.parseBoolean(params.get(EXTERNAL_TABLE_PROP)); |
There was a problem hiding this comment.
Nit: swap out params.get(EXTERNAL_TABLE_PROP) for externalParam?
There was a problem hiding this comment.
Nit 2: what about having flat if-case here?
if (params == null) {
...
} else if (params.get(EXTERNAL_TABLE_PROP) == null ) {
...
} else ...
There was a problem hiding this comment.
Ok for first comment. For the second if I actually follow the previous style it would be more sth like this:
if (params == null) {
return xxx
}
String externalParam = params.get(EXTERNAL_TABLE_PROP);
if (externalParams != null) {
return yyy;
}
return zzz;
In any case, both cannot be done at the same time
| @@ -1472,22 +1472,25 @@ public static FieldSchema getFieldSchemaFromTypeInfo(String fieldName, | |||
| "generated by TypeInfoUtils.getFieldSchemaFromTypeInfo"); | |||
| } | |||
|
|
|||
| private static boolean isTableTypeExternal(Table table) { | |||
| String tableType = table.getTableType(); | |||
| return tableType != null && tableType.equals(TableType.EXTERNAL_TABLE.name()); | |||
There was a problem hiding this comment.
Another way to do this is to return:
TableType.EXTERNAL_TABLE.name().equals(table.getTableType())
| return isTableTypeExternal(table); | ||
| } | ||
| else { | ||
| String externalParam = params.get(EXTERNAL_TABLE_PROP); |
There was a problem hiding this comment.
shorter version:
if (params == null || !params.containsKey(EXTERNAL_TABLE_PROP)) {
return isTableTypeExternal(table);
} else {
return Boolean.parseBoolean(params.get(EXTERNAL_TABLE_PROP));
}
A table is external in the following two cases:
tableTypeisEXTERNAL_TABLEand propertyEXTERNALis not falseEXTERNALis trueThis will allow to alter tables through DDL statements, which was not possible with previous behaviors.