Skip to content

Support query pass-through for BigQuery connector#12502

Merged
ebyhr merged 3 commits intomasterfrom
ebi/bigquery-query-pass-through
Aug 19, 2022
Merged

Support query pass-through for BigQuery connector#12502
ebyhr merged 3 commits intomasterfrom
ebi/bigquery-query-pass-through

Conversation

@ebyhr
Copy link
Copy Markdown
Member

@ebyhr ebyhr commented May 21, 2022

Description

Support query pass-through for BigQuery connector

Documentation

( ) No documentation is needed.
( ) Sufficient documentation is included in this PR.
( ) Documentation PR is available with #prnumber.
(x) Documentation issue #issuenumber is filed, and can be handled later.

Release notes

( ) No release notes entries required.
(x) Release notes entries required with the following suggested text:

# BigQuery
* Support query pass-through table function. ({issue}`12502`)

@cla-bot cla-bot bot added the cla-signed label May 21, 2022
@ebyhr ebyhr force-pushed the ebi/bigquery-query-pass-through branch 4 times, most recently from c60941a to 13654e7 Compare May 23, 2022 02:39
@ebyhr ebyhr marked this pull request as ready for review May 23, 2022 03:29
@ebyhr ebyhr force-pushed the ebi/bigquery-query-pass-through branch from 13654e7 to f61a341 Compare May 23, 2022 07:48
@mosabua
Copy link
Copy Markdown
Member

mosabua commented Jun 2, 2022

This will need docs .. @colebow can you maybe create a PR with the help of @ebyhr for technical input. Or at least figure out whats up. To some degree I think this does NOT yet need docs since its is related to table functions exposure and if we dont have any yet in the connector .. then this is just plumbing without user facing impact. @kasiafi might also be able to clarify

@mosabua mosabua added the needs-docs This pull request requires changes to the documentation label Jun 2, 2022
@kasiafi
Copy link
Copy Markdown
Member

kasiafi commented Jun 2, 2022

then this is just plumbing without user facing impact.

@mosabua @colebow This is a concrete Table Function that will be exposed by the BigQuery connector, so it definitely needs to be documented.

@mosabua
Copy link
Copy Markdown
Member

mosabua commented Jun 2, 2022

Thanks for confirming @kasiafi !

@mosabua
Copy link
Copy Markdown
Member

mosabua commented Jun 2, 2022

fyi @brianzhan

@ebyhr ebyhr force-pushed the ebi/bigquery-query-pass-through branch 2 times, most recently from 5ba6aa5 to 47fe830 Compare June 6, 2022 00:30
@ebyhr ebyhr requested review from findepi, hashhar and kasiafi June 6, 2022 01:27
@findepi findepi requested review from wendigo and removed request for findepi June 6, 2022 15:13
@ebyhr
Copy link
Copy Markdown
Member Author

ebyhr commented Jun 9, 2022

Removing syntax-needs-review label since JDBC connectors' PR was already merged.

@ebyhr ebyhr force-pushed the ebi/bigquery-query-pass-through branch from 47fe830 to 862e966 Compare June 25, 2022 02:27
@ebyhr ebyhr removed the needs-docs This pull request requires changes to the documentation label Jul 4, 2022
@ebyhr ebyhr force-pushed the ebi/bigquery-query-pass-through branch from 862e966 to 89744d0 Compare July 4, 2022 09:05
@github-actions github-actions bot added the docs label Jul 4, 2022
@ebyhr
Copy link
Copy Markdown
Member Author

ebyhr commented Jul 4, 2022

Updated documentation based on MySQL connector. Ready for review now.

Copy link
Copy Markdown
Member

@mosabua mosabua left a comment

Choose a reason for hiding this comment

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

I only looked at the docs .. they are good to go.

@ebyhr ebyhr force-pushed the ebi/bigquery-query-pass-through branch 2 times, most recently from 2b8704b to 80d95a0 Compare July 21, 2022 12:27
@ebyhr
Copy link
Copy Markdown
Member Author

ebyhr commented Jul 21, 2022

@hashhar @wendigo Gentle reminder.

Copy link
Copy Markdown
Member

@hashhar hashhar left a comment

Choose a reason for hiding this comment

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

Looks good to me however I would appreciate if @kasiafi skims as well.

The tests look good candidates for BCT now that we have query PTF in both JDBC and non-JDBC connectors - I'll see if I can come up with something.

Copy link
Copy Markdown
Contributor

@wendigo wendigo left a comment

Choose a reason for hiding this comment

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

Overall design looks good to me.

@ebyhr ebyhr force-pushed the ebi/bigquery-query-pass-through branch from 80d95a0 to c35a256 Compare August 17, 2022 08:43
@ebyhr
Copy link
Copy Markdown
Member Author

ebyhr commented Aug 18, 2022

Let me merge this PR if there's no additional comments until tomorrow.

@ebyhr ebyhr merged commit a52a964 into master Aug 19, 2022
@ebyhr ebyhr deleted the ebi/bigquery-query-pass-through branch August 19, 2022 22:33
@ebyhr ebyhr mentioned this pull request Aug 19, 2022
@github-actions github-actions bot added this to the 394 milestone Aug 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

5 participants