Skip to content

[Databricks] Now processing queries in batches in DatabricksDestinationHandler.kt - #63311

Merged
Francis Genet (frifriSF59) merged 4 commits into
masterfrom
frifri/databricks-std
Jul 15, 2025
Merged

[Databricks] Now processing queries in batches in DatabricksDestinationHandler.kt#63311
Francis Genet (frifriSF59) merged 4 commits into
masterfrom
frifri/databricks-std

Conversation

@frifriSF59

Copy link
Copy Markdown
Contributor

What

Since we are hitting an issue because we can only pass 256 parameters to a query, we need to process the streams in batches to avoid hitting that limit

Can this PR be safely reverted and rolled back?

  • YES 💚
  • NO ❌

@vercel

vercel Bot commented Jul 15, 2025

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
airbyte-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 15, 2025 7:00pm

@github-actions

Copy link
Copy Markdown
Contributor

👋 Greetings, Contributor!

Here are some helpful tips and reminders for your convenience.

Helpful Resources

PR Slash Commands

Airbyte Maintainers (that's you!) can execute the following slash commands on your PR:

  • /format-fix - Fixes most formatting issues.
  • /bump-version - Bumps connector versions.
    • You can specify a custom changelog by passing changelog. Example: /bump-version changelog="My cool update"
    • Leaving the changelog arg blank will auto-populate the changelog from the PR title.
  • /run-cat-tests - Runs legacy CAT tests (Connector Acceptance Tests)
  • /build-connector-images - Builds and publishes a pre-release docker image for the modified connector(s).

📝 Edit this welcome message.

@edgao Edward Gao (edgao) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

maybe worth testing - in AbstractDatabricksTypingDedupingTest, add override val manyStreamCount = 200 + run the testManyStreamsCompletion test case. Probably not worth actually committing that to git, but would at least verify that there isn't some other bug downstream from here.

(new CDK has an equivalent to this, but databricks is still on the old CDK)

Comment thread docs/integrations/destinations/databricks.md Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should we add a test case here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

There is already a test that runs a lot o stream (20 atm) I will be running a test locally with 300 streams, but as Ed mentioned, it is probably not a good idea to put that change in the repo

@github-actions

github-actions Bot commented Jul 15, 2025

Copy link
Copy Markdown
Contributor

destination-databricks Connector Test Results

  6 files   6 suites   30m 36s ⏱️
 82 tests 65 ✅ 17 💤 0 ❌
106 runs  89 ✅ 17 💤 0 ❌

Results for commit bb30fc5.

♻️ This comment has been updated with latest results.

@frifriSF59
Francis Genet (frifriSF59) merged commit 553a19e into master Jul 15, 2025
@frifriSF59
Francis Genet (frifriSF59) deleted the frifri/databricks-std branch July 15, 2025 20:44
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.

4 participants