Skip to content

fix: Support multiple tables#1037

Open
hhasija wants to merge 11 commits intomainfrom
support_multiple_tables
Open

fix: Support multiple tables#1037
hhasija wants to merge 11 commits intomainfrom
support_multiple_tables

Conversation

@hhasija
Copy link
Collaborator

@hhasija hhasija commented Mar 31, 2025

No description provided.

@hhasija hhasija changed the title fix : Support multiple tables fix: Support multiple tables Mar 31, 2025
vanshaj-bhatia
vanshaj-bhatia previously approved these changes Apr 2, 2025
Copy link
Collaborator

@vanshaj-bhatia vanshaj-bhatia left a comment

Choose a reason for hiding this comment

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

LGTM!

Comment on lines +190 to +191
source_tables_list = input_jdbc_table.split(";")
target_tables_list = big_query_table.split(";")
Copy link
Collaborator

Choose a reason for hiding this comment

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

instead of list can we accept dictionary for source:target.
Keep these 2 variables same to avoid breaking change but add another variable which accepts a key:value of source:target mapping so users do not have to worry about order

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Actually @surjits254 if we would do that, then we need to change our older pattern of accepting the input parameters. Below mentioned are the two older parameters :-

--jdbc.bigquery.input.table='test.demo'
--jdbc.bigquery.output.table='jdbctobq'

Thus, without changing any of the older support we need to come up with some solution. Hence, figured out this way. Do you think so we should change the older format to dictionary ?

@hhasija hhasija requested a review from surjits254 April 4, 2025 13:03
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.

4 participants

Comments