You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@hive.apache.org by GitBox <gi...@apache.org> on 2021/01/15 17:26:21 UTC

[GitHub] [hive] zabetak opened a new pull request #1875: HIVE-24601: Control CBO fallback behavior via property

zabetak opened a new pull request #1875:
URL: https://github.com/apache/hive/pull/1875


   ### What changes were proposed in this pull request?
   1. Add `hive.cbo.fallback.strategy` and plug it to the planner
   2. Replace usage of `hive.in.test` in planner with `hive.cbo.fallback.strategy`
   
   ### Why are the changes needed?
   1. Provide the means to fail-fast when CBO failures appear (even in production)
   2. Allow finer control on tests requiring to fail on CBO, legacy, or both
   
   ### Does this PR introduce _any_ user-facing change?
   No at its current state. 
   
   I was thinking to even drop the `CONSERVATIVE` option. The CBO error is always logged (not hidden) so I don't see why we should complicate the decision more.
   
   ### How was this patch tested?
   If we agree on the approach, I will add some tests for all options.
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] zabetak commented on pull request #1875: HIVE-24601: Control CBO fallback behavior via property

Posted by GitBox <gi...@apache.org>.
zabetak commented on pull request #1875:
URL: https://github.com/apache/hive/pull/1875#issuecomment-767761319


   Close/Reopen to retrigger tests.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] jcamachor commented on a change in pull request #1875: HIVE-24601: Control CBO fallback behavior via property

Posted by GitBox <gi...@apache.org>.
jcamachor commented on a change in pull request #1875:
URL: https://github.com/apache/hive/pull/1875#discussion_r564120756



##########
File path: ql/src/test/queries/clientpositive/analyze_npe.q
##########
@@ -1,3 +1,4 @@
+--! qt:disabled:HIVE-24656

Review comment:
       I think this can be enabled now that HIVE-24656 has been committed?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] jcamachor commented on pull request #1875: HIVE-24601: Control CBO fallback behavior via property

Posted by GitBox <gi...@apache.org>.
jcamachor commented on pull request #1875:
URL: https://github.com/apache/hive/pull/1875#issuecomment-767739852


   Can you trigger tests again? I think there was some failure?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] kgyrtkirk commented on a change in pull request #1875: HIVE-24601: Control CBO fallback behavior via property

Posted by GitBox <gi...@apache.org>.
kgyrtkirk commented on a change in pull request #1875:
URL: https://github.com/apache/hive/pull/1875#discussion_r565273113



##########
File path: ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java
##########
@@ -679,27 +680,15 @@ Operator genOPTree(ASTNode ast, PlannerContext plannerCtx) throws SemanticExcept
 
           // Determine if we should re-throw the exception OR if we try to mark plan as reAnayzeAST to retry
           // planning as non-CBO.
-          if (e instanceof CalciteSubquerySemanticException || e instanceof CalciteViewSemanticException
-              || e instanceof CalciteSubqueryRuntimeException) {
-            // Non-CBO path for CalciteSubquerySemanticException fails with completely different error
-            // and masks the original failure.
-            // Non-CBO path for CalciteViewSemanticException would fail in a similar way as CBO path.
-            throw new SemanticException(e);
-          }
-
-          boolean isHiveTest = conf.getBoolVar(ConfVars.HIVE_IN_TEST);
-          // At this point we retry with CBO off:
-          // 1) If this is not test mode (common case)
-          // 2) If we are in test mode and we are missing stats
-          // 3) if we are in test mode and a CalciteSemanticException is generated
-          reAnalyzeAST = (!isHiveTest || isMissingStats ||  e instanceof CalciteSemanticException);

Review comment:
       quesiton is what we want to do when a plain `RuntimeException` is thrown? do we want to retry it with the non-cbo path or not?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] zabetak commented on pull request #1875: HIVE-24601: Control CBO fallback behavior via property

Posted by GitBox <gi...@apache.org>.
zabetak commented on pull request #1875:
URL: https://github.com/apache/hive/pull/1875#issuecomment-766808576


   @kgyrtkirk , @jcamachor : Tests are green. Can you have a look and if we are good I can complete the javadoc to merge it.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] kgyrtkirk commented on a change in pull request #1875: HIVE-24601: Control CBO fallback behavior via property

Posted by GitBox <gi...@apache.org>.
kgyrtkirk commented on a change in pull request #1875:
URL: https://github.com/apache/hive/pull/1875#discussion_r566176263



##########
File path: ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java
##########
@@ -679,27 +680,15 @@ Operator genOPTree(ASTNode ast, PlannerContext plannerCtx) throws SemanticExcept
 
           // Determine if we should re-throw the exception OR if we try to mark plan as reAnayzeAST to retry
           // planning as non-CBO.
-          if (e instanceof CalciteSubquerySemanticException || e instanceof CalciteViewSemanticException
-              || e instanceof CalciteSubqueryRuntimeException) {
-            // Non-CBO path for CalciteSubquerySemanticException fails with completely different error
-            // and masks the original failure.
-            // Non-CBO path for CalciteViewSemanticException would fail in a similar way as CBO path.
-            throw new SemanticException(e);
-          }
-
-          boolean isHiveTest = conf.getBoolVar(ConfVars.HIVE_IN_TEST);
-          // At this point we retry with CBO off:
-          // 1) If this is not test mode (common case)
-          // 2) If we are in test mode and we are missing stats
-          // 3) if we are in test mode and a CalciteSemanticException is generated
-          reAnalyzeAST = (!isHiveTest || isMissingStats ||  e instanceof CalciteSemanticException);

Review comment:
       okay we could add a new mode to experiment with this - but I'm not sure how usefull that would be; we can start with whitelisting the case when column stats are missing - but I think that will not get us much further...probably enabling that mode will expose in which other cases we should fall back...




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] zabetak commented on pull request #1875: HIVE-24601: Control CBO fallback behavior via property

Posted by GitBox <gi...@apache.org>.
zabetak commented on pull request #1875:
URL: https://github.com/apache/hive/pull/1875#issuecomment-766808576


   @kgyrtkirk , @jcamachor : Tests are green. Can you have a look and if we are good I can complete the javadoc to merge it.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] zabetak closed pull request #1875: HIVE-24601: Control CBO fallback behavior via property

Posted by GitBox <gi...@apache.org>.
zabetak closed pull request #1875:
URL: https://github.com/apache/hive/pull/1875


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] zabetak commented on pull request #1875: HIVE-24601: Control CBO fallback behavior via property

Posted by GitBox <gi...@apache.org>.
zabetak commented on pull request #1875:
URL: https://github.com/apache/hive/pull/1875#issuecomment-769156199


   @kgyrtkirk @jcamachor : If we are good with the current state of the PR maybe we should merge it so that it does not get stale.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] zabetak commented on a change in pull request #1875: HIVE-24601: Control CBO fallback behavior via property

Posted by GitBox <gi...@apache.org>.
zabetak commented on a change in pull request #1875:
URL: https://github.com/apache/hive/pull/1875#discussion_r564722814



##########
File path: ql/src/test/queries/clientpositive/analyze_npe.q
##########
@@ -1,3 +1,4 @@
+--! qt:disabled:HIVE-24656

Review comment:
       Yeap, I reenabled the test and rebased the PR.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] zabetak commented on a change in pull request #1875: HIVE-24601: Control CBO fallback behavior via property

Posted by GitBox <gi...@apache.org>.
zabetak commented on a change in pull request #1875:
URL: https://github.com/apache/hive/pull/1875#discussion_r559627202



##########
File path: common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
##########
@@ -1806,6 +1806,12 @@ private static void populateLlapDaemonVarsSet(Set<String> llapDaemonVarsSetLocal
 
     // CBO related
     HIVE_CBO_ENABLED("hive.cbo.enable", true, "Flag to control enabling Cost Based Optimizations using Calcite framework."),
+    HIVE_CBO_FALLBACK_STRATEGY("hive.cbo.fallback.strategy", "CONSERVATIVE", new StringSet("NEVER, CONSERVATIVE, ALWAYS, TEST"),

Review comment:
       Good catch @kgyrtkirk ! Fixed in https://github.com/apache/hive/pull/1875/commits/a0ed3355de858b9aa49ae6989966451b04029353 and added tests in https://github.com/apache/hive/pull/1875/commits/bb7c60ff43257f6a9de4db125a71e6f2328017ee and https://github.com/apache/hive/pull/1875/commits/2d28db68a1830c2c4c9212d3650cd78db5375a4e.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] kgyrtkirk commented on a change in pull request #1875: HIVE-24601: Control CBO fallback behavior via property

Posted by GitBox <gi...@apache.org>.
kgyrtkirk commented on a change in pull request #1875:
URL: https://github.com/apache/hive/pull/1875#discussion_r565349741



##########
File path: ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java
##########
@@ -679,27 +680,15 @@ Operator genOPTree(ASTNode ast, PlannerContext plannerCtx) throws SemanticExcept
 
           // Determine if we should re-throw the exception OR if we try to mark plan as reAnayzeAST to retry
           // planning as non-CBO.
-          if (e instanceof CalciteSubquerySemanticException || e instanceof CalciteViewSemanticException
-              || e instanceof CalciteSubqueryRuntimeException) {
-            // Non-CBO path for CalciteSubquerySemanticException fails with completely different error
-            // and masks the original failure.
-            // Non-CBO path for CalciteViewSemanticException would fail in a similar way as CBO path.
-            throw new SemanticException(e);
-          }
-
-          boolean isHiveTest = conf.getBoolVar(ConfVars.HIVE_IN_TEST);
-          // At this point we retry with CBO off:
-          // 1) If this is not test mode (common case)
-          // 2) If we are in test mode and we are missing stats
-          // 3) if we are in test mode and a CalciteSemanticException is generated
-          reAnalyzeAST = (!isHiveTest || isMissingStats ||  e instanceof CalciteSemanticException);

Review comment:
       thank you for the explanation - yes it will be good
   
   about the `RuntimeException` stuff - I think in case we encounter something unknown; we should just stop the show.
   But because the approach is to blacklist some exception types - and retry everything else. Which probably aligns better to what our users want most of the time - get the resultset one way or the other.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] zabetak commented on a change in pull request #1875: HIVE-24601: Control CBO fallback behavior via property

Posted by GitBox <gi...@apache.org>.
zabetak commented on a change in pull request #1875:
URL: https://github.com/apache/hive/pull/1875#discussion_r565310416



##########
File path: ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java
##########
@@ -679,27 +680,15 @@ Operator genOPTree(ASTNode ast, PlannerContext plannerCtx) throws SemanticExcept
 
           // Determine if we should re-throw the exception OR if we try to mark plan as reAnayzeAST to retry
           // planning as non-CBO.
-          if (e instanceof CalciteSubquerySemanticException || e instanceof CalciteViewSemanticException
-              || e instanceof CalciteSubqueryRuntimeException) {
-            // Non-CBO path for CalciteSubquerySemanticException fails with completely different error
-            // and masks the original failure.
-            // Non-CBO path for CalciteViewSemanticException would fail in a similar way as CBO path.
-            throw new SemanticException(e);
-          }
-
-          boolean isHiveTest = conf.getBoolVar(ConfVars.HIVE_IN_TEST);
-          // At this point we retry with CBO off:
-          // 1) If this is not test mode (common case)
-          // 2) If we are in test mode and we are missing stats
-          // 3) if we are in test mode and a CalciteSemanticException is generated
-          reAnalyzeAST = (!isHiveTest || isMissingStats ||  e instanceof CalciteSemanticException);

Review comment:
       Many thanks for keeping an eye @kgyrtkirk !
   
   In production, (now `CONSERVATIVE` mode) `isHiveTest` is `false` so `!isHiveTest` becomes `true` thus no matter what happens with `isMissingStats` and `CalciteSemanticException`, we fallback to legacy optimizer.
   
   In tests, before these changes we fallaback to legacy when the problem is due to missing stats or CSE. I see this mostly as a convenient way to not update a big amount of existing tests which otherwise will fail. From my perspective the fact that CBO fails is not something that should be considered normal. When writting a test if we know that we cannot handle it with CBO then I think it would be better to explicitly disable CBO instead of relying on this fallback mechanism. After these changes, future tests might (depends on the kind of exception that they come with) fail if they are missing stats.
   
   Now regarding the `RuntimeException`:
   
   - in `ALWAYS` mode we fallback to legacy;
   - in `CONSERVATIVE` mode we fallback to legacy (same as before);
   - in `NEVER` mode we stop and fail the query;
   - in `TEST` mode we stop and fail the query (slightly different than before where we could retry if `RuntimeException` was paired with a missing stats problem).




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] kgyrtkirk merged pull request #1875: HIVE-24601: Control CBO fallback behavior via property

Posted by GitBox <gi...@apache.org>.
kgyrtkirk merged pull request #1875:
URL: https://github.com/apache/hive/pull/1875


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] kgyrtkirk commented on a change in pull request #1875: HIVE-24601: Control CBO fallback behavior via property

Posted by GitBox <gi...@apache.org>.
kgyrtkirk commented on a change in pull request #1875:
URL: https://github.com/apache/hive/pull/1875#discussion_r565271825



##########
File path: ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java
##########
@@ -679,27 +680,15 @@ Operator genOPTree(ASTNode ast, PlannerContext plannerCtx) throws SemanticExcept
 
           // Determine if we should re-throw the exception OR if we try to mark plan as reAnayzeAST to retry
           // planning as non-CBO.
-          if (e instanceof CalciteSubquerySemanticException || e instanceof CalciteViewSemanticException
-              || e instanceof CalciteSubqueryRuntimeException) {
-            // Non-CBO path for CalciteSubquerySemanticException fails with completely different error
-            // and masks the original failure.
-            // Non-CBO path for CalciteViewSemanticException would fail in a similar way as CBO path.
-            throw new SemanticException(e);
-          }
-
-          boolean isHiveTest = conf.getBoolVar(ConfVars.HIVE_IN_TEST);
-          // At this point we retry with CBO off:
-          // 1) If this is not test mode (common case)
-          // 2) If we are in test mode and we are missing stats
-          // 3) if we are in test mode and a CalciteSemanticException is generated
-          reAnalyzeAST = (!isHiveTest || isMissingStats ||  e instanceof CalciteSemanticException);

Review comment:
       there is a "small" change here - the new `CONSERVATIVE` doesn't contain this `isMissingStats` ; `e instanceof CSE` part.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] kgyrtkirk commented on a change in pull request #1875: HIVE-24601: Control CBO fallback behavior via property

Posted by GitBox <gi...@apache.org>.
kgyrtkirk commented on a change in pull request #1875:
URL: https://github.com/apache/hive/pull/1875#discussion_r559550634



##########
File path: common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
##########
@@ -1806,6 +1806,12 @@ private static void populateLlapDaemonVarsSet(Set<String> llapDaemonVarsSetLocal
 
     // CBO related
     HIVE_CBO_ENABLED("hive.cbo.enable", true, "Flag to control enabling Cost Based Optimizations using Calcite framework."),
+    HIVE_CBO_FALLBACK_STRATEGY("hive.cbo.fallback.strategy", "CONSERVATIVE", new StringSet("NEVER, CONSERVATIVE, ALWAYS, TEST"),

Review comment:
       this seem like the stringset has only one value which is `NEVER, CONSERVATIVE, ALWAYS, TEST`




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] jcamachor commented on a change in pull request #1875: HIVE-24601: Control CBO fallback behavior via property

Posted by GitBox <gi...@apache.org>.
jcamachor commented on a change in pull request #1875:
URL: https://github.com/apache/hive/pull/1875#discussion_r564120756



##########
File path: ql/src/test/queries/clientpositive/analyze_npe.q
##########
@@ -1,3 +1,4 @@
+--! qt:disabled:HIVE-24656

Review comment:
       I think this can be enabled now that HIVE-24656 has been committed?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org


[GitHub] [hive] zabetak commented on a change in pull request #1875: HIVE-24601: Control CBO fallback behavior via property

Posted by GitBox <gi...@apache.org>.
zabetak commented on a change in pull request #1875:
URL: https://github.com/apache/hive/pull/1875#discussion_r565377888



##########
File path: ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java
##########
@@ -679,27 +680,15 @@ Operator genOPTree(ASTNode ast, PlannerContext plannerCtx) throws SemanticExcept
 
           // Determine if we should re-throw the exception OR if we try to mark plan as reAnayzeAST to retry
           // planning as non-CBO.
-          if (e instanceof CalciteSubquerySemanticException || e instanceof CalciteViewSemanticException
-              || e instanceof CalciteSubqueryRuntimeException) {
-            // Non-CBO path for CalciteSubquerySemanticException fails with completely different error
-            // and masks the original failure.
-            // Non-CBO path for CalciteViewSemanticException would fail in a similar way as CBO path.
-            throw new SemanticException(e);
-          }
-
-          boolean isHiveTest = conf.getBoolVar(ConfVars.HIVE_IN_TEST);
-          // At this point we retry with CBO off:
-          // 1) If this is not test mode (common case)
-          // 2) If we are in test mode and we are missing stats
-          // 3) if we are in test mode and a CalciteSemanticException is generated
-          reAnalyzeAST = (!isHiveTest || isMissingStats ||  e instanceof CalciteSemanticException);

Review comment:
       Personally, I would prefer to make the `CONSERVATIVE` mode fail for `RuntimeException`. This would given an incentive to the client to report the unknown problem (which could be serious) and at the same time it is not blocking since they could switch temporarily to the more permissive `ALWAYS` mode. If others feel the same then we can go for the change otherwise we leave it as is.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org