offset + last bug #7441

Closed
kostya opened this Issue Aug 24, 2012 · 10 comments

Comments

Projects
None yet
6 participants
@kostya

kostya commented Aug 24, 2012

Word.offset(4).last
SELECT "words".* FROM "words" OFFSET 4
Word.last :offset => 4
SELECT "words".* FROM "words" OFFSET 4

Not as expected. Missed order by id desc and limit 1;

(model is empty, project empty 3.2.8, pg)

@steveklabnik

This comment has been minimized.

Show comment Hide comment
@steveklabnik

steveklabnik Aug 24, 2012

Member

Not as expected.

What did you expect?

Member

steveklabnik commented Aug 24, 2012

Not as expected.

What did you expect?

@kostya

This comment has been minimized.

Show comment Hide comment
@kostya

kostya Aug 24, 2012

to work as in 2.3 rails

SELECT * FROM "words" ORDER BY words.id DESC LIMIT 1 OFFSET 4

4 record from the end

kostya commented Aug 24, 2012

to work as in 2.3 rails

SELECT * FROM "words" ORDER BY words.id DESC LIMIT 1 OFFSET 4

4 record from the end

@justinko

This comment has been minimized.

Show comment Hide comment
@justinko

justinko Aug 25, 2012

Contributor

Is this a bug? LIMIT definitely makes sense here.

Contributor

justinko commented Aug 25, 2012

Is this a bug? LIMIT definitely makes sense here.

@rafaelfranca

This comment has been minimized.

Show comment Hide comment
@rafaelfranca

rafaelfranca Aug 25, 2012

Member

Yes, it seems a bug

Member

rafaelfranca commented Aug 25, 2012

Yes, it seems a bug

@kostya

This comment has been minimized.

Show comment Hide comment
@kostya

kostya Sep 1, 2012

I think this test is wrong, because test wrong behaviour.

  def test_find_last_with_offset_gives_same_result_when_loaded_and_unloaded
    scope = Topic.offset(3)
    unloaded_last = scope.last
    loaded_last = scope.all.last
    assert_equal loaded_last, unloaded_last
  end

kostya commented Sep 1, 2012

I think this test is wrong, because test wrong behaviour.

  def test_find_last_with_offset_gives_same_result_when_loaded_and_unloaded
    scope = Topic.offset(3)
    unloaded_last = scope.last
    loaded_last = scope.all.last
    assert_equal loaded_last, unloaded_last
  end
@kostya

This comment has been minimized.

Show comment Hide comment
@kostya

kostya Sep 1, 2012

bug appears in 92c1076

kostya commented Sep 1, 2012

bug appears in 92c1076

@senny

This comment has been minimized.

Show comment Hide comment
@senny

senny Mar 7, 2013

Member

@kostya did you submit a PR?

Member

senny commented Mar 7, 2013

@kostya did you submit a PR?

@kostya

This comment has been minimized.

Show comment Hide comment
@kostya

kostya Mar 7, 2013

no
i can submit it, but it is necessary to decide, is it bug or feature?

kostya commented Mar 7, 2013

no
i can submit it, but it is necessary to decide, is it bug or feature?

@senny

This comment has been minimized.

Show comment Hide comment
@senny

senny Mar 7, 2013

Member

Is it that relevant? I think it's something we should include. I guess I would consider it a bug but given this issue has been open for a long time it's not something that people use often.

Member

senny commented Mar 7, 2013

Is it that relevant? I think it's something we should include. I guess I would consider it a bug but given this issue has been open for a long time it's not something that people use often.

@makaroni4

This comment has been minimized.

Show comment Hide comment
@makaroni4

makaroni4 Sep 30, 2013

Contributor

@kostya

Word.last offset: 4

# DEPRECATION WARNING: Relation#last with finder options is deprecated. Please build a scope and then call #last on it instead.

And for proper query:

Subscription.offset(1).last

# SELECT "subscriptions".* FROM "subscriptions" LIMIT -1 OFFSET 1

Is this issue is still valid?

Contributor

makaroni4 commented Sep 30, 2013

@kostya

Word.last offset: 4

# DEPRECATION WARNING: Relation#last with finder options is deprecated. Please build a scope and then call #last on it instead.

And for proper query:

Subscription.offset(1).last

# SELECT "subscriptions".* FROM "subscriptions" LIMIT -1 OFFSET 1

Is this issue is still valid?

laurocaetano added a commit to laurocaetano/rails that referenced this issue Dec 3, 2013

fluxusfrequency added a commit to fluxusfrequency/rails that referenced this issue Dec 4, 2013

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