You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by st...@apache.org on 2021/03/30 01:30:46 UTC
[impala] 02/04: IMPALA-10597: Enable setting 'iceberg.file_format'
This is an automated email from the ASF dual-hosted git repository.
stigahuang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
commit dbc2fc14d86f84b25670ee9af7cedbbc3cd18b9f
Author: Zoltan Borok-Nagy <bo...@cloudera.com>
AuthorDate: Fri Mar 19 18:57:48 2021 +0100
IMPALA-10597: Enable setting 'iceberg.file_format'
Currently we prohibit setting the following properties:
* iceberg.catalog
* iceberg.catalog_location
* iceberg.file_format
* iceberg.table_identifier
This patch enables setting 'iceberg.file_format', therefore if
a table was created by another engine, but using HiveCatalog,
we'll be able to set the data file format to the proper value
and make the table readable by Impala. Setting the other
properties are not needed for HiveCatalog tables.
If the table wasn't created by HiveCatalog, then we cannot load the
table, therefore we cannot invoke any ALTER TABLE statement at all.
In that case we need to create an external table.
If the table already contains data files, then Impala checks if
all of them have the proper file format. If not, the ALTER TABLE
statement fails.
Before this patch a CREATE TABLE statement accepted any string
for 'iceberg.file_format', and in case of invalid file formats the
frontend silently used Parquet. This patch also adds a check to only
allow valid file formats.
Testing:
* added e2e test
Change-Id: I4b3506be4562a1ace3e6435867aadb3bdde7a8e2
Reviewed-on: http://gerrit.cloudera.org:8080/17207
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
.../analysis/AlterTableSetTblProperties.java | 37 ++++++++++++++++++++-
.../apache/impala/analysis/CreateTableStmt.java | 3 ++
.../org/apache/impala/catalog/IcebergTable.java | 1 +
.../queries/QueryTest/iceberg-alter.test | 38 ++++++++++++++++++++++
.../queries/QueryTest/iceberg-negative.test | 19 ++++++++---
5 files changed, 92 insertions(+), 6 deletions(-)
diff --git a/fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java b/fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java
index b934b88..3091188 100644
--- a/fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java
+++ b/fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java
@@ -24,6 +24,7 @@ import java.util.Map;
import org.apache.avro.SchemaParseException;
import org.apache.hadoop.hive.metastore.api.hive_metastoreConstants;
import org.apache.hadoop.hive.serde2.avro.AvroSerdeUtils;
+import org.apache.iceberg.DataFile;
import org.apache.impala.authorization.AuthorizationConfig;
import org.apache.impala.catalog.FeFsTable;
import org.apache.impala.catalog.FeHBaseTable;
@@ -32,6 +33,7 @@ import org.apache.impala.catalog.FeKuduTable;
import org.apache.impala.catalog.FeTable;
import org.apache.impala.catalog.IcebergTable;
import org.apache.impala.catalog.KuduTable;
+import org.apache.impala.catalog.TableLoadingException;
import org.apache.impala.common.AnalysisException;
import org.apache.impala.common.Pair;
import org.apache.impala.thrift.TAlterTableParams;
@@ -41,6 +43,7 @@ import org.apache.impala.thrift.TSortingOrder;
import org.apache.impala.thrift.TTablePropertyType;
import org.apache.impala.util.AvroSchemaParser;
import org.apache.impala.util.AvroSchemaUtils;
+import org.apache.impala.util.IcebergUtil;
import org.apache.impala.util.MetaStoreUtil;
import com.google.common.base.Preconditions;
@@ -147,11 +150,13 @@ public class AlterTableSetTblProperties extends AlterTableSetStmt {
private void analyzeIcebergTable(Analyzer analyzer) throws AnalysisException {
//Cannot set these properties related to metadata
- icebergPropertyCheck(IcebergTable.ICEBERG_FILE_FORMAT);
icebergPropertyCheck(IcebergTable.ICEBERG_CATALOG);
icebergPropertyCheck(IcebergTable.ICEBERG_CATALOG_LOCATION);
icebergPropertyCheck(IcebergTable.ICEBERG_TABLE_IDENTIFIER);
icebergPropertyCheck(IcebergTable.METADATA_LOCATION);
+ if (tblProperties_.containsKey(IcebergTable.ICEBERG_FILE_FORMAT)) {
+ icebergTableFormatCheck(tblProperties_.get(IcebergTable.ICEBERG_FILE_FORMAT));
+ }
}
private void icebergPropertyCheck(String property) throws AnalysisException {
@@ -161,6 +166,36 @@ public class AlterTableSetTblProperties extends AlterTableSetStmt {
}
}
+ private void icebergTableFormatCheck(String fileformat) throws AnalysisException {
+ Preconditions.checkState(getTargetTable() instanceof FeIcebergTable);
+ Preconditions.checkState(fileformat != null);
+ if (IcebergUtil.getIcebergFileFormat(fileformat) == null) {
+ throw new AnalysisException("Invalid fileformat for Iceberg table: " + fileformat);
+ }
+ try {
+ FeIcebergTable iceTable = (FeIcebergTable)getTargetTable();
+ List<DataFile> dataFiles = IcebergUtil.getIcebergDataFiles(iceTable,
+ new ArrayList<>());
+ if (dataFiles.isEmpty()) return;
+ DataFile firstFile = dataFiles.get(0);
+ String errorMsg = "Attempt to set Iceberg data file format to %s, but found data " +
+ "file %s with file format %s.";
+ if (!firstFile.format().name().equalsIgnoreCase(fileformat)) {
+ throw new AnalysisException(String.format(errorMsg, fileformat, firstFile.path(),
+ firstFile.format().name()));
+ }
+ //TODO(IMPALA-10610): Iceberg tables with mixed file formats are not readable.
+ for (DataFile df : dataFiles) {
+ if (df.format() != firstFile.format()) {
+ throw new AnalysisException(String.format(errorMsg, fileformat, df.path(),
+ df.format().name()));
+ }
+ }
+ } catch (TableLoadingException e) {
+ throw new AnalysisException(e);
+ }
+ }
+
/**
* Check that Avro schema provided in avro.schema.url or avro.schema.literal is valid
* Json and contains only supported Impala types. If both properties are set, then
diff --git a/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java b/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
index 1477536..752a226 100644
--- a/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
+++ b/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
@@ -605,6 +605,9 @@ public class CreateTableStmt extends StatementBase {
IcebergTable.ICEBERG_STORAGE_HANDLER);
String fileformat = getTblProperties().get(IcebergTable.ICEBERG_FILE_FORMAT);
+ if (fileformat != null && IcebergUtil.getIcebergFileFormat(fileformat) == null) {
+ throw new AnalysisException("Invalid fileformat for Iceberg table: " + fileformat);
+ }
if (fileformat == null || fileformat.isEmpty()) {
putGeneratedKuduProperty(IcebergTable.ICEBERG_FILE_FORMAT, "parquet");
}
diff --git a/fe/src/main/java/org/apache/impala/catalog/IcebergTable.java b/fe/src/main/java/org/apache/impala/catalog/IcebergTable.java
index 0cfce43..9f3cc95 100644
--- a/fe/src/main/java/org/apache/impala/catalog/IcebergTable.java
+++ b/fe/src/main/java/org/apache/impala/catalog/IcebergTable.java
@@ -262,6 +262,7 @@ public class IcebergTable extends Table implements FeIcebergTable {
loadSchemaFromIceberg(metadata);
// Loading hdfs table after loaded schema from Iceberg,
// in case we create external Iceberg table skipping column info in sql.
+ icebergFileFormat_ = Utils.getIcebergFileFormat(msTbl);
hdfsTable_.load(false, msClient, msTable_, true, true, false, null, null, reason);
pathHashToFileDescMap_ = Utils.loadAllPartition(this);
loadAllColumnStats(msClient);
diff --git a/testdata/workloads/functional-query/queries/QueryTest/iceberg-alter.test b/testdata/workloads/functional-query/queries/QueryTest/iceberg-alter.test
index cfa980c..938e11d 100644
--- a/testdata/workloads/functional-query/queries/QueryTest/iceberg-alter.test
+++ b/testdata/workloads/functional-query/queries/QueryTest/iceberg-alter.test
@@ -111,3 +111,41 @@ SELECT * FROM iceberg_rename;
---- CATCH
Could not resolve table reference: 'iceberg_rename'
====
+---- QUERY
+CREATE TABLE iceberg_changing_fileformats (i int)
+STORED AS ICEBERG
+TBLPROPERTIES('iceberg.file_format'='orc');
+DESCRIBE FORMATTED iceberg_changing_fileformats;
+---- RESULTS: VERIFY_IS_SUBSET
+'','iceberg.file_format ','orc '
+---- TYPES
+string, string, string
+====
+---- QUERY
+ALTER TABLE iceberg_changing_fileformats set TBLPROPERTIES('iceberg.file_format'='parquet');
+DESCRIBE FORMATTED iceberg_changing_fileformats;
+---- RESULTS: VERIFY_IS_SUBSET
+'','iceberg.file_format ','parquet '
+---- TYPES
+string, string, string
+====
+---- QUERY
+INSERT INTO iceberg_changing_fileformats values (123);
+SELECT * FROM iceberg_changing_fileformats;
+---- RESULTS
+123
+---- TYPES
+INT
+====
+---- QUERY
+ALTER TABLE iceberg_changing_fileformats set TBLPROPERTIES('iceberg.file_format'='ORC');
+---- CATCH
+Attempt to set Iceberg data file format to ORC
+====
+---- QUERY
+DESCRIBE FORMATTED iceberg_changing_fileformats;
+---- RESULTS: VERIFY_IS_SUBSET
+'','iceberg.file_format ','parquet '
+---- TYPES
+string, string, string
+====
diff --git a/testdata/workloads/functional-query/queries/QueryTest/iceberg-negative.test b/testdata/workloads/functional-query/queries/QueryTest/iceberg-negative.test
index 38a61a6..d6f0969 100644
--- a/testdata/workloads/functional-query/queries/QueryTest/iceberg-negative.test
+++ b/testdata/workloads/functional-query/queries/QueryTest/iceberg-negative.test
@@ -188,11 +188,6 @@ ALTER TABLE iceberg_table_hadoop_catalog RENAME TO iceberg_table_hadoop_catalog_
UnsupportedOperationException: Cannot rename Iceberg tables that use 'hadoop.catalog' as catalog.
====
---- QUERY
-ALTER TABLE iceberg_table_hadoop_catalog set TBLPROPERTIES('iceberg.file_format'='orc');
----- CATCH
-AnalysisException: Changing the 'iceberg.file_format' table property is not supported for Iceberg table.
-====
----- QUERY
ALTER TABLE iceberg_table_hadoop_catalog set TBLPROPERTIES('iceberg.catalog'='hadoop.tables');
---- CATCH
AnalysisException: Changing the 'iceberg.catalog' table property is not supported for Iceberg table.
@@ -328,3 +323,17 @@ STORED AS ICEBERG;
---- CATCH
Syntax error in line
====
+---- QUERY
+CREATE TABLE iceberg_wrong_fileformat (i int)
+STORED AS ICEBERG
+TBLPROPERTIES('iceberg.file_format'='or');
+---- CATCH
+Invalid fileformat for Iceberg table: or
+====
+---- QUERY
+CREATE TABLE iceberg_set_wrong_fileformat (i int)
+STORED AS ICEBERG;
+ALTER TABLE iceberg_set_wrong_fileformat SET TBLPROPERTIES ('iceberg.file_format'='parq');
+---- CATCH
+Invalid fileformat for Iceberg table: parq
+====