Is there a plugin to crash when dataset methods are ignored?

[fdr]

Hello everyone,

Contrast these two situations:

DB["select * from pg_class"].where { oid > 7 }.sql
=> "select * from pg_class"

DB[:pg_class].where { oid > 7 }.sql
=> "SELECT * FROM "pg_class" WHERE ("oid" > $1::int4); [7]"

I seem to remember figuring out in the past that, if I wanted this to do what was expected, I wanted the "implicit_subquery" plugin. But, I felt the amount of bloating to enable implicit_subquery all the time was perhaps excessive.

Now, it occurs to me: something that seems better would be a plugin that crashes/raises an error when dataset method chaining is ignored.

Is there a plugin like that?

[jeremyevans]

It would be fairly easy to make a copy of the implicit_subquery plugin that raises instead of using an implicit subquery. Is that what you want? It's just as bloating as implicit_subquery, but honestly, I don't think the amount of bloating is excessive (was it a bottleneck in your application)?

For a general case, it's expected that Sequel dataset methods will only use the parts of the dataset they are expected to use, and not check for parts they are not intended to use:

  ds = DB[:table].insert_ignore
  ds.insert(:a=>1) # insert_ignore setting used
  ds.all # insert_ignore setting not used

[fdr]

I was thinking more it makes the queries themselves a bit longer. It's not so much about system overhead, as the legibility of logs and the like:

> puts DB["select * from pg_class"].where { oid > 7 }.sql
SELECT * FROM (select * from pg_class) AS "t1" WHERE ("oid" > $1::int4)

So far, I've done some small restructuring to do more raw SQL stuff to avoid subquery wrapping. I was thinking I would use from_self on my own when necessary. But, on and off in the last few months, it occurred to me that there is some danger of ignored conditions on raw SQL queries that could cause an information disclosure problem. I never got around to writing my concern until now.

Perhaps I should enable implicit_subquery on those grounds alone...

[jeremyevans]

Good point. It's by design that Database#run does not accept placeholders. It is possible to combine them:

  db.run(db["GRANT CREATE ON SCHEMA public TO ?", ph_user.to_sym].sql)

Maybe we should add an extension for Dataset#run so you can do?:

  db["GRANT CREATE ON SCHEMA public TO ?", ph_user.to_sym].run

[fdr]

I think adding .run makes sense. It's has a meaningful symmetry with db.run, it scans a bit more authoritatively that the result set is not checked vs .first, or .all. You can imagine this also being useful with INSERT/UPDATE/DELETE as well. Though in principle these can have interesting return values (whether the count is interesting, or with RETURNING), for many cases, the error/exceptions are all the programmer needs.

[jeremyevans]

OK. I'll add a dataset_run extension for that in 5.85 (too late for 5.84). Thanks for the discussion!

[jeremyevans]

I've pushed some commits based on this discussion:

• Added dataset_run extension (26a2243)
• Make Dataset#get without arguments run query directly (without subquery) when using implicit_subquery extension (45b7f37)
• Updated documentation for Custom SQL section of querying guide (0f4cce2)

Thanks for the feedback and discussion!

[fdr]

thanks for your constant and skilled maintenance and design of Sequel (and the others)