Hive: New test case for writing to the table with projection and column specification #2121
Conversation
|
@marton-bod, @lcspinter: Could you please review? Thanks, |
mr/src/test/java/org/apache/iceberg/mr/hive/TestHiveIcebergStorageHandlerWithEngine.java
Outdated
Show resolved
Hide resolved
|
@rdblue: Could you please take a look at this PR? Only adds a single test case which failed for tez writes. Nothing fancy, but good to have a test case for MR as well. Thanks, |
| @@ -186,6 +186,9 @@ private HiveConf initializeConf() { | |||
| // Tez configuration | |||
| hiveConf.setBoolean("tez.local.mode", true); | |||
|
|
|||
| // Disable vectorization for HiveIcebergInputFormat | |||
| hiveConf.setBoolVar(HiveConf.ConfVars.HIVE_VECTORIZATION_ENABLED, false); | |||
There was a problem hiding this comment.
I think this is fine, but I'm curious: do we plan to add vectorized writes? Or is this not something that we think is important?
There was a problem hiding this comment.
I think it is important, but no hard data on yet. Will work on read/write performance in the near future, so would guess that vectorization will be something we check too.
mr/src/test/java/org/apache/iceberg/mr/hive/TestHiveIcebergStorageHandlerWithEngine.java
Outdated
Show resolved
Hide resolved
|
Overall, this looks fine. One minor question about why it creates an extra list. Otherwise +1. |
|
Thanks, @pvary! |
|
Thanks for the review and the merge @rdblue! |
During the test found several issues with vectorization for which I raised #2120
There are no issues with vectorization turned off, but having a test case for it helps finding possible issues later