Skip to content

GH-50219: [R] Fix duckdb test for dbplyr 2.6.0#50220

Merged
raulcd merged 2 commits into
apache:mainfrom
thisisnic:GH-50219-newdbplyr
Jun 22, 2026
Merged

GH-50219: [R] Fix duckdb test for dbplyr 2.6.0#50220
raulcd merged 2 commits into
apache:mainfrom
thisisnic:GH-50219-newdbplyr

Conversation

@thisisnic

@thisisnic thisisnic commented Jun 18, 2026

Copy link
Copy Markdown
Member

Rationale for this change

dbplyr 2.6.0 changed remote_con() to read from x$con instead of x$src$con. The duckdb test that simulates a non-DuckDB connection was overriding the old path, so the expected error was no longer thrown.

What changes are included in this PR?

Update the test to override ds_rt$con instead of ds_rt$src$con.

Are these changes tested?

This is a test fix — the test itself verifies the behavior.

Are there any user-facing changes?

No.

@thisisnic thisisnic requested a review from jonkeane as a code owner June 18, 2026 14:27
Copilot AI review requested due to automatic review settings June 18, 2026 14:27
@thisisnic

Copy link
Copy Markdown
Member Author

@github-actions crossbow submit test-r-depsource-system test-r-gcc-11 test-r-gcc-12 test-r-m1-san test-r-offline-maximal test-r-versions

Copilot AI 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.

Pull request overview

Fixes an R test failure caused by dbplyr 2.6.0 changing how remote_con() retrieves the connection from a tbl.

Changes:

  • Update the DuckDB integration test to override ds_rt$con (instead of ds_rt$src$con) so the simulated “non-DuckDB connection” path still triggers the expected to_arrow() error with dbplyr 2.6.0.

@github-actions

Copy link
Copy Markdown

Revision: a468619

Submitted crossbow builds: ursacomputing/crossbow @ actions-74af3d833c

Task Status
test-r-depsource-system GitHub Actions
test-r-gcc-11 GitHub Actions
test-r-gcc-12 GitHub Actions
test-r-m1-san GitHub Actions
test-r-offline-maximal GitHub Actions
test-r-versions GitHub Actions

@thisisnic thisisnic marked this pull request as draft June 18, 2026 15:05
# Now check errors
# dbplyr 2.6.0 added "con" to the allowed $ fields on tbl_lazy;
# older versions only allow "src" and "lazy_query"
skip_if(packageVersion("dbplyr") < "2.6.0")

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Needs skipping or fails on CI with cached dbplyr with old syntax

@thisisnic thisisnic marked this pull request as ready for review June 18, 2026 16:42
Copilot AI review requested due to automatic review settings June 18, 2026 16:42
@github-actions github-actions Bot added awaiting review Awaiting review awaiting changes Awaiting changes and removed awaiting committer review Awaiting committer review labels Jun 18, 2026

Copilot AI 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.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Comment thread r/tests/testthat/test-duckdb.R
@github-actions github-actions Bot added awaiting changes Awaiting changes and removed awaiting review Awaiting review awaiting changes Awaiting changes labels Jun 18, 2026

@raulcd raulcd left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

not an expert but this looks reasonable and fixes CI, @jonkeane do you want to take a quick look before I merge?

@github-actions github-actions Bot added awaiting merge Awaiting merge and removed awaiting changes Awaiting changes labels Jun 19, 2026
@raulcd raulcd merged commit d6bba6b into apache:main Jun 22, 2026
20 checks passed
@raulcd raulcd removed the awaiting merge Awaiting merge label Jun 22, 2026
@conbench-apache-arrow

Copy link
Copy Markdown

After merging your PR, Conbench analyzed the 4 benchmarking runs that have been run so far on merge-commit d6bba6b.

There were 2 benchmark results indicating a performance regression:

The full Conbench report has more details. It also includes information about 216 possible false positives for unstable benchmarks that are known to sometimes produce them.

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.

3 participants