Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inserting to a column CLOB, fill all clob with x00 #187

Open
abamara opened this issue Oct 23, 2024 · 14 comments
Open

Inserting to a column CLOB, fill all clob with x00 #187

abamara opened this issue Oct 23, 2024 · 14 comments
Labels
bug Something isn't working

Comments

@abamara
Copy link

abamara commented Oct 23, 2024

Hi

Inserting to a CLOB column, fill all clob with x00 at the end.

When inserting a row, using bindParameters with and targeting a table with a CLOB it add 00 to fill all clob size.
If my CLOB is defined with a size 1MB, select LENGTH(MYCLOB) from MYTABLE return 1MB and not the size of my actual data.

If I use a 2D array bindParameters( [ ['mytext', db.IN, db.SQL_ALL_TYPE] ] ) it fill the full CLOB with x00.
So the select LENGTH(MYCLOB) form MYTABLE returns 1MB

If I use a 1D array bindParameters( ['mytext'] ) it doesn't fill the CLOB column with x00
So the select LENGTH(MYCLOB) form MYTABLE returns only the actual data I inserted.

@abamara abamara added the bug Something isn't working label Oct 23, 2024
@abamara
Copy link
Author

abamara commented Oct 23, 2024

In the dbstmt.cc code

When using a 2D array for parameters, for a clob we need to use 0 or 1 bind indicator, since there is no CLOB indicator

at line ~2601

 if (bindIndicator == 0 || bindIndicator == 1)
      { //Parameter is string or clob
        std::string string = value.ToString().Utf8Value();
        size_t str_length = string.length();
        const char *cString = string.c_str();
        param[i].valueType = SQL_C_CHAR;
        // CLI does not honor the buffer size parameter or the output size in the indicator
        // Ensure the buffer size is at least the size of parameter or the size of the string

        param[i].buf = (char *)calloc(std::max(static_cast<size_t>(param[i].paramSize), str_length) + 1, sizeof(char));
        // Set the indicator to be SQL_NTS
        // this helps in edge cases like empty string where indicator is set 0 (which CLI doesn't like for some reason)
        param[i].ind = SQL_NTS;
        if (param[i].io != SQL_PARAM_OUTPUT) {
            // for SQL_PARAM_INPUT and SQL_PARAM_INPUT_OUTPUT
            // copy the string to the buffer
            strcpy((char*)param[i].buf, cString);
        }
      }

I see that param[i].ind = SQL_NTS; can be the part of the problem
I think that param[i].ind should be set to str_length for CLOBS as it is done for BLOBS

@kadler
Copy link
Member

kadler commented Oct 23, 2024

When inserting a row, using bindParameters with and targeting a table with a CLOB it add 00 to fill all clob size.

Are you adding the 00 to fill to clob size or are you saying that nodejs-idb-connector is?

@abamara
Copy link
Author

abamara commented Oct 23, 2024

nodejs-idb-connector/cli is adding the clob x00 to fill all the clob.
Since it is working well when I use 1D array parameters but not 2D array parameters

@kadler
Copy link
Member

kadler commented Oct 23, 2024

I see that param[i].ind = SQL_NTS; can be the part of the problem I think that param[i].ind should be set to str_length for CLOBS as it is done for BLOBS

        std::string string = value.ToString().Utf8Value();
        const char *cString = string.c_str();

        param[i].buf = (char *)calloc(std::max(static_cast<size_t>(param[i].paramSize), str_length) + 1, sizeof(char));
        if (param[i].io != SQL_PARAM_OUTPUT) {
            strcpy((char*)param[i].buf, cString);
        }

I the code you referenced, the data that is copied in from Node.js is converted to a std::string and then we access it as a C-style null-terminated string, then we copy it to an internal buffer which is initialized to all null. The copy is also a C-style string copy. And we also set the indicator to tell CLI that the data is null-terminated. Any single one of these steps would mess up inserting null bytes, but all together there's overwhelmingly no way this would cause the data to be padded with null bytes.

Though, I could see maybe this should be changed so that if a user did put embedded null bytes in their JS string that they would get inserted whereas today the data would be truncated incorrectly which is the opposite of your problem.

It must be somewhere else within the idb-connector code which is causing your issue. Or else the problem is within CLI itself.

@abamara
Copy link
Author

abamara commented Oct 23, 2024

The problem I end with a lot of x'00' values, and I was only inserting a small string.
If my CLOB is 10 MB, it will always end with 10MB written. I was only inserting a smaller string.

I think the problem is that when you use 1D array params, the CLOB is written to the correct size with no many nulls embeded at the end.

For 1D array params it end at this case :

case SQL_CLOB:
      {
        param[i].valueType = SQL_C_CHAR;
        param[i].buf = (char *)calloc(param[i].paramSize + 1, sizeof(char));
        if(value.IsString())
        {
          std::string string = value.ToString().Utf8Value();
          const char *cString = string.c_str();
          if(strlen(cString) > 0) 
          {
            strcpy((char *)param[i].buf, cString);
            param[i].ind = strlen(cString);
            break;
          }
        }
        // value is '' -> output param
        param[i].ind = param[i].paramSize;
      }

The only difference I see is that param[i].ind has a different value than SQL_NTS.

@kadler
Copy link
Member

kadler commented Oct 23, 2024

Must be this code then:

        // value is '' -> output param
        param[i].ind = param[i].paramSize;

Since the code within the if(value.IsString()) block is doing all using null-terminated strings.

You are passing in something like null for the value?

@abamara
Copy link
Author

abamara commented Oct 23, 2024

I tested with the same javascript string, I got different results ( works well with 1D array params but not with 2 array params )

@kadler
Copy link
Member

kadler commented Oct 23, 2024

Can you provide example JS code which can recreate the problem?

@abamara
Copy link
Author

abamara commented Oct 23, 2024

const db = require('idb-connector');

wDbconn = new db.dbconn();
...

dbStmt = new db.dbstmt(wDbconn);

var sql = "INSERT INTO MYTABLE (MYCLOB) VALUES ( ? ) WITH NONE\n";
 
var params = [
               ["MyTest", db.IN, db.SQL_ALL_TYPES]
             ];

dbStmt.prepare(sql, (error) => {
        if (error) {
          throw error;
        }
        dbStmt.bindParameters(params, (error) => {
          if (error) {
            throw error;
          }
          dbStmt.execute((out, error) => {
            if (error) {
              throw error;
            }
            const result = dbStmt.commit();
          });
        });
      });

In this case 'select LENGTH(MYCLOB) from MYTABLE' returns me 1MB.

@abmusse
Copy link
Member

abmusse commented Oct 23, 2024

var params = [
               ["MyTest", db.IN, db.SQL_ALL_TYPES]
             ];

Hello @abamara 👋🏿

Why are you are using db.SQL_ALL_TYPES as the type?

There should be db.CLOB that can be used with 2D Array to indicate that the type is a CLOB.

@abamara
Copy link
Author

abamara commented Oct 23, 2024

Hi,

If I use db.SQL_CLOB, I get this error ; BIND INDICATOR FOR PARAMETER 4IS INVALID

db.SQL_ALL_TYPES is equivalent to 0.
db.CLOB is defined as 0 ?

@kadler
Copy link
Member

kadler commented Oct 23, 2024

@abmusse I can't recall how the idb-connector bind code works, but if the type passed in by the user is passed as the target type on SQLBindCol, then this will not work properly for SQL_CLOB as that requires binding as a LOB struct, IIRC.

@abmusse
Copy link
Member

abmusse commented Oct 23, 2024

db.SQL_ALL_TYPES is equivalent to 0.
db.CLOB is defined as 0 ?

Using db.SQL_CLOB wouldn't work as you've seen.

Yes SQL_ALL_TYPES and CLOB alias are equivalent both are 0 so these would go down same code path. I was unsure what SQL_ALL_TYPES was defined to but looks like its the same as CLOB.

@kadler

In the case of 2-D array the user passes in 0 to indicate I want to bind a clob and Looks like the indicator is set as SQL_NTS like @abamara mentioned earlier.

This will later be used in the SQLBindParameter call.

if (bindIndicator == 0 || bindIndicator == 1)
{ //Parameter is string or clob
std::string string = value.ToString().Utf8Value();
size_t str_length = string.length();
const char *cString = string.c_str();
param[i].valueType = SQL_C_CHAR;
// CLI does not honor the buffer size parameter or the output size in the indicator
// Ensure the buffer size is at least the size of parameter or the size of the string
param[i].buf = (char *)calloc(std::max(static_cast<size_t>(param[i].paramSize), str_length) + 1, sizeof(char));
// Set the indicator to be SQL_NTS
// this helps in edge cases like empty string where indicator is set 0 (which CLI doesn't like for some reason)
param[i].ind = SQL_NTS;
if (param[i].io != SQL_PARAM_OUTPUT) {
// for SQL_PARAM_INPUT and SQL_PARAM_INPUT_OUTPUT
// copy the string to the buffer
strcpy((char*)param[i].buf, cString);
}
}

In the case of the 1D Array we handle CLOBs differently:

case SQL_CLOB:
{
param[i].valueType = SQL_C_CHAR;
param[i].buf = (char *)calloc(param[i].paramSize + 1, sizeof(char));
if(value.IsString())
{
std::string string = value.ToString().Utf8Value();
const char *cString = string.c_str();
if(strlen(cString) > 0)
{
strcpy((char *)param[i].buf, cString);
param[i].ind = strlen(cString);
break;
}
}
// value is '' -> output param
param[i].ind = param[i].paramSize;

We set the indicator to the parameter size that was retrieved from SQLDescribeParam call.

So perhaps using SQL_NTS is causing the weird behavior?

@kadler
Copy link
Member

kadler commented Oct 23, 2024

Well, I see that we don't set ind in the case where strlen was <= 0, but that shouldn't affect this case with "MyTest" which should be set to its length (6). I think we need to try to recreate this and debug.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants