Skip to content

Commit

Permalink
fix(duckdb): fix create_table() in databases with spaces in the name (#…
Browse files Browse the repository at this point in the history
…9817)

If you tried

```python
import ibis

conn = ibis.duckdb.connect("with space.duckdb")
conn.create_table("foo", {"a": [0,1,2]})
```

then you would get the DDL `CREATE TABLE with space.main."foo" ("a"
BIGINT)`. Now, the database name is quoted.

I went through the create_table() method in all the backends and looked
for where we forgot to use "quoted", but I almost definitely missed some
cases.

I need to add a test for this, but I would love your help with this:
- should it add/modify a create_table() test to check for a database
with spaces in the name?
- should it go somewhere near other tests that check for spaces? I think
we have those?
- or should we go larger, and modify the `con` fixture to sometimes be a
database with spaces in the name?
  • Loading branch information
NickCrews authored Aug 26, 2024
1 parent a402095 commit 9da3c9f
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 9 deletions.
2 changes: 1 addition & 1 deletion ibis/backends/clickhouse/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -673,7 +673,7 @@ def create_table(
obj = ibis.memtable(obj, schema=schema)

this = sge.Schema(
this=sg.table(name, db=database),
this=sg.table(name, db=database, quoted=self.compiler.quoted),
expressions=[
sge.ColumnDef(
this=sg.to_identifier(name, quoted=self.compiler.quoted),
Expand Down
12 changes: 4 additions & 8 deletions ibis/backends/duckdb/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -191,10 +191,8 @@ def create_table(
else:
temp_name = name

initial_table = sge.Table(
this=sg.to_identifier(temp_name, quoted=self.compiler.quoted),
catalog=catalog,
db=database,
initial_table = sg.table(
temp_name, catalog=catalog, db=database, quoted=self.compiler.quoted
)
target = sge.Schema(this=initial_table, expressions=column_defs)

Expand All @@ -205,10 +203,8 @@ def create_table(
)

# This is the same table as initial_table unless overwrite == True
final_table = sge.Table(
this=sg.to_identifier(name, quoted=self.compiler.quoted),
catalog=catalog,
db=database,
final_table = sg.table(
name, catalog=catalog, db=database, quoted=self.compiler.quoted
)
with self._safe_raw_sql(create_stmt) as cur:
if query is not None:
Expand Down
14 changes: 14 additions & 0 deletions ibis/backends/duckdb/tests/test_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,20 @@ def test_connect_named_in_memory_db():
assert "ork" not in default_memory_db.list_tables()


@pytest.mark.parametrize(
"database_file",
[
"with spaces.ddb",
"space catalog.duckdb.db",
],
)
def test_create_table_quoting(database_file, tmp_path):
conn = ibis.duckdb.connect(tmp_path / database_file)
t = conn.create_table("t", {"a": [0, 1, 2]})
result = set(conn.execute(t.a))
assert result == {0, 1, 2}


@pytest.mark.parametrize(
("url", "method_name"),
[
Expand Down

0 comments on commit 9da3c9f

Please sign in to comment.