HIVE-28618: TestGenericUDTFGetSQLSchema to run on Tez #5777
Conversation
|
okumin
changed the title
| @@ -87,9 +87,6 @@ public void testWithSimpleTypes() throws Exception { | |||
|
|
|||
| @Test | |||
| public void testWithDDL() throws Exception { | |||
| // Set the execution engine to mr to avoid the NPE exception in stats flow | |||
| // TODO: HIVE-28618: TestGenericUDTFGetSQLSchema to run on Tez | |||
| conf.set("hive.execution.engine", "mr"); | |||
There was a problem hiding this comment.
As far as I know, MR is still supported. Can you add a new test instead of modifying an existing one? Now we don't know if it works with MR.
There was a problem hiding this comment.
This is a part of the decommission of Hive on MapReduce. In my understanding, we've been migrating exceptional test cases that were excluded here only to Tez, which means testWithSimpleTypes in this file is already not tested with MR. It is also not an unfair proposal to keep MR test cases as long as MR is not removed! In my personal opinion, we don't have CI power to test every feature on the deprecated MapReduce.
@abstractdog Do you have a suggestion about the work under the umbrella?
What changes were proposed in this pull request?
Let
TestGenericUDTFGetSQLSchema.testWithDDLon Tez.https://issues.apache.org/jira/browse/HIVE-28618
On Tez, we see the NPE when we run StatsRulesProcFactory, which is triggered by TezCompiler.
SHOW TABLESis a DDL that doesn't scan a regular table. So, some attributes are missing.On MapReduce, StatsRulesProcFactory is not enabled unless isExplainSkipExecution is enabled. As far as I tested it, it is skipped when we pass
SHOW TABLESto the UDF. I've not detailed the intention as MR is already deprecated.This change will let the UDF choose a correct SemanticAnalyzer based on the statement. I think this change is safe as long as I see the usage of
ParseUtils. parseQueryAndGetSchema.Why are the changes needed?
To discontinue Hive on MR.
Does this PR introduce any user-facing change?
The behavior of GenericUDTFGetSQLSchema will be consistent with MapReduce.
How was this patch tested?
Updated the unit test