Skip to content

Fix whitespace handling #53

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.

Sign up for GitHub

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

Closed
satirebird wants to merge 2 commits into from
Closed

Fix whitespace handling #53

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
18690cb
Fix whitespace handling
Mar 1, 2020
e7799fd
Fix test case
Mar 1, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions src/query/write_query.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ impl Display for Type {
Float(x) => write!(f, "{}", x),
SignedInteger(x) => write!(f, "{}", x),
UnsignedInteger(x) => write!(f, "{}", x),
Text(text) => write!(f, "\"{text}\"", text = text),
Text(text) => write!(f, "{text}", text = text.replace(" ", "\\ ")),
Copy link
Collaborator

@Empty2k12 Empty2k12 Mar 2, 2020

Choose a reason for hiding this comment

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

What was the reason for removing these quotes? InfuxDb Docs says on quoting

Element Double quotes Single quotes
Timestamp Never Never
Measurements, tag keys, tag values, field keys Never* Never*
Field values Double quote string field values. Do not double quote floats, integers, or Booleans. Never

* InfluxDB line protocol allows users to double and single quote measurement names, tag keys, tag values, and field keys. It will, however, assume that the double or single quotes are part of the name, key, or value. This can complicate query syntax (see the example below).

I think it would be best if these guidelines were implemented. This will possibly break existing uses of this library as quoting field names is not recommended, but we did so far.

For escaping special characters, InfluxDb Docs suggest the following:

In string field values, you must escape:

  • double quotes
  • backslash character

In tag keys, tag values, and field keys, you must escape:

  • commas
  • equal signs
  • spaces

For example, , escapes a comma.

In measurements, you must escape:

  • commas
  • spaces

You do not need to escape other special characters.

That means we can safely ignore \r, \n, and \t.

Copy link
Author

@satirebird satirebird Mar 2, 2020

Choose a reason for hiding this comment

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

I wanted to replace my existing python application and I wanted to keep the existing measurements. The python library doesn't add any kind of quotes. Whitespaces will be escaped with backslashes.
As the InfluxDB docs says, the quotes are part of the name. If I use the rust library I cannot append any data with the same tag key as before. The double quotes are always added. This was the reason for removing the quotes.
On the other hand, if someone updates the library to the new version the user must add the quotes in the application (if necessary). But this should be easy. I think adding the double quotes in the library code is an unnecessary restriction.

Copy link
Author

@satirebird satirebird Mar 3, 2020

Choose a reason for hiding this comment

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

But...
there is a problem with the field values. The Type formatter is also used for the field values. If the field value is a string it must be quoted with double quotes. I think this is actually wrong and should be fixed prior.
Also the escaping should be completed.

}
}
}
Expand Down Expand Up @@ -250,12 +250,13 @@ mod tests {
.add_field("temperature", 82)
.add_tag("location", "us-midwest")
.add_tag("season", "summer")
.add_tag("space", "escape test")
.build();

assert!(query.is_ok(), "Query was empty");
assert_eq!(
query.unwrap(),
"weather,location=\"us-midwest\",season=\"summer\" temperature=82 11"
"weather,location=us-midwest,season=summer,space=escape\\ test temperature=82 11"
);
}

Expand Down