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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
