Skip to content
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

fix(DBAPI): keep cursor hot when statement is a script #386

Closed
wants to merge 1 commit into from
Closed

fix(DBAPI): keep cursor hot when statement is a script #386

wants to merge 1 commit into from

Conversation

mistercrunch
Copy link
Contributor

@mistercrunch mistercrunch commented Nov 12, 2020 •
edited

all the details here: #377

I guess technically a script isn't DML :trollface:

@product-auto-label product-auto-label bot added the api: bigquery Issues related to the googleapis/python-bigquery API. label Nov 12, 2020
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Nov 12, 2020
@mistercrunch mistercrunch mentioned this pull request Nov 12, 2020
Closed
@tswast
Copy link
Contributor

tswast commented Nov 12, 2020

Makes sense to me.

Actually, we can probably consider removing the is_dml logic altogether now that we moved to getQueryResults instead of tabledata.list (#375). The destination table of a DML query would return a non-null value that was unreadable or would return nonsense with DML (and some DDL) queries. Before we do that, I'll want to double-check that we have a system test for this case. (I think a DDL to CREATE VIEW would be a good choice for that)

@mistercrunch
Copy link
Contributor Author

Superseded by #387

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
api: bigquery Issues related to the googleapis/python-bigquery API. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@mistercrunch @tswast