Skip to content
This repository was archived by the owner on Mar 31, 2026. It is now read-only.

fix: support INSERT from SELECT clause with args#306

Merged
larkee merged 3 commits intogoogleapis:masterfrom
MaxxleLLC:insert_from_select
Apr 15, 2021
Merged

fix: support INSERT from SELECT clause with args#306
larkee merged 3 commits intogoogleapis:masterfrom
MaxxleLLC:insert_from_select

Conversation

@IlyaFaer
Copy link
Copy Markdown

Spanner supports doing INSERT with values takes from a SELECT clause. However, DB API is processing such a cases incorrectly, so such a query, for example:

INSERT INTO manual_pk (id, data)
SELECT manual_pk.id + %s AS anon_1, manual_pk.data
FROM manual_pk
WHERE manual_pk.data IN (%s, %s)

Will fail with a syntax error:

400 Syntax error: Illegal input character \"%\" [at 3:23]\nSELECT manual_pk.id + %s AS anon_1, manual_pk.data\n

Fixing this bug.

@IlyaFaer IlyaFaer added api: spanner Issues related to the googleapis/python-spanner API. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. labels Apr 12, 2021
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Apr 12, 2021
# Case b)
insert_sql = sanitize_literals_for_upload(insert_sql)
return {"sql_params_list": [(insert_sql, None)]}
return {"sql_params_list": [(insert_sql, params)]}
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like the previous author of the DB API didn't notice that INSERT from SELECT will be a case b), and it can include some args. Thus, passing None instead of args is not correct.

@IlyaFaer IlyaFaer marked this pull request as ready for review April 12, 2021 08:42
@IlyaFaer IlyaFaer requested review from a team and larkee April 12, 2021 08:42
Copy link
Copy Markdown
Contributor

@larkee larkee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Does the docstring need updating to reflect this change?

@larkee larkee merged commit 0dcda5e into googleapis:master Apr 15, 2021
@IlyaFaer IlyaFaer deleted the insert_from_select branch April 15, 2021 07:14
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

api: spanner Issues related to the googleapis/python-spanner API. cla: yes This human has signed the Contributor License Agreement. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants