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

Make r.iso8601 round, not truncate. #6574

Merged
merged 1 commit into from Dec 21, 2017

Conversation

Projects
None yet
2 participants
@srh
Copy link
Contributor

srh commented Dec 21, 2017

Description

Fixes #6069.

@srh srh added this to the 2.4 milestone Dec 21, 2017

@srh srh merged commit 72d500d into rethinkdb:v2.4.x Dec 21, 2017

@srh

This comment has been minimized.

Copy link
Contributor

srh commented Dec 21, 2017

Pushed
v2.4.x 72d500d
next 8927612

@srh srh deleted the srh:sam/iso8601 branch Dec 21, 2017

}
while (read++ < 3) {
out += '0';
if (reql_version < reql_version_t::v2_4) {

This comment has been minimized.

@daveisfera

daveisfera Dec 21, 2017

Why does version matter here? Can't it just be changed to always do this behavior?

This comment has been minimized.

@srh

srh Dec 21, 2017

Contributor

Regular queries run with the latest reql_version value, but old secondary indexes' functions need to run with whatever reql_version value they were created with, in order to work properly.

This comment has been minimized.

@daveisfera

daveisfera Dec 21, 2017

The upgrade process states that secondary indexes need to be rebuilt during upgrade, so I don't see why this needs to be mainted since it's part of 2.4 and will require an upgrade.

This comment has been minimized.

@srh

srh Dec 21, 2017

Contributor

It says that secondary indexes should be rebuilt.

This comment has been minimized.

@daveisfera

daveisfera Dec 21, 2017

Just trying to keep the code simpler and avoid potential bugs down the road. Sorry for the noise.

This comment has been minimized.

@srh

srh Dec 21, 2017

Contributor

It's no problem. Eventually the old pre-2.4 behavior will be dropped (being optimistic about future releases happening here...)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment