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 <[email protected]> Tested-by: Impala Public Jenkins <[email protected]> Project: http://git-wip-us.apache.org/repos/asf/impala/repo Commit: http://git-wip-us.apache.org/repos/asf/impala/commit/62885d8d Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/62885d8d Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/62885d8d Branch: refs/heads/2.x Commit: 62885d8dedf9994707ba3d30c6b7d2d640551702 Parents: e114778 Author: Fredy Wijaya <[email protected]> Authored: Thu Apr 19 23:51:56 2018 -0500 Committer: Impala Public Jenkins <[email protected]> Committed: Fri Apr 20 20:17:58 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/62885d8d/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/62885d8d/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,
