Skip to content

fix(insert): user can specify insert table in specified database #10285

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 11 commits into from
Oct 16, 2024
Merged

fix(insert): user can specify insert table in specified database #10285

merged 11 commits into from
Oct 16, 2024

Conversation

gforsyth
Copy link
Member

@gforsyth gforsyth commented Oct 8, 2024 •
edited by cpcloud
Loading

Description of changes

A user reported this issue on Zulip.

We weren't passing catalog.database information to get_schema inside
of _build_insert_from_table which caused inserts to fail if user had
specified a database argument, because we were looking in the wrong db
for the existing table to pull the schema from.

Resolves #10281.

@github-actions github-actions bot added tests Issues or PRs related to tests sql Backends that generate SQL labels Oct 8, 2024
@gforsyth
Copy link
Member Author

gforsyth commented Oct 8, 2024

Ah, looks like this was also reported in #10281 ?

Resolves #10281

@gforsyth gforsyth linked an issue Oct 8, 2024 that may be closed by this pull request
bug: mssql insert throw table not found error if specify schema and database #10281
Closed
1 task
@gforsyth gforsyth mentioned this pull request Oct 8, 2024
Closed
@cpcloud cpcloud force-pushed the fix_insert_other_db branch from 193e4cc to 894ab3d Compare October 9, 2024 10:05
@github-actions github-actions bot added the clickhouse The ClickHouse backend label Oct 9, 2024
@cpcloud cpcloud force-pushed the fix_insert_other_db branch from c92a4fa to 0b9ca54 Compare October 9, 2024 10:40
@cpcloud cpcloud added this to the 10.0 milestone Oct 9, 2024
@cpcloud cpcloud added the bug Incorrect behavior inside of ibis label Oct 9, 2024
@cpcloud cpcloud force-pushed the fix_insert_other_db branch 2 times, most recently from 619532f to e1c0e72 Compare October 9, 2024 12:23
@github-actions github-actions bot added the flink Issues or PRs related to Flink label Oct 9, 2024
@gforsyth
Copy link
Member Author

What do you think @cpcloud -- should we notyet the Flink backend for this one? I can't tell if Flink just doesn't like inserts at all, or if it's the cross-database insert in particular that's tripping it up.

@cpcloud
Copy link
Member

cpcloud commented Oct 16, 2024

I'll push up a commit to notyet it.

gforsyth and others added 11 commits October 16, 2024 10:43
@gforsyth @cpcloud
fix(insert): user can specify insert table in specified database
754a2b5 
A user reported this issue on Zulip.

We weren't passing `catalog.database` information to `get_schema` inside
of `_build_insert_from_table` which caused inserts to fail if user had
specified a `database` argument, because we were looking in the wrong db
for the existing table to pull the `schema` from.
@cpcloud
test(fixture): ensure that database clean up happens regardless of wh…
a77b670 
…at happens in the yielded-to function
@cpcloud
fix(clickhouse): remove completely bogus non-existent exception usage
e47b78a
@cpcloud
fixup! fix(clickhouse): remove completely bogus non-existent exceptio…
7113c97 
…n usage
@cpcloud
test: remove unnecessary default_backend monkeypatch
648a410
@cpcloud
test(clickhouse): add test for invalid catalog argument
78e16e4
@cpcloud
test(clickhouse): clean up unconditionally
a7a838a
@cpcloud
test(flink): always use temp=True for flink
aeeb42e
@cpcloud
test: do not use None for temp argument when testing insert
0a6a4d2
@cpcloud
chore: try using the entire table path for flink inserts
77a28f9
@cpcloud
test(flink): xfail on cross-database insert
8cc9d4f
@cpcloud cpcloud force-pushed the fix_insert_other_db branch from 606dccb to 8cc9d4f Compare October 16, 2024 14:46
@cpcloud
Copy link
Member

cpcloud commented Oct 16, 2024

Clouds are passing:

cloud in 🌐 falcon in …/ibis on  fix_insert_other_db is 📦 v9.5.0 via 🐍 v3.12.6 via ❄️  impure (ibis-3.12.6-env)
❯ pytest -m snowflake -n 8 --dist loadgroup -q && pytest -m bigquery -n auto --dist loadgroup -q
bringing up nodes...
s.s.....x..x...................x....x................xx..........x.xxxx..xxxxxxx.x.x......x...........x..x....s..........x..........................................x.........x........ [  9%]
...........x...................x.......x......x...xx.......x.............x...........x.........................x..........x.x.......x....x.....x........................x.x....x....... [ 18%]
xx..x.x.....x.........x..x.....................x..xx...x..x.........xx.x.x..x...x.x..x......xxx........x..x......x.x..x..........xxx........xx.....x..x......x................x........ [ 27%]
....xx....x...x............x.......x.x...x.......x.......xx.x.....xx..x...............x.x.......................x................x...x......................x.............x............ [ 36%]
............................................................x.......x..s......s.................................s...........s.......................................................... [ 45%]
......x...x.................................x..........xx.................................................x....................................x..x.......................x............ [ 54%]
...x...............x......................x.................x..x.....x.............x.................x.....xx...................................x......x..x..........x..x........x..... [ 63%]
.................x.........x.....x.......................x....................................x.x....x................x................................x....x...............x.......x.. [ 72%]
...x.....x..xx.x..x......x..xxxx...x....x.xxx.xx....x..x.xxx.xx..xx...xx...xx.x..x.xxx.xx.x.xx.x.x..x...xxxx..x.xxxxxxxx..xx.x....xxxx................................................. [ 81%]
.....................................s.................x.x.x....xxx.....x...................xxxxxxxxxxx........x......x................................................................ [ 90%]
....................................................s....s.....................................................................................................x....................... [ 99%]
.                                                                                                                                                                                       [100%]
1777 passed, 10 skipped, 227 xfailed in 256.00s (0:04:16)
bringing up nodes...
...........................xsssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss.........s................s.........x..sssssssssssssssss [  8%]
ssssx.x.s.........x......s.........xxx..s....x...x.........x.................x.xx.x...x...x......x..x...xxx.....x.x..............x..........xx.x.xx.x...x..x...x.....xx.......x..x....x [ 16%]
x..xxx..x...x...xx...xxx.x.xxxxxx.xxxx.xxx.xxxx.xxx.x.x.xxx..sxxxxxxxx.xxxxx.xxxxx.xxxxxxxxxx.xxxxxxx.xxxxxxxxxxxxx.xxxxxxxxxxxxxxxxxxxxx..xx.xxxxxxxxxxxxx.x...x.xxx.x.xx.x.xxx.xx..xx [ 24%]
X.....x...x..xx.x..X...x...xx....xx.....x..xxx....x.xx....x...x....x..x..x..............x.....Xx...x.xxx..x.........x.......x............xx.xx.X......x...x.x........................sx [ 32%]
..............x..x..x................................x..................x...............................x...............x.xxxx...x..xx..xx...xxx..xx.x.x............................... [ 40%]
x........x.......x..x..........x.............x....xxx....................x...........x.x........x.............xx.xxx...x..........x.....x........x....x...................x.....x.....x [ 48%]
x..........x....................x.......x...............x.....................x.......x..............x.....x.........x......x..x...x..x.....x.......s..............x...x.xx...........x [ 57%]
.........x..x.xx.......................x....................x.......x..............................................................x................................................... [ 65%]
.......x....................x..x.x..x.xx..x.xx.x.x..x.............x....x.....x..................x...xx..xx.x....x........x.x.x..x....x..xx.xx..x...x.x..........x............x..x...x.x [ 73%]
..x..x.......x..............x.x..xx...x....x.....................x........x..x....x...........x....x.x....x...........xx............................................................... [ 81%]
................................................................x....s...........s...s......s.......................x.................................................................. [ 89%]
....x........................................................................................................................................................................x......... [ 97%]
..............................................                                                                                                                                          [100%]
1751 passed, 132 skipped, 355 xfailed, 4 xpassed in 252.02s (0:04:12)

@cpcloud cpcloud merged commit 0e848d0 into ibis-project:main Oct 16, 2024
77 checks passed
@gforsyth gforsyth deleted the fix_insert_other_db branch October 16, 2024 20:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cpcloud cpcloud cpcloud approved these changes

Assignees
No one assigned
Labels
bug Incorrect behavior inside of ibis clickhouse The ClickHouse backend flink Issues or PRs related to Flink sql Backends that generate SQL tests Issues or PRs related to tests
Projects
None yet
Milestone
10.0
Development

Successfully merging this pull request may close these issues.

bug: mssql insert throw table not found error if specify schema and database
2 participants
@gforsyth @cpcloud