Suggestion : Query empty parameters warning

Discussion of open issues, suggestions and bugs regarding ODAC (Oracle Data Access Components) for Delphi, C++Builder, Lazarus (and FPC)
Post Reply
wchris
Posts: 51
Joined: Thu 09 Jun 2005 09:44

Suggestion : Query empty parameters warning

Post by wchris » Thu 12 Jul 2007 11:27

hello,

It's just an idea, perhaps you could find it interresting.

I discovered that in a big query with many parameters, if i forget to set a parameter odac and oracle do not complain. The query just executes succesfully returning no rows.

I wonder if you could add an option (or if it exists already perhaps) called "EmptyparamValueWarning" in the session or the query component that would open a window with a warning when the query executes.

This parameter could be set to false by default for compatibility.

awaiting comments.

Cheers

Plash
Devart Team
Posts: 2844
Joined: Wed 10 May 2006 07:09

Post by Plash » Fri 13 Jul 2007 12:35

We are not planning to add such option because in most cases it is correct when a programer sets a parameter value to NULL.

wchris
Posts: 51
Joined: Thu 09 Jun 2005 09:44

Post by wchris » Mon 16 Jul 2007 13:06

Plash wrote:We are not planning to add such option because in most cases it is correct when a programer sets a parameter value to NULL.
not sure .... because if i do those queries

SQL> select count(*) from psysequence where psqjourmanuel = null;

COUNT(*)
----------
0

SQL> select count(*) from psysequence where psqjourmanuel is null;

COUNT(*)
----------
132

i must use the IS operator, with the = opérator assigned here in my sample to a constant (but it woul also have the same effect with a parameter) the query executes but the result is wrong.

look like you cannot query a null value with parameters.


try the following

select * from psysequence where psqjourmanuel = :param1

will execute but return no rows (wrong result)

select * from psysequence where psqjourmanuel is :param1

will return an error requesting null keywork, cannot use parameters with IS

select * from psysequence where psqjourmanuel is null

will work but will not be parametric


I'm not tying to convince you, the feature is not absolutely necessary, i can live without :? . But i'm just wondering if null values can really be used in a query parameters ... maybe i'm wrong and you'll tell me :wink:

Do you know a case where setting NULL in a query parameter returns the rows where the column is null ? I'm myself not sure that's why i suggested the feature to be optionnal.

well, nevermind, thank you for reading me

jfudickar
Posts: 202
Joined: Fri 10 Mar 2006 13:03
Location: Oberursel / Germany

Post by jfudickar » Mon 16 Jul 2007 13:15

Code: Select all

select count(*) from psysequence where NVL(psqjourmanuel, -1) = NVL(:PARAM, -1); 
This would be a valid way.

But i would agree with you, it could be an helpfull OPTIONAL property.

Greetings
Jens[/code]

wchris
Posts: 51
Joined: Thu 09 Jun 2005 09:44

Post by wchris » Mon 16 Jul 2007 13:38

jfudickar wrote:

Code: Select all

select count(*) from psysequence where NVL(psqjourmanuel, -1) = NVL(:PARAM, -1); 
This would be a valid way.

But i would agree with you, it could be an helpfull OPTIONAL property.

Greetings
Jens[/code]
Thank you Jens. :lol: well done

i was near to find it myself, but you were faster !

Plash
Devart Team
Posts: 2844
Joined: Wed 10 May 2006 07:09

Post by Plash » Wed 18 Jul 2007 11:32

We will investigate possibility to add such option, but that is hardly to happen in the nearest future.

wchris
Posts: 51
Joined: Thu 09 Jun 2005 09:44

Post by wchris » Thu 26 Jul 2007 09:23

Plash wrote:We will investigate possibility to add such option, but that is hardly to happen in the nearest future.
Thank you

I know it won't happen soon, but i was thinking about 'how to do it' ...

if you do it someday, don't test if parameter value is null.

Internally set a flag called 'value_set_by_user'. When a parameter is assigned you probably have a method setvalue, there you could toggle 'value_set_by_user' to true. That way users could set parameters to null if they want. When the query executes, check if all 'value_set_by_user' from all parameters = True.
After the query, reset the 'value_set_by_user' to False for all the parameters of the query.

Perhaps you'll have a better idea... but i wanted to share mine :wink:

Best regards

wchris
Posts: 51
Joined: Thu 09 Jun 2005 09:44

Post by wchris » Fri 06 Jan 2012 16:46

Plash wrote:We will investigate possibility to add such option, but that is hardly to happen in the nearest future.
Hello,

does such an option now exist in latest odac ?

to resume:
you would add an option "checkifallassigned" or "check explicitparameterassignement" (maybe find a shorter name ;))
if the option is not set behaviour is unchanged.

each time a "parambyname :=" occurs you increase a setcounter
where executeproc you raise an exception "warning : all parameters not set" if setcounter does not match paramcount.
you must not test if teh parameter contains a value, but if a command to set the parameter was used. programemr can chhose to set the parameter to null but it must be explicit.

after executeproc you reset setcounter to 0.

this avoids sending "garbage" into parameters that are forgotten to set by a programmer A because programmer B did not tell him a parameter was added to the procedure.

cheers
happy new year :wink: :wink:

AlexP
Devart Team
Posts: 5530
Joined: Tue 10 Aug 2010 11:35

Post by AlexP » Wed 11 Jan 2012 12:11

Hello,

We will not implement check of parameters for the presence of the value assigned to parameters, as in many cases values of parameters are to be Null. You can check the values of parameters yourself before a query execution, for example, with the help of the following method, and display the related message

Code: Select all

function CheckParamValue(DataSet: TCustomDADataSet): boolean;
var
  i: integer;
begin
  Result := true;
  if not Assigned(DataSet) then Exit;
  for i:= 0 to DataSet.ParamCount - 1 do
    if DataSet.Params.IsNull then
    begin
      Result := false;
      exit;
    end;
end;

P.S. You can improve this method and have it also return amount, names, etc. unasked parameters.

wchris
Posts: 51
Joined: Thu 09 Jun 2005 09:44

Post by wchris » Thu 12 Jan 2012 11:11

AlexP wrote:Hello,

We will not implement check of parameters for the presence of the value assigned to parameters, as in many cases values of parameters are to be Null. You can check the values of parameters yourself before a query execution, for example, with the help of the following method, and display the related message.
It's not a problem if you don't do it, but you did not completely understand my point so i'll have to explain better. Before you say no you must understand what it is about.

Testing null like you did will not work. Because as you said parameters can be null. That's why my code sample above tests if an assignment is done with a counter and not the value.

as an example codegear is doing it with wsdlimp when importing soap wsdl. It produces such code :
procedure Setquantity(Index: Integer; const AINT2: INT2);
function quantity_Specified(Index: Integer): boolean;
procedure Setdesc(Index: Integer; const AED: ED);
function desc_Specified(Index: Integer): boolean;
procedure SetstatusCode(Index: Integer; const ACS2: CS2);
function statusCode_Specified(Index: Integer): boolean;
procedure SetexistenceTime(Index: Integer; const AIVL_TS: IVL_TS);
function existenceTime_Specified(Index: Integer): boolean;
where each element can be checked for beeing specified or not (IMPORTANT: THIS IS FROM BEEING NULL, ELEMENT CAN BE SPECIFIED TO NULL)

What happens to us is that we use a global datamodule with all storedprocs inside used by everyone. The parameters are never cleared, so each call to execproc must be preceded by an EXPLICIT assignment of all parameters even when the value is NULL. If you don't you get the value left by the previous call in the parameter and this can be horribly wrong.

One solution would be to override Torastoredproc and create our own component that does the check... but we would need to replace all Torastoredprocs while adding a simple option from your side would do the trick.

Sorry for my bad English. maybe I use the wrong words. Did you understand the difference between #1 "checking if all parameters are specified" and #2 '"checking if all parameters are assigned" ?
#1 is my request, #2 is what you suggested


I always use the word "assigned" instead of "specified", so it's my fault, i led you in the wrong way :roll:
cheers :wink:

PS: if I resurect this suggestion 4 years later, it is because every now and then we encouter a bug that could have been avoided with such an optional check. And it just happened again, one programmer added parameters to a Torastoredproc while others were not aware of the change and did not fill the new parameters.

AlexP
Devart Team
Posts: 5530
Joined: Tue 10 Aug 2010 11:35

Post by AlexP » Thu 12 Jan 2012 15:40

Hello,

In order to have parameters empty at every new call, you should clear them after execution

Code: Select all

for i := 0 to OraQuery.ParamCount -1 do
OraQuery.Params[i].clear
If you do not specify the parameters type, it will be ftUnknown before the first value setting

Code: Select all

for i := 0 to OraQuery.ParamCount -1 do
if OraQuery.Params[i].DataType = ftUnknown then

wchris
Posts: 51
Joined: Thu 09 Jun 2005 09:44

Post by wchris » Fri 13 Jan 2012 12:22

AlexP wrote:Hello,

In order to have parameters empty at every new call, you should clear them after execution
Hello Alex,

Clearing parameters solves only partially the problem. (It depends on what the procedure you are calling is doing with the parameters values.)

Imagine programmer A added a parameter C in procedure Z but did not tell programmer B.

Programmer A also modified procedure Z to update field CF from table T with parameter C value.

Now if programmer B does a .clear without filling correctly the parameter C then field Cf of table T will get updated to null and data lost. (this is better than without the .clear where random data would be in CF, but still not correct)

With an options that check (by counting) that each parameter is "explicitely specified" programmer B could have been warned and avoid data loss.

AlexP
Devart Team
Posts: 5530
Joined: Tue 10 Aug 2010 11:35

Post by AlexP » Fri 13 Jan 2012 14:31

Hello,

In such case, after every procedure call, you can set the parameters DataType property to ftUnknown, and check the parameter type for ftUnknown before execution. If it is so, then it means that parameter was not set.

Code: Select all

procedure ResetParams(DataSet: TCustomDADataSet);
var
  i: integer;
begin
  if not Assigned(DataSet) then Exit;
  for i:= 0 to DataSet.ParamCount - 1 do
    DataSet.Params[i].DataType := ftUnknown;
end;

procedure CheckParams(DataSet: TCustomDADataSet);
var
  i: integer;
begin
  if not Assigned(DataSet) then Exit;
  for i:= 0 to DataSet.ParamCount - 1 do
    if DataSet.Params[i].DataType = ftUnknown then
      Raise Exception.Create(Format('The %s parameter value was not set explicitly',[DataSet.Params[i].Name]));
end;

procedure TForm1.OraQuery1AfterExecute(Sender: TObject;
  Result: Boolean);
begin
    ResetParams(TCustomDADataSet(Sender));
end;

procedure TForm1.OraQuery1BeforeExecute(Sender: TObject);
begin
    CheckParams(TCustomDADataSet(Sender));
end;

procedure TForm1.Button1Click(Sender: TObject);
begin
  OraQuery1.Execute;
end;

wchris
Posts: 51
Joined: Thu 09 Jun 2005 09:44

Post by wchris » Tue 17 Jan 2012 14:05

AlexP wrote:Hello,

In such case, after every procedure call, you can set the parameters DataType property to ftUnknown, and check the parameter type for ftUnknown before execution. If it is so, then it means that parameter was not set.
This solution looks good.
I'll try this.
Thank you Alex
Cheers

Post Reply