Skip to content
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

Model instance method this returns dataset without a row_proc and thus returns a Hash #2280

Closed
onyxrev opened this issue Feb 26, 2025 · 4 comments

Comments

@onyxrev
Copy link

onyxrev commented Feb 26, 2025

Complete Description of Issue

Given some model Thing, I would expect the this method of an instance thing of Thing to return a dataset that would produce a Thing. But in my use it does not, it returns a Hash of the key/value pairs.

I investigated, and it appears that the dataset produced by the this method does not have a row_proc set in its opts.

Setting the row_proc to Thing will cause the dataset to return a Thing.

This is Sequel v5.89.0

Simplest Possible Self-Contained Example Showing the Bug

Is this a bug? It's not what I would expect.

[7] pry(#<...>)> thing.this.first.class
=> Hash
[8] pry(#<...>)> Thing.first(id: thing.id).class
=> Thing

[24] pry(#<...>)> foo = thing.this
=> #<Sequel::Postgres::Dataset: "SELECT * FROM \"entities\" WHERE (\"id\" = 123) LIMIT 1">
[25] pry(#<...>)> bar = Thing.where(id: thing.id).limit(1)
=> #<Sequel::Postgres::Dataset: "SELECT * FROM \"entities\" WHERE (\"id\" = 123) LIMIT 1">
[26] pry(#<...>)> foo.sql == bar.sql
=> true

[30] pry(#<...>)> foo.model
=> Thing
[31] pry(#<...>)> bar.model
=> Thing

[32] pry(#<...>)> foo.opts
=> {:from=>[:entities],
 :model=>Thing,
 :row_proc=>nil,
 :limit=>1,
 :skip_limit_check=>true,
 :where=>#<Sequel::SQL::BooleanExpression @op=>:"=", @args=>[:id, 123]>}
[33] pry(#<...>)> bar.opts
=> {:from=>[:entities], :model=>Thing, :row_proc=>Thing, :where=>#<Sequel::SQL::BooleanExpression @op=>:"=", @args=>[:id, 123]>, :limit=>1}

[41] pry(#<...>)> foo.with_row_proc(Thing).first.class
=> Thing

Full Backtrace of Exception (if any)

No response

SQL Log (if any)

No response

Ruby Version

3.0.6

Sequel Version

5.89.0

@jeremyevans
Copy link
Owner

The current behavior is expected. The documentation states: "Returns (naked) dataset that should return only this instance.". This has been true since the method was originally added to Sequel, as far as I know: https://github.com/jeremyevans/sequel/blob/1.0/sequel_model/lib/sequel_model/record.rb#L145

@onyxrev
Copy link
Author

onyxrev commented Feb 26, 2025

So an instance means a Hash? Seems like maybe confusing documentation?

@jeremyevans
Copy link
Owner

The (naked) part indicates it returns a Hash (no row_proc). I'm open to making the documentation more clear. How about: "Returns naked dataset that should return only the row related to this instance."? Or if you have other ideas, please let me know.

@onyxrev
Copy link
Author

onyxrev commented Feb 26, 2025

Yes, that is definitely better.

It makes sense that it's intuitive language for you but for someone less familiar with the library the term "naked" doesn't really communicate what it does.

However, if you'd like to rely on "naked dataset" to inform the user about the behavior, I would simply say "Returns a naked dataset for this instance."

"A naked dataset that should return only this instance" makes me think that the naked dataset would return an instance.

Thanks for the fix 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants