rails / arel

A Relational Algebra
2.06k stars 390 forks source link

Deprecation notices are confusing me #424

Closed codeodor closed 6 years ago

codeodor commented 8 years ago

As an example: https://github.com/rails/arel/blob/master/lib/arel/visitors/to_sql.rb#L874

I think my confusion stems from the fact that even if I edit the message to include the full call stack, I don't see anything in my application's path nor my library's path that is causing the notice to be printed.

I have the same issue with "passing a column to quote" and table_exists? -- I can't find what my code is doing that's causing it.

Is it possible to put the notices closer to the source of what causes them, rather than in the call that finally executes the query?

If you are passing user input to a predicate, you must either give an appropriate type caster object to the Arel::Table, or manually cast the value before passing it to Arel

Can this be adjusted to include an example like there is for the "if you know the value is good" case?

rafaelfranca commented 8 years ago

Is this problem still valid?

codeodor commented 8 years ago

I think it is.

I don't know Arel well enough to know if there is a solution to the problem though.

For example, we construct a relation in one file (file A), passing in a bad value. Then in another file (file B), we do something with the relation that causes the query to execute.

But the stack trace and deprecation message don't refer to any place (file A) that would need to change to fix the message.

This is especially awful as a gem maintainer because your own code may not be at fault and you have no way at all to know what is at fault. File A could be in any number of projects yours depends on.

What I would propose, is that instead of having the query check the values and print the deprecation notice, it would be great if the notice could be printed when you are actually constructing the relation. That would let you know where to fix it.

However, as I noted, I don't know Arel well enough to know whether that is doable, or if it is doable, how much effort it would be to do it.

codeodor commented 8 years ago

Perhaps another way to do this, rather than printing the message when the relation is being built, maybe all the calls that affect the query could have their files and line numbers logged, and that could be printed with the notice?

matthewd commented 6 years ago

Per #523, Arel development is moving to rails/rails.

If this issue is still relevant, please consider reopening it over there. (Note that the Rails repository does not accept feature request issues, and requires reproduction steps that rely on Active Record's documented API.)