Support all value types #1580

otoolep · 2015-02-12T00:30:59Z

No description provided.

otoolep · 2015-02-12T00:31:13Z

WIP.

otoolep · 2015-02-12T00:33:08Z

shard.go

 			binary.BigEndian.PutUint64(buf[1:9], math.Float64bits(v))
+		case bool:
+			buf = make([]byte, 2)


@benbjohnson -- from implementing this function, it seems to me we need to code more information in a (field ID, value) pair -- we must also encode the type. Can you confirm? unmarshalValues just assumes that it's a float type in the database, but that will no longer be true with these changes.

If I am correct, a single byte after the field ID would be sufficient.

You shouldn't have to encode the type as that's kept in the metastore. Field id, followed by value (if a fixed size type) or in the case of bytes and strings, field id followed by length, followed by value.

So why is this code casting it to a float before sending it up? Is that redundant? It did occur to me that the info is in the metastore, and we could decode at a higher level.

To be clear -- the conversion is happening in unmarshalValues.

It seems like I could preform the type conversion using the metastore around here:

https://github.com/influxdb/influxdb/blob/master/server.go#L1839

so the low-level conversion to float (and support for other types at this low-level as implied by the comments) may be unnecessary.

Ah -- unless your distinction is just between numeric (floats) with a fixed length and variable length like strings? I interpreted the comments to mean we needed a breakdown by all supported types at this low-level of the code.

I think the ReadSeries function is actually cruft left over from stuff that was in there for debugging purposes. The real way to read raw series data is through the query engine, which now actually works.

There's some other spot in the code where you can get a value based on the field ID. At some point someone needs to cast this to the correct data type. That should be done in the query engine before it gets yielded to the mappers.

Ah -- OK. I do recall @benbjohnson remarking that ReadSeries was debug. If casting strictly happens at the higher level, yeah, then at this low-level it really is just a bunch of bytes.

Let me take another pass.

otoolep · 2015-02-12T06:15:55Z

Just for the record, unmarshalValues is also part of the production code, and is called while iterating over the shard (I had assumed after the discussion above that it was actually just used by ReadSeries). So right now all type "conversion" is being done at the low-level, and no reference is made to the Metastore when performing the conversion, that I can see.

otoolep · 2015-02-12T06:16:40Z

Obviously using a single source of truth for field types would be best, of course.

Code now on Measurement type

otoolep · 2015-02-13T01:29:15Z

@pauldix @benbjohnson -- please review these latest changes and let me know what you think. This is still WIP, but I think it's close. I'd like some feedback at this point. Points to note:

I basically just lifted the "marshal" and "unmarshal" code from shard.go. There are some potential optimizations there -- and we should do them -- but wanted to get this working first.
I just added a new interface to the cursor, such that it expects a field decoder. I think this is fine, but for now I just slammed the Measurement object in there. Doesn't feel great.
@benbjohnson suggested a dedicated field codec, with a static of set of fields, presumably to avoid races. But we seem to have the same race with mapValues and there is no locking there. Is this code special, or we just haven't fixed up mapValues yet?
All integers are still stored internally as floats.
While the build is green, I haven't actually added any tests for booleans and strings yet.

pauldix · 2015-02-13T11:39:09Z

We need to also support raw bytes as a type.

On the mapValues part of things, almost all the aggregate functions look at only one field. It would be nice to have a function that takes this field ID and returns the raw value without marshaling it into a map. We should be able to just cast the raw bytes from the DB into the type.

This would save us a ton when aggregating a through a bunch of data by not allocating a bunch of maps.

otoolep · 2015-02-13T16:15:00Z

How would a user insert raw bytes?

benbjohnson · 2015-02-13T16:15:22Z

Materializing into a map was just to get it working. It definitely need to get optimized. As for the mapValues(), the lack of locking is a bug. Queries can potentially run for a while so we need to be able to copy the field metadata to the codec to be used during query execution.

pauldix · 2015-02-13T16:23:54Z

I'm looking at some other stuff on the query side in terms of raw data queries. I think it would be useful to have the iterators yield the raw bytes to the map functions. Then have the iterator have a method that will pull a field value from those bytes.

This will be useful for queries that need to access multiple fields in the map portion of the query.

…r field values.

…nd value

Conflicts: database.go server.go

otoolep · 2015-02-17T00:46:05Z

Field codec split into its own type, which allows the locking to be decoupled from the server lock. I still need to add support for other types, and have questions about interactions with the shard iterators.

Unit test indexing of strings.

otoolep · 2015-02-17T20:38:56Z

Unit tests added for string and boolean types.

…l_value_types

pauldix · 2015-02-17T21:10:00Z

LGTM. Drop it like it's hot. Where's the Snoop Dogg emoji when you need it?

otoolep · 2015-02-17T21:19:29Z

Thanks for the review @pauldix -- merging now.

Support all value types

otoolep added the 2 - Working label Feb 12, 2015

otoolep reviewed Feb 12, 2015
View reviewed changes

otoolep and others added 5 commits February 12, 2015 16:06

Error if field's type has changed

bf9bfd6

Add marshalling for more field types

aa6a45c

Remove marshalValues

5b8c074

Code now on Measurement type

Cursor now requires a Field decoder

e9880a4

Remove unmarshalValues

0ca98ea

otoolep force-pushed the support_all_value_types branch 3 times, most recently from c99b16a to 335bad2 Compare February 13, 2015 01:21

otoolep force-pushed the support_all_value_types branch from 335bad2 to e562716 Compare February 13, 2015 01:34

Support bool and string types

4baeee3

otoolep force-pushed the support_all_value_types branch from e562716 to 4baeee3 Compare February 13, 2015 01:35

pauldix and others added 6 commits February 13, 2015 14:04

WIP: updating the signature for iterators to pass around raw bytes fo…

b394fc2

…r field values.

WIP: update signature of iterator in tx.go

4e68828

Update interface on iterator.Next() to pass raw data along with key a…

4cd3494

…nd value

Merge branch 'master' into support_all_value_types

dcdb4b7

Conflicts: database.go server.go

Clarify use of ReadSeries()

8cdee6a

Implement codec's encode

6db74aa

otoolep force-pushed the support_all_value_types branch 2 times, most recently from b3cdf28 to 7ace67a Compare February 17, 2015 00:23

Implement codec DecodeByID

5f20af0

otoolep force-pushed the support_all_value_types branch from 7ace67a to 5f20af0 Compare February 17, 2015 00:31

otoolep added 2 commits February 16, 2015 16:39

Use dedicated decoded in cursor

571fde8

Use Field codec in debug code

ae81dff

otoolep mentioned this pull request Feb 17, 2015

Create dedicated codec for Fields #1612

Closed

otoolep added 3 commits February 17, 2015 12:04

Rename test case for clarity

b564b90

Decode strings correctly with correct end index

e825022

Unit test indexing of strings.

Unit test writing boolean types

3810fb5

Merge branch 'master' of github.com:influxdb/influxdb into support_al…

e74fadf

…l_value_types

otoolep added a commit that referenced this pull request Feb 17, 2015

Merge pull request #1580 from influxdb/support_all_value_types

cd87583

Support all value types

otoolep merged commit cd87583 into master Feb 17, 2015

otoolep removed the 2 - Working label Feb 17, 2015

otoolep deleted the support_all_value_types branch February 17, 2015 21:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support all value types #1580

Support all value types #1580

otoolep commented Feb 12, 2015

otoolep commented Feb 12, 2015

otoolep Feb 12, 2015

pauldix Feb 12, 2015

otoolep Feb 12, 2015

otoolep Feb 12, 2015

otoolep Feb 12, 2015

otoolep Feb 12, 2015

pauldix Feb 12, 2015

otoolep Feb 12, 2015

otoolep commented Feb 12, 2015

otoolep commented Feb 12, 2015

otoolep commented Feb 13, 2015

pauldix commented Feb 13, 2015

otoolep commented Feb 13, 2015

benbjohnson commented Feb 13, 2015

pauldix commented Feb 13, 2015

otoolep commented Feb 17, 2015

otoolep commented Feb 17, 2015

pauldix commented Feb 17, 2015

otoolep commented Feb 17, 2015

Support all value types #1580

Support all value types #1580

Conversation

otoolep commented Feb 12, 2015

otoolep commented Feb 12, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

otoolep commented Feb 12, 2015

otoolep commented Feb 12, 2015

otoolep commented Feb 13, 2015

pauldix commented Feb 13, 2015

otoolep commented Feb 13, 2015

benbjohnson commented Feb 13, 2015

pauldix commented Feb 13, 2015

otoolep commented Feb 17, 2015

otoolep commented Feb 17, 2015

pauldix commented Feb 17, 2015

otoolep commented Feb 17, 2015