aturoczy commented on code in PR #4467:
URL: https://github.com/apache/hive/pull/4467#discussion_r1256281733
##########
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java:
##########
@@ -1819,6 +1819,10 @@ public static enum ConfVars {
HIVE_STRICT_CHECKS_BUCKETING("hive.strict.checks.bucketing", true,
"Enabling strict bucketing checks disallows the following:\n" +
" Load into bucketed tables."),
+
HIVE_STRICT_CHECKS_OFFSET_NO_ORDERBY("hive.strict.checks.offset.no.orderby",
false,
+ "Enabling strict offset checks disallows the following:\n" +
+ " OFFSET without ORDER BY.\n" +
Review Comment:
2 white spaces. I think 1 would be enough :)
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java:
##########
@@ -11258,38 +11260,41 @@ private Operator genPostGroupByBodyPlan(Operator
curr, String dest, QB qb,
if (qbp.getOrderByForClause(dest) != null) {
genReduceSink = true;
+ runsSingleReducer = true;
hasOrderBy = true;
}
+ if (offset > 0) {
+ if (!hasOrderBy) {
+ final String error =
HiveConf.StrictChecks.checkOffsetWithoutOrderBy(conf);
Review Comment:
It is a conversation starter:
If the checkOffsetWithoutOrderBy return value is an error message, then why
not throw it inside the method? This is bit a C++ style error handling style
imho. It creates more cyclomatic complexity just for error handling.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
