Page 1 of 1

Suggestion : Query empty parameters warning

Posted: Thu 12 Jul 2007 11:27
by wchris
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

Posted: Fri 13 Jul 2007 12:35
by Plash
We are not planning to add such option because in most cases it is correct when a programer sets a parameter value to NULL.

Posted: Mon 16 Jul 2007 13:06
by wchris
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

Posted: Mon 16 Jul 2007 13:15
by jfudickar

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]

Posted: Mon 16 Jul 2007 13:38
by wchris
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 !

Posted: Wed 18 Jul 2007 11:32
by Plash
We will investigate possibility to add such option, but that is hardly to happen in the nearest future.

Posted: Thu 26 Jul 2007 09:23
by wchris
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

Posted: Fri 06 Jan 2012 16:46
by wchris
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:

Posted: Wed 11 Jan 2012 12:11
by AlexP
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.

Posted: Thu 12 Jan 2012 11:11
by wchris
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.

Posted: Thu 12 Jan 2012 15:40
by AlexP
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

Posted: Fri 13 Jan 2012 12:22
by wchris
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.

Posted: Fri 13 Jan 2012 14:31
by AlexP
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;

Posted: Tue 17 Jan 2012 14:05
by wchris
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