0

I have 2 oracle prodecures inside a package like this:

PROCEDURE INSERT_LOG (
    message_id      in VARCHAR2,
    mq_request      in VARCHAR2,
    req_timestamp   in VARCHAR2
)
IS
BEGIN
    INSERT INTO TESTSCHEMA.MY_MESSAGE_LOG (MESSAGE_ID,MQ_REQUEST,REQ_TIMESTAMP)
    VALUES(message_id,mq_request,TO_DATE(req_timestamp,'DD-MM-YYYY HH24:MI:SS'));
    commit;
EXCEPTION
  WHEN OTHERS
  THEN
     ROLLBACK;
     RAISE_APPLICATION_ERROR (-20000, 'Error: INSERT_LOG() '||SQLERRM);
END;

PROCEDURE UPDATE_LOG (
        message_id      IN VARCHAR2,
        mq_response     IN VARCHAR2,
        resp_identifier IN VARCHAR2,
        resp_timestamp  IN VARCHAR2,
        req_timestamp   IN VARCHAR2 
        )
IS
BEGIN
        UPDATE TESTSCHEMA.MY_MESSAGE_LOG A
        SET
            A.MQ_RESPONSE = mq_response,
            A.RESP_IDENTIFIER =resp_identifier,
            A.RESP_TIMESTAMP = TO_DATE(resp_timestamp,'DD-MM-YYYY HH24:MI:SS')
        WHERE    
            A.MESSAGE_ID = message_id
            and A.REQ_TIMESTAMP = TO_DATE(req_timestamp,'DD-MM-YYYY HH24:MI:SS');

        commit;
EXCEPTION
    WHEN OTHERS
    THEN
        ROLLBACK;
        RAISE_APPLICATION_ERROR (-20000, 'Error: UPDATE_LOG() '||SQLERRM);
END;

And I try to call these procedures from my C# code. The issue is that the insert procedure works fine, but the update procedure does not update any rows. On investigating, I noticed that although I set the variables for the stored procedure from the C# code, the values of these variables are reflecting as null or empty on the database side. Here is my C# code for the update procedure.

try
        {
            using (DbConnection connection = new DbConnection())
            {
                var comm = new OracleCommand
                {
                    Connection = connection.OpenUsbAppsSchema(),
                    CommandText = <My Procedure Name>,
                    CommandType = CommandType.StoredProcedure
                };

                var param = new OracleParameter[5];

                param[0] = new OracleParameter("message_id", OracleDbType.Varchar2, 500, ParameterDirection.Input) { Value = messageId };
                param[1] = new OracleParameter("mq_response", OracleDbType.Varchar2, 2000, ParameterDirection.Input) { Value = mqResponse };
                param[2] = new OracleParameter("resp_identifier", OracleDbType.Varchar2, 200, ParameterDirection.Input) { Value = identifier };
                param[3] = new OracleParameter("resp_timestamp", OracleDbType.Varchar2,500, ParameterDirection.Input) { Value = resposeTimeStamp };
                param[4] = new OracleParameter("req_timestamp", OracleDbType.Varchar2, 500, ParameterDirection.Input) {Value = requestTimeStamp};

                comm.Parameters.AddRange(param);
                comm.ExecuteNonQuery();

            }
        }
        catch (Exception ex)
        {
            throw ex;
        }

Any pointers on what am I doing wrong?

Kunal Nair
  • 93
  • 2
  • 8
  • JustinCave's answer is spot-on. I was going to say there is nothing wrong with the C# side, until I saw [that evil `catch` clause](http://stackoverflow.com/a/881489/158074). Besides that, everything is right, you just need to change those parameter names on `UPDATE_LOG`... – rsenna May 23 '14 at 18:25

1 Answers1

2

Generally, you don't want to name parameters to procedures something that could conflict with the names of columns in tables. That makes it far too easy to make the mistake that you appear to be making here where you are expecting identifiers to refer to parameters when Oracle is actually resolving them to be columns in the table.

Looking at your UPDATE statement,

UPDATE TESTSCHEMA.MY_MESSAGE_LOG A
   SET A.MQ_RESPONSE = mq_response,
       A.RESP_IDENTIFIER =resp_identifier,
       A.RESP_TIMESTAMP = TO_DATE(resp_timestamp,'DD-MM-YYYY HH24:MI:SS')
 WHERE A.MESSAGE_ID = message_id
   and A.REQ_TIMESTAMP = TO_DATE(req_timestamp,'DD-MM-YYYY HH24:MI:SS');

when Oracle sees the standalone identifier mq_response, it first looks to resolve that using the columns in the table. Since there is an mq_response column in the table, that's what Oracle uses. It doesn't look to see if there is a local variable named mq_response or a parameter named mq_response unless the check of the columns in the tables fail. This happens on every unqualified identifier in your query so your query ends up being (logically) this query which updates every row in the table but doesn't change any values

UPDATE TESTSCHEMA.MY_MESSAGE_LOG A
   SET A.MQ_RESPONSE = a.mq_response,
       A.RESP_IDENTIFIER =a.resp_identifier,
       A.RESP_TIMESTAMP = TO_DATE(a.resp_timestamp,'DD-MM-YYYY HH24:MI:SS')
 WHERE A.MESSAGE_ID = a.message_id
   and A.REQ_TIMESTAMP = TO_DATE(a.req_timestamp,'DD-MM-YYYY HH24:MI:SS');

The most common way to solve this is to adopt a standard naming convention for parameters and local variables to differentiate them from columns in tables. Personally, I use p_ as the prefix for parameters and l_ as the prefix for local variables which is pretty common. If you do that, you'd have something like

PROCEDURE UPDATE_LOG (
        p_message_id      IN VARCHAR2,
        p_mq_response     IN VARCHAR2,
        p_resp_identifier IN VARCHAR2,
        p_resp_timestamp  IN VARCHAR2,
        p_req_timestamp   IN VARCHAR2 
        )
IS
BEGIN
        UPDATE TESTSCHEMA.MY_MESSAGE_LOG A
           SET A.MQ_RESPONSE     = p_mq_response,
               A.RESP_IDENTIFIER = p_resp_identifier,
               A.RESP_TIMESTAMP  = TO_DATE(p_resp_timestamp,'DD-MM-YYYY HH24:MI:SS')
         WHERE A.MESSAGE_ID    = p_message_id
           and A.REQ_TIMESTAMP = TO_DATE(p_req_timestamp,'DD-MM-YYYY HH24:MI:SS');

        commit;
EXCEPTION
    WHEN OTHERS
    THEN
        ROLLBACK;
        RAISE_APPLICATION_ERROR (-20000, 'Error: UPDATE_LOG() '||SQLERRM);
END;

The alternative would be to explicitly qualify all the parameter names in your query. You can do that by using the name of your function as the qualifier, i.e.

UPDATE TESTSCHEMA.MY_MESSAGE_LOG A
   SET A.MQ_RESPONSE = update_log.mq_response,
       A.RESP_IDENTIFIER =update_log.resp_identifier,
       A.RESP_TIMESTAMP = TO_DATE(update_log.resp_timestamp,'DD-MM-YYYY HH24:MI:SS')
 WHERE A.MESSAGE_ID = update_log.message_id
   and A.REQ_TIMESTAMP = TO_DATE(update_log.req_timestamp,'DD-MM-YYYY HH24:MI:SS');

Personally, I find the prefixes to be much more readable and much less error-prone.

Taking a look at your code, I'd strongly recommend taking out the commit statements and removing the exception handlers. When you have commit statements in a procedure, those procedures instantly become far less reusable because they can't be used if a piece of code is in the middle of a transaction. Otherwise, your commit is going to commit all the work that the caller may be in the middle of performing. Exception handlers should only catch exceptions if your code can do something useful with the exception or if your custom exception is going to provide more information to the caller. In this case, though, all your exception handler is doing is throwing away the error stack and preventing the caller from easily checking the error code to determine what went wrong. You may not be losing too much debugging information with code this simple but going forward, you're setting yourself up for a world of trouble trying to debug your code if you code exception handlers like this.

Justin Cave
  • 227,342
  • 24
  • 367
  • 384