Joe McDonnell has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15990 )

Change subject: IMPALA-9673: Add external warehouse dir variable in E2E test
......................................................................


Patch Set 5:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/15990/5//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/15990/5//COMMIT_MSG@7
PS5, Line 7: Add external warehouse dir variable in E2E test
           :
           : In minicluster, we have default values of hive.create.as.acid and
           : hive.create.as.insert.only which are false. So by default hive 
creates
           : external type table located in external warehouse directory.
           : Due to HIVE-22995, desc db returns external warehouse directory.
I think we should mention in the commit message that we are updating the CDP 
build number. Then, the other parts can explain why these other changes are 
needed to adapt to changes in Hive.

Larger context, Hive made changes to its "create database" statement splitting 
out location and managed location. I think we will want a followup JIRA to make 
Impala's functionality match (i.e. being able to take a managed location in 
"create database" and also display it back when using "describe database"). 
Given that "location" from Hive's perspective is the external location, this 
fix still makes sense on its own.

There's one semantic that we know we want to maintain. And that is that if we 
do "create database ... location X" and then "describe database ...", then the 
location is X. I don't see a test like that. If there isn't one, then I think 
we should add one.


http://gerrit.cloudera.org:8080/#/c/15990/4/testdata/datasets/functional/functional_schema_template.sql
File testdata/datasets/functional/functional_schema_template.sql:

http://gerrit.cloudera.org:8080/#/c/15990/4/testdata/datasets/functional/functional_schema_template.sql@2805
PS4, Line 2805: CREATE MATERIALIZED VIEW IF NOT EXISTS 
{db_name}{db_suffix}.{table_name}
> The command exists previously, I just moved to end of file, which has done
I agree with Sahil. It would be good to have a comment here describing why it 
is later in the file than expected.

My understanding is that the issue comes from the materialized view being 
created before any managed table is created in that database. My guess is that 
it has something to do with creating the database's managed directory. Moving 
this statement down pushes it past the create statements for some managed 
tables (insert_only_transactional_bucketed_table, bucketed_table) and avoids 
the issue.

In general, it would be nice for this to be as close to its original location 
as possible. I believe it should work to put this after bucketed_table and 
before uncomp_src_alltypes.



--
To view, visit http://gerrit.cloudera.org:8080/15990
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I57926babf4caebfd365e6be65a399f12ea68687f
Gerrit-Change-Number: 15990
Gerrit-PatchSet: 5
Gerrit-Owner: Xiaomeng Zhang <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Joe McDonnell <[email protected]>
Gerrit-Reviewer: Sahil Takiar <[email protected]>
Gerrit-Reviewer: Xiaomeng Zhang <[email protected]>
Gerrit-Comment-Date: Mon, 01 Jun 2020 05:02:01 +0000
Gerrit-HasComments: Yes

Reply via email to