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

feat: add support for bigquery in core #10447

Merged
merged 4 commits into from
Feb 3, 2025
Merged

Conversation

fontanierh
Copy link
Contributor

@fontanierh fontanierh commented Jan 31, 2025

Description

Adds support for BigQuery remote databases in core.

The implementation supports all required features for a remote DB:

  • ✅ Query execution
  • ✅ Query "greenlighting" (i.e, check whether the query only uses allowed tables & doesn't mutate any data)
  • ✅ Obtain a TableSchema for a list of tables
  • ✅ Rows streaming and error if we go over a certain result set size

What is not covered here:

  • ❌ Proper distinct error types based on error case (generic errors eveywhere

Focused on merging quickly so we can test end to end and debug

Tests

Manual

Risk

N/A (pretty much no change to non-bigquery flow)

Deploy Plan

Deploy core

@fontanierh fontanierh force-pushed the feat/core-support-bigquery branch from 8fa9d6d to 4ee69bc Compare February 3, 2025 15:23
@fontanierh fontanierh changed the title WIP feat: add support for bigquery in core feat: add support for bigquery in core Feb 3, 2025

query_result_size = query_result_size
+ res
.total_bytes_processed
Copy link
Contributor

Choose a reason for hiding this comment

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

I think, this is the number of bytes read from bigquery data, not necessary the size of the payload, is it really what you want ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah interesting, not at all what I want and I think you might be right. Going to replace with the same approach we use for SF

Copy link
Contributor Author

Choose a reason for hiding this comment

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

reverting to nb of rows for now

Copy link
Contributor

@Fraggle Fraggle left a comment

Choose a reason for hiding this comment

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

LGTM, double check the bytes thing

@fontanierh fontanierh merged commit 49e56ac into main Feb 3, 2025
5 checks passed
@fontanierh fontanierh deleted the feat/core-support-bigquery branch February 3, 2025 17:15
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.

2 participants