Skip to content

Enable "fromNext" by default from druid-0.10.0#2767

Closed
navis wants to merge 2 commits intoapache:masterfrom
navis:paging-fromnext-enabled
Closed

Enable "fromNext" by default from druid-0.10.0#2767
navis wants to merge 2 commits intoapache:masterfrom
navis:paging-fromnext-enabled

Conversation

@navis
Copy link
Copy Markdown
Contributor

@navis navis commented Apr 1, 2016

Based on #2576. As discussed in #2576 (comment), enable "fromNext" by default from druid-0.10.0.

@navis navis added this to the 0.10.0 milestone Apr 1, 2016
@navis navis changed the title Paging fromnext enabled Enable "fromNext" by default from druid-0.10.0 Apr 1, 2016
@fjy fjy modified the milestones: 0.10.0, 0.10.1 Dec 13, 2016
@fjy
Copy link
Copy Markdown
Contributor

fjy commented Dec 13, 2016

👍

@fjy
Copy link
Copy Markdown
Contributor

fjy commented Dec 13, 2016

@navis can we fix merge conflicts?

Copy link
Copy Markdown
Member

@nishantmonu51 nishantmonu51 left a comment

Choose a reason for hiding this comment

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

👍

@nishantmonu51
Copy link
Copy Markdown
Member

@navis: can you fix conflicts ?

@gianm
Copy link
Copy Markdown
Contributor

gianm commented Feb 14, 2017

IMO, if we're going to change the default of fromNext, we should also make it mandatory to specify either fromNext: false or fromNext: true. This means that people that don't pass fromNext in their queries will get failures starting in 0.10.0, which I think is better than getting wrong results.

@gianm
Copy link
Copy Markdown
Contributor

gianm commented Feb 21, 2017

Hmm, actually, my comment was kind of nonsensical. There's no such thing as "changing the default" for something that's mandatory. I still think that silent wrong results on older client code is bad, but also, making fromNext mandatory is kind of silly.

I would be ok with adding a runtime property that controls the default behavior, like druid.query.select.fromNextDefault and have that default to "true" in 0.10.0. Then users that need to support older client code could set it to "false" to get the old Druid behavior.

The important thing, imo, is that many Druid deployers are not the same people as the ones that write the queries, and we have to recognize that dynamic when making changes like this.

@fjy @nishantmonu51 @navis what do you think?

@nishantmonu51
Copy link
Copy Markdown
Member

@gianm: your suggestion seems good to me.

@fjy
Copy link
Copy Markdown
Contributor

fjy commented Feb 28, 2017

@gianm 👍

@jon-wei
Copy link
Copy Markdown
Contributor

jon-wei commented Mar 1, 2017

Closing this, discussion can continue at #3986

@jon-wei jon-wei closed this Mar 1, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants