You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@hive.apache.org by "mdayakar (via GitHub)" <gi...@apache.org> on 2024/04/05 12:20:20 UTC
[PR] HIVE-28143: After HIVE-27492 fix, some HPLSQL built-in functions like trim, lower are not working when used in insert statement. [hive]
mdayakar opened a new pull request, #5182:
URL: https://github.com/apache/hive/pull/5182
HIVE-28143: After HIVE-27492 fix, some HPLSQL built-in functions like trim, lower are not working when used in insert statement.
<!--
Thanks for sending a pull request! Here are some tips for you:
1. If this is your first time, please read our contributor guidelines: https://cwiki.apache.org/confluence/display/Hive/HowToContribute
2. Ensure that you have created an issue on the Hive project JIRA: https://issues.apache.org/jira/projects/HIVE/summary
3. Ensure you have added or run the appropriate tests for your PR:
4. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP]HIVE-XXXXX: Your PR title ...'.
5. Be sure to keep the PR description updated to reflect all changes.
6. Please write your PR title to summarize what this PR proposes.
7. If possible, provide a concise example to reproduce the issue for a faster review.
-->
### What changes were proposed in this pull request?
As a part of HIVE-27492 issue fix, removed the UDFs which are present in Hive as it may become duplicate but HPLSQL can support local and offline mode which requires the HPLSQL supported UDFs must present in HPLSQL itself so added them back and also fixed issues related to those UDFs.
<!--
Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue.
If possible, please consider writing useful notes for better and faster reviews in your PR. See the examples below.
1. If you refactor some codes with changing classes, showing the class hierarchy will help reviewers.
2. If you fix some SQL features, you can provide some references of other DBMSes.
3. If there is design documentation, please add the link.
4. If there is a discussion in the mailing list, please add the link.
-->
### Why are the changes needed?
To fix HIVE-28143 as all UDFs which are supported by HPLSQL are not working when they used other select statements.
<!--
Please clarify why the changes are needed. For instance,
1. If you propose a new API, clarify the use case for a new API.
2. If you fix a bug, you can clarify why it is a bug.
-->
### Does this PR introduce _any_ user-facing change?
No
<!--
Note that it means *any* user-facing change including all aspects such as the documentation fix.
If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description, screenshot and/or a reproducable example to show the behavior difference if possible.
If possible, please also clarify if this is a user-facing change compared to the released Hive versions or within the unreleased branches such as master.
If no, write 'No'.
-->
### Is the change a dependency upgrade?
No
<!--
If yes, please attach a file with output from mvn dependency:tree to validate a complete upgrade of dependency.
If no, write 'No'.
-->
### How was this patch tested?
_mvn test -Dtest=TestHplsqlOffline
mvn test -Dtest=TestHplsqlLocal
mvn test -Dtest=TestHplSqlViaBeeLine -pl itests/hive-unit -Pitests_
<!--
If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
If tests were not added, please describe why they were not added and/or why it was difficult to add.
-->
--
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-28143: After HIVE-27492 fix, some HPLSQL built-in functions like trim, lower are not working when used in insert statement. [hive]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #5182:
URL: https://github.com/apache/hive/pull/5182#issuecomment-2107758848
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5182) **Quality Gate passed**
Issues
 [6 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5182&resolved=false&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5182&metric=new_accepted_issues&view=list)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5182&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=5182)
--
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-28143: After HIVE-27492 fix, some HPLSQL built-in functions like trim, lower are not working when used in insert statement. [hive]
Posted by "kasakrisz (via GitHub)" <gi...@apache.org>.
kasakrisz merged PR #5182:
URL: https://github.com/apache/hive/pull/5182
--
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-28143: After HIVE-27492 fix, some HPLSQL built-in functions like trim, lower are not working when used in insert statement. [hive]
Posted by "mdayakar (via GitHub)" <gi...@apache.org>.
mdayakar commented on code in PR #5182:
URL: https://github.com/apache/hive/pull/5182#discussion_r1600529809
##########
hplsql/src/main/java/org/apache/hive/hplsql/Expression.java:
##########
@@ -110,7 +110,17 @@ else if (ctx.interval_item() != null) {
}
else {
visitChildren(ctx);
- sql.append(exec.stackPop().toString());
+ Var value = exec.stackPop();
+ if (value.type == Type.NULL && sql.toString().length() == 0) {
+ exec.stackPush(new Var());
+ return;
+ } else if (exec.buildSql && value.type == Type.DATE) {
+ sql.append(String.format("DATE '%s'", value.toString()));
+ } else if (exec.buildSql && value.type == Type.TIMESTAMP) {
+ sql.append(String.format("TIMESTAMP '%s'", value.toString()));
+ } else {
+ sql.append(value.toString());
+ }
Review Comment:
Added Var.toSqlString(boolean isBuildSql) API and using the same.
##########
hplsql/src/main/java/org/apache/hive/hplsql/Expression.java:
##########
@@ -565,7 +575,11 @@ public void operatorConcatSql(HplsqlParser.Expr_concatContext ctx) {
sql.append("CONCAT(");
int cnt = ctx.expr_concat_item().size();
for (int i = 0; i < cnt; i++) {
- sql.append(evalPop(ctx.expr_concat_item(i)).toString());
+ String concatStr = evalPop(ctx.expr_concat_item(i)).toString();
+ if (!concatStr.startsWith("'")) {
Review Comment:
Fixed
--
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-28143: After HIVE-27492 fix, some HPLSQL built-in functions like trim, lower are not working when used in insert statement. [hive]
Posted by "mdayakar (via GitHub)" <gi...@apache.org>.
mdayakar commented on code in PR #5182:
URL: https://github.com/apache/hive/pull/5182#discussion_r1570924896
##########
hplsql/src/main/java/org/apache/hive/hplsql/functions/FunctionMisc.java:
##########
@@ -211,6 +269,16 @@ public void partCount(HplsqlParser.Expr_spec_funcContext ctx) {
query.close();
}
+ public void modulo(HplsqlParser.Expr_func_paramsContext ctx) {
+ if (ctx.func_param().size() == 2) {
+ int a = evalPop(ctx.func_param(0).expr()).intValue();
+ int b = evalPop(ctx.func_param(1).expr()).intValue();
+ evalInt(a % b);
+ } else {
+ evalNull();
Review Comment:
These are the APIs removed as a part of [HIVE-27492](https://issues.apache.org/jira/browse/HIVE-27492) fix. So just added back them again.Didn't do any code changes.
--
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-28143: After HIVE-27492 fix, some HPLSQL built-in functions like trim, lower are not working when used in insert statement. [hive]
Posted by "mdayakar (via GitHub)" <gi...@apache.org>.
mdayakar commented on code in PR #5182:
URL: https://github.com/apache/hive/pull/5182#discussion_r1570920471
##########
hplsql/src/main/java/org/apache/hive/hplsql/functions/FunctionMisc.java:
##########
@@ -59,6 +64,43 @@ public void register(BuiltinFunctions f) {
void activityCount(HplsqlParser.Expr_spec_funcContext ctx) {
evalInt(Long.valueOf(exec.getRowCount()));
}
+
+ /**
+ * CAST function
+ */
+ void cast(HplsqlParser.Expr_spec_funcContext ctx) {
+ if (ctx.expr().size() != 1) {
+ evalNull();
+ return;
+ }
Review Comment:
These are the APIs removed as a part of [HIVE-27492](https://issues.apache.org/jira/browse/HIVE-27492) fix. So just added back them again.Didn't do any code changes.
--
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-28143: After HIVE-27492 fix, some HPLSQL built-in functions like trim, lower are not working when used in insert statement. [hive]
Posted by "kasakrisz (via GitHub)" <gi...@apache.org>.
kasakrisz commented on code in PR #5182:
URL: https://github.com/apache/hive/pull/5182#discussion_r1575886634
##########
hplsql/src/main/java/org/apache/hive/hplsql/functions/FunctionDatetime.java:
##########
@@ -162,6 +174,35 @@ void toTimestamp(HplsqlParser.Expr_func_paramsContext ctx) {
}
}
+ /**
+ * FROM_UNIXTIME() function (convert seconds since 1970-01-01 00:00:00 to timestamp)
+ */
+ void fromUnixtime(HplsqlParser.Expr_func_paramsContext ctx) {
+ int cnt = BuiltinFunctions.getParamCount(ctx);
+ if (cnt == 0) {
+ evalNull();
+ return;
+ }
+ Var value = evalPop(ctx.func_param(0).expr());
+ if (value.type != Var.Type.BIGINT) {
+ Var newVar = new Var(Var.Type.BIGINT);
+ value = newVar.cast(value);
+ }
+ long epoch = value.longValue();
+ String format = "yyyy-MM-dd HH:mm:ss";
+ if (cnt > 1) {
+ format = Utils.unquoteString(evalPop(ctx.func_param(1).expr()).toString());
+ }
+ evalString(Utils.quoteString(new SimpleDateFormat(format).format(new Date(epoch * 1000))));
Review Comment:
IIUC by reintroducing this function consecutive calls may return different results depending on which implementation is used: Hive or HPL/SQL
IMHO it should be consistent.
https://github.com/apache/hive/pull/4965/files#r1444226220
##########
hplsql/src/main/java/org/apache/hive/hplsql/functions/FunctionMisc.java:
##########
@@ -113,7 +155,9 @@ void currentUser(HplsqlParser.Expr_spec_funcContext ctx) {
}
public static Var currentUser() {
- return new Var("CURRENT_USER()");
+ String currentUser = System.getProperty("user.name");
+ currentUser = Utils.quoteString(currentUser);
+ return new Var(currentUser);
Review Comment:
Sorry, I'm missing something. IIUC instances of `Var` are the internal representations of an SQL literal. In case of string the single quote characters in the beginning and the end should be removed at parse time and also single quote characters part of the text should be unescaped.
If the original SQL form of the literal is needed there is https://github.com/apache/hive/blob/1a969f6642d4081027040f97e6a689bf2e687db3/hplsql/src/main/java/org/apache/hive/hplsql/Var.java#L620-L631
##########
hplsql/src/main/java/org/apache/hive/hplsql/functions/FunctionMisc.java:
##########
@@ -59,6 +64,43 @@ public void register(BuiltinFunctions f) {
void activityCount(HplsqlParser.Expr_spec_funcContext ctx) {
evalInt(Long.valueOf(exec.getRowCount()));
}
+
+ /**
+ * CAST function
+ */
+ void cast(HplsqlParser.Expr_spec_funcContext ctx) {
+ if (ctx.expr().size() != 1) {
+ evalNull();
+ return;
+ }
Review Comment:
If it should not be removed why not consider reverting HIVE-27492 ?
##########
hplsql/src/main/java/org/apache/hive/hplsql/Expression.java:
##########
@@ -565,7 +575,11 @@ public void operatorConcatSql(HplsqlParser.Expr_concatContext ctx) {
sql.append("CONCAT(");
int cnt = ctx.expr_concat_item().size();
for (int i = 0; i < cnt; i++) {
- sql.append(evalPop(ctx.expr_concat_item(i)).toString());
+ String concatStr = evalPop(ctx.expr_concat_item(i)).toString();
+ if (!concatStr.startsWith("'")) {
Review Comment:
How about
```
SELECT '#' || TRIM(''' Hello ') || '#';
```
trim result is: `' Hello`
--
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-28143: After HIVE-27492 fix, some HPLSQL built-in functions like trim, lower are not working when used in insert statement. [hive]
Posted by "mdayakar (via GitHub)" <gi...@apache.org>.
mdayakar commented on code in PR #5182:
URL: https://github.com/apache/hive/pull/5182#discussion_r1600530827
##########
hplsql/src/main/java/org/apache/hive/hplsql/functions/FunctionDatetime.java:
##########
@@ -162,6 +174,35 @@ void toTimestamp(HplsqlParser.Expr_func_paramsContext ctx) {
}
}
+ /**
+ * FROM_UNIXTIME() function (convert seconds since 1970-01-01 00:00:00 to timestamp)
+ */
+ void fromUnixtime(HplsqlParser.Expr_func_paramsContext ctx) {
+ int cnt = BuiltinFunctions.getParamCount(ctx);
+ if (cnt == 0) {
+ evalNull();
+ return;
+ }
+ Var value = evalPop(ctx.func_param(0).expr());
+ if (value.type != Var.Type.BIGINT) {
+ Var newVar = new Var(Var.Type.BIGINT);
+ value = newVar.cast(value);
+ }
+ long epoch = value.longValue();
+ String format = "yyyy-MM-dd HH:mm:ss";
+ if (cnt > 1) {
+ format = Utils.unquoteString(evalPop(ctx.func_param(1).expr()).toString());
+ }
+ evalString(Utils.quoteString(new SimpleDateFormat(format).format(new Date(epoch * 1000))));
Review Comment:
Fixed. Now using all java.time.* APIs.
--
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-28143: After HIVE-27492 fix, some HPLSQL built-in functions like trim, lower are not working when used in insert statement. [hive]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #5182:
URL: https://github.com/apache/hive/pull/5182#issuecomment-2112520756
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5182) **Quality Gate passed**
Issues
 [6 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5182&resolved=false&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5182&metric=new_accepted_issues&view=list)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5182&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=5182)
--
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-28143: After HIVE-27492 fix, some HPLSQL built-in functions like trim, lower are not working when used in insert statement. [hive]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #5182:
URL: https://github.com/apache/hive/pull/5182#issuecomment-2262569869
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5182) **Quality Gate passed**
Issues
 [33 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5182&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5182&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5182&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5182&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5182&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5182)
--
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-28143: After HIVE-27492 fix, some HPLSQL built-in functions like trim, lower are not working when used in insert statement. [hive]
Posted by "mdayakar (via GitHub)" <gi...@apache.org>.
mdayakar commented on code in PR #5182:
URL: https://github.com/apache/hive/pull/5182#discussion_r1570919635
##########
hplsql/src/main/java/org/apache/hive/hplsql/functions/FunctionDatetime.java:
##########
@@ -162,6 +174,35 @@ void toTimestamp(HplsqlParser.Expr_func_paramsContext ctx) {
}
}
+ /**
+ * FROM_UNIXTIME() function (convert seconds since 1970-01-01 00:00:00 to timestamp)
+ */
+ void fromUnixtime(HplsqlParser.Expr_func_paramsContext ctx) {
+ int cnt = BuiltinFunctions.getParamCount(ctx);
+ if (cnt == 0) {
+ evalNull();
+ return;
+ }
+ Var value = evalPop(ctx.func_param(0).expr());
+ if (value.type != Var.Type.BIGINT) {
+ Var newVar = new Var(Var.Type.BIGINT);
+ value = newVar.cast(value);
+ }
+ long epoch = value.longValue();
+ String format = "yyyy-MM-dd HH:mm:ss";
+ if (cnt > 1) {
+ format = Utils.unquoteString(evalPop(ctx.func_param(1).expr()).toString());
+ }
+ evalString(Utils.quoteString(new SimpleDateFormat(format).format(new Date(epoch * 1000))));
Review Comment:
These are the APIs removed as a part of [HIVE-27492](https://issues.apache.org/jira/browse/HIVE-27492) fix. So just added back them again.Didn't do any code changes.
--
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-28143: After HIVE-27492 fix, some HPLSQL built-in functions like trim, lower are not working when used in insert statement. [hive]
Posted by "mdayakar (via GitHub)" <gi...@apache.org>.
mdayakar commented on code in PR #5182:
URL: https://github.com/apache/hive/pull/5182#discussion_r1570916828
##########
hplsql/src/main/java/org/apache/hive/hplsql/Var.java:
##########
@@ -612,7 +613,7 @@ else if (type == Type.TIMESTAMP) {
if (t.length() > len) {
t = t.substring(0, len);
}
- return String.format("TIMESTAMP '%s'", t);
+ return t;
Review Comment:
As explained in https://github.com/apache/hive/pull/5182#discussion_r1570337451 comment, this was added by me only just reverted back the change done as a part of [HIVE-27492](https://issues.apache.org/jira/browse/HIVE-27492) fix.
##########
hplsql/src/main/java/org/apache/hive/hplsql/Var.java:
##########
@@ -601,7 +602,7 @@ else if (type == Type.STRING) {
return (String)value;
}
else if (type == Type.DATE) {
- return String.format("DATE '%s'", value);
+ return ((Date)value).toString();
Review Comment:
As explained in https://github.com/apache/hive/pull/5182#discussion_r1570337451 comment, this was added by me only just reverted back the change done as a part of [HIVE-27492](https://issues.apache.org/jira/browse/HIVE-27492) fix.
--
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-28143: After HIVE-27492 fix, some HPLSQL built-in functions like trim, lower are not working when used in insert statement. [hive]
Posted by "kasakrisz (via GitHub)" <gi...@apache.org>.
kasakrisz commented on code in PR #5182:
URL: https://github.com/apache/hive/pull/5182#discussion_r1602810089
##########
hplsql/src/main/java/org/apache/hive/hplsql/Var.java:
##########
@@ -629,6 +630,31 @@ else if (type == Type.STRING) {
}
return toString();
}
+
+ public String toSqlString(boolean isBuildSql, boolean handleStringType) {
Review Comment:
Why do we need these parameters `boolean isBuildSql, boolean handleStringType`?
IIUC this method should convert the value represented by this instance to the sql text representation which is a 1 to 1 mapping. I mean every type has one sql string representation.
I think this is related: why are the quotes not removed here?
https://github.com/apache/hive/blob/0e84fe2000c026afd0a49f4e7c7dd5f54fe7b1ec/hplsql/src/main/java/org/apache/hive/hplsql/Exec.java#L2573-L2576
IMHO the internal representation of a string value in the `Var` instance should not contain the single quotes since those are not part of the string value hence we should add them when converting back to sql text.
I also discovered that we use the Var class instances to store partially built SQL statements. These are not string literals and should not be quoted. Maybe a new type should be introduced to represent these
https://github.com/apache/hive/blob/0e84fe2000c026afd0a49f4e7c7dd5f54fe7b1ec/hplsql/src/main/java/org/apache/hive/hplsql/Exec.java#L303-L305
##########
hplsql/src/main/java/org/apache/hive/hplsql/Var.java:
##########
@@ -629,6 +630,31 @@ else if (type == Type.STRING) {
}
return toString();
}
+
+ public String toSqlString(boolean isBuildSql, boolean handleStringType) {
+ if (type == Type.IDENT) {
+ return name;
+ } else if (handleStringType && value == null) {
+ return "NULL";
+ } else if (type == Type.TIMESTAMP && isBuildSql) {
+ int len = 19;
+ String t = ((Timestamp) value).toString(); // .0 returned if the fractional part not set
+ if (scale > 0) {
+ len += scale + 1;
+ }
+ if (t.length() > len) {
+ t = t.substring(0, len);
+ }
+ return String.format("TIMESTAMP '%s'", t);
+ } else if (type == Type.DATE && isBuildSql) {
+ return String.format("DATE '%s'", ((Date) value).toString());
+ } else if (handleStringType && isBuildSql && type == Type.STRING && (!((String) value).startsWith(
+ "'") || !((String) value).endsWith("'"))) {
+ return Utils.quoteString(((String) value));
+ } else {
+ return toString();
Review Comment:
This is going to return `'Cloud'` for both inputs:
```
"p1('786', 'Cloud');\n" +
"p1('786', '''Cloud''');\n" +
```
but these inputs are not equal.
--
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-28143: After HIVE-27492 fix, some HPLSQL built-in functions like trim, lower are not working when used in insert statement. [hive]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #5182:
URL: https://github.com/apache/hive/pull/5182#issuecomment-2040448911
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5182) **Quality Gate passed**
Issues
 [6 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5182&resolved=false&inNewCodePeriod=true)
 [0 Accepted issues](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5182&metric=new_accepted_issues&view=list)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5182&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=5182)
--
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-28143: After HIVE-27492 fix, some HPLSQL built-in functions like trim, lower are not working when used in insert statement. [hive]
Posted by "mdayakar (via GitHub)" <gi...@apache.org>.
mdayakar commented on code in PR #5182:
URL: https://github.com/apache/hive/pull/5182#discussion_r1570337451
##########
hplsql/src/main/java/org/apache/hive/hplsql/Expression.java:
##########
@@ -110,7 +110,17 @@ else if (ctx.interval_item() != null) {
}
else {
visitChildren(ctx);
- sql.append(exec.stackPop().toString());
+ Var value = exec.stackPop();
+ if (value.type == Type.NULL && sql.toString().length() == 0) {
+ exec.stackPush(new Var());
+ return;
+ } else if (exec.buildSql && value.type == Type.DATE) {
+ sql.append(String.format("DATE '%s'", value.toString()));
+ } else if (exec.buildSql && value.type == Type.TIMESTAMP) {
+ sql.append(String.format("TIMESTAMP '%s'", value.toString()));
+ } else {
+ sql.append(value.toString());
+ }
Review Comment:
As a part of HIVE-27492 fix, added these format in Var.toString() API itself but this format is required only if the UDF used in as a select statement (DATE '2024-04-18' is valid where as 2024-04-18 is not a valid value in selection) so changed accordingly(now added only for select flow).
--
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-28143: After HIVE-27492 fix, some HPLSQL built-in functions like trim, lower are not working when used in insert statement. [hive]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #5182:
URL: https://github.com/apache/hive/pull/5182#issuecomment-2261302570
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5182) **Quality Gate passed**
Issues
 [33 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5182&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5182&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5182&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5182&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5182&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5182)
--
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-28143: After HIVE-27492 fix, some HPLSQL built-in functions like trim, lower are not working when used in insert statement. [hive]
Posted by "mdayakar (via GitHub)" <gi...@apache.org>.
mdayakar commented on code in PR #5182:
URL: https://github.com/apache/hive/pull/5182#discussion_r1570924035
##########
hplsql/src/main/java/org/apache/hive/hplsql/functions/FunctionMisc.java:
##########
@@ -113,7 +155,9 @@ void currentUser(HplsqlParser.Expr_spec_funcContext ctx) {
}
public static Var currentUser() {
- return new Var("CURRENT_USER()");
+ String currentUser = System.getProperty("user.name");
+ currentUser = Utils.quoteString(currentUser);
+ return new Var(currentUser);
Review Comment:
if the UDF is getting called as a part of select then without quotes it will throw error. Ex: `SELECT Bob ` where as `SELECT 'Bob'` is. a valid statement. So adding single quote.
--
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-28143: After HIVE-27492 fix, some HPLSQL built-in functions like trim, lower are not working when used in insert statement. [hive]
Posted by "mdayakar (via GitHub)" <gi...@apache.org>.
mdayakar commented on code in PR #5182:
URL: https://github.com/apache/hive/pull/5182#discussion_r1570900994
##########
hplsql/src/main/java/org/apache/hive/hplsql/Stmt.java:
##########
@@ -787,8 +787,19 @@ public Integer insertValues(HplsqlParser.Insert_stmtContext ctx) {
for (int i = 0; i < rows; i++) {
HplsqlParser.Insert_stmt_rowContext row =ctx.insert_stmt_rows().insert_stmt_row(i);
int cols = row.expr().size();
- for (int j = 0; j < cols; j++) {
- String value = evalPop(row.expr(j)).toSqlString();
+ for (int j = 0; j < cols; j++) {
+ Var var = evalPop(row.expr(j));
+ String value = null;
+ if (var.type == Type.TIMESTAMP) {
+ value = String.format("TIMESTAMP '%s'", var.toString());
+ } else if (var.type == Type.DATE) {
+ value = String.format("DATE '%s'", var.toString());
+ } else {
+ value = var.toSqlString();
+ }
Review Comment:
As explained at https://github.com/apache/hive/pull/5182#discussion_r1570337451 comment, this format is required for insert also otherwise it throws exception. Example: insert into test values('Bob', 2024-04-17 10:20:30) throws exception here even the the type of second column is timestamp.
--
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-28143: After HIVE-27492 fix, some HPLSQL built-in functions like trim, lower are not working when used in insert statement. [hive]
Posted by "mdayakar (via GitHub)" <gi...@apache.org>.
mdayakar commented on code in PR #5182:
URL: https://github.com/apache/hive/pull/5182#discussion_r1570912666
##########
hplsql/src/main/java/org/apache/hive/hplsql/Stmt.java:
##########
@@ -787,8 +787,19 @@ public Integer insertValues(HplsqlParser.Insert_stmtContext ctx) {
for (int i = 0; i < rows; i++) {
HplsqlParser.Insert_stmt_rowContext row =ctx.insert_stmt_rows().insert_stmt_row(i);
int cols = row.expr().size();
- for (int j = 0; j < cols; j++) {
- String value = evalPop(row.expr(j)).toSqlString();
+ for (int j = 0; j < cols; j++) {
+ Var var = evalPop(row.expr(j));
+ String value = null;
+ if (var.type == Type.TIMESTAMP) {
+ value = String.format("TIMESTAMP '%s'", var.toString());
+ } else if (var.type == Type.DATE) {
+ value = String.format("DATE '%s'", var.toString());
+ } else {
+ value = var.toSqlString();
+ }
+ if (value.startsWith("''")) {
Review Comment:
if the `value` has already a quote then `var.toSqlString();` add another one so remove that extra quote added this check. Anyway I changed the code to add the quote if it is not present.
--
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-28143: After HIVE-27492 fix, some HPLSQL built-in functions like trim, lower are not working when used in insert statement. [hive]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #5182:
URL: https://github.com/apache/hive/pull/5182#issuecomment-2065140555
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5182) **Quality Gate passed**
Issues
 [6 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5182&resolved=false&inNewCodePeriod=true)
 [0 Accepted issues](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5182&metric=new_accepted_issues&view=list)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5182&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=5182)
--
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-28143: After HIVE-27492 fix, some HPLSQL built-in functions like trim, lower are not working when used in insert statement. [hive]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #5182:
URL: https://github.com/apache/hive/pull/5182#issuecomment-2125617085
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5182) **Quality Gate passed**
Issues
 [19 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5182&resolved=false&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5182&resolutions=WONTFIX)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5182&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=5182)
--
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-28143: After HIVE-27492 fix, some HPLSQL built-in functions like trim, lower are not working when used in insert statement. [hive]
Posted by "mdayakar (via GitHub)" <gi...@apache.org>.
mdayakar commented on code in PR #5182:
URL: https://github.com/apache/hive/pull/5182#discussion_r1570658422
##########
hplsql/src/main/java/org/apache/hive/hplsql/Expression.java:
##########
@@ -565,7 +575,11 @@ public void operatorConcatSql(HplsqlParser.Expr_concatContext ctx) {
sql.append("CONCAT(");
int cnt = ctx.expr_concat_item().size();
for (int i = 0; i < cnt; i++) {
- sql.append(evalPop(ctx.expr_concat_item(i)).toString());
+ String concatStr = evalPop(ctx.expr_concat_item(i)).toString();
+ if (!concatStr.startsWith("'")) {
Review Comment:
For example below example
`SELECT '#' || TRIM(' Hello ') || '#';`
Here HPLSQL internally calls concat and when `TRIM(' Hello ')` is evaluated it will become `Hello` without quotes so here concat will throw error. To avoid it if the concat item is not starting with single quote then enclosing it with single quotes.
--
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-28143: After HIVE-27492 fix, some HPLSQL built-in functions like trim, lower are not working when used in insert statement. [hive]
Posted by "mdayakar (via GitHub)" <gi...@apache.org>.
mdayakar commented on code in PR #5182:
URL: https://github.com/apache/hive/pull/5182#discussion_r1600530114
##########
hplsql/src/main/java/org/apache/hive/hplsql/Stmt.java:
##########
@@ -787,8 +787,19 @@ public Integer insertValues(HplsqlParser.Insert_stmtContext ctx) {
for (int i = 0; i < rows; i++) {
HplsqlParser.Insert_stmt_rowContext row =ctx.insert_stmt_rows().insert_stmt_row(i);
int cols = row.expr().size();
- for (int j = 0; j < cols; j++) {
- String value = evalPop(row.expr(j)).toSqlString();
+ for (int j = 0; j < cols; j++) {
+ Var var = evalPop(row.expr(j));
+ String value = null;
+ if (var.type == Type.TIMESTAMP) {
+ value = String.format("TIMESTAMP '%s'", var.toString());
+ } else if (var.type == Type.DATE) {
+ value = String.format("DATE '%s'", var.toString());
+ } else {
+ value = var.toSqlString();
+ }
Review Comment:
Added Var.toSqlString(boolean isBuildSql) API and using the same.
--
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-28143: After HIVE-27492 fix, some HPLSQL built-in functions like trim, lower are not working when used in insert statement. [hive]
Posted by "mdayakar (via GitHub)" <gi...@apache.org>.
mdayakar commented on code in PR #5182:
URL: https://github.com/apache/hive/pull/5182#discussion_r1600532609
##########
hplsql/src/main/java/org/apache/hive/hplsql/functions/FunctionMisc.java:
##########
@@ -59,6 +64,43 @@ public void register(BuiltinFunctions f) {
void activityCount(HplsqlParser.Expr_spec_funcContext ctx) {
evalInt(Long.valueOf(exec.getRowCount()));
}
+
+ /**
+ * CAST function
+ */
+ void cast(HplsqlParser.Expr_spec_funcContext ctx) {
+ if (ctx.expr().size() != 1) {
+ evalNull();
+ return;
+ }
Review Comment:
As a part [HIVE-27492](https://issues.apache.org/jira/browse/HIVE-27492) fix, we fixed some issues like UDF using in select statement, that fix should be present so not reverting back entire [HIVE-27492](https://issues.apache.org/jira/browse/HIVE-27492) fix only the removed UDF cde is getting added and corrected for the functionality.
--
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-28143: After HIVE-27492 fix, some HPLSQL built-in functions like trim, lower are not working when used in insert statement. [hive]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #5182:
URL: https://github.com/apache/hive/pull/5182#issuecomment-2065929338
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5182) **Quality Gate passed**
Issues
 [6 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5182&resolved=false&inNewCodePeriod=true)
 [0 Accepted issues](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5182&metric=new_accepted_issues&view=list)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5182&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=5182)
--
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-28143: After HIVE-27492 fix, some HPLSQL built-in functions like trim, lower are not working when used in insert statement. [hive]
Posted by "mdayakar (via GitHub)" <gi...@apache.org>.
mdayakar commented on code in PR #5182:
URL: https://github.com/apache/hive/pull/5182#discussion_r1570148189
##########
hplsql/src/main/antlr4/org/apache/hive/hplsql/Hplsql.g4:
##########
@@ -1563,7 +1563,7 @@ non_reserved_words : // Tokens that are not reserved words
| T_TO
| T_TOP
| T_TRANSACTION
- | T_TRIM
+ // | T_TRIM
Review Comment:
No. Now uncommented 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.
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-28143: After HIVE-27492 fix, some HPLSQL built-in functions like trim, lower are not working when used in insert statement. [hive]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #5182:
URL: https://github.com/apache/hive/pull/5182#issuecomment-2111060813
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5182) **Quality Gate passed**
Issues
 [6 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5182&resolved=false&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5182&metric=new_accepted_issues&view=list)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5182&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=5182)
--
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-28143: After HIVE-27492 fix, some HPLSQL built-in functions like trim, lower are not working when used in insert statement. [hive]
Posted by "kasakrisz (via GitHub)" <gi...@apache.org>.
kasakrisz commented on code in PR #5182:
URL: https://github.com/apache/hive/pull/5182#discussion_r1557648667
##########
hplsql/src/main/java/org/apache/hive/hplsql/Var.java:
##########
@@ -601,7 +602,7 @@ else if (type == Type.STRING) {
return (String)value;
}
else if (type == Type.DATE) {
- return String.format("DATE '%s'", value);
+ return ((Date)value).toString();
Review Comment:
Is it safe to remove the `DATE` keyword? Let's say we have `2024-04-09`. Is it a string or a date literal?
##########
hplsql/src/main/java/org/apache/hive/hplsql/Expression.java:
##########
@@ -565,7 +575,11 @@ public void operatorConcatSql(HplsqlParser.Expr_concatContext ctx) {
sql.append("CONCAT(");
int cnt = ctx.expr_concat_item().size();
for (int i = 0; i < cnt; i++) {
- sql.append(evalPop(ctx.expr_concat_item(i)).toString());
+ String concatStr = evalPop(ctx.expr_concat_item(i)).toString();
+ if (!concatStr.startsWith("'")) {
Review Comment:
AFAIK `concat` expects string parameters. The actual parameters can be a
* string literals which needs quotation
* or an string typed expressions which should not be quoted
Could you please share an example when quotation is missing but required at this point?
##########
hplsql/src/main/java/org/apache/hive/hplsql/Stmt.java:
##########
@@ -787,8 +787,19 @@ public Integer insertValues(HplsqlParser.Insert_stmtContext ctx) {
for (int i = 0; i < rows; i++) {
HplsqlParser.Insert_stmt_rowContext row =ctx.insert_stmt_rows().insert_stmt_row(i);
int cols = row.expr().size();
- for (int j = 0; j < cols; j++) {
- String value = evalPop(row.expr(j)).toSqlString();
+ for (int j = 0; j < cols; j++) {
+ Var var = evalPop(row.expr(j));
+ String value = null;
+ if (var.type == Type.TIMESTAMP) {
+ value = String.format("TIMESTAMP '%s'", var.toString());
+ } else if (var.type == Type.DATE) {
+ value = String.format("DATE '%s'", var.toString());
+ } else {
+ value = var.toSqlString();
+ }
+ if (value.startsWith("''")) {
Review Comment:
Which case can we have "double" quoted value?
##########
hplsql/src/main/java/org/apache/hive/hplsql/functions/FunctionDatetime.java:
##########
@@ -162,6 +174,35 @@ void toTimestamp(HplsqlParser.Expr_func_paramsContext ctx) {
}
}
+ /**
+ * FROM_UNIXTIME() function (convert seconds since 1970-01-01 00:00:00 to timestamp)
+ */
+ void fromUnixtime(HplsqlParser.Expr_func_paramsContext ctx) {
+ int cnt = BuiltinFunctions.getParamCount(ctx);
+ if (cnt == 0) {
+ evalNull();
+ return;
+ }
+ Var value = evalPop(ctx.func_param(0).expr());
+ if (value.type != Var.Type.BIGINT) {
+ Var newVar = new Var(Var.Type.BIGINT);
+ value = newVar.cast(value);
+ }
+ long epoch = value.longValue();
+ String format = "yyyy-MM-dd HH:mm:ss";
+ if (cnt > 1) {
+ format = Utils.unquoteString(evalPop(ctx.func_param(1).expr()).toString());
+ }
+ evalString(Utils.quoteString(new SimpleDateFormat(format).format(new Date(epoch * 1000))));
Review Comment:
Can the newer Java Date Time API be used? `java.time.*`
##########
hplsql/src/main/antlr4/org/apache/hive/hplsql/Hplsql.g4:
##########
@@ -1563,7 +1563,7 @@ non_reserved_words : // Tokens that are not reserved words
| T_TO
| T_TOP
| T_TRANSACTION
- | T_TRIM
+ // | T_TRIM
Review Comment:
Is `trim` a reserved word?
##########
hplsql/src/main/java/org/apache/hive/hplsql/Expression.java:
##########
@@ -110,7 +110,17 @@ else if (ctx.interval_item() != null) {
}
else {
visitChildren(ctx);
- sql.append(exec.stackPop().toString());
+ Var value = exec.stackPop();
+ if (value.type == Type.NULL && sql.toString().length() == 0) {
+ exec.stackPush(new Var());
+ return;
+ } else if (exec.buildSql && value.type == Type.DATE) {
+ sql.append(String.format("DATE '%s'", value.toString()));
+ } else if (exec.buildSql && value.type == Type.TIMESTAMP) {
+ sql.append(String.format("TIMESTAMP '%s'", value.toString()));
+ } else {
+ sql.append(value.toString());
+ }
Review Comment:
Can `Var.toString()` be used instead? it seems that not just date and timestamp needs custom string format:
https://github.com/apache/hive/blob/be857bf83c00e41a315a7cc8e013832cf169ef18/hplsql/src/main/java/org/apache/hive/hplsql/Var.java#L590-L618
##########
hplsql/src/main/java/org/apache/hive/hplsql/Stmt.java:
##########
@@ -787,8 +787,19 @@ public Integer insertValues(HplsqlParser.Insert_stmtContext ctx) {
for (int i = 0; i < rows; i++) {
HplsqlParser.Insert_stmt_rowContext row =ctx.insert_stmt_rows().insert_stmt_row(i);
int cols = row.expr().size();
- for (int j = 0; j < cols; j++) {
- String value = evalPop(row.expr(j)).toSqlString();
+ for (int j = 0; j < cols; j++) {
+ Var var = evalPop(row.expr(j));
+ String value = null;
+ if (var.type == Type.TIMESTAMP) {
+ value = String.format("TIMESTAMP '%s'", var.toString());
+ } else if (var.type == Type.DATE) {
+ value = String.format("DATE '%s'", var.toString());
+ } else {
+ value = var.toSqlString();
+ }
Review Comment:
How about `var.toString()`?
##########
hplsql/src/main/java/org/apache/hive/hplsql/functions/FunctionMisc.java:
##########
@@ -59,6 +64,43 @@ public void register(BuiltinFunctions f) {
void activityCount(HplsqlParser.Expr_spec_funcContext ctx) {
evalInt(Long.valueOf(exec.getRowCount()));
}
+
+ /**
+ * CAST function
+ */
+ void cast(HplsqlParser.Expr_spec_funcContext ctx) {
+ if (ctx.expr().size() != 1) {
+ evalNull();
+ return;
+ }
Review Comment:
Does it mean that the user specified wrong parameters? Should we throw exception?
##########
hplsql/src/main/java/org/apache/hive/hplsql/functions/FunctionMisc.java:
##########
@@ -211,6 +269,16 @@ public void partCount(HplsqlParser.Expr_spec_funcContext ctx) {
query.close();
}
+ public void modulo(HplsqlParser.Expr_func_paramsContext ctx) {
+ if (ctx.func_param().size() == 2) {
+ int a = evalPop(ctx.func_param(0).expr()).intValue();
+ int b = evalPop(ctx.func_param(1).expr()).intValue();
+ evalInt(a % b);
+ } else {
+ evalNull();
Review Comment:
Should exception be thrown?
##########
hplsql/src/main/java/org/apache/hive/hplsql/functions/FunctionMisc.java:
##########
@@ -113,7 +155,9 @@ void currentUser(HplsqlParser.Expr_spec_funcContext ctx) {
}
public static Var currentUser() {
- return new Var("CURRENT_USER()");
+ String currentUser = System.getProperty("user.name");
+ currentUser = Utils.quoteString(currentUser);
+ return new Var(currentUser);
Review Comment:
Why do we need quotes here? The variable has string type:
https://github.com/apache/hive/blob/be857bf83c00e41a315a7cc8e013832cf169ef18/hplsql/src/main/java/org/apache/hive/hplsql/Var.java#L77-L80
##########
hplsql/src/main/java/org/apache/hive/hplsql/Var.java:
##########
@@ -612,7 +613,7 @@ else if (type == Type.TIMESTAMP) {
if (t.length() > len) {
t = t.substring(0, len);
}
- return String.format("TIMESTAMP '%s'", t);
+ return t;
Review Comment:
Is it safe to remove the `TIMESTAMP`?
A few lines above a comment says the value can be `.0`. Is it a float or a timestamp?
https://github.com/apache/hive/blob/be857bf83c00e41a315a7cc8e013832cf169ef18/hplsql/src/main/java/org/apache/hive/hplsql/Var.java#L608C54-L608C56
--
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-28143: After HIVE-27492 fix, some HPLSQL built-in functions like trim, lower are not working when used in insert statement. [hive]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #5182:
URL: https://github.com/apache/hive/pull/5182#issuecomment-2039795751
## [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=5182) **Quality Gate passed**
Issues
 [22 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=5182&resolved=false&inNewCodePeriod=true)
 [0 Accepted issues](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=5182&metric=new_accepted_issues&view=list)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=5182&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=5182)
--
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