Re: [naviserver-devel] commit/nsdbi: 3 new changesets

[Stephen <yo...@gr...>]

On Sat, Feb 1, 2014 at 10:42 AM, Bitbucket <com...@bi...>wrote:

> 3 new commits in nsdbi:
>
> https://bitbucket.org/naviserver/nsdbi/commits/c81a764b2dad/
> Changeset:   c81a764b2dad
> User:        gustafn
> Date:        2014-02-01 11:41:41
> Summary:     - added "-result flat|sets|dicts?" to dbi_rows to specify
> output  format
> - added "dict" to allowed option values in "-bind" (allowed are
> array|set|dict?)
> - added the option "-autonull" to the options applicable to all
>    queries. This option automatically converts missing bind variables
>    as null values in the SQL statement
> - extended regression test
> - updated documentation
> Affected #:  4 files


Regarding the back 'n forth on these changes...


https://bitbucket.org/naviserver/nsdbi/commits/c81a764b2dad/#Ltclcmds.cT657

The original concern was that a generic utility function which wraps
dbi_rows and uses the typical foreach pattern might have it's private
variables clobbered by column names it can't predict. Returning a list of
dicts from dbi_rows is one way around that.

Looks like there's a couple of different situations where wrappers are
wanted:

1) Legacy ACS db_* functions

Looks like a lot of that code unpacks row ns_sets into local vars anyway.
db_multirow even has an -unclobber switch which restores the original
values of any vars which clash with column names.

It seems like a bit of a backwards step to add legacy sets to the dbi api
only to have the legacy db_* stuff unpack it again.

2) New generic xo functions

I'm not following how the implementation as it is now, which returns a list
of lists with duplicated column names, is faster than just returning a list
of dicts. You have to feed the lists to dict create before you use it,
which is extra work and always going to be slower.

For any other purpose nested lists are no different than the original flat
list with column list, and don't address the problem of variable clobbering.

How about this:

    dbi_dicts cols rows

Returns a list of dicts by zipping up the cols and rows lists returned by
dbi_rows. It's faster than the current implementation as dbi_rows only has
to construct a flat list rather than the nested list with duplicated keys,
and that means less data to cache in ns_cache, and parse again when
deserialising.

It could have more utility:

  dbi_dicts cols rows ?dict-name body?

...would evaluate body with a dict containing the current row, so that
rownum dicts do not have to exist in memory at once.

Generalising, it could be something like:

  dbi_foreach ?-dict name | -array name | -set name? cols rows ?body?

I avoided adding the obvious implementation of this because it encourages
stuff like nested queries. With dbi_rows you either get a list result or a
template filled with subst, but no eval. There's only ever one query active
and no visible handle management.