Skip to content

Conversation

@DinosL
Copy link
Contributor

@DinosL DinosL commented Jan 12, 2026

Some tests currently fail when using the curl backend. These tests are temporarily forced to use httplib instead.

url_encode.test: should be fixed by #193

@carlopi
Copy link
Collaborator

carlopi commented Jan 12, 2026

Locally I got a:

GEN=ninja make
./build/release/test/unittest
1. test/sql/logging/http_logging.test:25
================================================================================
Wrong result in query! (test/sql/logging/http_logging.test:25)!
================================================================================

I am not sure what's different between local and CI, I'll give it a look.

I think locally httplib I have all passing, while curl 1 failure on logging, that's potentially less relevant:

./build/release/test/unittest --skip-error-messages "[]" --statically-loaded-extensions "[httpfs,parquet,core_functions]" --on-init "SET httpfs_client_implementation='httplib';"
./build/release/test/unittest --skip-error-messages "[]" --statically-loaded-extensions "[httpfs,parquet,core_functions]" --on-init "SET httpfs_client_implementation='curl';"

@DinosL
Copy link
Contributor Author

DinosL commented Jan 12, 2026

Can you show me the full error?

@carlopi
Copy link
Collaborator

carlopi commented Jan 12, 2026

Sure:

================================================================================
Wrong result in query! (test/sql/logging/http_logging.test:25)!
================================================================================
SELECT request.headers['Range'], response.headers['Content-Range']
FROM duckdb_logs_parsed('HTTP')
WHERE request.type='GET';
================================================================================
Mismatch on row 1, column response.headers['Content-Range'](index 2)
NULL <> bytes 0-1275/1276
================================================================================
Expected result:
================================================================================
bytes=0-1275	bytes 0-1275/1276

================================================================================
Actual result:
================================================================================
bytes=0-1275	NULL


~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
unittest is a Catch v2.13.7 host application.
Run with -? for options

-------------------------------------------------------------------------------
test/sql/logging/http_logging.test
-------------------------------------------------------------------------------

@DinosL
Copy link
Contributor Author

DinosL commented Jan 12, 2026

That's strange! I cannot reproduce locally and CI doesn't seem to fail either.

@DinosL DinosL marked this pull request as draft January 12, 2026 15:09
@DinosL DinosL force-pushed the curl_missing_url_error branch from 822863a to 49a5654 Compare January 12, 2026 15:28
@DinosL DinosL marked this pull request as ready for review January 12, 2026 16:01
carlopi added a commit to carlopi/duckdb_httpfs that referenced this pull request Jan 13, 2026
DinosL added a commit to DinosL/duckdb-httpfs that referenced this pull request Jan 14, 2026
@carlopi carlopi force-pushed the curl_missing_url_error branch from 49a5654 to 19559ef Compare January 15, 2026 07:09
@carlopi
Copy link
Collaborator

carlopi commented Jan 15, 2026

@DinosL: I rebased this on origin/main, I hope I have not messed up too much your workflow. Idea is checking this + other PR (now landed) where it stands.

@carlopi
Copy link
Collaborator

carlopi commented Jan 15, 2026

After merging other 2 PRs and rebase, this looks great.

Could you review the test modifications, what needs to stay in and what could be reverted?

  • test/sql/copy/s3/http_secret.test
  • test/sql/copy/s3/url_encode.test
  • test/sql/httpfs/hffs.test_slow
  • test/sql/secrets/create_secret_gcs.test_slow
  • test/sql/full_file_download_fallback.test

I think at least some have been solved now.

@DinosL
Copy link
Contributor Author

DinosL commented Jan 15, 2026

Agreed, they should be fixed now. I reverted them and hope CI agrees. 😃

@carlopi
Copy link
Collaborator

carlopi commented Jan 15, 2026

Thanks!!

@carlopi carlopi merged commit b914a30 into duckdb:main Jan 15, 2026
25 checks passed
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