oracle / node-oracledb

Oracle Database driver for Node.js maintained by Oracle Corp.
http://oracle.github.io/node-oracledb/
Other
2.26k stars 1.08k forks source link

Binding temporary LOBs as procedure inbound parameters leads to "Uncaught Error: NJS-022: invalid Lob" #1064

Closed wallride closed 5 years ago

wallride commented 5 years ago

I've got some problems with creating LOBs and passing them into PL/SQL procedure parameters.

Here's some primitive procedure for testing:

procedure mock_lobs(P_CLOB  IN OUT CLOB, P_BLOB  IN OUT BLOB) AS BEGIN
      P_CLOB := P_CLOB;
END mock_lobs;

Here's some simplified code to create LOBs (typescript):

function createLob(connection: any, type: number): Promise<oracle.Lob> {
    // For some reason "createLob" method is not provided in typescript definitions ...
    // so I have to make this dirty hack to avoid transpiling errors
    return <Promise<oracle.Lob>> connection.createLob(type);
}

const clob = await createLob(connection, oracle.CLOB);
const blob = await createLob(connection, oracle.BLOB);

// Fill lobs with data (about 800 000 characters / 1 200 000 bytes)
fs.createReadStream(fileDataPath, {encoding: 'utf8'})
    .on('data', function (chunk) {
        clob.write(chunk);
        blob.write(chunk, 'utf8');
    })

Problem №1: Module @types/oracledb does not describe method Connection.createLob()

Then I pass these LOB objects to execution binds...

await connection.execute(
    'begin "TEST"."PKG".mock_lobs(P_CLOB => :clob, P_BLOB => :blob); end;', 
    {
        clob: {type: oracle.CLOB, dir: oracle.BIND_INOUT, val: clob,
        blob: {type: oracle.BLOB, dir: oracle.BIND_INOUT, val: blob,
    }, 
    {outFormat: oracle.OBJECT}
);

Problem №2: Execution fires Uncaught Error: NJS-022: invalid Lob and I have no idea what it means.

Do you have ideas or explanation?

cjbj commented 5 years ago

It's late for me here so this is a comment in passing: if you give a testcase that actually runs (even with errors!) it would be easier to look at.

wallride commented 5 years ago

Sure. I'll write a plain js script reproducing the error

wallride commented 5 years ago
const oracle = require('oracledb');
const fs = require('fs');

oracle.createPool({connectString:'odb_app', user: 'system', password: 'oracle'}, (err, pool) => {
    pool.getConnection((err, connection) => {
        connection.execute(
            `CREATE OR REPLACE PROCEDURE mock_lobs(P_CLOB IN OUT CLOB) AS BEGIN P_CLOB := P_CLOB; END;`,
            (err)=>{
                if (err) {console.error(err); return;}

                connection.createLob(oracle.CLOB, (err, clob) => {
                    if (err) {console.error(err); return;}

                    for (let i = 0; i < 1000; i++)
                        clob.write('Hello world! '.repeat(10));

                    connection.execute(
                        'begin mock_lobs(P_CLOB => :clob); end;',
                        {clob: {type: oracle.CLOB, dir: oracle.BIND_INOUT, val: clob}},
                        {outFormat: oracle.OBJECT},
                        (err, res) => {
                            clob.close();

                            if (err) {console.error(err); return;}

                            console.log(res.outBinds);

                        }
                    );
                })
            }
        )
    })
});

Produces this error:

"C:\Program Files\nodejs\node.exe" C:\!work\_pswitch\lib\im-oracle\lobs-error-test.js
events.js:183
      throw er; // Unhandled 'error' event
      ^

Error: NJS-022: invalid Lob

Process finished with exit code 1

lobs-error-test.zip

dmcghan commented 5 years ago

@wallride You've stumbeled upon an inconsistency in the APIs. When working with Result Sets, you have the option of using programatic methods (like getRows) or converting to a Query Stream to leverage steaming/evented APIs. The programatic APIs are great in that they play nice with async/await - which can greatly simplify how code is written in Node.js.

LOBs, on the other hand, only support the streaming/evented APIs (unless you're using fetchAsString or fetchAsBuffer). So, when you call write, that's the write method of a stream, which writes to an internal stream buffer. Behind the scenes, the driver is moving the data from that buffer into the database. The pushing and pulling are somewhat disconnected.

To signal to the driver that you're done pushing, you need invoke the end method on the LOB. Then the driver signals back that it's done moving the data by emitting the finish event. You'll need to wait for that event before moving on to executing the PL/SQL code that binds in the temp LOB.

Many thanks to @anthony-tuininga for helping me wrap my head around all of this! :)

I've rewritten your test using async/await and evented APIs. I removed the createPool call and the creation of the procedure since those weren't specific to the LOB code.

Given this procedure:

create or replace procedure mock_lobs(
  p_clob in out clob
) 

as 

begin 

  null;

end;

The following should work:

const oracledb = require('oracledb');
const config = require('./dbConfig.js');

async function runTest() {
  let conn;

  try {
    conn = await oracledb.getConnection(config);

    const clob = await conn.createLob(oracledb.CLOB);

    for (let i = 0; i < 1000; i++) {
      clob.write('Hello world! '.repeat(10));
    }

    await clob.end();

    clob.on('finish', async () => {
      try {
        const result = await conn.execute(
          'begin mock_lobs(P_CLOB => :clob); end;',
          {
            clob: {
              type: oracledb.CLOB, 
              dir: oracledb.BIND_INOUT, 
              val: clob
            }
          }
        );

        console.log(result); // Out lob is on result.outBinds.clob
      } catch (err) {
        console.error(err);
      } finally {
        try {
          await clob.close();
        } catch (err) {
          console.log(err);
        }

        try {
          await conn.close();
        } catch (err) {
          console.error(err);
        }
      }
    });
  } catch (err) {
    console.error(err);
  }
}

runTest();

As written, this code is less than perfect because it's ignoring the return value of the write method. This dissables backpressure support in Node.js which means the internal stream buffer may grow much larger than expected. Depending on the amount of data you're writing and how many concurrent users you have, this could cause Node.js to run out of memory. I'm not sure if this would really affect your usecase.

In the future, I'd like to see some LOB APIs that support promises and async/await. Then we could simplify the code some:

const oracledb = require('oracledb');
const config = require('./dbConfig.js');

async function runTest() {
  let conn;
  let clob;

  try {
    conn = await oracledb.getConnection(config);

    clob = await conn.createLob(oracledb.CLOB);

    for (let i = 0; i < 1000; i++) {
      await clob.writeDirect('Hello world! '.repeat(10));
    }

    await clob.end();

    const result = await conn.execute(
      'begin mock_lobs(P_CLOB => :clob); end;',
      {
        clob: {
          type: oracledb.CLOB, 
          dir: oracledb.BIND_INOUT, 
          val: clob
        }
      }
    );
  } catch (err) {
    console.error(err);
  } finally {
    if (conn) {
      try {
        await conn.close();
      } catch (err) {
        console.error(err);
      }
    }
  }
}

runTest();
wallride commented 5 years ago

@dmcghan , many thanks for explanation. I'm a bit ashamed for missing the finish event. I had no experience with streams before. :) The finish event on LOB object did help me out. In your example you close LOB object before passing it to execute - is it actually correct? My implementation based on docs closes lobs after execution. Is it wrong?

This approach make things work. Though when I write a pair of LOBs of ~1MB the execution of that procedure takes TOO long, more than 5 minutes, whereas simple passing thsese string and buffer as plain (non-stream) data executed immediately. Execution with 12kb streamed lobs works fine. And when I raise the amount to 120kb it takes much more time and gives some weird behaviour that I've not investigated yet... Does this case look familiar to you?

@cjbj , I guess everyone would be happy to get more detailed errors in such cases, like "your LOB object is not finished" or something :)

dmcghan commented 5 years ago

@wallride I don't close the LOB, I invoke clob.end(). As explained, this is what signals to the driver that you're done writing to the LOB.

cjbj commented 5 years ago

@dmcghan I'm looking forward to that LOB blog series you are working on. We should simplify lobinserttemp.js with your snippt above.

@wallride can you submit a PR for the Typescript file?

Are your LOBs really too big to pass as string/buffer? There are some definite performance advantages.

It's been awhile since I played with LOB streaming performance and the implementation has changed a bit, but some performance notes (about non-Temporary LOBs) from our test team that may come in handy are:

Let us know what you find.

wallride commented 5 years ago

Many thanks to all for support. I think I've figured out that case. It was improper use of streams.

wallride commented 5 years ago

Hi again! :) I've got another problem while trying to play with LOBs. It's a problem with unicode characters (tat take more then 1 byte to store). When I write such a character to a CLOB an additional space character (or may be just 0x00 byte) appears after the written chunk. The more special symbols, the more spaces I see... Google and experiments with specifying 'utf8' encoding gave nothing...

Here's a code example to reproduce the problem:

const oracle = require('oracledb');

// PL/SQL Procedure
// CREATE PROCEDURE mock_lob(P_CLOB IN OUT CLOB) AS BEGIN NULL; END;

const originString = 'dddЩЩЩ';

oracle.getConnection({connectString:'odb_app', user: 'system', password: 'oracle'}, (err, connection) => {
    connection.createLob(oracle.CLOB, (err, clob) => {
        if (err) {
            console.error(err);
            return;
        }

        console.log('CLOB created');

        // Write originString to clob many times

        let sentData = '';

        for (let i = 0; i < 1000; i++) {
            clob.write(originString);
            sentData += originString;
        }
        clob.end();

        clob.on('finish', async () => {
            console.log('Write to CLOB complete');
            let outClob;

            // Now execute: pass the populated clob to procedure and receive another clob that is expected to equal origin one

            try {
                const result = await connection.execute(
                    'begin mock_lob(P_CLOB => :clob); end;',
                    {clob: {type: oracle.CLOB, dir: oracle.BIND_INOUT, val: clob}},
                    {outFormat: oracle.OBJECT}
                );
                outClob = result.outBinds['clob'];
            } catch(e) {
                console.error(e);
                return;
            }
            finally {
                clob.close();
            }

            // Read the received clob object an check data consistency

            let receivedData = '';
            outClob.on('data', chunk => {
                const slice = sentData.slice(receivedData.length, receivedData.length + chunk.length);

                if (slice !== chunk)
                    throw new Error(`Unequal part ${receivedData.length}-${receivedData.length + chunk.length}\n${slice}\n${chunk}`);

                receivedData += chunk;
            })
        });
    });
});

Can anyone explain what am I missing?

anthony-tuininga commented 5 years ago

You're not missing anything, it turns out. This is a bug. If you open a new issue we can track it there.

dmcghan commented 5 years ago

@wallride Please try the following test...

const oracledb = require('oracledb');
const config = require('./dbConfig.js');

async function runTest() {
  let conn;

  try {
    const testString = 'dddЩЩЩ';

    conn = await oracledb.getConnection(config);

    const clob = await conn.createLob(oracledb.CLOB);

    clob.write(testString);

    await clob.end();

    clob.on('finish', async () => {
      try {
        const result = await conn.execute(
          'begin mock_lobs(p_clob => :clob); end;',
          {
            clob: {
              type: oracledb.CLOB, 
              dir: oracledb.BIND_INOUT, 
              val: clob
            }
          }
        );

        const clobOut = await readClob(result.outBinds.clob);

        if (clobOut === testString) {
          console.log('Success!');
        } else {
          console.log('Something broke...');
          console.log(clobOut);
        }
      } catch (err) {
        console.error(err);
      } finally {
        try {
          await conn.close();
        } catch (err) {
          console.error(err);
        }
      }
    });
  } catch (err) {
    console.error(err);
  }
}

runTest();

function readClob(clob) {
  return new Promise((resolve, reject) => {
    let clobData = '';

    clob.on('data', chunk => {
      clobData += chunk;
    });

    clob.on('error', err => {
      reject(err);
    });

    clob.on('end', () => {
      resolve(clobData);
    });
  });
}
dmcghan commented 5 years ago

@wallride Nevermind, sorry. Once I add the loop it doesn't work... Please see @anthony-tuininga's comment.