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

Implement chdb v3.0 Connect method #16

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

agoncear-mwb
Copy link
Contributor

This pr implements the new statefull session created in the v3.0.0 release.

For backward compatibility, at the moment, i kept the old session mechanism still avaiable, but i guess in future release we can remove it in favor of the new ChdbConnection which is equivalent in methods but handle the connection with the underlying clickhouse local in a better way.

@auxten
Copy link
Member

auxten commented Jan 30, 2025

The old impl of query and the new one can not co-exist in one process.
As the new one need to keep some global stuff, but the old one tend to cleanup everything on finished.

@auxten
Copy link
Member

auxten commented Feb 3, 2025

@agoncear-mwb are you still working on this?

We should use the Connection base queryConn here instead.
And the Query should also use the the Connection base queryConn
See also: https://github.com/chdb-io/chdb/blob/d26d2ce84190e9f9e2dbbde024ac09e9739198eb/chdb/__init__.py#L75

@agoncear-mwb
Copy link
Contributor Author

ok, so we should remove the old one?
I'll have a double check today, and push the mods ASAP

@auxten
Copy link
Member

auxten commented Feb 3, 2025

We need to try our best to keep the user side API stable, so the plan is:

@agoncear-mwb
Copy link
Contributor Author

Perfect, i'll try to keep the implementation as close as possible with the python binding

@agoncear-mwb
Copy link
Contributor Author

@auxten the mac build has never started, if you could please manually re-run it we can proceed. Thanks

@auxten
Copy link
Member

auxten commented Feb 10, 2025

It seems there is no macos-12 runner. I tried to use macos-15, but it seems the -ldflags '-extldflags "-Wl,-rpath,/usr/local/lib"' not work here #17
@agoncear-mwb Do you have any clue for the bloody macos arm64 linking issue?

@agoncear-mwb
Copy link
Contributor Author

Maybe is due to this? StackOverflow

Looking at the logs, it seems that it ignored the directive of where to search for the lib.

@auxten
Copy link
Member

auxten commented Feb 12, 2025

Tried several ways to fix the dyn lib searching path. Only DYLD_LIBRARY_PATH=/usr/local/lib works, but it's too dirty. Some one impl a new chdb binding without cgo, How do you think? https://discord.com/channels/1098133460310294528/1184432370431049809/1338879169198817341

@agoncear-mwb
Copy link
Contributor Author

We can give it a try. Do you want to create the base yourself or should i go and try to create a new pr?

@auxten
Copy link
Member

auxten commented Feb 12, 2025

We can give it a try. Do you want to create the base yourself or should i go and try to create a new pr?

Please go with a new PR😀

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.

2 participants