Forum OpenACS Development: a hack to the db_nextval hack

Collapse
Posted by Dan Chak on
On postgres, the divergence of sequences between those that use the view hack and those that do not is starting to cause a lot of problems. One big problem that is cropping up in a number of places is that the abstraction of the view hack is broken between the sql and tcl layers. db_nextval assumes you are using the view hack, and fails on any sequences that use the postgres syntax (what we would expect new OACS postgres developers to use by default, right...?) I am still searching for an end-all solution to this problem which _doesn't_ require splitting all queries that use nextval into -postgresql and -oracle files. For now, I have a hack to the hack. Here is my proposed new db_nextval for the file acs-tcl/tcl/00-database-procs-postgresql.tcl:
proc_doc db_nextval { sequence } { Returns the next value for a
sequence. This can utilize a pool of sequence values to save hits to
the database. } {

    set sequence_datatype_hack [db_string get_hack_type "select
relkind from pg_class where relname='${sequence}'"]

    switch "$sequence_datatype_hack" {
        "S" {
            return [db_string nextval "select nextval('${sequence}')"]
        }
        "v" {
            return [db_string nextval "select ${sequence}.nextval"]
        }
        default {
            ns_log notice "db_nextval error: uh oh.. ${sequence} does
not appear to be of type sequence OR view."
            return
        }
    }
}

This function checks to see if the sequence is a real sequence (return val 'S') or a view (return val 'v'). It then runs the correct query to get the nextval. If there are no objections, I am going to commit this into the tree. If someone has a good end-all solution to the nextval problem, I'd love to hear that, too.
Collapse
Posted by Don Baccus on
Good end-all solution:
  • drop view "foo"
  • create sequence "foo" with the value of "nextval('t_foo')" (when there's only one user running!)
  • drop sequence t_foo
That's the gist of an upgrade SQL file.

Then find all references of "foo.nextval" and replace with "nextval('foo')" making sure they're in PG, not generic, query files

Repeat for every package in the system.

I think we need your hacked db_nextval for now but it would be nice if we could get folks who want to work on PG packages to take the time to remove the hack entirely on a package-by-package basis.

Collapse
Posted by Stephen . on
The original idea was to make postgres look like oracle so that queries could remain the same, right?  Why not instead make oracle look like postgres by defining an oracle nextval function?  Haven't tried this, don't know if there's problems...
Collapse
Posted by Dan Chak on
Stephen, as a pro-pg-lobbyist, I tend to like your suggestion, though I don't think it helps in this case.

The following function is the result of some chatting with DanW on this issue:

proc_doc db_nextval { sequence } { 
    Returns the next value for a sequence. 
    This can utilize a pool of sequence values to save hits to the database. 
} {
    # the following query will return a nextval if the sequnce
    # is of relkind = 'S' (a sequnce).  if it is not of relkind = 'S'
    # we will try querying it as a view
    db_0or1row nextval_sequence "select nextval('${sequence}') as nextval
                                  where (select relkind 
                                           from pg_class 
                                          where relname = '${sequence}') = 'S'"
    if {[info exists nextval]} {
        return $nextval
    } else {
        ns_log notice "db_nextval: sequence($sequence) is not a real sequence.  perhaps it uses the view hack."
        db_0or1row nextval_view "select ${sequence}.nextval as nextval"
        return $nextval
    }
}
If the sequence is a true PG sequence, this modifed version of the function only uses one query instead of two. Might provide some speed improvement, so we are going to go with this one for now.

I am committing this change now, so that more pages will start to work on postgres (such as customizing portlets). This isn't set in stone, of course -- so continue to post comments/criticisms -- but we do need a temporary bandaid for now...

Collapse
Posted by Don Baccus on
How about the slightly simplified
proc_doc db_nextval { sequence } { 
    Returns the next value for a sequence. 
    This can utilize a pool of sequence values to save hits to the database. 
} {
    # the following query will return a nextval if the sequnce
    # is of relkind = 'S' (a sequnce).  if it is not of relkind = 'S'
    # we will try querying it as a view
    if { [db_0or1row nextval_sequence "select nextval('${sequence}') as nextval
                                  where (select relkind 
                                           from pg_class 
                                          where relname = '${sequence}') = 'S'"] } {
        return $nextval
    } else {
        ns_log notice "db_nextval: sequence($sequence) is not a real sequence.  perhaps it uses the view hack."
        db_0or1row nextval_view "select ${sequence}.nextval as nextval"
        return $nextval
    }
}
I'd leave out the log or change it to the debug level though, we create a lot of objects and that's done using the view hack currently. This would be the first one to get rid of if someone wants to tackle this issue.

Also the comment about pools of sequences can go away. The object sequence is pooled at the SQL level and in the very beginning of the porting effort I got rid of pooling at the Tcl level.