You are viewing a plain text version of this content. The canonical link for it is here.
Posted to codereview@trafodion.apache.org by Weixin-Xu <gi...@git.apache.org> on 2017/11/27 11:40:46 UTC

[GitHub] incubator-trafodion pull request #1311: [TRAFODION-2818] For Server 2008, fu...

GitHub user Weixin-Xu opened a pull request:

    https://github.com/apache/incubator-trafodion/pull/1311

    [TRAFODION-2818] For Server 2008, function pow() throws STATUS_ILLEGAL_INSTRUCTION

    SQL_NUMERIC to SQL_C_CHAR
    ***ERROR: SQLGetData: Expected: SQL_SUCCESS Actual: SQL_ERROR
    State: HY000
    Native Error: 29
    Error: [TRAF][Trafodion ODBC Driver] General error.EXCEPTION: UNKNOWN - Exception Code = c000001d

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/Weixin-Xu/incubator-trafodion master

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/incubator-trafodion/pull/1311.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 #1311
    
----
commit 2e1f26f7499bd51f03ba28f89cdf563432278603
Author: Weixin-Xu <we...@esgyn.cn>
Date:   2017-11-27T11:37:06Z

    [TRAFODION-2818] For Server 2008, function pow() throws
    STATUS_ILLEGAL_INSTRUCTION

----


---

[GitHub] incubator-trafodion pull request #1311: [TRAFODION-2818] For Server 2008, fu...

Posted by Weixin-Xu <gi...@git.apache.org>.
Github user Weixin-Xu commented on a diff in the pull request:

    https://github.com/apache/incubator-trafodion/pull/1311#discussion_r153976065
  
    --- Diff: win-odbc64/odbcclient/drvr35/sqltocconv.cpp ---
    @@ -36,6 +36,34 @@
     
     #define MAXCHARLEN 32768 //32K
     
    +// for server2008 when using function pow() then throws STATUS_ILLEGAL_INSTRUCTION
    +__int64 pow(int base, short scale)
    +{
    +	DWORD dwVersion = 0;
    +	DWORD dwBuild = 0;
    +
    +	dwVersion = GetVersion();
    +
    +	// Get the build number.
    +
    +	if (dwVersion < 0x80000000)
    +		dwBuild = (DWORD)(HIWORD(dwVersion));
    +
    +	__int64 retValue = 1;
    +	if (dwBuild == 7600)
    +	{
    +		scale = scale > 18 ? 18 : scale;
    +		for (int i = 0; i < scale; i++)
    +			retValue = retValue * 10;
    +	}
    +	else
    +	{
    +		retValue = pow((double)base, scale);
    --- End diff --
    
    When using the standard pow function in ODBC under server2008, it throws STATUS_ILLEGAL_INSTRUCTION.
    For the standard pow function, the type of parameter base is _Common_float_type including float, double and long double.
    So using type int  as the parameter in pow() can lead linker to call this function first.


---

[GitHub] incubator-trafodion pull request #1311: [TRAFODION-2818] For Server 2008, fu...

Posted by DaveBirdsall <gi...@git.apache.org>.
Github user DaveBirdsall commented on a diff in the pull request:

    https://github.com/apache/incubator-trafodion/pull/1311#discussion_r153246992
  
    --- Diff: win-odbc64/odbcclient/drvr35/sqltocconv.cpp ---
    @@ -36,6 +36,34 @@
     
     #define MAXCHARLEN 32768 //32K
     
    +// for server2008 when using function pow() then throws STATUS_ILLEGAL_INSTRUCTION
    +__int64 pow(int base, short scale)
    +{
    +	DWORD dwVersion = 0;
    +	DWORD dwBuild = 0;
    +
    +	dwVersion = GetVersion();
    +
    +	// Get the build number.
    +
    +	if (dwVersion < 0x80000000)
    +		dwBuild = (DWORD)(HIWORD(dwVersion));
    +
    +	__int64 retValue = 1;
    +	if (dwBuild == 7600)
    +	{
    +		scale = scale > 18 ? 18 : scale;
    +		for (int i = 0; i < scale; i++)
    +			retValue = retValue * 10;
    --- End diff --
    
    So, if the scale was 19, we would return 10^18? Is this correct? Shouldn't there be some sort of overflow error instead?


---

[GitHub] incubator-trafodion pull request #1311: [TRAFODION-2818] For Server 2008, fu...

Posted by selvaganesang <gi...@git.apache.org>.
Github user selvaganesang commented on a diff in the pull request:

    https://github.com/apache/incubator-trafodion/pull/1311#discussion_r153837812
  
    --- Diff: win-odbc64/odbcclient/drvr35/sqltocconv.cpp ---
    @@ -36,6 +36,34 @@
     
     #define MAXCHARLEN 32768 //32K
     
    +// for server2008 when using function pow() then throws STATUS_ILLEGAL_INSTRUCTION
    +__int64 pow(int base, short scale)
    +{
    +	DWORD dwVersion = 0;
    +	DWORD dwBuild = 0;
    +
    +	dwVersion = GetVersion();
    +
    +	// Get the build number.
    +
    +	if (dwVersion < 0x80000000)
    +		dwBuild = (DWORD)(HIWORD(dwVersion));
    +
    +	__int64 retValue = 1;
    +	if (dwBuild == 7600)
    +	{
    +		scale = scale > 18 ? 18 : scale;
    +		for (int i = 0; i < scale; i++)
    +			retValue = retValue * 10;
    +	}
    +	else
    +	{
    +		retValue = pow((double)base, scale);
    --- End diff --
    
    Suggest to look at the errno to return proper errors.  I wonder due to compatible method signature, there is possibility that linker might resolve to the standard pow function instead of calling this function.


---

[GitHub] incubator-trafodion pull request #1311: [TRAFODION-2818] For Server 2008, fu...

Posted by selvaganesang <gi...@git.apache.org>.
Github user selvaganesang commented on a diff in the pull request:

    https://github.com/apache/incubator-trafodion/pull/1311#discussion_r155901708
  
    --- Diff: win-odbc64/odbcclient/drvr35/sqltocconv.cpp ---
    @@ -36,6 +36,34 @@
     
     #define MAXCHARLEN 32768 //32K
     
    +// for server2008 when using function pow() then throws STATUS_ILLEGAL_INSTRUCTION
    +__int64 pow(int base, short scale)
    +{
    +	DWORD dwVersion = 0;
    +	DWORD dwBuild = 0;
    +
    +	dwVersion = GetVersion();
    +
    +	// Get the build number.
    +
    +	if (dwVersion < 0x80000000)
    +		dwBuild = (DWORD)(HIWORD(dwVersion));
    +
    +	__int64 retValue = 1;
    +	if (dwBuild == 7600)
    +	{
    +		scale = scale > 18 ? 18 : scale;
    +		for (int i = 0; i < scale; i++)
    +			retValue = retValue * 10;
    +	}
    +	else
    +	{
    +		retValue = pow((double)base, scale);
    --- End diff --
    
    I would suggest to retain double data type for return type to enable a large result to be accommodated without raising numeric overflow. Hence, I would suggest Trafodion function can have an extra parameter like to pass the errno as in
    double pow (double base, double exponent, int *error)
    
    Pass the errno to be returned to the caller for it handle it. This would also take care of any ambiguity in resolution of the method
     


---

[GitHub] incubator-trafodion pull request #1311: [TRAFODION-2818] For Server 2008, fu...

Posted by Weixin-Xu <gi...@git.apache.org>.
Github user Weixin-Xu commented on a diff in the pull request:

    https://github.com/apache/incubator-trafodion/pull/1311#discussion_r156836851
  
    --- Diff: win-odbc64/odbcclient/drvr35/sqltocconv.cpp ---
    @@ -36,6 +36,34 @@
     
     #define MAXCHARLEN 32768 //32K
     
    +// for server2008 when using function pow() then throws STATUS_ILLEGAL_INSTRUCTION
    +__int64 pow(int base, short scale)
    +{
    +	DWORD dwVersion = 0;
    +	DWORD dwBuild = 0;
    +
    +	dwVersion = GetVersion();
    +
    +	// Get the build number.
    +
    +	if (dwVersion < 0x80000000)
    +		dwBuild = (DWORD)(HIWORD(dwVersion));
    +
    +	__int64 retValue = 1;
    +	if (dwBuild == 7600)
    +	{
    +		scale = scale > 18 ? 18 : scale;
    +		for (int i = 0; i < scale; i++)
    +			retValue = retValue * 10;
    --- End diff --
    
    For the function used only in sqltocconv.cpp, server always gives a scale less than 19.
    The function is only used to figure up the fractional length of numeric whose precision is not more than 18.
    Actually, it need not to be compared with 18.


---