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

refactor: expose built-in engines to sqllogictest-engines crate #154

Merged
merged 7 commits into from
Jan 20, 2023

Conversation

BugenZhao
Copy link
Collaborator

@BugenZhao BugenZhao commented Jan 18, 2023

In this PR, we add a new workspace member of sqllogictest-engine, which contains pre-built engines like PostgresSimple, PostgresExtended, and ExternalDriver. Developers can directly depend on the sqllogictest-engine crate to utilize them.

Signed-off-by: Bugen Zhao <i@bugenzhao.com>
Signed-off-by: Bugen Zhao <i@bugenzhao.com>
Signed-off-by: Bugen Zhao <i@bugenzhao.com>
Signed-off-by: Bugen Zhao <i@bugenzhao.com>
Signed-off-by: Bugen Zhao <i@bugenzhao.com>
@BugenZhao
Copy link
Collaborator Author

cc @xxchan @wangrunji0408 @skyzh PTAL. 🥰

BTW, should I do something else for adding a new crate?

@BugenZhao
Copy link
Collaborator Author

It seems the file postgres_extended_test.slt is unused... 🤔

@@ -0,0 +1,33 @@
[package]
name = "sqllogictest-engine"
version = { workspace = true }
Copy link
Member

Choose a reason for hiding this comment

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

We can begin to use separated versions? 🤪

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm not sure, but it seems not bad for using a unified version 🤣 which is less ambiguous for users.

serde_json = "1"
sqllogictest = { path = "../sqllogictest", version = "0.11" }
thiserror = "1"
tokio = { version = "1", features = [
Copy link
Member

Choose a reason for hiding this comment

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

Do we have to depend on tokio 🤔

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm struggling with this too. Depending on tokio means that this cannot be directly used in madsim. 🥺 We need to fork it and replace tokio with madsim-tokio.

sqllogictest-engine/Cargo.toml Outdated Show resolved Hide resolved
sqllogictest-engine/Cargo.toml Outdated Show resolved Hide resolved
Signed-off-by: Bugen Zhao <i@bugenzhao.com>
@BugenZhao BugenZhao changed the title refactor: expose pre-built engines to sqllogictest-engine crate refactor: expose built-in engines to sqllogictest-engine crate Jan 19, 2023
@BugenZhao BugenZhao changed the title refactor: expose built-in engines to sqllogictest-engine crate refactor: expose built-in engines to sqllogictest-engines crate Jan 19, 2023
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.

consider exposing pre-built engines public
4 participants