DateTime field support #1665

evanxg852000 · 2022-06-24T15:20:57Z

Implements the DateTime field support
updated tantivy version to include DateTime changes
Closes Datetime field #1328

guilload · 2022-07-26T17:50:38Z

quickwit-doc-mapper/Cargo.toml

nit: keep the deps sorted

guilload · 2022-07-26T18:04:37Z

quickwit-doc-mapper/src/default_doc_mapper/date_time_type.rs

This intermediate representation exists because we want to build the parsers only once? Are there other reasons?

guilload · 2022-07-26T18:09:22Z

quickwit-doc-mapper/src/default_doc_mapper/date_time_type.rs

Would be nicer to display the formats attempted:
"Failed to parse datetime `foo` using the specified formats `unix_ts_secs, unix_ts_millis`."

guilload · 2022-07-26T20:35:36Z

docs/configuration/index-config.md

I wonder whether we should have date and datetime. Thoughts?

We could definitely have both: date being based on datetime. For better support (query parsing, display). I think we need to start from tantivy. Would you suggest renaming the date type as datetime now?

Yes, let's rename everything Datetime for now. We can add a proper Date type later with better tantivy support.

guilload · 2022-07-26T21:07:33Z

quickwit-doc-mapper/src/default_doc_mapper/date_time_type.rs

I don't think those function types are buying us anything. We could just iterate over the list of formats and get away with it. Less code, no dynamic dispatch. That would also remove the need for QuickwitDateOptionsDeser:

guilload · 2022-07-27T18:53:56Z

docs/configuration/index-config.md

Suggested change

- `Strftime`: Parsing dates using the unix [strftime](https://man7.org/linux/man-pages/man3/strftime.3.html) format.

- `strftime`: Parsing dates using the Unix [strftime](https://man7.org/linux/man-pages/man3/strftime.3.html) format.

guilload · 2022-07-27T18:54:41Z

quickwit-doc-mapper/src/default_doc_mapper/date_time_type.rs

Suggested change

pub struct QuickwitDateOptions {

pub struct QuickwitDateTimeOptions {

guilload · 2022-07-27T18:55:41Z

quickwit-doc-mapper/src/default_doc_mapper/date_time_type.rs

Looking much better! Thank you for the refactor.

guilload · 2022-07-27T19:09:33Z

quickwit-doc-mapper/src/default_doc_mapper/date_time_type.rs

We may want to implement as_str DateTimeFormat?

guilload · 2022-07-27T19:09:53Z

quickwit-doc-mapper/src/default_doc_mapper/date_time_type.rs

and then implement Display for DateTimeFormat instead.

guilload · 2022-07-27T19:47:05Z

quickwit-doc-mapper/src/default_doc_mapper/date_time_type.rs

Suggested change

use tantivy::{DatePrecision, DateTime};

use tantivy::{DatePrecision as DateTimePrecision, DateTime};

guilload · 2022-07-27T19:49:20Z

quickwit-doc-mapper/src/default_doc_mapper/mapping_tree.rs

Remove the unwrap and return proper error: "Failed to blabla".

guilload · 2022-07-27T19:51:30Z

quickwit-doc-mapper/src/query_builder.rs

guilload · 2022-07-27T19:52:46Z

quickwit-indexing/Cargo.toml

lol, just review a PR from Paul where he added the trailing comma. This makes me feel better about myself, I felt I was the only one OCD on the team.

quickwit-directories/src/bundle_directory.rs

guilload · 2022-07-28T16:06:19Z

docs/configuration/index-config.md

+- `unix_ts_secs`, `unix_ts_millis`, `unix_ts_micros`: Parsing dates from numbers (timestamp). Only one can be used in configuration. `unix_ts_secs` is added to the list by default if none is specified. 
+
+:::info
+When accepting multiple formats, the corresponding parsers are tried in order they are declared. 


Suggested change

When accepting multiple formats, the corresponding parsers are tried in order they are declared.

When specifying multiple input formats, the corresponding parsers are tried in the order they are declared.

guilload · 2022-07-28T16:13:55Z

quickwit-doc-mapper/src/default_doc_mapper/date_time_type.rs

+
+    pub(crate) fn parse_number(&self, value: i64) -> Result<OffsetDateTime, String> {
+        for format in self.input_formats.iter() {
+            match format {


if let DateTimeFormat::Timestamp(precision) = format { return unix_timestamp_parse(value, precision) }

guilload · 2022-07-28T16:14:53Z

quickwit-doc-mapper/src/default_doc_mapper/date_time_type.rs

+}
+
+/// Recognizes numbers as unix timestamp with a precision.
+fn unix_timestamp_parse(


Suggested change

fn unix_timestamp_parse(

fn parse_unix_timestamp(

guilload · 2022-07-28T16:16:27Z

quickwit-doc-mapper/src/default_doc_mapper/date_time_type.rs

+            DateTimeFormat::RFC2822 => "rfc2822",
+            DateTimeFormat::ISO8601 => "iso8601",
+            DateTimeFormat::Strftime(format) => format,
+            DateTimeFormat::Timestamp(precision) => match precision {


DateTimeFormat::Timestamp(DateTimePrecision::Seconds) => "unix_ts_secs" ...

guilload · 2022-07-28T16:18:01Z

quickwit-doc-mapper/src/default_doc_mapper/date_time_type.rs

+}
+
+impl Display for DateTimeFormat {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {


The codebase aligns on

fn fmt(&self, f: &mut std::fmt::Formatter)

guilload · 2022-07-28T16:19:07Z

quickwit-doc-mapper/src/default_doc_mapper/date_time_type.rs

+}
+
+/// Parses DateTime strings using the unix strftime formatting.
+fn strftime_parse(value: &str, format: &str) -> Result<OffsetDateTime, String> {


Suggested change

fn strftime_parse(value: &str, format: &str) -> Result<OffsetDateTime, String> {

fn parse_strftime(value: &str, format: &str) -> Result<OffsetDateTime, String> {

The codebase aligns on putting the verb first.

guilload · 2022-07-28T16:31:38Z

quickwit-doc-mapper/src/default_doc_mapper/mapping_tree.rs

+                (self.get_type(), json_val.as_i64())
+            {
+                let date_time_str =
+                    timestamp_to_datetime_str(timestamp, &options.precision).unwrap();


This is probably an expect, right?

evanxg852000 added 7 commits June 17, 2022 14:54

implemented parsing logic

6946920

now querying with lame (timestamp) format.

85dcfd8

integrating DateTime from tantivy

abdf9e2

Merge branch 'main' into fix-1328

b58e25a

fixed serialisation bridge

8c698b4

Merge branch 'main' into fix-1328

7c1eb29

working on datetime

68006f2

evanxg852000 changed the title ~~Fix 1328 [W.I.P]~~ DateTime field support [W.I.P] Jul 22, 2022

evanxg852000 added 4 commits July 22, 2022 11:56

implements datetime and upgraded tantivy

26bbfb8

Merge branch 'main' into fix-1328

c8aeeca

fix license header

d155726

added tests and fixed format duplication in BTreeSet

34044ce

evanxg852000 marked this pull request as ready for review July 25, 2022 13:07

evanxg852000 requested review from fulmicoton and guilload July 25, 2022 13:07

evanxg852000 changed the title ~~DateTime field support [W.I.P]~~ DateTime field support Jul 25, 2022

guilload reviewed Jul 26, 2022

View reviewed changes

fixed PR comment and switched to IndexSet as we need insertion ordering

de49c3a

evanxg852000 force-pushed the fix-1328 branch from 7198828 to de49c3a Compare July 27, 2022 12:15

guilload reviewed Jul 27, 2022

View reviewed changes

evanxg852000 force-pushed the fix-1328 branch from 078b858 to 50c9d88 Compare July 27, 2022 22:16

fix pr comments

1a3d398

evanxg852000 force-pushed the fix-1328 branch from 9023a3d to 1a3d398 Compare July 28, 2022 08:13

Merge branch 'main' into fix-1328

3cf77d6

guilload approved these changes Jul 28, 2022

View reviewed changes

fix pr comments

101dbfe

evanxg852000 merged commit 37976c9 into main Jul 28, 2022

evanxg852000 deleted the fix-1328 branch July 28, 2022 18:04

	- `Strftime`: Parsing dates using the unix [strftime](https://man7.org/linux/man-pages/man3/strftime.3.html) format.
	- `strftime`: Parsing dates using the Unix [strftime](https://man7.org/linux/man-pages/man3/strftime.3.html) format.

	pub struct QuickwitDateOptions {
	pub struct QuickwitDateTimeOptions {

	use tantivy::{DatePrecision, DateTime};
	use tantivy::{DatePrecision as DateTimePrecision, DateTime};

	When accepting multiple formats, the corresponding parsers are tried in order they are declared.
	When specifying multiple input formats, the corresponding parsers are tried in the order they are declared.

	fn strftime_parse(value: &str, format: &str) -> Result<OffsetDateTime, String> {
	fn parse_strftime(value: &str, format: &str) -> Result<OffsetDateTime, String> {

DateTime field support #1665

DateTime field support #1665

Uh oh!

Conversation

evanxg852000 commented Jun 24, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

evanxg852000 commented Jun 24, 2022 •

edited

Loading