Skip to content
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

Line protocol integer #3526

Merged
merged 2 commits into from
Jul 31, 2015
Merged

Line protocol integer #3526

merged 2 commits into from
Jul 31, 2015

Conversation

corylanou
Copy link
Contributor

Line protocol now requires a trailing i for the type to be written as in int.

@@ -488,8 +488,9 @@ func isNumeric(b byte) bool {
// scanNumber returns the end position within buf, start at i after
// scanning over buf for an integer, or float. It returns an
// error if a invalid number is scanned.
func scanNumber(buf []byte, i int) (int, []byte, error) {
func scanNumber(buf []byte, i int) (int, error) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made this change as the []byte of the return was ignored and only called in one spot.

@pauldix
Copy link
Member

pauldix commented Jul 31, 2015

+1

1 similar comment
@juddgaddie
Copy link

+1

corylanou added a commit that referenced this pull request Jul 31, 2015
@corylanou corylanou merged commit 42c4034 into master Jul 31, 2015
@corylanou corylanou deleted the line-protocol-integer branch July 31, 2015 17:29
desa added a commit that referenced this pull request Aug 5, 2015
As noted in #3526 writing
integer values now requires a tailing i. This commit updates the README
appropriately
@beckettsean
Copy link
Contributor

@corylanou looks like there's a minor mismatch with the original #3519 from @pauldix.

cpu,host=A value=23.2i
# the precision after the decimal will be dropped on that last one

@mjdesa got a parser error, not a silent dropping of the fractional part. Either one can be the true implementation, but let's agree on the right one before we document.

@corylanou
Copy link
Contributor Author

@beckettsean correct. This was a decision based on conversation that I had with @pauldix and we decided to just return an error for now. If the community feels strongly, we can add more code around it to just truncate it. In general, it feels really odd that i would force a type conversion for them in the line protocol, but that's just my opinion.

@gunnaraasen
Copy link
Member

To me, adding an i to the field value implies a float-to-int type conversion, but I don't feel strongly either way. I could see this causing errors in naive clients. Just my two cents.

@beckettsean
Copy link
Contributor

Thanks for the context, @corylanou. I agree with you and Paul, let's leave it as is for now, and @mjdesa please document it as such. If someone opens an issue to complain about the writes throwing errors instead of silently recasting and truncating we can have the discussion then.

@beckettsean
Copy link
Contributor

@pauldix I updated your comment in #3519 to be consistent with the current implementation, just to reduce any confusion from folks who read that issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants