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

Temporary LOB caching #10

Closed
Dronablo opened this Issue Apr 26, 2017 · 1 comment

Comments

Projects
None yet
2 participants
@Dronablo
Copy link

Dronablo commented Apr 26, 2017

Dear Anthony,

There is a problem in the folowing function (dpiLob.c:1427):

int dpiOci__lobCreateTemporary(dpiLob *lob, dpiError *error)
{
    uint8_t lobType;
    int status;

    DPI_OCI_LOAD_SYMBOL("OCILobCreateTemporary",
            dpiOciSymbols.fnLobCreateTemporary)
    if (lob->type->oracleTypeNum == DPI_ORACLE_TYPE_BLOB)
        lobType = DPI_OCI_TEMP_BLOB;
    else lobType = DPI_OCI_TEMP_CLOB;
    status = (*dpiOciSymbols.fnLobCreateTemporary)(lob->conn->handle,
            error->handle, lob->locator, DPI_OCI_DEFAULT,
            lob->type->charsetForm, lobType, 0, DPI_OCI_DURATION_SESSION);
    return dpiError__check(error, status, lob->conn, "create temporary LOB");
}

It creates temporary lob with cache=false, which leads to unnecessary physical io every time such a variable is created.

For example, if we try to pass empty string from python code to plsql procedure with CLOB in parameter using cx_Oracle and ODPI, it would generate physical IO (could be easily seen in 10046 trace).

I believe it should be changed to chache=true, or, which is probably better, make this parameter configurable.

Regards,
Andrey

@anthony-tuininga

This comment has been minimized.

Copy link
Member

anthony-tuininga commented Apr 27, 2017

You make an excellent point. Thanks. I have changed cache=True in this function as suggested. Only very large temporary LOBs (multiple GB) would benefit from cache=False and these are unlikely to be used. The parameter could be added to make it configurable with the documentation indicating that the value True should be used in almost all cases -- but that seems an unnecessary complication so I'll leave it the way it is for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment