KurtYoung commented on a change in pull request #15155:
URL: https://github.com/apache/flink/pull/15155#discussion_r611144110
##########
File path:
flink-connectors/flink-connector-hive/src/main/java/org/apache/flink/table/catalog/hive/HiveCatalog.java
##########
@@ -822,18 +843,15 @@ public void createPartition(
checkNotNull(partitionSpec, "CatalogPartitionSpec cannot be null");
checkNotNull(partition, "Partition cannot be null");
- boolean isGeneric =
-
Boolean.valueOf(partition.getProperties().get(CatalogPropertiesUtil.IS_GENERIC));
-
- if (isGeneric) {
- throw new CatalogException("Currently only supports non-generic
CatalogPartition");
- }
-
Table hiveTable = getHiveTable(tablePath);
+ ensurePartitionedTable(tablePath, hiveTable);
- ensureTableAndPartitionMatch(hiveTable, partition);
+ // partition inherits 'is_generic' from table
Review comment:
wrong comment? we don't use is_generic anymore
##########
File path:
flink-connectors/flink-connector-hive/src/main/java/org/apache/flink/connectors/hive/HiveTableFactory.java
##########
@@ -51,11 +51,10 @@ public TableSource
createTableSource(TableSourceFactory.Context context) {
CatalogTable table = checkNotNull(context.getTable());
Preconditions.checkArgument(table instanceof CatalogTableImpl);
- boolean isGeneric =
-
Boolean.parseBoolean(table.getOptions().get(CatalogPropertiesUtil.IS_GENERIC));
+ boolean isHiveTable = HiveCatalog.isHiveTable(table.getOptions());
- // temporary table doesn't have the IS_GENERIC flag but we still
consider it generic
- if (!isGeneric && !context.isTemporary()) {
+ // temporary table is considered generic
+ if (isHiveTable && !context.isTemporary()) {
throw new UnsupportedOperationException(
"Hive table should be resolved by
HiveDynamicTableFactory.");
Review comment:
throw a more meaningful message
##########
File path:
flink-connectors/flink-connector-hive/src/main/java/org/apache/flink/table/catalog/hive/HiveDatabaseUtil.java
##########
@@ -44,7 +43,9 @@ static Database instantiateHiveDatabase(String databaseName,
CatalogDatabase dat
Map<String, String> properties = database.getProperties();
- boolean isGeneric = isGenericForCreate(properties);
+ properties.putIfAbsent(CatalogPropertiesUtil.IS_GENERIC, "true");
Review comment:
why set is_generic=true here?
##########
File path:
flink-connectors/flink-connector-hive/src/main/java/org/apache/flink/table/catalog/hive/HiveCatalog.java
##########
@@ -587,9 +588,16 @@ public void alterTable(
newCatalogTable.getOptions(),
hiveTable.getSd());
}
+ } else {
+ hiveTable =
+ HiveTableUtil.alterTableViaCatalogBaseTable(
+ tablePath, newCatalogTable, hiveTable, hiveConf);
}
- disallowChangeIsGeneric(isGeneric,
isGenericForGet(hiveTable.getParameters()));
+ disallowChangeIsGeneric(isHiveTable,
isHiveTable(hiveTable.getParameters()));
Review comment:
modify the method name
##########
File path:
flink-connectors/flink-connector-hive/src/main/java/org/apache/flink/connectors/hive/HiveTableFactory.java
##########
@@ -68,11 +67,10 @@ public TableSink createTableSink(TableSinkFactory.Context
context) {
CatalogTable table = checkNotNull(context.getTable());
Preconditions.checkArgument(table instanceof CatalogTableImpl);
- boolean isGeneric =
-
Boolean.parseBoolean(table.getOptions().get(CatalogPropertiesUtil.IS_GENERIC));
+ boolean isHiveTable = HiveCatalog.isHiveTable(table.getOptions());
// temporary table doesn't have the IS_GENERIC flag but we still
consider it generic
- if (!isGeneric && !context.isTemporary()) {
+ if (isHiveTable && !context.isTemporary()) {
throw new UnsupportedOperationException(
"Hive table should be resolved by
HiveDynamicTableFactory.");
Review comment:
throw a more meaningful message
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
