Skip to content

Conversation

@timsaucer
Copy link
Member

Which issue does this PR close?

None but addresses the comment in #1337 (comment)

Rationale for this change

The current unit test for keyboard interrupts uses a long running query. This moves over to a simple query that uses a timer to make it run for longer than the keyboard interrupt checks. This matters because we are getting differences in how long it takes for the long running unit test to return based on upstream updates. The desired behavior is just that the query takes a long time, so we do not want to be dependent on performance improvements upstream.

What changes are included in this PR?

Adds a slow_udf that has a 2 second timeout in it.
Uses this instead of a long running join.

Are there any user-facing changes?

No, only a test is updated.

@timsaucer timsaucer requested a review from kosiew January 11, 2026 15:30
@timsaucer
Copy link
Member Author

FYI @nuno-faria and @kosiew

@nuno-faria
Copy link
Contributor

nuno-faria commented Jan 11, 2026

@timsaucer I agree that it would be better to have something more deterministic. However, I think we still need a test to interrupt queries that continuously generate new batches, and therefore do not have the opportunity to check for interrupts (#1337 (comment)). For example, a really long generator (select * from generate_series(1, 1000000000000000000)).

@timsaucer
Copy link
Member Author

@timsaucer I agree that it would be better to have something more deterministic. However, I think we still need a test to interrupt queries that continuously generate new batches, and therefore do not have the opportunity to check for interrupts (#1337 (comment)). For example, a really long generator (select * from generate_series(1, 1000000000000000000)).

This is a good point. I updated the unit test and combined them to check for fast/slow responses and either collect or stream.

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