Fix!: Improve handling of the --where option in table_diff
#2975
Conversation
sqlmesh/core/table_diff.py
Outdated
| @@ -272,7 +272,22 @@ def _column_expr(name: str, table: str) -> exp.Expression: | |||
| def name(e: exp.Expression) -> str: | |||
| return e.args["alias"].sql(identify=True) | |||
|
|
|||
| query = ( | |||
| source_query = ( | |||
| exp.select(*(exp.column(c) for c in source_schema.keys())) | |||
There was a problem hiding this comment.
you can just pass c, if it’s a string
you don’t need keys(), dict is iterable
There was a problem hiding this comment.
It looks like I still need to use exp.column() due to this effect:
>>> exp.select("country code").sql()
'SELECT country AS code'
>>> exp.select(exp.column("country code")).sql()
'SELECT "country code"'
however i've removed .keys()
sqlmesh/core/table_diff.py
Outdated
| .where(self.where) | ||
| ) | ||
| target_query = ( | ||
| exp.select(*(exp.column(t) for t in target_schema.keys())) |
sqlmesh/core/table_diff.py
Outdated
| join_alias="t", | ||
| ) | ||
| .where(self.where) | ||
| .from_(exp.alias_(source_identifier, "s")) |
There was a problem hiding this comment.
doesn’t seem right since it’s an identifier, is there no error here?
There was a problem hiding this comment.
>>> exp.alias_(exp.to_identifier("__source"), "s").sql()
'__source AS s'
It got the correct result. What should I have done?
| .from_(exp.alias_(source_identifier, "s")) | ||
| .join(exp.alias_(target_identifier, "t"), on=self.on, join_type="FULL") | ||
| ) | ||
|
|
There was a problem hiding this comment.
i don’t like manually creating with, can you use a builder api or use subqueries?
There was a problem hiding this comment.
Is the builder API documented anywhere? There didnt seem to be a .with(..)
There was a problem hiding this comment.
ah, I think I found it. There appears to be no top-level with_() like there is for select() but doing exp.Select().with_(...) gets me there.
Is that better?
Prior to this change, running
sqlmesh table_diffwith something like--where "foo = 'bar'"would throw an error like:This was because the where clause was injected verbatim without automatically qualifying the columns. The user had to know that table_diff used a subselect internally and supply
--where "s.foo = 'bar' and t.foo = 'bar'"in order to get this to work.This PR refactors the generated table_diff query to use CTE's to build the source / target / stats datasets so the
--whereclause can still be injected verbatim but it's now scoped to each dataset so there are no more ambiguous column references