New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GET key id [WITHFIELDS] has a bug #169

Closed
icewukong opened this Issue Apr 19, 2017 · 8 comments

Comments

4 participants
@icewukong

icewukong commented Apr 19, 2017

Hi,@tidwall
It looks like a bug:

When executed:
SET fleet truck1 FIELD speed 0 FIELD age 21 POINT 33.5123 -112.2693
and
get fleet truck1 withfields

the field who's value is equal to 0, will not be returned by "GET" command, but returned by "SCAN" command.

Can you check it?

@Lars-Meijer

This comment has been minimized.

Show comment
Hide comment
@Lars-Meijer

Lars-Meijer Apr 19, 2017

I can confirm this behavior, more specifically, any field that is set to 0 will not get returned by the GET command. So it looks like this is indeed a bug.

I can confirm this behavior, more specifically, any field that is set to 0 will not get returned by the GET command. So it looks like this is indeed a bug.

@m1ome

This comment has been minimized.

Show comment
Hide comment
@m1ome

m1ome Apr 19, 2017

Contributor

As far as i remember @tidwall have a reason why we don't show fields that contains 0.
We definitely should ask him. I prepared PR fixing this issue.

Dunno if we need it or it's normal behaviour.

Contributor

m1ome commented Apr 19, 2017

As far as i remember @tidwall have a reason why we don't show fields that contains 0.
We definitely should ask him. I prepared PR fixing this issue.

Dunno if we need it or it's normal behaviour.

@Lars-Meijer

This comment has been minimized.

Show comment
Hide comment
@Lars-Meijer

Lars-Meijer Apr 19, 2017

Dunno if we need it or it's normal behaviour.

Maybe it has to do with unsetting a field? Is there a way to remove a field from a key with this change? In my opinion there is a difference between being 0 and having no value. E.G. a car has a speed of 0, or a car has an unknown speed.

Dunno if we need it or it's normal behaviour.

Maybe it has to do with unsetting a field? Is there a way to remove a field from a key with this change? In my opinion there is a difference between being 0 and having no value. E.G. a car has a speed of 0, or a car has an unknown speed.

@tidwall

This comment has been minimized.

Show comment
Hide comment
@tidwall

tidwall Apr 19, 2017

Owner

Dunno if we need it or it's normal behaviour.

This is normal, in terms that it's by design.

The zero value is omitted by design. This is due to the manner in which a collection manages the layout of the field data.

Each field value is stored at a specified index in a []float64 array. Each object has it's own field array. The objects do not store the names of the fields, only the values. The parent collection has a map[string]int that maps the name to an index. This is to keep the field data as packed as possible, and the collection memory usage as low as possible.

When a field is added to an object in a collection the object allocates a map entry with one element.

set mykey id1 field a 1 point 10 10
mykey NAMES    -> ["a"]
id1   FIELDS   -> [1]
scan mykey points
{"ok":true,"fields":["a"],"points":[
  {"id":"id1","point":{"lat":10,"lon":10},"fields":[1]}
],"count":1,"cursor":0,"elapsed":"40.402µs"}

Then when a second field is added to a second object, that object allocates a []float64 with two elements. The first element is a zero value and the second is the new field value.

set mykey id2 field b 2 point 20 20
set mykey id3 field a 3 point 30 30
set mykey id4 point 40 40
mykey NAMES    -> ["a","b"]
id1   FIELDS   -> [1]
id2   FIELDS   -> [0,2]
id3   FIELDS   -> [3]
id4   FIELDS   -> []
scan mykey points
{"ok":true,"fields":["a","b"],"points":[
  {"id":"id1","point":{"lat":10,"lon":10},"fields":[1,0]},
  {"id":"id2","point":{"lat":20,"lon":20},"fields":[0,2]},
  {"id":"id3","point":{"lat":30,"lon":30},"fields":[3,0]},
  {"id":"id4","point":{"lat":40,"lon":40},"fields":[0,0]}
],"count":1,"cursor":0,"elapsed":"40.402µs"}

During a scan all possible field names are written ahead of the objects, and each object has a matching number of the field values as names. The collection does not know ahead of time if an object has a field value or not.

Thus in the SCAN above, you'll notice that id4 has [0,0] even though in memory the values are in a empty []. The SCAN filled the extra array values on-the-fly.

Also id2 stored a zero at index one (the "a" field) even though it never had the "a" field set. For this reason I decided that it was best to always omit zero values in GET calls, even when using WITHFIELDS.

Maybe it has to do with unsetting a field?

That's a part of it. Unsetting a field or deleting an object will never alter the collection fieldname map.

In my opinion there is a difference between being 0 and having no value. E.G. a car has a speed of 0, or a car has an unknown speed.

Valid point.

Owner

tidwall commented Apr 19, 2017

Dunno if we need it or it's normal behaviour.

This is normal, in terms that it's by design.

The zero value is omitted by design. This is due to the manner in which a collection manages the layout of the field data.

Each field value is stored at a specified index in a []float64 array. Each object has it's own field array. The objects do not store the names of the fields, only the values. The parent collection has a map[string]int that maps the name to an index. This is to keep the field data as packed as possible, and the collection memory usage as low as possible.

When a field is added to an object in a collection the object allocates a map entry with one element.

set mykey id1 field a 1 point 10 10
mykey NAMES    -> ["a"]
id1   FIELDS   -> [1]
scan mykey points
{"ok":true,"fields":["a"],"points":[
  {"id":"id1","point":{"lat":10,"lon":10},"fields":[1]}
],"count":1,"cursor":0,"elapsed":"40.402µs"}

Then when a second field is added to a second object, that object allocates a []float64 with two elements. The first element is a zero value and the second is the new field value.

set mykey id2 field b 2 point 20 20
set mykey id3 field a 3 point 30 30
set mykey id4 point 40 40
mykey NAMES    -> ["a","b"]
id1   FIELDS   -> [1]
id2   FIELDS   -> [0,2]
id3   FIELDS   -> [3]
id4   FIELDS   -> []
scan mykey points
{"ok":true,"fields":["a","b"],"points":[
  {"id":"id1","point":{"lat":10,"lon":10},"fields":[1,0]},
  {"id":"id2","point":{"lat":20,"lon":20},"fields":[0,2]},
  {"id":"id3","point":{"lat":30,"lon":30},"fields":[3,0]},
  {"id":"id4","point":{"lat":40,"lon":40},"fields":[0,0]}
],"count":1,"cursor":0,"elapsed":"40.402µs"}

During a scan all possible field names are written ahead of the objects, and each object has a matching number of the field values as names. The collection does not know ahead of time if an object has a field value or not.

Thus in the SCAN above, you'll notice that id4 has [0,0] even though in memory the values are in a empty []. The SCAN filled the extra array values on-the-fly.

Also id2 stored a zero at index one (the "a" field) even though it never had the "a" field set. For this reason I decided that it was best to always omit zero values in GET calls, even when using WITHFIELDS.

Maybe it has to do with unsetting a field?

That's a part of it. Unsetting a field or deleting an object will never alter the collection fieldname map.

In my opinion there is a difference between being 0 and having no value. E.G. a car has a speed of 0, or a car has an unknown speed.

Valid point.

@tidwall

This comment has been minimized.

Show comment
Hide comment
@tidwall

tidwall Apr 19, 2017

Owner

It's clear to me now that field system, as it stands today, is confusing. It may need to be revised for v2.0 to address case for nonexistent values.

Owner

tidwall commented Apr 19, 2017

It's clear to me now that field system, as it stands today, is confusing. It may need to be revised for v2.0 to address case for nonexistent values.

@icewukong

This comment has been minimized.

Show comment
Hide comment
@icewukong

icewukong Apr 20, 2017

It's not a big problem. And it will be clear by statute to illustrate the usage of field.
@Lars-Meijer,@m1ome,@tidwall, thank you for your explanation!

It's not a big problem. And it will be clear by statute to illustrate the usage of field.
@Lars-Meijer,@m1ome,@tidwall, thank you for your explanation!

@tidwall

This comment has been minimized.

Show comment
Hide comment
@tidwall

tidwall Apr 20, 2017

Owner

@icewukong You're welcome.

Owner

tidwall commented Apr 20, 2017

@icewukong You're welcome.

@tidwall

This comment has been minimized.

Show comment
Hide comment
@tidwall

tidwall Apr 20, 2017

Owner

I'll explore alternatives for the next major point release, but for now I feel that it's best to leave the behavior as is.

I added a little extra info regarding to the zero behavior on the Tile38 website.

http://tile38.com/commands/get
http://tile38.com/commands/intersects#fields

If anyone has other ideas around this topic please feel free to reopen this issue.

Owner

tidwall commented Apr 20, 2017

I'll explore alternatives for the next major point release, but for now I feel that it's best to leave the behavior as is.

I added a little extra info regarding to the zero behavior on the Tile38 website.

http://tile38.com/commands/get
http://tile38.com/commands/intersects#fields

If anyone has other ideas around this topic please feel free to reopen this issue.

@tidwall tidwall closed this Apr 20, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment