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 2024/05/02 16:40:25 UTC
[PR] HIVE-28208: WITH column list doesn't work with CTE materialization [hive]
okumin opened a new pull request, #5232:
URL: https://github.com/apache/hive/pull/5232
### What changes were proposed in this pull request?
ANSI SQL supports column aliases for CTEs. Hive also supports it but it fails when CTEs are materialized.
https://issues.apache.org/jira/browse/HIVE-28208
### Why are the changes needed?
This patch allows materialized CTEs to obey the standard.
### Does this PR introduce _any_ user-facing change?
No. The affected queries never succeed now.
### Is the change a dependency upgrade?
No.
### How was this patch tested?
I added test queries to `cte_mat_1.q`.
--
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
Re: [PR] HIVE-28208: WITH column list doesn't work with CTE materialization [hive]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #5232: HIVE-28208: WITH column list doesn't work with CTE materialization
URL: https://github.com/apache/hive/pull/5232
--
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
Re: [PR] HIVE-28208: WITH column list doesn't work with CTE materialization [hive]
Posted by "okumin (via GitHub)" <gi...@apache.org>.
okumin commented on code in PR #5232:
URL: https://github.com/apache/hive/pull/5232#discussion_r1587967323
##########
ql/src/test/queries/clientpositive/cte_mat_1.q:
##########
@@ -1,10 +1,37 @@
--! qt:dataset:src
set hive.mapred.mode=nonstrict;
set hive.optimize.cte.materialize.threshold=-1;
-set hive.explain.user=true;
explain
with q1(srcKey, srcValue) as (select * from src where key= '5')
+select a.srcKey, b.srcValue
+from q1 a join q1 b
+on a.srcKey=b.srcKey;
+
+set hive.optimize.cte.materialize.threshold=2;
+set hive.optimize.cte.materialize.full.aggregate.only=false;
+-- Use a format that retains column names
+set hive.default.fileformat=parquet;
Review Comment:
The default one positionally accesses physical columns, and we can't find some kinds of errors.
--
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
Re: [PR] HIVE-28208: WITH column list doesn't work with CTE materialization [hive]
Posted by "okumin (via GitHub)" <gi...@apache.org>.
okumin commented on code in PR #5232:
URL: https://github.com/apache/hive/pull/5232#discussion_r1587966282
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java:
##########
@@ -8379,7 +8402,9 @@ private ColsAndTypes deriveFileSinkColTypes(
String colName = colInfo.getInternalName(); //default column name
if (columns != null) {
FieldSchema col = new FieldSchema();
- if (!("".equals(nm[0])) && nm[1] != null) {
+ if (i < withColList.size()) {
Review Comment:
Using a random name when <with column list> is shorter than the input. This follows the behavior of non-materialized CTEs.
If I understand correctly, with-column-list is supported only when the number of columns is exactly equal to that of the query expression. So, it could be an option to disallow `colInfos.size() != withColList.size()`.
--
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
Re: [PR] HIVE-28208: WITH column list doesn't work with CTE materialization [hive]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #5232:
URL: https://github.com/apache/hive/pull/5232#issuecomment-2171444503
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5232) **Quality Gate passed**
Issues
 [6 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5232&resolved=false&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5232&resolutions=WONTFIX)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5232&resolved=false&sinceLeakPeriod=true)
 No data about Coverage
 No data about Duplication
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5232)
--
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
Re: [PR] HIVE-28208: WITH column list doesn't work with CTE materialization [hive]
Posted by "okumin (via GitHub)" <gi...@apache.org>.
okumin commented on code in PR #5232:
URL: https://github.com/apache/hive/pull/5232#discussion_r1587966282
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java:
##########
@@ -8379,7 +8402,9 @@ private ColsAndTypes deriveFileSinkColTypes(
String colName = colInfo.getInternalName(); //default column name
if (columns != null) {
FieldSchema col = new FieldSchema();
- if (!("".equals(nm[0])) && nm[1] != null) {
+ if (i < withColList.size()) {
Review Comment:
Using an input name when <with column list> is shorter than the input. This follows the behavior of non-materialized CTEs.
If I understand correctly, with-column-list is supported only when the number of columns is exactly equal to that of the query expression. So, it could be an option to disallow `colInfos.size() != withColList.size()`.
--
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
Re: [PR] HIVE-28208: WITH column list doesn't work with CTE materialization [hive]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #5232:
URL: https://github.com/apache/hive/pull/5232#issuecomment-2091865431
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5232) **Quality Gate passed**
Issues
 [6 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5232&resolved=false&inNewCodePeriod=true)
 [0 Accepted issues](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5232&metric=new_accepted_issues&view=list)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5232&resolved=false&inNewCodePeriod=true)
 No data about Coverage
 No data about Duplication
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5232)
--
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
Re: [PR] HIVE-28208: WITH column list doesn't work with CTE materialization [hive]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #5232:
URL: https://github.com/apache/hive/pull/5232#issuecomment-2354255005
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
Feel free to reach out on the dev@hive.apache.org list if the patch is in need of reviews.
--
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
Re: [PR] HIVE-28208: WITH column list doesn't work with CTE materialization [hive]
Posted by "okumin (via GitHub)" <gi...@apache.org>.
okumin commented on code in PR #5232:
URL: https://github.com/apache/hive/pull/5232#discussion_r1587966282
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java:
##########
@@ -8379,7 +8402,9 @@ private ColsAndTypes deriveFileSinkColTypes(
String colName = colInfo.getInternalName(); //default column name
if (columns != null) {
FieldSchema col = new FieldSchema();
- if (!("".equals(nm[0])) && nm[1] != null) {
+ if (i < withColList.size()) {
Review Comment:
Using a random name when <with column list> is shorter than the input. This follows the behavior of non-materialized CTEs.
If I understand correctly, with-column-list is supported only when the number of columns is exactly equal to that of the query expression. So, it could be an option to make a query fail if `colInfos.size() != withColList.size()`.
--
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
Re: [PR] HIVE-28208: WITH column list doesn't work with CTE materialization [hive]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #5232:
URL: https://github.com/apache/hive/pull/5232#issuecomment-2172327910
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5232) **Quality Gate passed**
Issues
 [6 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5232&resolved=false&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5232&resolutions=WONTFIX)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5232&resolved=false&sinceLeakPeriod=true)
 No data about Coverage
 No data about Duplication
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5232)
--
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
Re: [PR] HIVE-28208: WITH column list doesn't work with CTE materialization [hive]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on PR #5232:
URL: https://github.com/apache/hive/pull/5232#issuecomment-2237354747
cc @zabetak
--
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