Feat(tsql): improve transpilation of temp table DDLs #1958
Conversation
| return sep.join(self.sql(e) for e in expressions) | ||
| return sep.join(sql for sql in (self.sql(e) for e in expressions) if sql) |
There was a problem hiding this comment.
I think this was a bug @tobymao: if an expression yields "" for its sql(...), then we'll include the separator anyway even though there's no meaningful text for it. It happened to me with TemporaryProperty and I was getting CREATE TABLE ... for a T-SQL test case (notice the added whitespace).
|
Local and Global temp table support is huge. TSQL and Snowflake report temp table variables but I haven't encountered them much. Doing these later is good. |
| @@ -2426,6 +2427,9 @@ def expressions( | |||
| result_sqls = [] | |||
| for i, e in enumerate(expressions): | |||
| sql = self.sql(e, comment=False) | |||
| if not sql: | |||
There was a problem hiding this comment.
Probably just didn't have a use case where an expression was generating an empty string. This came up naturally in the case of table properties, because we can discard one such property by generating "" for it.
There was a problem hiding this comment.
The test case I tried was actually mis-formatted due to this line: https://github.com/tobymao/sqlglot/pull/1958/files#diff-362498362ae539e216cd83ebfe510977107648742f1a1bee7b2db7b5b7d27974L992: TemporaryProperty generates "" for T-SQL and so even though expressions is simply [""] in this section, we still output a redundant space separator.
I mentioned this in a comment above, and you can easily see that if we had more properties in the DDL we'd hit the same issue due to the sep.join(...) in the expressions method.
@dmoore247 this covers most of the tests you wrote, but I haven't done something about table vars yet. Could continue in a future PR or feel free to take a stab yourself too.
Also considered creating a couple of new expressions instead of adding new args in
Identifier, but the resulting code with this approach looked a bit cleaner I think. Don't feel strongly about it, so happy to pivot if needed.