You are viewing a plain text version of this content. The canonical link for it is here.
Posted to codereview@trafodion.apache.org by traflm <gi...@git.apache.org> on 2018/01/13 14:52:47 UTC
[GitHub] trafodion pull request #1397: [TRAFODION-2904] optionally add leading space ...
GitHub user traflm opened a pull request:
https://github.com/apache/trafodion/pull/1397
[TRAFODION-2904] optionally add leading space when get a number colum…
…n to display
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/traflm/trafodion j2904
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/trafodion/pull/1397.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1397
----
commit 56e9a807ed98b5f304befe834b13d4e62cbebb36
Author: Liu Ming <ov...@...>
Date: 2018-01-13T09:37:45Z
[TRAFODION-2904] optionally add leading space when get a number column to display
----
---
[GitHub] trafodion pull request #1397: [TRAFODION-2904] optionally add leading space ...
Posted by DaveBirdsall <gi...@git.apache.org>.
Github user DaveBirdsall commented on a diff in the pull request:
https://github.com/apache/trafodion/pull/1397#discussion_r161409312
--- Diff: core/sql/optimizer/BindItemExpr.cpp ---
@@ -3654,12 +3654,10 @@ ItemExpr *Concat::bindNode(BindWA *bindWA)
else if (convType == 2)
{
Parser parser(bindWA->currentCmpContext());
- char buf[1000];
+ char buf[128];
- // right justify the string representation of numeric operand
- // and then do the concat
- sprintf(buf, "CAST(SPACE(%d - CHAR_LENGTH(CAST(@A1 AS VARCHAR(%d)))) || CAST(@A1 AS VARCHAR(%d)) AS VARCHAR(%d))",
- dLen, dLen, dLen, dLen);
+ sprintf(buf, "CAST(CAST(@A1 AS VARCHAR(%d)) AS VARCHAR(%d))",
--- End diff --
Just curious: Why do we need two casts? Would one be enough? (Looks like we use the same length for both.)
---
[GitHub] trafodion pull request #1397: [TRAFODION-2904] optionally add leading space ...
Posted by traflm <gi...@git.apache.org>.
Github user traflm commented on a diff in the pull request:
https://github.com/apache/trafodion/pull/1397#discussion_r161424099
--- Diff: core/sql/optimizer/BindItemExpr.cpp ---
@@ -3654,12 +3654,10 @@ ItemExpr *Concat::bindNode(BindWA *bindWA)
else if (convType == 2)
{
Parser parser(bindWA->currentCmpContext());
- char buf[1000];
+ char buf[128];
- // right justify the string representation of numeric operand
- // and then do the concat
- sprintf(buf, "CAST(SPACE(%d - CHAR_LENGTH(CAST(@A1 AS VARCHAR(%d)))) || CAST(@A1 AS VARCHAR(%d)) AS VARCHAR(%d))",
- dLen, dLen, dLen, dLen);
+ sprintf(buf, "CAST(CAST(@A1 AS VARCHAR(%d)) AS VARCHAR(%d))",
--- End diff --
That is my question as well :-)
I don't really understand the syntax here, if without the out CAST, the parser will fail to parse this string.
There are many such cases, I wish to understand the special syntax here later. But this is what it required for now.
---
[GitHub] trafodion pull request #1397: [TRAFODION-2904] optionally add leading space ...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/trafodion/pull/1397
---