flightaware / speedtables

Speed tables is a high-performance memory-resident database. The speed table compiler reads a table definition and generates a set of C access routines to create, manipulate and search tables containing millions of rows. Currently oriented towards Tcl.
https://flightaware.github.io/speedtables/
BSD 3-Clause "New" or "Revised" License
66 stars 15 forks source link

"share set" vs "share get" does not match for values that are lists #12

Closed bovine closed 13 years ago

bovine commented 13 years ago

An extra list is introduced around values that are already lists:

positions share set moo "1234 2345 23423"
ok {}
positions share get moo
ok {{1234 2345 23423}}

Non-lists seem to be correctly handled:

positions share set moo 1234
ok {}
positions share get moo
ok 1234
resuna commented 13 years ago

This is expected behaviour. "get" returns a list of requested fields.

Try this:

positions share set moo {1234 5678 23423} positions share set woof 1234 positions share get moo woof ok {{1234 5678 23423} 1234}

You're being led astray by the fact that the printable form of a single-element list is the same as the first element.

bovine commented 13 years ago

Make "share get" not introduce extra list around the value if it is already a list and only one value is being requested.

[master 38f110c] Make "share get" not introduce extra list around the value if it is already a list and only one value is being requested. (issue 12) 3 files changed, 41 insertions(+), 2 deletions(-)

bovine commented 13 years ago

hmm, didn't see your post before I committed... Let me consider that.

bovine commented 13 years ago

I did notice that "share get" allows requesting multiple values when I was making my fix. My above commit makes it return a single value if you are requesting only one variable, or the old behavior if requesting 2 or more values. What do you think about that?

resuna commented 13 years ago

Changing it will break any existing code that uses the idiom [lindex [$table get key] 0]. I know I've written code that uses it. None of my code that does is live any more, but I would be surprised if there was no code in all of Flightaware that used it.

resuna commented 13 years ago

Karl and I have discussed this before and decided that the existing behavior was less surprising than having the result change from an element to a list depending on the number of elements requested.

bovine commented 13 years ago

Actually, FA doesn't use any other shared memory speedtables currently. I can change it back if we think it's better that way though... It just seemed more convenient to avoid needing to use lindex/lassign if you're only requesting one variable.

resuna commented 13 years ago

You're not just changing shared memory ctables. This change effects all ctables code, and also needs to be changed in master-server ctables and stapi.

resuna commented 13 years ago

It also creates an inconsistency between "table get" and "table search -get".

bovine commented 13 years ago

okay you have some good arguments, so I've restored the original behavior.

However, I don't seem to actually be able to use "share get" for a non shmem table. It always returns empty string.

package require speedtable

CExtension bullseye 1.0 {
    CTable Barn {
        varstring alpha
        varstring beta indexed 1
        varstring delta
        varstring gamma
    }
}

package require Bullseye
Barn create mybarn

mybarn share set alpha alfa
if {[mybarn share get alpha] != [list "alfa"]} {
    error "mismatch1"
}
mybarn share set beta "bovo bravo"
if {[mybarn share get beta] != [list "bovo bravo"]} {
    error "mismatch2"
}
mybarn share set delta deltalina gamma "gamma gamma gamma"
if {[mybarn share get gamma] != [list "gamma gamma gamma"]} {
    error "mismatch3"
}
if {[mybarn share get beta delta] != [list "bovo bravo" "deltalina"]} {
    error "mismatch4"
}
resuna commented 13 years ago

Aha, I get it! I'm mixing up "share get" with "get". You're right, the "share" command is only there for shared memory ctables. It should be an error for everything else.

How are you using "share set" and "share get"? What are you using them for?

resuna commented 13 years ago

Anyway, for the "share get" command, I don't see anything that would actually break with your change, but it's a change that makes me nervous.

resuna commented 13 years ago

On further thought I would be happier making "share get" not even accept more than one argument, rather than break the symmetry of a list and a single value.

bovine commented 13 years ago

good point. I've made a commit to make all "share" commands now return error rather than silently doing nothing:

[master 64f322e] make "share" commands return error (not TCL_OK) when invoked on a non-shared table. 1 files changed, 4 insertions(+), 2 deletions(-)

I'm using the "share set/get" for communicating the last replayed timestamp from the master to the readers. This works well for small variables that are not complex enough to warrant a dedicated speedtable.

I think I will change "share get" to only accept one argument, and then add a "share multiget" to let you specify multiple arguments and get the old behavior...

bovine commented 13 years ago

[master a1e1a50] make "share get" accept only one argument, add "share multiget" for multiple arguments and returning a list. (Issue 12) 4 files changed, 91 insertions(+), 16 deletions(-)