Re: Bad Code Alert

Birgitta, must disagree strongly.

One. Not checking the open status. The error that needs to be reported is
the error that is returned from the Open statement, not from the fetch. I
again reemphasize , each statement except a declare needs to be error
checked and processing stopped if an error is detected.

Two. I specifically that warning should be reported as errors. You used as
an example of a warning that not enough variables provides. How is that
less of a disaster? Your program is expecting three variables but you only
load two. That could cause all kinds of problems. If a specific warning
message should be ignore, it should specifically be coded. You just cannot
ignore warning message. They are indicators of bigger problems.



On Sun, Mar 18, 2018 at 11:20 PM, Birgitta Hauser <Hauser@xxxxxxxxxxxxxxx>
wrote:

Alan,

Not sure what was your problem.
First it was a code snippet with the focus on how to handle NULL values in
an embedded SQL RPG Program.
And second I added a comment (//Handle SQL Error) where to handle an SQL
error.
I could have included another check after the OPEN, but If the OPEN fails,
the FETCH will fail, too.

If you already want to include an error handling, than do it correctly.
If you want to check the SQL State, have check the status group (the first
2
characters) and not the complete SQLSTATE (may be in exceptions).
If the first 2 characters are 00 than everything is ok, that does not mean
it must be always 00000.
If the first 2 characters are 01 than a warning occurs. That means data is
returned, but something should be considered (for example SQLSTATE 01503 -
The number of result columns is larger than the number of variables
provided.)
If the first 2 characters are 02 than no record is returned.
All other status groups mark an SQL Error.

From the IBM i Knowledge Center:
The class code of an SQLSTATE value indicates whether the SQL statement was
executed successfully (class codes 00 and 01) or unsuccessfully (all other
class codes).

For checking an SQL Error, I prefer the SQLCODE because it is easier to
check for < *Zeros and = 100, than substringing the SQLSTATE.

Instead of throwing an Dummy SQL Message, I'd suggest to trap the real SQL
Error Message. ... whether you want add the SQLSTATE or not is your
decision.
DCL-S LocErrMsg VarChar(256);
...
If %Subst(SQLSTATE: 1: 2) <> '00'
and %Subst(SQLSTATE: 1: 2) <> '01'
and %Subst(SQLSTATE: 1: 2) <> '02';
Exec SQL Get Diagnostics Condition 1 :LocErrMsg = MESSAGE_TEXT;
YourErrorMsgHandling(%Trim(LocErrMsg) + ' ' + SQLSTATE);
End-If;

We even check the SQLCODE in the SQL Error Handling Procedure. If an error
occurred an escape message is sent, otherwise the procedure is left
immediately.

Mit freundlichen Grüßen / Best regards

Birgitta Hauser

"Shoot for the moon, even if you miss, you'll land among the stars." (Les
Brown)
"If you think education is expensive, try ignorance." (Derek Bok)
"What is worse than training your staff and losing them? Not training them
and keeping them!"
„Train people well enough so they can leave, treat them well enough so they
don't want to.“ (Richard Branson)


-----Original Message-----
From: RPG400-L <rpg400-l-bounces@xxxxxxxxxxxx> On Behalf Of Alan Campin
Sent: Sonntag, 18. März 2018 23:56
To: RPG programming on the IBM i / System i <rpg400-l@xxxxxxxxxxxx>
Subject: Bad Code Alert

Brigitta, this is not a personal attack on you. I see this error all over
the place at my company and I keep trying to get it stopped.

Recently the following code snippet was presented.

Exec SQL Declare CsrC01 cursor for
Select TS1, VarChar1, TS2, Decimal1
From ...
Where ...;

Exec SQL Open CsrC01;

DoU 1=0;
Exec SQL Fetch Next From CsrC01
into :LocTS1 :LocTS1Ind, :MyVarChar1,
:LocTS2 :LocTS2Ind, :MYDec1;
If SQLCODE = 100
or SQLCODE < *Zeros;
//Handle SQL Error;
Exec SQL Close CsrC01;
Leave;
EndIf;

This code represents a severe problem. The problem. No SQL error checking.

It is absolutely critical that SQL statement be error checked. If an error
occurs on a File I/O, an error is thrown but unless the error is critical
the only thing that happens in SQL is that an error code is returned in
SQLSTATE (I use SQLSTATE. SQLCODE can also be used).

This is all that happens. If an SQL error is not checked the program will
simply continue on. I cannot tell you the number of times that I have gone
into SQL programs running in production that I determined where failing
and
had been failing for years. The effect from those errors was minor to
severe.

The above code should look like the following.I am using my standard error
handler for these examples because it throws an exception message and stops
processing. . Note that I have no error checking on Declare. It does not
generate any machine code and does not need to be error checked.

dcl-c SQL_STATE_NO_ROW '02000';
dcl-c SQL_STATE_OK '00000';

Exec SQL Declare CsrC01 cursor for
Select TS1, VarChar1, TS2, Decimal1
From ...
Where ...;

Exec SQL Open CsrC01;
If SqlState <> SQL_STATE_OK;
// Report error and stop processing.
XVERRH_Throw('CPF9898':
CPF_MESSAGE_FILE_NAME:
'Open Cursor CsrC01 Failed! SqlState = ' + SqlState); EndIf;

DoU 1=0;
Exec SQL Fetch Next From CsrC01
into :LocTS1 :LocTS1Ind, :MyVarChar1,
:LocTS2 :LocTS2Ind, :MYDec1;
Select;
When SqlState = SQL_STATE_NO_ROW;
Exec SQL Close CsrC01;
Leave;
When SqlState = SQL_STATE_OK;
// process record.
Other;
// Report error and stop processing.
XVERRH_Throw('CPF9898':
CPF_MESSAGE_FILE_NAME:
'Fetch Next From CsrC01 failed! Sql State = ' +
SqlState);
EndSl;
EndDo;

I want to emphasize again the importance of stopping processing. If an
error
occurs, we must stop processing. If we continue, we are just compounding
the
problem.

Please note also that I am stopping processing for both warning and
errors.It has been my experience that warning error are always masking some
form of programming error and need to be investigated.

I appreciate their may be certain rare condition were we want processing to
continue after a warning but they are the exception.

I again apologize if I have hurt anyone feeling but I felt it was important
to bring this to the groups attention. SQL error checking is absolutely
critical.
--
This is the RPG programming on the IBM i (AS/400 and iSeries) (RPG400-L)
mailing list To post a message email: RPG400-L@xxxxxxxxxxxx To subscribe,
unsubscribe, or change list options,
visit: https://lists.midrange.com/mailman/listinfo/rpg400-l
or email: RPG400-L-request@xxxxxxxxxxxx
Before posting, please take a moment to review the archives at
https://archive.midrange.com/rpg400-l.

Please contact support@xxxxxxxxxxxx for any subscription related
questions.

Help support midrange.com by shopping at amazon.com with our affiliate
link:
http://amzn.to/2dEadiD

--
This is the RPG programming on the IBM i (AS/400 and iSeries) (RPG400-L)
mailing list
To post a message email: RPG400-L@xxxxxxxxxxxx
To subscribe, unsubscribe, or change list options,
visit: https://lists.midrange.com/mailman/listinfo/rpg400-l
or email: RPG400-L-request@xxxxxxxxxxxx
Before posting, please take a moment to review the archives
at https://archive.midrange.com/rpg400-l.

Please contact support@xxxxxxxxxxxx for any subscription related
questions.

Help support midrange.com by shopping at amazon.com with our affiliate
link: http://amzn.to/2dEadiD