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.

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

Make DBAPI implementation use session, fix execute for none-returning queries #169

Merged
merged 3 commits into from
Dec 22, 2023

Conversation

yunyu
Copy link
Contributor

@yunyu yunyu commented Dec 21, 2023

Fixes #163 #162

Changelog category (leave one):

  • Bug Fix (user-visible misbehavior in an official stable release)

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):

Make dbapi implementation use session, which is stateful (most consumers of dbapi likely want statefulness). Also fix execute to not throw an exception for None-returning queries like CREATE TABLE.

Documentation entry for user-facing changes

  • Documentation is written (mandatory for new features)

@CLAassistant
Copy link

CLAassistant commented Dec 21, 2023

CLA assistant check
All committers have signed the CLA.

lmangani
lmangani previously approved these changes Dec 21, 2023
@yunyu
Copy link
Contributor Author

yunyu commented Dec 21, 2023

Hmm, I signed the CLA but the comment isn't updating:
image

Also not sure why the bot is saying "Yunyu Lin seems not to be a GitHub user"

@lmangani
Copy link
Contributor

@yunyu this usually means the commits within the PR were pushed to your branch with a different user (or no user)

@yunyu
Copy link
Contributor Author

yunyu commented Dec 21, 2023

Got it, CLA fixed!

@yunyu
Copy link
Contributor Author

yunyu commented Dec 21, 2023

Btw @lmangani do you know why the test fails with ENGINE = Memory (which should persist data, given it's the same session?) but not with ENGINE = Log?

@lmangani
Copy link
Contributor

Btw @lmangani do you know why the test fails with ENGINE = Memory (which should persist data, given it's the same session?) but not with ENGINE = Log?

Sessions are files. My guess the Memory engine bypasses the session persistence entirely. @auxten can confirm best.

@auxten
Copy link
Member

auxten commented Dec 22, 2023

@lmangani is right. chDB session relies on persistence of database storage files.
But, supporting session on ENGINE = Memory is reasonable. I will find a way to support that.

@auxten auxten requested a review from laodouya December 22, 2023 02:54
Copy link
Contributor

@laodouya laodouya left a comment

Choose a reason for hiding this comment

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

LGTM

@auxten auxten merged commit 46436c4 into chdb-io:main Dec 22, 2023
1 check passed
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DBAPI implementation loses all Clickhouse state on every query
5 participants