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

[Feature] Hive Source/Sink support multiple table #5929

Merged
merged 2 commits into from
Apr 30, 2024

Conversation

ruanwenjun
Copy link
Member

Purpose of this pull request

  • Use Factory API to create Hive Source/Sink
  • Hive Source/Sink support multiple table

Does this PR introduce any user-facing change?

How was this patch tested?

Check list

@ruanwenjun ruanwenjun force-pushed the dev_wenjun_hiveSupportMultipleTable branch 4 times, most recently from c5b18f7 to b88017e Compare November 28, 2023 14:09
@ruanwenjun ruanwenjun force-pushed the dev_wenjun_hiveSupportMultipleTable branch from b88017e to a3aa9c8 Compare December 1, 2023 05:40
@ruanwenjun ruanwenjun added the feature New feature label Dec 1, 2023
```bash

Hive {
tables_configs = [
Copy link
Contributor

Choose a reason for hiding this comment

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

A small suggestion is that this is better named using table_list, maximizing uniform parameter naming There should be a unified specification for the naming of table granularity to better reduce the difficulty of user use

Copy link
Member Author

@ruanwenjun ruanwenjun Apr 8, 2024

Choose a reason for hiding this comment

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

Use tables_configs is consistent with other file connectors. We need to change tables_configs to table_list in another PR. I create a new PR for this #6659

@hailin0 hailin0 added the no update The owner doesn't provide further feedback. label Dec 19, 2023
@EricJoy2048
Copy link
Member

Hi, @ruanwenjun Is there any new progress in this PR?

@ruanwenjun ruanwenjun force-pushed the dev_wenjun_hiveSupportMultipleTable branch from a3aa9c8 to 24da46a Compare April 8, 2024 02:39
@ruanwenjun ruanwenjun removed the no update The owner doesn't provide further feedback. label Apr 8, 2024
@ruanwenjun ruanwenjun self-assigned this Apr 8, 2024
@ruanwenjun ruanwenjun force-pushed the dev_wenjun_hiveSupportMultipleTable branch from 24da46a to 0b9b4c5 Compare April 9, 2024 01:55
@ruanwenjun
Copy link
Member Author

ruanwenjun commented Apr 9, 2024

Hi, @ruanwenjun Is there any new progress in this PR?

I have resolved the conflicts.

@ruanwenjun ruanwenjun force-pushed the dev_wenjun_hiveSupportMultipleTable branch 5 times, most recently from 1f55316 to 35eb5e3 Compare April 9, 2024 06:02
@hailin0
Copy link
Contributor

hailin0 commented Apr 9, 2024

Please add test case

@ruanwenjun ruanwenjun force-pushed the dev_wenjun_hiveSupportMultipleTable branch 4 times, most recently from 557ba63 to f65d86d Compare April 10, 2024 09:20
@@ -25,23 +25,30 @@ env {

source {
# This is a example source plugin **only for test and demonstrate the feature source plugin**
FakeSource {
Hive {
Copy link
Contributor

Choose a reason for hiding this comment

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

revert this file and add e2e

@ruanwenjun ruanwenjun force-pushed the dev_wenjun_hiveSupportMultipleTable branch from f65d86d to 4ac29af Compare April 16, 2024 08:38
@ruanwenjun ruanwenjun force-pushed the dev_wenjun_hiveSupportMultipleTable branch from 4ac29af to 8d8ced0 Compare April 28, 2024 04:30
@hailin0
Copy link
Contributor

hailin0 commented Apr 30, 2024

We can add test cases later
cc @EricJoy2048 @Hisoka-X

@TyrantLucifer
Copy link
Member

Could you please offer some snapshots to verify it worked? For new hive sink/source has no e2e cases, so I think offer some screenshots would be better.

I remember before, the community was formulating relevant regulations. If the merged code can write e2e, e2e should be written. If it cannot be written, relevant information should be provided to explain why it cannot be written. At the same time, some evidence should be provided to prove that this PR is effective and can be used in production.

cc @EricJoy2048 @hailin0 @Hisoka-X

@wwd-wwp
Copy link

wwd-wwp commented Apr 30, 2024

I have successfully applied this solution and the configuration file is as follows
image

After a period of operation, it runs stably
The table data information is as follows
table one
image
table two
image
I have also tried PG and Mongodb as data sources and have been successful

@hailin0
Copy link
Contributor

hailin0 commented Apr 30, 2024

I have successfully applied this solution and the configuration file is as follows image

After a period of operation, it runs stably The table data information is as follows table one image table two image I have also tried PG and Mongodb as data sources and have been successful

Thanks, LGTM
cc @TyrantLucifer

@hailin0 hailin0 merged commit 4d9287f into apache:dev Apr 30, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants