feat: add Expr.to_sql() #11357
feat: add Expr.to_sql() #11357
Conversation
|
I like this, and often find myself wanting to continue chaining. |
ibis/expr/types/core.py
Outdated
| """ | ||
| return self._find_backend().compile( | ||
| return self._find_backend(use_default=True).compile( |
There was a problem hiding this comment.
Is this a necessary change? It was explicitly left as False (the default) to avoid people compiling to a dialect that they haven't specified.
There was a problem hiding this comment.
I want to be able to compile unbound expressions to sql:
import ibis
ibis.literal("foo").compile()This change allows that. Though I agree, the implicitness is not good. Really I think we should add a dialect optional kwarg argument to Expr.compile() and then use that if given, or fall back to the default. Similar to how the new Expr.to_sql() works.
Note that I think it should be called dialect and not backend, because I should be able to supply the string "postgres" and it should work without an actual postgres instance connected.
But, you are right, I think this is out of scope for this PR, let's leave this change out.
There was a problem hiding this comment.
Thanks for fixing this up! looks good.
I also would love to add a toplevel util method like ibis.to_dialect(dialect: str | ibis.BaseBackend) -> sqlglot.compiler | OurPolarsCompiler or similar to make it more friendly when specifying the dialect. Maybe should be called to_compiler() and we actually add a public Compiler class/Protocol that the sqlglot and polars compilers inherit from/implement
ibis/expr/types/core.py
Outdated
| """ | ||
| return self._find_backend().compile( | ||
| return self._find_backend(use_default=True).compile( |
There was a problem hiding this comment.
I want to be able to compile unbound expressions to sql:
import ibis
ibis.literal("foo").compile()This change allows that. Though I agree, the implicitness is not good. Really I think we should add a dialect optional kwarg argument to Expr.compile() and then use that if given, or fall back to the default. Similar to how the new Expr.to_sql() works.
Note that I think it should be called dialect and not backend, because I should be able to supply the string "postgres" and it should work without an actual postgres instance connected.
But, you are right, I think this is out of scope for this PR, let's leave this change out.
This is mostly an ergonomics thing.
I don't want to have to
import ibis; ibis.to_sql(e), I want to just doe.to_sql()This also makes the docstring to compile() less confusing, and adds these methods to Columns and Values in the docs.