You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@hive.apache.org by "okumin (via GitHub)" <gi...@apache.org> on 2023/07/21 11:34:33 UTC
[GitHub] [hive] okumin opened a new pull request, #4511: [WIP] HIVE-27480: Add test cases
okumin opened a new pull request, #4511:
URL: https://github.com/apache/hive/pull/4511
Do not review this PR yet
### What changes were proposed in this pull request?
TODO
### Why are the changes needed?
TODO
### Does this PR introduce _any_ user-facing change?
TODO
### Is the change a dependency upgrade?
TODO
### How was this patch tested?
TODO
--
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: gitbox-unsubscribe@hive.apache.org
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] okumin commented on a diff in pull request #4511: HIVE-27480: OFFSET without ORDER BY generates wrong results
Posted by "okumin (via GitHub)" <gi...@apache.org>.
okumin commented on code in PR #4511:
URL: https://github.com/apache/hive/pull/4511#discussion_r1274902625
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java:
##########
@@ -11258,38 +11259,47 @@ private Operator genPostGroupByBodyPlan(Operator curr, String dest, QB qb,
if (qbp.getOrderByForClause(dest) != null) {
genReduceSink = true;
+ numReducers = 1;
hasOrderBy = true;
}
+ if (offset > 0 && !hasOrderBy) {
+ HiveConf.StrictChecks.checkOffsetWithoutOrderBy(conf);
+
+ assert limit != null : "OFFSET is always paired with LIMIT";
Review Comment:
I don't have immediate access to the standard and MySQL is the only one I found so far.
Trino 413.
```
trino> SELECT * FROM tpcds.tiny.item OFFSET 10;
(Returned the result)
```
Spark 3.4.1.
```
spark-sql (default)> SELECT * FROM item OFFSET 1990;
(Returned the result)
```
MySQL 8.
```
mysql> SELECT * FROM item OFFSET 2;
ERROR 1064 (42000): You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '2' at line 1
```
I guess we can support it by updating the grammar and steps related to OFFSET.
https://github.com/apache/hive/blob/rel/release-4.0.0-alpha-2/parser/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g#L2779-L2785
--
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: gitbox-unsubscribe@hive.apache.org
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] okumin commented on a diff in pull request #4511: HIVE-27480: OFFSET without ORDER BY generates wrong results
Posted by "okumin (via GitHub)" <gi...@apache.org>.
okumin commented on code in PR #4511:
URL: https://github.com/apache/hive/pull/4511#discussion_r1271477976
##########
ql/src/test/results/clientpositive/llap/limit_join_transpose.q.out:
##########
@@ -1099,7 +1116,7 @@ limit 1 offset 1
POSTHOOK: type: QUERY
POSTHOOK: Input: default@src
#### A masked pattern was here ####
-86 val_86 86 val_86
+238 val_238 238 val_238
Review Comment:
This is OK because the test query picks up any single row.
```
select *
from src src1 left outer join src src2
on src1.key = src2.key
limit 1 offset 1
```
##########
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java:
##########
@@ -7015,6 +7022,12 @@ public static String checkBucketing(Configuration conf) {
return isAllowed(conf, ConfVars.HIVE_STRICT_CHECKS_BUCKETING) ? null : NO_BUCKETING_MSG;
}
+ public static void checkOffsetWithoutOrderBy(Configuration conf) throws SemanticException {
+ if (!isAllowed(conf, ConfVars.HIVE_STRICT_CHECKS_OFFSET_NO_ORDERBY)) {
+ throw new SemanticException(NO_OFFSET_WITHOUT_ORDERBY_MSG);
Review Comment:
This directly throws an Exception following @aturoczy's suggestion. I'm thinking to update other usages in another ticket if we agree this manner is better.
https://github.com/apache/hive/pull/4467#discussion_r1256310558
##########
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java:
##########
@@ -1819,6 +1820,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" +
+ "OFFSET is mostly meaningless when a result set doesn't have a total order."),
Review Comment:
Let me explain why I want to add this parameter.
I have received a number of inquiries from different users so far. They always said like "My `LIMIT 100 OFFSET 10000` returned empty rows even though the table has more than 100,000 records".
Hive actually has a bug as I reported. However, listening to the users carefully, 99.9% of them mistakenly tried to use the feature in order to paginate their tables as below(Note this is a pseudo-code, and they actually use workflow engines or something). This is not deterministic.
```
int offset = 0;
while (true) {
List<String> rows = submit("SELECT * FROM tbl LIMIT 1000000 OFFSET " + i);
if (rows.isEmpty) {
break;
}
doSomething(rows);
i += 1000000;
}
```
I guess OFFSET w/o ORDER BY doesn't have so many valid use cases while users can very easily make the mistake. So, I'd like to have an option to reject this usage by default.
--
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: gitbox-unsubscribe@hive.apache.org
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] sonarcloud[bot] commented on pull request #4511: HIVE-27480: OFFSET without ORDER BY generates wrong results
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #4511:
URL: https://github.com/apache/hive/pull/4511#issuecomment-1710576977
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=4511)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4511&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4511&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4511&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4511&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4511&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4511&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4511&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4511&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4511&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4511&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4511&resolved=false&types=CODE_SMELL) [8 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4511&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4511&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4511&metric=duplicated_lines_density&view=list) No Duplication information
 The version of Java (11.0.8) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17.
Read more [here](https://docs.sonarcloud.io/appendices/scanner-environment/)
--
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: gitbox-unsubscribe@hive.apache.org
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] kasakrisz commented on a diff in pull request #4511: HIVE-27480: OFFSET without ORDER BY generates wrong results
Posted by "kasakrisz (via GitHub)" <gi...@apache.org>.
kasakrisz commented on code in PR #4511:
URL: https://github.com/apache/hive/pull/4511#discussion_r1318267190
##########
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java:
##########
@@ -1819,6 +1820,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" +
+ "OFFSET is mostly meaningless when a result set doesn't have a total order."),
Review Comment:
Ok. Please add the warning and I will approve this patch.
--
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: gitbox-unsubscribe@hive.apache.org
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] sonarcloud[bot] commented on pull request #4511: HIVE-27480: OFFSET without ORDER BY generates wrong results
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #4511:
URL: https://github.com/apache/hive/pull/4511#issuecomment-1711414109
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=4511)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4511&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4511&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4511&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4511&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4511&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4511&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4511&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4511&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4511&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4511&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4511&resolved=false&types=CODE_SMELL) [8 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4511&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4511&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4511&metric=duplicated_lines_density&view=list) No Duplication information
 The version of Java (11.0.8) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17.
Read more [here](https://docs.sonarcloud.io/appendices/scanner-environment/)
--
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: gitbox-unsubscribe@hive.apache.org
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] okumin commented on a diff in pull request #4511: HIVE-27480: OFFSET without ORDER BY generates wrong results
Posted by "okumin (via GitHub)" <gi...@apache.org>.
okumin commented on code in PR #4511:
URL: https://github.com/apache/hive/pull/4511#discussion_r1274884229
##########
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java:
##########
@@ -1819,6 +1820,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" +
+ "OFFSET is mostly meaningless when a result set doesn't have a total order."),
Review Comment:
I personally agree with the idea as long as the incompatibility is introduced to the new major version. In my experience providing a managed Hive service, the usage of OFFSET without ORDER BY is not so high, and the ratio of mistakes in the small number of use cases is high. Warning is a more conservative approach, though.
--
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: gitbox-unsubscribe@hive.apache.org
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] kasakrisz commented on a diff in pull request #4511: HIVE-27480: OFFSET without ORDER BY generates wrong results
Posted by "kasakrisz (via GitHub)" <gi...@apache.org>.
kasakrisz commented on code in PR #4511:
URL: https://github.com/apache/hive/pull/4511#discussion_r1274662033
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java:
##########
@@ -11258,38 +11259,47 @@ private Operator genPostGroupByBodyPlan(Operator curr, String dest, QB qb,
if (qbp.getOrderByForClause(dest) != null) {
genReduceSink = true;
+ numReducers = 1;
hasOrderBy = true;
}
+ if (offset > 0 && !hasOrderBy) {
+ HiveConf.StrictChecks.checkOffsetWithoutOrderBy(conf);
+
+ assert limit != null : "OFFSET is always paired with LIMIT";
Review Comment:
Is this Hive specific? Postgres allows
```
select * from test offset 10;
```
Could you please check the standard?
--
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: gitbox-unsubscribe@hive.apache.org
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] kasakrisz merged pull request #4511: HIVE-27480: OFFSET without ORDER BY generates wrong results
Posted by "kasakrisz (via GitHub)" <gi...@apache.org>.
kasakrisz merged PR #4511:
URL: https://github.com/apache/hive/pull/4511
--
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: gitbox-unsubscribe@hive.apache.org
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] okumin commented on a diff in pull request #4511: HIVE-27480: OFFSET without ORDER BY generates wrong results
Posted by "okumin (via GitHub)" <gi...@apache.org>.
okumin commented on code in PR #4511:
URL: https://github.com/apache/hive/pull/4511#discussion_r1274902625
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java:
##########
@@ -11258,38 +11259,47 @@ private Operator genPostGroupByBodyPlan(Operator curr, String dest, QB qb,
if (qbp.getOrderByForClause(dest) != null) {
genReduceSink = true;
+ numReducers = 1;
hasOrderBy = true;
}
+ if (offset > 0 && !hasOrderBy) {
+ HiveConf.StrictChecks.checkOffsetWithoutOrderBy(conf);
+
+ assert limit != null : "OFFSET is always paired with LIMIT";
Review Comment:
I don't have immediate access to the standard and MySQL is the only one I found so far.
Trino 413.
```
trino> SELECT * FROM tpcds.tiny.item OFFSET 10;
```
Spark 3.4.1.
```
spark-sql (default)> SELECT * FROM item OFFSET 1990;
```
MySQL 8.
```
mysql> SELECT * FROM item OFFSET 2;
ERROR 1064 (42000): You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '2' at line 1
```
I guess we can support it by updating the grammar and steps related to OFFSET.
https://github.com/apache/hive/blob/rel/release-4.0.0-alpha-2/parser/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g#L2779-L2785
--
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: gitbox-unsubscribe@hive.apache.org
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] okumin commented on a diff in pull request #4511: HIVE-27480: OFFSET without ORDER BY generates wrong results
Posted by "okumin (via GitHub)" <gi...@apache.org>.
okumin commented on code in PR #4511:
URL: https://github.com/apache/hive/pull/4511#discussion_r1318509711
##########
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java:
##########
@@ -7015,6 +7022,12 @@ public static String checkBucketing(Configuration conf) {
return isAllowed(conf, ConfVars.HIVE_STRICT_CHECKS_BUCKETING) ? null : NO_BUCKETING_MSG;
}
+ public static void checkOffsetWithoutOrderBy(Configuration conf) throws SemanticException {
+ if (!isAllowed(conf, ConfVars.HIVE_STRICT_CHECKS_OFFSET_NO_ORDERBY)) {
+ throw new SemanticException(NO_OFFSET_WITHOUT_ORDERBY_MSG);
Review Comment:
@kasakrisz I found a utility method and used it for standardization. This is an example.
```
$ beeline -e "select * from test limit 10, 10" --hiveconf hive.fetch.task.conversion=none --hiveconf tez.grouping.min-size=1 --hiveconf tez.grouping.max-size=1
...
INFO : Warning: OFFSET without ORDER BY is mostly non-deterministic and meaningless. Please make sure that you really don't need ORDER BY
```
--
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: gitbox-unsubscribe@hive.apache.org
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] okumin commented on pull request #4511: HIVE-27480: OFFSET without ORDER BY generates wrong results
Posted by "okumin (via GitHub)" <gi...@apache.org>.
okumin commented on PR #4511:
URL: https://github.com/apache/hive/pull/4511#issuecomment-1711708332
Thanks for your review!
--
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: gitbox-unsubscribe@hive.apache.org
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] kasakrisz commented on a diff in pull request #4511: HIVE-27480: OFFSET without ORDER BY generates wrong results
Posted by "kasakrisz (via GitHub)" <gi...@apache.org>.
kasakrisz commented on code in PR #4511:
URL: https://github.com/apache/hive/pull/4511#discussion_r1317111691
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java:
##########
@@ -11258,38 +11259,47 @@ private Operator genPostGroupByBodyPlan(Operator curr, String dest, QB qb,
if (qbp.getOrderByForClause(dest) != null) {
genReduceSink = true;
+ numReducers = 1;
hasOrderBy = true;
}
+ if (offset > 0 && !hasOrderBy) {
+ HiveConf.StrictChecks.checkOffsetWithoutOrderBy(conf);
+
+ assert limit != null : "OFFSET is always paired with LIMIT";
Review Comment:
It can be done in a separate patch if necessary.
--
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: gitbox-unsubscribe@hive.apache.org
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] sonarcloud[bot] commented on pull request #4511: [WIP] HIVE-27480: WIP
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #4511:
URL: https://github.com/apache/hive/pull/4511#issuecomment-1646575476
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=4511)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4511&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4511&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4511&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4511&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4511&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4511&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4511&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4511&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4511&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4511&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4511&resolved=false&types=CODE_SMELL) [8 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4511&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4511&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4511&metric=duplicated_lines_density&view=list) No Duplication information
--
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: gitbox-unsubscribe@hive.apache.org
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] okumin commented on pull request #4511: HIVE-27480: OFFSET without ORDER BY generates wrong results
Posted by "okumin (via GitHub)" <gi...@apache.org>.
okumin commented on PR #4511:
URL: https://github.com/apache/hive/pull/4511#issuecomment-1647076586
@kasakrisz Could you please take a look when you have a chance? This PR is related to #4471.
--
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: gitbox-unsubscribe@hive.apache.org
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] kasakrisz commented on a diff in pull request #4511: HIVE-27480: OFFSET without ORDER BY generates wrong results
Posted by "kasakrisz (via GitHub)" <gi...@apache.org>.
kasakrisz commented on code in PR #4511:
URL: https://github.com/apache/hive/pull/4511#discussion_r1274659576
##########
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java:
##########
@@ -7015,6 +7022,12 @@ public static String checkBucketing(Configuration conf) {
return isAllowed(conf, ConfVars.HIVE_STRICT_CHECKS_BUCKETING) ? null : NO_BUCKETING_MSG;
}
+ public static void checkOffsetWithoutOrderBy(Configuration conf) throws SemanticException {
+ if (!isAllowed(conf, ConfVars.HIVE_STRICT_CHECKS_OFFSET_NO_ORDERBY)) {
+ throw new SemanticException(NO_OFFSET_WITHOUT_ORDERBY_MSG);
Review Comment:
Could you please add a warning when it is allowed to let the user know that offset without order by gives non-deterministic results.
Here is an example how to write to console. AFAIK this should be seen in beeline:
https://github.com/apache/hive/blob/25826cc9e0dfdc9de2edf19c2ab9962ba20267d8/ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java#L14370C11-L14370C44
--
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: gitbox-unsubscribe@hive.apache.org
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] sonarcloud[bot] commented on pull request #4511: [WIP] HIVE-27480: WIP
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #4511:
URL: https://github.com/apache/hive/pull/4511#issuecomment-1646104930
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=4511)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4511&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4511&resolved=false&types=BUG) [1 Bug](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4511&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4511&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4511&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4511&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4511&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4511&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4511&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4511&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4511&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4511&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4511&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4511&metric=duplicated_lines_density&view=list) No Duplication information
--
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: gitbox-unsubscribe@hive.apache.org
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] sonarcloud[bot] commented on pull request #4511: HIVE-27480: OFFSET without ORDER BY generates wrong results
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #4511:
URL: https://github.com/apache/hive/pull/4511#issuecomment-1710985811
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=4511)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4511&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4511&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4511&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4511&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4511&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4511&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4511&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4511&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4511&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4511&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4511&resolved=false&types=CODE_SMELL) [8 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4511&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4511&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4511&metric=duplicated_lines_density&view=list) No Duplication information
 The version of Java (11.0.8) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17.
Read more [here](https://docs.sonarcloud.io/appendices/scanner-environment/)
--
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: gitbox-unsubscribe@hive.apache.org
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] okumin commented on a diff in pull request #4511: HIVE-27480: OFFSET without ORDER BY generates wrong results
Posted by "okumin (via GitHub)" <gi...@apache.org>.
okumin commented on code in PR #4511:
URL: https://github.com/apache/hive/pull/4511#discussion_r1317290046
##########
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java:
##########
@@ -1819,6 +1820,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" +
+ "OFFSET is mostly meaningless when a result set doesn't have a total order."),
Review Comment:
@kasakrisz Now, I think it is more moderate if (1) we keep the current behavior and (2) we show a warning as you mentioned below. We can change the default anytime later. What do you think? I am going to add the warning anyway.
--
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: gitbox-unsubscribe@hive.apache.org
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] kasakrisz commented on a diff in pull request #4511: HIVE-27480: OFFSET without ORDER BY generates wrong results
Posted by "kasakrisz (via GitHub)" <gi...@apache.org>.
kasakrisz commented on code in PR #4511:
URL: https://github.com/apache/hive/pull/4511#discussion_r1274669025
##########
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java:
##########
@@ -1819,6 +1820,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" +
+ "OFFSET is mostly meaningless when a result set doesn't have a total order."),
Review Comment:
I'm thinking about setting this true by default like the others. It would not be backward compatible but otherwise users may continue using it accidentally. Or printing a warning is enough? WDT?
--
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: gitbox-unsubscribe@hive.apache.org
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] sonarcloud[bot] commented on pull request #4511: [WIP] HIVE-27480: WIP
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #4511:
URL: https://github.com/apache/hive/pull/4511#issuecomment-1646456478
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=4511)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4511&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4511&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4511&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4511&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4511&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4511&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4511&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4511&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4511&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4511&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4511&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4511&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4511&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4511&metric=duplicated_lines_density&view=list) No Duplication information
--
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: gitbox-unsubscribe@hive.apache.org
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