Error Use functions in for each statement

[bigwill]

Hi all

Something strange here. Is this a progress bug or a limitation.
I am having problem using call to functions within a for statment. Using 10.2b at the moment.
Is this a bug in Progress ?

function GetRecId returns recid (input cSpotype as char,
                                input dOrderdate as date):
  /* function that returns recid */
end function.   
 
procedure BufferCopyOnCreateNew:
 
  /* This works */
  def var r as recid no-undo.
  r = GetRecId(oldSpotype,today).
  for first SpotypeEvent no-lock
    where recid(SpotypeEvent) = r:
    /* do something */
  end.
 
  /* This don't work */
  for first SpotypeEvent no-lock
    where recid(SpotypeEvent) = GetRecId(oldSpotype,today):
    /* do something */
  end.
 
end procedure.

 

[Rob Fitzpatrick]

I don't know what you're actually trying to accomplish. But I'll throw in a few observations.

• You don't explain where or how oldSpotype is defined or initialized.
• In your second FOR block you are using a user-defined function in a where clause. This is very bad practice because it prevents the compiler from bracketing on the selected index, resulting in a table scan.
• A recid is unique in a table so you don't need FOR FIRST. You can just do something like:

find mytable where recid(mytable) = somevalue.
/* do something with mytable buffer */

 

[UncleNel]

I agree with Rob, you don't use user defined functions in a for each select. It is my understanding that a user-defined function will execute only on the first iteration and that result will be used for all other iterations. If you need to use the function, then do it within the for-each block, not in the for-each select.

 

[bigwill]

Ok,
I did not specify how "oldSpotype" was defined, it is a normal string with some value, but i don't think it is vital to my exampel. Neither is how i use Find first.

My main problem is that i can not use a user-defined function in a where clause. Maybe it is bad practice in progress, but it is good practice in .NET which i also use.
But even if it is bad practice in progress, should't it work ?

Tried to fixed my example to run:

function GetRecId returns recid ():
 
  def var r as recid no-undo.
  def buffer bSpotype for spotype.
  find first bSpotype no-lock no-error.
  if avail bSpotype then
    r = recid(bSpotype).
 
  return r.
 
end function.   
 
 
/* This works */
def var r as recid no-undo.
r = GetRecId().
for first spotype no-lock
  where recid(spotype) = r:
  message 'hello using variable' view-as alert-box.
end.
 
/* This don't work */
for first spotype no-lock
  where recid(spotype) = GetRecId():
  message "hellow using function direct" view-as alert-box.
end.

 

[Rob Fitzpatrick]

• You're still using a FOR block where a FIND is advised.
• FOR FIRST is generally considered bad practice and may surprise you with its results.
• If you have a version of the code that works, then use it. But change the FOR FIRST to a FIND. It will make your code more readable and correctly document the fact that the query must be unique.
• If you do make this change, add error-handling to deal with the situation where your FIND returns no record.

 

[UncleNel]

Just to clarify my statements, I hack over your sample. Changing the for-first to a for-each will demonstrate why you don't use functions in the where clause.
<code>
def temp-table tt no-undo
field ok as log.

def var i as int.

create tt.
tt.ok = true.
create tt.
tt.ok = true.
create tt.
tt.ok = true.
create tt.
tt.ok = true.
create tt.
tt.ok = true.
create tt.
tt.ok = true.

function GetValue returns log():
message i view-as alert-box.
if i gt 0 then return false.
else return yes.
end function.

def var l as log init no no-undo.

l = getValue().
for first tt no-lock
where tt.ok = l:
message 'hello using variable' view-as alert-box.
end.

/* This don't work */
for each /*first*/ tt no-lock
where tt.ok = GetValue():
message 'hello using function direct' GetValue() view-as alert-box.
i = i + 1.
end.
</code>

 

[bigwill]

First, thank you for helping me. I have gotten this to work, but i don't understand why it is not working. Maybe bad practice but it should work.
I have rewritten my exampel to use find table with recid and it still don't work:

function GetRecId returns recid ():
 
  def var r as recid no-undo.
  def buffer bSpotype for spotype.
  find first bSpotype no-lock no-error.
  if avail bSpotype then
    r = recid(bSpotype).
 
  return r.
 
end function.   
 
 
/* This works */
def var r as recid no-undo.
r = GetRecId().
find spotype no-lock
  where recid(spotype) = r no-error.
if avail spotype then
  message 'hello using variable' view-as alert-box.
 
/* This don't work */
find spotype no-lock
  where recid(spotype) = GetRecId() no-error.
if avail spotype then
  message "hellow using function direct" view-as alert-box.

Now i do not use a for each or a for first. Now i use a find table using recid. My method returns the correct recid, but it still do not work when using a user-defined function in my where.

 

[TomBascom]

Ok,
I did not specify how "oldSpotype" was defined, it is a normal string with some value, but i don't think it is vital to my exampel. Neither is how i use Find first.

Actually it is relevant. You might, for instance, be using a "borrowed buffer" inside the function. And using FOR FIRST with a RECID shows that you're not really up to speed with how Progress queries work. Which reinforces the idea that maybe something int that function is causing an issue.

You also have not actually stated what the problem is. Is there an error message? If so, what error? If it is a n issue of unexpected functionality then --what did you expect and what was the unexpected result?

If it is "unexpected results" -- you're using FOR FIRST. Expect the unexpected. You won't be disappointed.

FOR FIRST is so bad that IMHO should be stricken from the language. You are looking for a single unique record. Use FIND, not FOR.

My main problem is that i can not use a user-defined function in a where clause. Maybe it is bad practice in progress, but it is good practice in .NET which i also use.
But even if it is bad practice in progress, should't it work ?

That depends. Describe the problem a bit more than "I cannot use".

Tried to fixed my example to run:

function GetRecId returns recid ():
 
  def var r as recid no-undo.
  def buffer bSpotype for spotype.
  find first bSpotype no-lock no-error.
  if avail bSpotype then
    r = recid(bSpotype).
 
  return r.
 
end function. 
 
 
/* This works */
def var r as recid no-undo.
r = GetRecId().
for first spotype no-lock
  where recid(spotype) = r:
  message 'hello using variable' view-as alert-box.
end.
 
/* This don't work */
for first spotype no-lock
  where recid(spotype) = GetRecId():
  message "hellow using function direct" view-as alert-box.
end.

[/quote]

On the bright side -- your function properly scopes the record so side-effects from calling the function aren't an issue.

Anyhow -- try this:

function getRecid returns recid():
  define variable r as recid no-undo.
  /* whatever */
  return r.
end.
 
find spotType no-lock where recid( spotType ) = getRecid() no-error.
if available spotType then
  message "found it!".
 else
 message "missing :(".

 

[TomBascom]

So, presumably, you're complaining that you get error 7254? "Illegal FIND, FOR EACH or OPEN QUERY in User-defined function"?

The error message is more descriptive in version 11:

A FIND, FOR EACH or OPEN QUERY statement has been encountered in a user-defined function, method or constructor 'getRecid
/home/tom/tmp/p23342_getrecid.ped' which was called from a WHERE clause. This statement cannot be executed in this context. (7254)

The long and the short of it:

Don't use user-defined functions in WHERE clauses.

 

[bigwill]

Ok. Just have to live with it then
Tom: Your last code is very similar to my last example, where i use find, but even that don't work

So, no worries. If this is bad practice i just have to get used to always use variables in for first, for each.

Thank you for all help.

 

[Cringer]

It has been explained further up but essentially the reason why it's bad practise is that it is incredibly inefficient. Progress decides what indexes will be used at compile time, therefore if there's a function in the where clause it has no way of evaluating that and so it will probably end up doing a whole table read for each iteration of the query.
To be honest, if you're going to be writing Progress code, you need to forget what is good and bad practise in .NET and learn what is good and bad practise for Progress. Trying to write Progress code from a .NET perspective will land you in a lot of pain.

 

[TomBascom]

Yes, I saw that right after I posted it.

The thread could have been a lot shorter if you had specified your error message -- then we could have gone directly to the problem for you instead of playing 20 questions to try to figure out what you meant

On the other hand we wouldn't have had so many opportunities to argue about worst practices...

You keep referring to FOR FIRST. If you code with FOR FIRST you will, eventually, come to regret it.

Your particular code snippet is best solved with a simple unique FIND. Adding "FIRST" to a FIND statement is usually bad (there are a few rare exceptions) and often an indication of poorly understood query concepts. Unfortunately there are vast bodies of vendor provided code where FIND FIRST is actually their standard. Don't give in -- never add FIRST to a FIND without getting my permission in writing (that should give you an idea of how rare it /should/ be...).

FOR EACH has many uses when sets of records are being iterated. FIND is appropriate when a single, unique record is needed.

User defined functions in WHERE clauses have many downsides:

1) They are only ever evaluated once. They are not evaluated for each record that is returned. So if the WHERE is part of a FOR EACH or an OPEN QUERY and you are passing any of the record data to the function you probably will not get the expected results.
2) They are evaluated on the client -- the server cannot evaluate them when it is selecting data (this is probably related to #1, although which is cauise, and which is effect is not clear to me).
3) If the UDF contains a query it blows up (this it what you saw happening)

Simple user-defined functions that do not contain queries can be useful in the WHERE clause of a FIND statement or in cases where you really do just want the function evaluated a single time (as if it were a variable). But you avoid accidentally running into the issues above by avoiding their use in WHERE clauses.