Repository navigation
fix(sql): export the SingleStoreDB compiler module - #12164
Conversation
`ibis.to_sql` resolves a dialect name by attribute lookup on `ibis.backends.sql.compilers`, which never imported the `singlestoredb` module. `ibis.to_sql(expr, dialect="singlestoredb")` therefore raised "Unknown dialect" unless the backend had already been imported. Add a test that compiles a trivial table for every SQL backend name. Assisted-by: Claude Fable 5.1 <noreply@anthropic.com>
github-actions
Bot
added
tests
| # `ibis.to_sql` looks the dialect up on `ibis.backends.sql.compilers`, so | ||
| # every SQL backend's compiler module has to be imported there | ||
| expr = ibis.table({"a": "int64"}, name="t") | ||
| assert ibis.to_sql(expr, dialect=dialect) |
There was a problem hiding this comment.
@claude Why is this an assert? Is the idea that we're happy as long as some non-empty SQL is produced (and there's no error along the way)?
There was a problem hiding this comment.
yesh I think this should be a little more strict, eg even a very basicassert "from" in sql
NickCrews
left a comment
NickCrews
left a comment
There was a problem hiding this comment.
Looks pretty inoffensive and a move in the right direction! I don't really care that much about the comment thread and would be happy with pretty much anything.
| # `ibis.to_sql` looks the dialect up on `ibis.backends.sql.compilers`, so | ||
| # every SQL backend's compiler module has to be imported there | ||
| expr = ibis.table({"a": "int64"}, name="t") | ||
| assert ibis.to_sql(expr, dialect=dialect) |
There was a problem hiding this comment.
yesh I think this should be a little more strict, eg even a very basicassert "from" in sql
Sounds good; I'll just leave it rather than worrying about "from" capitalization or waiting for another round of CI. :) Also, since have another PR which vendored this fix. |
2 participants
Description of changes
ibis.to_sqllooks a dialect name up as an attribute ofibis.backends.sql.compilers, but that package never imported thesinglestoredbcompiler module. Soibis.to_sql(expr, dialect="singlestoredb")raisedValueError: Unknown dialect singlestoredbunless something had importedibis.backends.singlestoredbfirst. Our backend test jobs always import the backend, which is why CI never caught it.This imports the module and adds
SingleStoreDBCompilerto__all__, matching every other SQL backend. The new test compiles a trivial table for each SQL backend name, so a future backend that misses this list fails in the core test job.Found while moving compile-only tests around in #12153.
🤖 Generated with Claude Code