Forum OpenACS Q&A: what if we adjust empty_string_p

Collapse
Posted by David Richards on
It seems that empty_string_p has either been grossly missused, or
worked differently in the pure ACS.  All over the place, it is used on
a numerical field that is set to 0 when it doesn't have a value.  I
started adding extra conditionals where things were blowing up to test
for 0.  This is inellegant, and probably unnecessary.  I changed my
version of empty_string_p to read:

    if { [string compare $query_string ""] == 0 || [string compare
$query_string 0] == 0 } {
        return 1
    } else {
        return 0
    }

It seems to fix my problems, and I haven't seen anything else blow up
yet.  Does anyone know why this shouldn't be done?

Collapse
Posted by Roberto Mello on
The Tcl portions of the ACS are independent of implementation, Classic or OpenACS. What varies is that Oracle and PostgreSQL do different things under the same circunstances sometimes.

For example, I remember Don or Ben commented sometime ago on the behaviour of NULLs in both flavors of SQL. I think Oracle returns NULLs when a numeric field has nothing in it, whereas PostgreSQL correctly returns a 0 (am I on the right track Ben, Don ?)

If what I said above is true, then your modified empty_string_p makes sense and should be added to the CVS tree.

Collapse
Posted by Ben Adida on
Can you give an example of this happening? Which version of Postgres are you using? There are times when Postgres will see a 0 where there shouldn't be, but I don't think this is the right way to fix this. Give us a couple of examples where this is happening. Thanks!
Collapse
Posted by David Richards on
I saw this first in /education/class/one.tcl.  It's also in /education/class/assignment-info.tcl.  Each of these have three or four places where the version_id is returning 0 when null.  Also, there were some new problems with /education/class/admin/*.  When I started seeing trouble there, I just changed empty_string_p to take care of things globally.
Collapse
Posted by Don Baccus on
The difference between Oracle and Postgres regarding NULL has to do with the empty string '', which Oracle incorrectly treats as being equivalent to NULL in insert/update statements.

So this problem with zero you're seeing is due to something else.  Rather than gloss over it with empty_string_p, I'd rather have you point out just where you're running into them (as you've done in response to Ben's post) - they may be ACS Classic bugs, not just Postgres bugs.  If they're Postgres-only, perhaps some queries got mistranslated in a way that would be educational to us, allowing us to  improve the porting document to help others avoid such mistakes in the future.

Collapse
Posted by Dan Wickstrom on
Don,

I've come across this problem before. I think I even posted it on your site as one of those postgres odities. Here is an example:

Welcome to psql, the PostgreSQL interactive terminal.

Type:  copyright for distribution terms
       h for help with SQL commands
       ? for help on internal slash commands
       g or terminate with semicolon to execute query
       q to quit

acspg=# create table test (
acspg(# x integer);
CREATE
acspg=# insert into test values ('');
INSERT 1541897 1
acspg=# insert into test values (1);
INSERT 1541898 1
acspg=# select * from test;
 x 
---
 0
 1
(2 rows)

acspg=#
It appears that postgres converts an empty string into a zero, which in alot of cases has caused me problems. In the past, I've gone for the oracle behavior by adding triggers to check for empty strings on insert or update and convert them to nulls. I don't know if this is necessarily the best approach to fixing this problem, but at least it gives the same behavior as oracle.

Collapse
Posted by Michael A. Cleverly on
David, where exactly are you experiencing a problem with the existing empty_string_p calls? Your modified version of empty_string_p returns true (indicating that the string is empty) either when the string is empty, or when the string happens to be "0", which would seem (to me) to be more aptly named empty_string_or_zero_p.
Collapse
Posted by Ben Adida on
Bingo, Dan's got it. I remember I had seen this problem before, and so it seems that the empty string gets interpreted as "0" for the integer type. Don, we should report this as a bug, it really should throw an error, which would make our porting effort easier here.

I don't think the solution is to change empty_string_p, because the problem is with the data layer: it's wrong to keep a 0 in the database where you should have a NULL. It means more work to be done on our end, but it's better to fix the data entry than patch the data selection.

Collapse
Posted by David Richards on
Don, Michael, and Ben,

Thanks for your ideas.  I see the problem.  I like Ben's solution.  I was experiencing the problem in many places, which is why I thought I ought to just change the procedure to work for now.  However, it messes up other parts of the openACS (like the login), so I've undone my changes, and I live with the bugs, or fix them one at a time.  However, I agree that Michael's naming convention would make more sense if I were to be using this function in new development.

At the end of the day, if Postgres fixes this error, we'll all be better off.

Collapse
Posted by Don Baccus on
Ahhh, man, I wasn't aware of this '' == 0 thing (or had forgotten Dan's posting of it earlier).  Yes, it is a bug and should be reported  as such.

Which brings up a point.  Dan alluded to my old "postgres oddities" forum I had up a few months ago on my old P200 at home.  I dropped it when Ben put up the "official" OpenACS site.

Would there be value in setting this up again?  If so, it should go at  openacs.org.  Ben - I had it set up as an "editorial", not "Q&A" style forum in order to encourage rather complete descriptions of problems with comments appearing on their own page.  The idea was to build up something like an interactive porting hints/bugs/petty annoyances FAQ for Postgres in the ACS context.