You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by sa...@apache.org on 2018/04/23 17:39:22 UTC
[4/9] impala git commit: IMPALA-6896: NullPointerException in
DESCRIBE FORMATTED on views
IMPALA-6896: NullPointerException in DESCRIBE FORMATTED on views
This patch fixes an issue where in ALTER VIEW the storage descriptor
is created with a new instance instead of reusing the existing
storage descriptor. This causes an issue where some HMS attributes
become nullable causing a NullPointerException.
The patch also differentiates between updating view attributes for
CREATE VIEW and ALTER VIEW.
Testing:
- Ran all front-end tests
- Added a new end-to-end test
- Ran the all end-to-end metadata tests
Change-Id: Ica2fb0c4f4b09cdf36eeb4911a1cbe7e98381d9e
Reviewed-on: http://gerrit.cloudera.org:8080/10132
Reviewed-by: Alex Behm <al...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
Project: http://git-wip-us.apache.org/repos/asf/impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/impala/commit/34b2f218
Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/34b2f218
Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/34b2f218
Branch: refs/heads/master
Commit: 34b2f218411155de076d3e5463376c585bcff4f3
Parents: 8e86678
Author: Fredy Wijaya <fw...@cloudera.com>
Authored: Thu Apr 19 23:51:56 2018 -0500
Committer: Impala Public Jenkins <im...@cloudera.com>
Committed: Fri Apr 20 11:01:44 2018 +0000
----------------------------------------------------------------------
.../impala/service/CatalogOpExecutor.java | 29 +++++++++++++-----
tests/metadata/test_ddl.py | 32 ++++++++++++++++++++
2 files changed, 54 insertions(+), 7 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/impala/blob/34b2f218/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java b/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
index 87513aa..fdee124 100644
--- a/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
+++ b/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
@@ -702,7 +702,7 @@ public class CatalogOpExecutor {
}
// Set the altered view attributes and update the metastore.
- setViewAttributes(params, msTbl);
+ setAlterViewAttributes(params, msTbl);
if (LOG.isTraceEnabled()) {
LOG.trace(String.format("Altering view %s", tableName));
}
@@ -1802,7 +1802,7 @@ public class CatalogOpExecutor {
// Create new view.
org.apache.hadoop.hive.metastore.api.Table view =
new org.apache.hadoop.hive.metastore.api.Table();
- setViewAttributes(params, view);
+ setCreateViewAttributes(params, view);
LOG.trace(String.format("Creating view %s", tableName));
if (!createTable(view, params.if_not_exists, null, response)) {
addSummary(response, "View already exists.");
@@ -1901,9 +1901,10 @@ public class CatalogOpExecutor {
}
/**
- * Sets the given params in the metastore table as appropriate for a view.
+ * Sets the given params in the metastore table as appropriate for a
+ * create view operation.
*/
- private void setViewAttributes(TCreateOrAlterViewParams params,
+ private void setCreateViewAttributes(TCreateOrAlterViewParams params,
org.apache.hadoop.hive.metastore.api.Table view) {
view.setTableType(TableType.VIRTUAL_VIEW.toString());
view.setViewOriginalText(params.getOriginal_view_def());
@@ -1912,12 +1913,11 @@ public class CatalogOpExecutor {
view.setTableName(params.getView_name().getTable_name());
view.setOwner(params.getOwner());
if (view.getParameters() == null) view.setParameters(new HashMap<String, String>());
- if (params.isSetComment() && params.getComment() != null) {
+ if (params.isSetComment() && params.getComment() != null) {
view.getParameters().put("comment", params.getComment());
}
-
- // Add all the columns to a new storage descriptor.
StorageDescriptor sd = new StorageDescriptor();
+ // Add all the columns to a new storage descriptor.
sd.setCols(buildFieldSchemaList(params.getColumns()));
// Set a dummy SerdeInfo for Hive.
sd.setSerdeInfo(new SerDeInfo());
@@ -1925,6 +1925,21 @@ public class CatalogOpExecutor {
}
/**
+ * Sets the given params in the metastore table as appropriate for an
+ * alter view operation.
+ */
+ private void setAlterViewAttributes(TCreateOrAlterViewParams params,
+ org.apache.hadoop.hive.metastore.api.Table view) {
+ view.setViewOriginalText(params.getOriginal_view_def());
+ view.setViewExpandedText(params.getExpanded_view_def());
+ if (params.isSetComment() && params.getComment() != null) {
+ view.getParameters().put("comment", params.getComment());
+ }
+ // Add all the columns to a new storage descriptor.
+ view.getSd().setCols(buildFieldSchemaList(params.getColumns()));
+ }
+
+ /**
* Appends one or more columns to the given table, optionally replacing all existing
* columns.
*/
http://git-wip-us.apache.org/repos/asf/impala/blob/34b2f218/tests/metadata/test_ddl.py
----------------------------------------------------------------------
diff --git a/tests/metadata/test_ddl.py b/tests/metadata/test_ddl.py
index 27748bd..0830060 100644
--- a/tests/metadata/test_ddl.py
+++ b/tests/metadata/test_ddl.py
@@ -26,6 +26,7 @@ from tests.common.parametrize import UniqueDatabase
from tests.common.skip import SkipIf, SkipIfADLS, SkipIfLocal
from tests.common.test_dimensions import create_single_exec_option_dimension
from tests.util.filesystem_utils import WAREHOUSE, IS_HDFS, IS_S3, IS_ADLS
+from tests.common.impala_cluster import ImpalaCluster
# Validates DDL statements (create, drop)
class TestDdlStatements(TestDdlBase):
@@ -361,6 +362,37 @@ class TestDdlStatements(TestDdlBase):
| 01:SCAN HDFS [functional.alltypes b]
00:SCAN HDFS [functional.alltypestiny a]""" in '\n'.join(plan.data)
+ def test_views_describe(self, vector, unique_database):
+ # IMPALA-6896: Tests that altered views can be described by all impalads.
+ impala_cluster = ImpalaCluster()
+ impalads = impala_cluster.impalads
+ first_client = impalads[0].service.create_beeswax_client()
+ try:
+ self.execute_query_expect_success(first_client,
+ "create view {0}.test_describe_view as "
+ "select * from functional.alltypes"
+ .format(unique_database), {'sync_ddl': 1})
+ self.execute_query_expect_success(first_client,
+ "alter view {0}.test_describe_view as "
+ "select * from functional.alltypesagg"
+ .format(unique_database))
+ finally:
+ first_client.close()
+
+ for impalad in impalads:
+ client = impalad.service.create_beeswax_client()
+ try:
+ while True:
+ result = self.execute_query_expect_success(
+ client, "describe formatted {0}.test_describe_view"
+ .format(unique_database))
+ if any("select * from functional.alltypesagg" in s.lower()
+ for s in result.data):
+ break
+ time.sleep(1)
+ finally:
+ client.close()
+
@UniqueDatabase.parametrize(sync_ddl=True)
def test_functions_ddl(self, vector, unique_database):
self.run_test_case('QueryTest/functions-ddl', vector, use_db=unique_database,