When a HTTPFileHandle has an ETag, and that changes over time, consider the read unreliable

[carlopi]

I am not completely sure what should be the mitigation strategy here, should there be a setting to disable this check?

[carlopi]

This solves https://github.com/duckdblabs/duckdb-internal/issues/5786, where if a file is changed mid-read we would not detect it.

Currently there is a precedence for just throwing a:

HTTP GET error on 'https://testbucket-break-the-duck.s3.us-east-2.amazonaws.com/somefile.db' (HTTP 416) This could mean the file was changed. Try disabling the duckdb http metadata cache if enabled, and confirm the server supports range requests.

when a file is queried on an range that is not compatible with new length.

I think throwing HTTPException is OK here, main question I have is if we need to allow this to be skipped (say a "unreliable_etag" option).

[Tmonster]

@carlopi can you merge this to v1.4-andium? Or does this need to also be in main?

[Tmonster]

Do we have other ways to test this for existing httpfs workflows? Is it possible there are sometimes false positives?

Otherwise just the error on the comment

[Tmonster]

nit:

\nTry to restart the read or close the file-handle and read the file again (e.g. `DETACH` in the file is a database file).

[lnkuiper]

This should probably remove the file metadata from the HTTPFS cache so that we are sure to do a new HEAD request upon the next time we read the file (important for when enable_http_metadata_cache is enabled).

[carlopi]

@carlopi can you merge this to v1.4-andium? Or does this need to also be in main?

I would probably prefer this out in nightly for 1.3.2, error mode is potentially very bad AND allows to test this already.

Do we have other ways to test this for existing httpfs workflows?

That's a big question, I am not sure to what degree this can be really tested.

Is it possible there are sometimes false positives?

I am not sure, one positive thing is that httpfs can be patched independently.

[Tmonster]

I would probably prefer this out in nightly for 1.3.2, error mode is potentially very bad AND allows to test this already.

I think main points to the v1.4 branch, so nightlies should stil be built/tested. Or are nightlies still building the v1.3-ossivalis branch? Wouldn't it require a bump in submodule pointer anyway?

I am not sure, one positive thing is that httpfs can be patched independently.
That is true, but maybe still good to run a tpch test over s3 or something just to make sure

[carlopi]

@samansmink's: we do add an option, and error message