Skip to content

Support Min/Max for Timestamp - #3299

Merged
fjy merged 4 commits into
apache:masterfrom
sirpkt:timestamp-min-max
Nov 15, 2016
Merged

Support Min/Max for Timestamp#3299
fjy merged 4 commits into
apache:masterfrom
sirpkt:timestamp-min-max

Conversation

@sirpkt

@sirpkt sirpkt commented Jul 28, 2016

Copy link
Copy Markdown
Contributor

Related with #3298.

  • Add an extension which provides two aggregator timeMax and timeMin.
  • Internally, it stores timestamp value as Long so that we can have DateTime with group by query by finalizeComputing() but only get Long value with select query.
  • To support parsing of string represented timestamp, TimestampSpec is slightly modified.
  • Incremental index is also modified to accept many different type of timestamp at ingestion time.

@fjy

fjy commented Aug 24, 2016

Copy link
Copy Markdown
Contributor

@sirpkt does this conflict with #3168?

Can we get some docs on what timeMax and timeMin do? I'm not entirely clear from reading the description

fjy
fjy reviewed Aug 27, 2016
View reviewed changes
Comment thread extensions-contrib/time-min-max/src/test/resources/druid.sample.tsv Outdated

@fjy fjy Aug 27, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can we gzip this file?

@sirpkt

sirpkt commented Aug 31, 2016

Copy link
Copy Markdown
Contributor Author

@fjy This is different from #3168.
Its resolution is the same as query granularity because __time is, while this patch returns exact min/max value.
#3168 returns one min/max time for the entire group of (maybe filtered) data while this patch returns separate min/max time for each row of aggregated data.

@fjy fjy added the Feature label Oct 31, 2016
@fjy fjy added this to the 0.9.3 milestone Oct 31, 2016
@fjy

fjy commented Oct 31, 2016

Copy link
Copy Markdown
Contributor

👍

@jon-wei

jon-wei commented Nov 7, 2016

Copy link
Copy Markdown
Contributor

I think this still needs docs, looks good to me aside from that

@fjy

fjy commented Nov 8, 2016

Copy link
Copy Markdown
Contributor

@sirpkt can we add docs and also an entry in the list of extensions?

@sirpkt

sirpkt commented Nov 15, 2016

Copy link
Copy Markdown
Contributor Author

@fjy I added docs.

@fjy
fjy merged commit 094f5b8 into apache:master Nov 15, 2016
fundead pushed a commit to fundead/druid that referenced this pull request Dec 7, 2016
@sirpkt fundead
Support Min/Max for Timestamp (apache#3299)
25946cc 
* Min/Max aggregator for Timestamp

* remove unused imports and method

* rebase and zip the test data

* add docs
dgolitsyn pushed a commit to metamx/druid that referenced this pull request Feb 14, 2017
@sirpkt @dgolitsyn
Support Min/Max for Timestamp (apache#3299)
95745d3 
* Min/Max aggregator for Timestamp

* remove unused imports and method

* rebase and zip the test data

* add docs
@gianm gianm mentioned this pull request Feb 28, 2017
Closed
@leventov leventov mentioned this pull request Aug 7, 2017
Closed
seoeun25 pushed a commit to seoeun25/incubator-druid that referenced this pull request Feb 25, 2022
@navis
apache#3299 Minor refactoring on value desc
d14a1ec
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

Feature Release Notes

Projects

None yet

Milestone

0.10.0

Development

Successfully merging this pull request may close these issues.

4 participants

@sirpkt @fjy @jon-wei @gianm