Updating column is not working in Oracle PLSQL

Question

I have a Stored procedure in PLSQL which Inserts and Updates records on the basis of some condition.

Now here the issue is. While Inserting the record for the first time, it inserts records properly as required but

while updating it doesn't updates the record of the table.

Below is SP

PROCEDURE INSERT_INTO_VSAT_MST_DATA
  (
  P_SAPID IN NVARCHAR2,
  P_CIRCLE IN NVARCHAR2,
  P_CANDIDATEID IN NVARCHAR2,
  P_SITEID IN NVARCHAR2,
  P_PRIORITYID IN NVARCHAR2,
  P_SITENAME IN NVARCHAR2,
  P_LATITUDE IN NVARCHAR2,
  P_LONGITUDE IN NVARCHAR2,
  P_CONTACT_DETAILS IN CLOB,
  P_SITETYPE IN NVARCHAR2,
  P_SITE_PLOT_DIMENSION IN NUMBER,
  P_TECHNOLOGY IN NVARCHAR2
 )
  AS

  V_COUNT NUMBER:=0;
  V_PANAROMICIMG_COUNT NUMBER:=0;
  V_SATELLITEIMG_COUNT NUMBER:=0;
  V_SITEPLOTIMG_COUNT NUMBER:=0;
  V_VSAT_DETAIL_ID NUMBER:=0;

    BEGIN

SELECT COUNT(VSAT_DETAIL_ID) INTO V_COUNT FROM TBL_VSAT_MST_DETAIL WHERE SAP_ID = P_SAPID AND CANDIDATE_ID =  P_CANDIDATEID;

IF V_COUNT > 0 THEN

SELECT VSAT_DETAIL_ID INTO TBL_INSERT FROM TBL_VSAT_MST_DETAIL WHERE SAP_ID = P_SAPID AND CANDIDATE_ID =  P_CANDIDATEID;  
UPDATE  TBL_VSAT_MST_DETAIL SET  
                            CIRCLE = P_CIRCLE,
                            CONTACT_DETAILS = P_CONTACT_DETAILS,
                            SITE_TYPE = P_SITETYPE,
                            SITE_DETAILS_DIMENSION = P_SITE_PLOT_DIMENSION,
                            SITE_DETAILS_TECHNOLOGY = P_TECHNOLOGY
                            WHERE VSAT_DETAIL_ID = V_VSAT_DETAIL_ID
                RETURNING VSAT_DETAIL_ID INTO TBL_INSERT;          
ELSE     
INSERT INTO TBL_VSAT_MST_DETAIL 
                                 (
                                    SAP_ID,
                                    CIRCLE,
                                    CANDIDATE_ID,
                                    SITE_ID,
                                    PRIORITY,
                                    SITE_NAME,
                                    LATITUDE,
                                    LONGITUDE,
                                    CONTACT_DETAILS,
                                    SITE_TYPE,
                                    SITE_DETAILS_DIMENSION,
                                    SITE_DETAILS_TECHNOLOGY                                     
VALUES
                                 (
                                    P_SAPID,
                                    P_CIRCLE,
                                    P_CANDIDATEID,
                                    P_SITEID,
                                    P_PRIORITYID,
                                    P_SITENAME,
                                    P_LATITUDE,
                                    P_LONGITUDE,
                                    P_CONTACT_DETAILS,
                                    P_SITETYPE,
                                    P_SITE_PLOT_DIMENSION,
                                    P_TECHNOLOGY
                                    ) RETURNING VSAT_DETAIL_ID INTO TBL_INSERT;
END IF;    
IF TBL_INSERT > 0 THEN
BEGIN                
    SELECT COUNT(*) INTO V_PANAROMICIMG_COUNT FROM TBL_VSAT_IMAGE_DETAIL WHERE IMG_TYPE = 'Panaromic' AND IMG_ID = TBL_INSERT;        
    SELECT COUNT(*) INTO V_SATELLITEIMG_COUNT FROM TBL_VSAT_IMAGE_DETAIL WHERE IMG_TYPE = 'Satellite' AND IMG_ID = TBL_INSERT;        
    SELECT COUNT(*) INTO V_SITEPLOTIMG_COUNT FROM TBL_VSAT_IMAGE_DETAIL WHERE IMG_TYPE = 'SitePlot' AND IMG_ID = TBL_INSERT;        
    IF V_PANAROMICIMG_COUNT > 0 THEN
    BEGIN        
        DELETE FROM TBL_VSAT_IMAGE_DETAIL WHERE IMG_TYPE = 'Panaromic' AND IMG_ID = TBL_INSERT;   
    END;        
    END IF;           
    IF V_SATELLITEIMG_COUNT > 0 THEN
    BEGIN        
        DELETE FROM TBL_VSAT_IMAGE_DETAIL WHERE IMG_TYPE = 'Satellite' AND IMG_ID = TBL_INSERT;   
    END;  
    END IF;           
    IF V_SITEPLOTIMG_COUNT > 0 THEN
    BEGIN        
        DELETE FROM TBL_VSAT_IMAGE_DETAIL WHERE IMG_TYPE = 'SitePlot' AND IMG_ID = TBL_INSERT;   
    END;
    END IF;  
    FOR PMULTIFIELDS IN (SELECT REGEXP_SUBSTR(P_PANORAMIC_IMAGES,'[^,]+', 1, LEVEL) AS IMAGES FROM DUAL
                              CONNECT BY REGEXP_SUBSTR(P_PANORAMIC_IMAGES, '[^,]+', 1, LEVEL) IS NOT NULL
                            )
    LOOP
                INSERT INTO TBL_VSAT_IMAGE_DETAIL
                            (
                              IMG_ID,
                              IMG_NAME,
                              IMG_TYPE,
                              IMG_UPLOADED_DATE,
                              UPLOADED_BY
                            )
                            VALUES
                            (
                              TBL_INSERT,
                              PMULTIFIELDS.IMAGES,
                              'Panaromic',
                              SYSDATE,
                              P_CREATEDBY
                            );
    END LOOP;
    FOR PSATELLITEIMG IN (SELECT REGEXP_SUBSTR(P_SATELLITE_IMAGES,'[^,]+', 1, LEVEL) AS IMAGES FROM DUAL
                                  CONNECT BY REGEXP_SUBSTR(P_SATELLITE_IMAGES, '[^,]+', 1, LEVEL) IS NOT NULL
                                )
    LOOP
                INSERT INTO TBL_VSAT_IMAGE_DETAIL
                            (
                              IMG_ID,
                              IMG_NAME,
                              IMG_TYPE,
                              IMG_UPLOADED_DATE,
                              UPLOADED_BY
                            )
                            VALUES
                            (
                              TBL_INSERT,
                              PSATELLITEIMG.IMAGES,
                              'Satellite',
                              SYSDATE,
                              P_CREATEDBY
                            );
    END LOOP;

  IF P_SITEPLOT_IMAGES IS NOT NULL THEN
  BEGIN      
    INSERT INTO TBL_VSAT_IMAGE_DETAIL
                              (
                                IMG_ID,
                                IMG_NAME,
                                IMG_TYPE,
                                IMG_UPLOADED_DATE,
                                UPLOADED_BY
                              )
                              VALUES
                              (
                                TBL_INSERT,
                                P_SITEPLOT_IMAGES,
                                'SitePlot',
                                SYSDATE,
                                P_CREATEDBY
                              ); 
       END;   
      END IF;      
         END;
         END IF;      
     COMMIT;    
    EXCEPTION WHEN OTHERS THEN    
     ROLLBACK;

NOTE

While updating the record my TBL_INSERT returns as NULL

`tbl_insert` doesn't seem to be defined, and you're missing a closing parenthesis before line 54, but those would be compilation errors. Can you confirm `vsat_detail_id` is being set via a trigger? You're probably getting an exception, but because of your `when others` catch you're suppressing/hiding it. It's not generally a good idea to commit/rollback inside a procedure anyway, but if you must catch the exception, your should re-raise it. (add `raise;` after the `rollback`). Also look into savepoints.... — Alex Poole, Jul 11 '17 at 09:07
@AlexPoole: Really thanks for your comment, yes `vsat_detail_id` is being set via a trigger. Also how i will be able to know that it is giving me an `exception` as I am unable to find it. IT would be great if you let me know to deal with this.! — Nad, Jul 11 '17 at 09:11
I already said, re-raise the exception after caching it with `when others`. But as user7294900 pointed out, the value you're using for your update is set to zero, not the value you expect - from the preceding `select`.... But you could still be getting an exception, e.g. if `v_count` is greater than one, rather than exactly 1. — Alex Poole, Jul 11 '17 at 09:15
@AlexPoole: oh, What I really want is I want to update the table record on the basis of `vsat_detail_id` but when I go for updating it my `v_count` always comes to `0` and it doesnt updates the record. On the other hand from server code my `TBL_INSERT` always comes as `NULL` on update. currently I am trying to raise the exception as you told. Do let me know so that I can go step by step and resolve this — Nad, Jul 11 '17 at 09:21
If you applied some basic formatting to the code I guarantee you would feel more in control of the program structure. It is really worth getting into the habit. — William Robertson, Jul 16 '17 at 10:11

Alex Poole · Accepted Answer · 2017-07-11T09:49:59.357

Expanding on what @user7294900 pointed you towards... in the declare section you have:

V_VSAT_DETAIL_ID NUMBER:=0;

then if v_count > 0 you do:

SELECT VSAT_DETAIL_ID INTO TBL_INSERT FROM TBL_VSAT_MST_DETAIL WHERE SAP_ID = P_SAPID AND CANDIDATE_ID =  P_CANDIDATEID;  
UPDATE  TBL_VSAT_MST_DETAIL SET  
                            CIRCLE = P_CIRCLE,
                            CONTACT_DETAILS = P_CONTACT_DETAILS,
                            SITE_TYPE = P_SITETYPE,
                            SITE_DETAILS_DIMENSION = P_SITE_PLOT_DIMENSION,
                            SITE_DETAILS_TECHNOLOGY = P_TECHNOLOGY
                            WHERE VSAT_DETAIL_ID = V_VSAT_DETAIL_ID
                RETURNING VSAT_DETAIL_ID INTO TBL_INSERT;

The select is setting TBL_INSERT to the ID value from your table. But when you do the update your filter is using V_VSAT_DETAIL_ID, which is still set to its initial value of zero.

You probably meant to do:

SELECT VSAT_DETAIL_ID INTO V_VSAT_DETAIL_ID FROM TBL_VSAT_MST_DETAIL WHERE SAP_ID = P_SAPID AND CANDIDATE_ID =  P_CANDIDATEID;

although you could still with your current select, and use that in the update too (making the returning into a bit redundant.

Be aware though that if v_count is not exactly 1, i.e. you have more than one row matching the P_SAPID and P_CANDIDATEID values, the select will get a too-many-rows exception. You won't see that because you are silently squashing any errors you get at run time.

It's usually not a good idea to commit or rollback inside a procedure anyway; it should be up to the caller to decide what to do, as this could be one of a series of statements and calls that you really want to treat as an atomic transaction. (You may be interested in savepoints.)

If you really, really want to rollback on exception within the procedure, you should at least re-raise the exception so the caller knows there was a problem:

EXCEPTION WHEN OTHERS THEN    
  ROLLBACK;
  RAISE;
END INSERT_INTO_VSAT_MST_DATA;

but I would avoid when others if you can.

You could also combine a few steps by getting the ID at the start (again kind of assuming there is at most one matching row), and dropping the separate count and v_count variable:

SELECT MAX(VSAT_DETAIL_ID) INTO V_VSAT_DETAIL_ID
FROM TBL_VSAT_MST_DETAIL
WHERE SAP_ID = P_SAPID AND CANDIDATE_ID =  P_CANDIDATEID;

IF V_VSAT_DETAIL_ID IS NOT NULL THEN

  UPDATE  TBL_VSAT_MST_DETAIL SET  
                            CIRCLE = P_CIRCLE,
                            CONTACT_DETAILS = P_CONTACT_DETAILS,
                            SITE_TYPE = P_SITETYPE,
                            SITE_DETAILS_DIMENSION = P_SITE_PLOT_DIMENSION,
                            SITE_DETAILS_TECHNOLOGY = P_TECHNOLOGY
                            WHERE VSAT_DETAIL_ID = V_VSAT_DETAIL_ID
                RETURNING VSAT_DETAIL_ID INTO TBL_INSERT;          
  ELSE     
...

And I'm not sure why you're doing counts before your deletes later on, and it looks like all your tbl_insert references could/should be v_vast_detail_id - there doesn't seem an obvious reason to have two variables for that. Passing in a comma-delimited string that you then have to tokenize is also a bit painful - you should consider passing in a collection of values instead, if whatever calls this can manage that.

As also pointed out, you could use merge instead of the separate insert/update logic.

my `select` statement is good i guess. I would be interested in making `returning into` more logical as my scenario wholly depends on it. on the raising exception part, i will try to reraise it as per the standard exception — Nad, Jul 11 '17 at 09:31
thanks for your update in answer. One more thing here. is it fine for raising exception ? `EXCEPTION WHEN OTHERS THEN ROLLBACK; RAISE; ROLLBACK;` Or do I need to modify something ? — Nad, Jul 11 '17 at 09:44
You will never reach the second `rollback`, the raise exits the block (the procedure, in this case). — Alex Poole, Jul 11 '17 at 09:47
ohh, so I dont have to look much into raising the exception part. But still i will look into the `merge` part if I get time — Nad, Jul 11 '17 at 10:01

Ori Marko · Answer 2 · 2017-07-11T09:14:57.033

0

You don't assign value to V_VSAT_DETAIL_ID which is used in your update as a key.

You should use merge for this kind of operations

edited Jul 11 '17 at 09:14

answered Jul 11 '17 at 08:58

Ori Marko

56,308
23
131
233

`vsat_detail_id ` is being initialised to zero, but it looks like you intended to either use `tbl_insert` in the update, or you meant to select into `v_vsat_detail_id` instead of `tbl_insert` in the line before the update. – Alex Poole Jul 11 '17 at 09:14
1

You should use merge for this kind of operations – Ori Marko Jul 11 '17 at 09:15

Updating column is not working in Oracle PLSQL

2 Answers2