dilipbiswal commented on a change in pull request #28750:
URL: https://github.com/apache/spark/pull/28750#discussion_r436808519
##########
File path:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/StringUtils.scala
##########
@@ -122,8 +122,8 @@ object StringUtils extends Logging {
val available = maxLength - length
val stringToAppend = if (available >= sLen) s else s.substring(0,
available)
strings.append(stringToAppend)
+ length += sLen
}
- length += sLen
Review comment:
@srowen I think, the original intent of keeping this length outside the
if block (which i missed and @MaxGekk pointed out) is to keep the true length
of the input. So when we are producing a SQL plan, we want to tell users how
much was that the size of the explain string and how much we truncated based on
max length specification. So caller could call append method even after we have
gone past the max.
cc @MaxGekk
##########
File path:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/StringUtils.scala
##########
@@ -122,8 +122,8 @@ object StringUtils extends Logging {
val available = maxLength - length
val stringToAppend = if (available >= sLen) s else s.substring(0,
available)
strings.append(stringToAppend)
+ length += sLen
}
- length += sLen
Review comment:
@srowen You are right that, we may under report when the length becomes
more than MAX_ROUNDED_ARRAY_LENGTH. I did consider changing the message to
cover the case to say something like "greater than or equal to N". But thought
against it as we are adding code to cover may be a extremely corner case.
Please let me know if you have any suggestion here.
About changing to long, even if we did it, we can in theory hit the long
limit, right ?
##########
File path:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/StringUtils.scala
##########
@@ -122,8 +122,8 @@ object StringUtils extends Logging {
val available = maxLength - length
val stringToAppend = if (available >= sLen) s else s.substring(0,
available)
strings.append(stringToAppend)
+ length += sLen
}
- length += sLen
Review comment:
Thank you @srowen.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
