HIVE-25331: Create database query doesn't create MANAGEDLOCATION dire… #2478
Conversation
kgyrtkirk
added
tests pending
tests unstable
tests failed
tests passed
and removed
tests pending
tests unstable
tests failed
labels
| # Queries ran by both MiniLlapLocal and MiniTez | ||
| minitez.query.files.shared=\ | ||
| compressed_skip_header_footer_aggr.q,\ | ||
| create_database.q,\ |
There was a problem hiding this comment.
let's use TestMiniLlapLocalCliDriver if the patch has nothing specific to tez container mode
There was a problem hiding this comment.
Got it. Good to know :)
There was a problem hiding this comment.
Removed the Tez tests and squashed the commits.
kgyrtkirk
added
tests pending
tests failed
and removed
tests passed
tests pending
labels
kgyrtkirk
added
tests pending
tests failed
and removed
tests failed
tests pending
labels
kgyrtkirk
added
tests failed
tests pending
tests unstable
and removed
tests pending
tests failed
labels
kgyrtkirk
added
tests pending
tests passed
and removed
tests unstable
tests pending
labels
| POSTHOOK: type: DESCDATABASE | ||
| POSTHOOK: Input: database:newdb | ||
| newdb location/in/test hive_test_user USER | ||
| #### A masked pattern was here #### |
There was a problem hiding this comment.
we need to double-check in what way the output changed which triggered a q.out masking
There was a problem hiding this comment.
The managedlocation is like "file:/home/robbie/hive/itests/qtest/target/localfs/warehouse/newdb.db" which matches "file:" in QOutProcessor.maskIfContains.
There was a problem hiding this comment.
I see, I created a followup ticket for this: HIVE-25473 (in order to get useful info back)
There was a problem hiding this comment.
Actually the original output is like:
newdb location/in/test file:/home/robbie/hive/itests/qtest/target/localfs/warehouse/newdb.db hive_test_user USER
The managedlocation is not empty. Because of the pattern "file:/", QOutProcessor masks the whole line.
| replicatedDbName.toLowerCase() + ".db").toUri().getPath()); | ||
| } else { | ||
| Assert.assertNotEquals(managedCustLocOnSrc, null); | ||
| Assert.assertEquals(replDatabase.getManagedLocationUri(), null); |
There was a problem hiding this comment.
@pkumarsinha: could you please confirm if removing this assertion won't hide any problems with replication?
this patch takes care of setting managed location uri if it's not set by create database
There was a problem hiding this comment.
@ujc714 Rather than removing, I think we should assert that replDatabase.getManagedLocationUri() is in default warehouse location, i.e it should be equal to new Path(replica.warehouseRoot, replicatedDbName.toLowerCase() + ".db")
There was a problem hiding this comment.
@pkumarsinha Made the change as you suggested :)
| Table Type: EXTERNAL_TABLE | ||
| Table Parameters: | ||
| COLUMN_STATS_ACCURATE {\"BASIC_STATS\":\"true\"} | ||
| COLUMN_STATS_ACCURATE {\"BASIC_STATS\":\"true\",\"COLUMN_STATS\":{\"id\":\"true\",\"value\":\"true\"}} |
There was a problem hiding this comment.
is this because of the patch? looks unrelated
There was a problem hiding this comment.
You are right. The two iceberg qfiles are updated in the master branch. I rebased and recommitted the changes again.
kgyrtkirk
added
tests pending
tests passed
and removed
tests passed
tests pending
labels
kgyrtkirk
added
tests pending
tests passed
and removed
tests passed
tests pending
labels
|
change looks good to me, all tests passed +1 |
…ctory (apache#2478) (Robbie Zhang reviewed by Laszlo Bodor and Pravin Kumar Sinha)
…ctory (apache#2478) (Robbie Zhang reviewed by Laszlo Bodor and Pravin Kumar Sinha)
4 participants
What changes were proposed in this pull request?
Use a default directory for MANAGEDLOCATION if it's not assigned in CREATE DATABASE query.
Why are the changes needed?
HMS doesn't create MANAGEDLOCATION directory if it's NULL. If we run a CTAS query immediately after the CREATE DATABASE query and the staging directory is not under the MANAGEDLOCATION directory, the CTAS query will fail in MOVE task.
Does this PR introduce any user-facing change?
No.
How was this patch tested?
mvn test -Dtest=TestMiniLlapLocalCliDriver -Dqfile=create_database.q