-
Notifications
You must be signed in to change notification settings - Fork 763
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
feat(hive) add support to query simple hive table #5895
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Thanks for the contribution! Please review the labels and make any necessary changes. |
@mergify update |
✅ Branch has been successfully updated |
Hi @sandflee, really appreciate your contribution!!
It will be better if we put them "into" the Tablemeta::engine_options" since they are hive table-specific options, the
|
@dantengsky , thanks for you advice!, |
Hi @sandflee That's great to see you are working on the hive. Thank you! |
yes, add sql tests is very useful and necessary, I try some methodes to work with docker env but failed in my local machine, the main reason is : |
@sandflee Roger please feel free to modify the docker script at https://github.com/datafuselabs/databend/tree/main/docker/it-hive if necessary. e.g. exports the necessary ports in the script: https://github.com/datafuselabs/databend/blob/main/docker/it-hive/hive-docker-compose.yml. but it looks like that the ports of datanodes and namenodes are accessible (I might be completely wrong since I do not know exactly the setup of your local dev):
And as you might already know, the |
thanks @dantengsky , the port of my local hdfs conflicts with hdfs in docker, :( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
cc @Xuanwo We have an issue here in the CI setup of hdfs storage, would you please give us some hints?
https://github.com/datafuselabs/databend/runs/6892054183?check_suite_focus=true#step:3:1109 |
We need to set up java runtime to make them work like https://github.com/datafuselabs/opendal/blob/main/.github/workflows/service_test_hdfs.yml#L20-L28 But I don't know if this action works under our build tool. Ping @everpcpc for a look. |
I added java env,but seems JAVA_HOME is not set。 3b3fe5f |
https://github.com/datafuselabs/databend/blob/main/scripts/setup/dev_setup.sh#L491 |
It seems working: https://github.com/datafuselabs/databend/runs/6892054183?check_suite_focus=true#step:3:118
will the above action also set the |
@mergify update |
✅ Branch has been successfully updated |
@mergify update |
✅ Branch has been successfully updated |
@mergify update |
✅ Branch has been successfully updated |
Hi @sandflee, docker image for building has been updated, the but we still have to tweak the The basic idea is, in the stateful hive test environment, we need to set up a JDK, and export the necessary env vars, so that And I think, we only need to install JDK, and export LD_LIBRARY_PATH properly at this stage (later, if files are accessed by hdfs protocol, some extra jars and env var should be installed/exported). thus, my suggestions are :
A reference from opendal I've not tested the above idea, not sure if it works; but if it seems reasonable to you, would you please have a try? @Xuanwo in this PR,
is it enough that we just install the JDK and setup the LD_LIBRARY_PATH properly, to bring up |
Yep. |
Sorry for the late, I have approved this workflow run. |
thanks, @dantengsky @Xuanwo , databend could startup now, but failed to query hive data, for the storage is "fs" not "hdfs". we may run though test_stateful_hive_standalone by changing the storage. |
@sandflee Seems the build and run-time dependencies issues are resolved. Failure of
|
ok, remove storage-hdfs depends in latest code |
using a track way to run the hive ci, by replacing hdfs storage with local storage |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work!
format_path | ||
} | ||
|
||
#[cfg(test)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Databend unit test style is not following the Rust in the same file, we separate them to a single folder tests
.
We can move theis unit test to tests/it/catalogs
next time, like https://github.com/datafuselabs/databend/tree/main/query/tests/it/catalogs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, I'll move the test code to tests in another PR.
I hereby agree to the terms of the CLA available at: https://databend.rs/dev/policies/cla/
Summary
add support to query simple hive table
Changelog
Related Issues
#4826