Fix: saving compressed load files with .gz extension #2835

anuunchin · 2025-07-03T08:47:28Z

Description

This PR adds the .gz extension to compressed files based on a new config disable_extension that is set to True by default.

In a nutshell:

is_compression_disabled() is removed and destination load jobs have access to BufferedDataWriterConfiguration.
Since the task involves getting rid of tricks like FileStorage.is_gzipped(), solely relying on BufferedDataWriterConfiguration is not sufficient when it comes to imported files, since they must not be compressed in any situation. For this reason, a new context class FileImportContext was introduced that tracks whether a file is an imported file and, thus, does not require compression. The is_compressed_file attribute of FileImportContext is then accessed by the duckdb and clickhouse load jobs to correctly set the compression type.

Related Issues

Resolves Save compressed load files with .gz extension #925

netlify · 2025-07-03T08:47:34Z

✅ Deploy Preview for dlt-hub-docs canceled.

Name	Link
🔨 Latest commit	`633bcd6`
🔍 Latest deploy log	https://app.netlify.com/projects/dlt-hub-docs/deploys/688a07dfd32516000813add9

dlt/common/data_writers/buffered.py

dlt/destinations/impl/duckdb/sql_client.py

tests/common/storages/test_compressed_files.py

rudolfix

the direction is good and the only complication is backward compat for filesystem destination. I think @sh-rp wants to chip-in here.

with this PR we can remove some of weird things we do to discover if files are zipped.

is_compression_disabled() - this function should go away and whenever it is used there's something wrong. we are better if we use extension to guess if file is zipped.

another so so pattern used for example in clikckhouse.py

if ext == "jsonl":
                compression = "gz" if FileStorage.is_gzipped(file_path) else "none"

we actually probe the file to see if it is zipped

we remove all those tricks and just use extensiosn. the only place is filesystem destination where we must decide how/if we do backward compatibility

sh-rp · 2025-07-09T08:12:24Z

I did not read the PR, but just from a user / compatibility perspective, I think dlt should behave the exact same way with regards to adding these extensions after updating if you do not change any settings. This means:

adding the gzip extension must be configurable
This setting must be set to off by default
It would be nicer to have it switched on by default, but then we would need to make this change in dlt 2.0 and mark it as breaking

sh-rp

Thanks for the updates, it is already looking good :) I have a bunch of changes that are related to making it clearer what is going on with better naming and less double code.

About maintaining backwards compatibility: I think the decision was to not have this setting but rather to maintain a version marker in the init file of the filesystem destination, and always add this extension for new filesystem datasets and maintain the old behavior for existing ones. Right now the init marker is just an empty file. Going forward, it should contain something l like "{'version': 1}" or similar. Depending on what it says in this file, the extensions will be added or not. I'll add details later today.

dlt/common/data_writers/buffered.py

dlt/common/storages/load_package.py

dlt/destinations/job_impl.py

dlt/destinations/path_utils.py

docs/website/docs/dlt-ecosystem/destinations/filesystem.md

sh-rp · 2025-07-17T13:53:46Z

After more discussion, I think we will need to maintain a filesystem versioning. So the proposal is this:

Remove this flag that governs wether to add the extension or not
Version the filesystem, we keep the version in the base init file. If this file is empty, we assume its content to be {'version': 1} if it does not exist on initialize_storage, we set it to {'version': CURRENT_VERSION} which is a constant that is 2. If we encounter anything else in this file (non empty and parsing it does not have a version 1 or 2) we raise.
An initialized filesystem instance should always know its current version, for now we do not do any migrations or anything like that, older datasets will have version 1, newer ones have version 2.
datasets with version 1 will continue to save the files without this extension.
datasets with version 2 will add this extension
We need to test this behavior with an older dlt version

anuunchin · 2025-07-22T14:56:40Z

dlt/destinations/impl/filesystem/sql_client.py

+                if (
+                    table_name not in dlt_table_names
+                    and self.remote_client.get_storage_version() == 1
+                    and not is_compression_disabled()
+                ):
+                    from_statement = from_statement[:-1] + ", compression = 'gzip')"
+


We still use this is_compression_disabled() trick because solely knowing the storage version is not enough as the files inside it may or may not be compressed. Without this if condition, the duckdb reader won't work with filesystem datasets of version 1.

PS: I put this as separately as possible, so that we can maybe remove this in the future 👀

kudos for spotting this

sh-rp

Very cool! Just a few minor changes and cleanups :)

dlt/common/data_writers/buffered.py

dlt/destinations/impl/filesystem/filesystem.py

dlt/destinations/path_utils.py

docs/website/docs/dlt-ecosystem/destinations/filesystem.md

rudolfix

this still need some work.

also AFAIK we are missing a test where we import both compressed and uncompressed file and then check if extensions was correctly added (ie. up to filesystem storage)

dlt/destinations/impl/filesystem/filesystem.py

dlt/destinations/impl/bigquery/bigquery.py

dlt/destinations/impl/filesystem/filesystem.py

tests/pipeline/test_dlt_versions.py

tests/load/utils.py

rudolfix · 2025-07-28T15:31:55Z

dlt/destinations/impl/filesystem/sql_client.py

+                if (
+                    table_name not in dlt_table_names
+                    and self.remote_client.get_storage_version() == 1
+                    and not is_compression_disabled()
+                ):
+                    from_statement = from_statement[:-1] + ", compression = 'gzip')"
+


kudos for spotting this

--amend

rudolfix

LGTM!

* enable_gz_extension added to client configs --amend * docs added * Unnecessary flag removed, fs storage versioning added * Redundancies removed, storage version cached * Test for imported files improved * Initial version stored separately

anuunchin self-assigned this Jul 3, 2025

anuunchin force-pushed the fix/gzip branch from 9111b5f to 7f34192 Compare July 3, 2025 14:20

anuunchin closed this Jul 3, 2025

anuunchin force-pushed the fix/gzip branch from 7f34192 to 3a23b66 Compare July 3, 2025 14:29

anuunchin reopened this Jul 3, 2025

anuunchin force-pushed the fix/gzip branch 7 times, most recently from 8118936 to 41285c5 Compare July 4, 2025 13:44

anuunchin marked this pull request as ready for review July 7, 2025 07:48

anuunchin force-pushed the fix/gzip branch 6 times, most recently from 879abc7 to e95ebee Compare July 7, 2025 12:15

anuunchin commented Jul 7, 2025

View reviewed changes

dlt/common/data_writers/buffered.py Outdated Show resolved Hide resolved

anuunchin commented Jul 8, 2025

View reviewed changes

dlt/destinations/impl/duckdb/sql_client.py Show resolved Hide resolved

anuunchin requested a review from sh-rp July 8, 2025 08:20

anuunchin commented Jul 8, 2025

View reviewed changes

tests/common/storages/test_compressed_files.py Outdated Show resolved Hide resolved

rudolfix reviewed Jul 8, 2025

View reviewed changes

anuunchin force-pushed the fix/gzip branch 4 times, most recently from 756528a to 04eff17 Compare July 11, 2025 10:53

sh-rp requested changes Jul 17, 2025

View reviewed changes

anuunchin force-pushed the fix/gzip branch 4 times, most recently from ba2cf07 to f2bdc37 Compare July 22, 2025 14:53

anuunchin marked this pull request as ready for review July 22, 2025 14:53

anuunchin commented Jul 22, 2025

View reviewed changes

anuunchin force-pushed the fix/gzip branch from f2bdc37 to 86ae7c1 Compare July 23, 2025 07:01

anuunchin requested a review from sh-rp July 23, 2025 08:05

sh-rp requested changes Jul 24, 2025

View reviewed changes

anuunchin force-pushed the fix/gzip branch 4 times, most recently from 57ed81b to 771e978 Compare July 27, 2025 12:57

rudolfix requested changes Jul 28, 2025

View reviewed changes

anuunchin added 5 commits July 29, 2025 16:16

enable_gz_extension added to client configs

db7136e

--amend

docs added

337b9f1

Unnecessary flag removed, fs storage versioning added

ac6657c

Redundancies removed, storage version cached

02df5b0

Test for imported files improved

f43668a

anuunchin force-pushed the fix/gzip branch 2 times, most recently from 74b6b7f to 864bd2f Compare July 30, 2025 09:04

Initial version stored separately

633bcd6

anuunchin force-pushed the fix/gzip branch from 864bd2f to 633bcd6 Compare July 30, 2025 11:54

anuunchin requested a review from rudolfix July 31, 2025 11:09

rudolfix approved these changes Aug 4, 2025

View reviewed changes

rudolfix merged commit 007953a into devel Aug 4, 2025
115 of 118 checks passed

rudolfix deleted the fix/gzip branch August 4, 2025 10:10

Fix: saving compressed load files with .gz extension #2835

Fix: saving compressed load files with .gz extension #2835

Uh oh!

Conversation

anuunchin commented Jul 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issues

Uh oh!

netlify bot commented Jul 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for dlt-hub-docs canceled.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rudolfix left a comment

Choose a reason for hiding this comment

Uh oh!

sh-rp commented Jul 9, 2025

Uh oh!

sh-rp left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sh-rp commented Jul 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

anuunchin Jul 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rudolfix Jul 28, 2025

Choose a reason for hiding this comment

Uh oh!

sh-rp left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rudolfix left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rudolfix Jul 28, 2025

Choose a reason for hiding this comment

Uh oh!

rudolfix left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

anuunchin commented Jul 3, 2025 •

edited

Loading

netlify bot commented Jul 3, 2025 •

edited

Loading

sh-rp commented Jul 17, 2025 •

edited

Loading

anuunchin Jul 22, 2025 •

edited

Loading