Add quote style and trimming to csv writier by xanderbailey · Pull Request #20813 · apache/datafusion

xanderbailey · 2026-03-08T21:11:31Z

Which issue does this PR close?

Closes Add quote-style parameter for CSV options #10669

Related arrow-rs PRs apache/arrow-rs#8960 and apache/arrow-rs#9004

Rationale for this change

The CSV writer was missing support for quote_style, ignore_leading_whitespace, and ignore_trailing_whitespace options that are available on the underlying arrow WriterBuilder. This meant users couldn't control quoting behaviour or whitespace trimming when writing CSV files.

What changes are included in this PR?

Adds three new CSV writer options wired through the full stack:

quote_style — controls when fields are quoted (Always, Necessary, NonNumeric, Never). Modelled as a protobuf enum (CsvQuoteStyle).
ignore_leading_whitespace — trims leading whitespace from string values on write.
ignore_trailing_whitespace — trims trailing whitespace from string values on write.

Are these changes tested?

Yes — sqllogictest coverage added in csv_files.slt

Are there any user-facing changes?

Three new format.* options available in COPY TO and CREATE EXTERNAL TABLE for CSV:

format.quote_style (string: Always, Necessary, NonNumeric, Never)
format.ignore_leading_whitespace (boolean)
format.ignore_trailing_whitespace (boolean)

xanderbailey · 2026-03-08T21:26:35Z

+LOCATION 'test_files/scratch/csv_files/quote_style_always.csv'
+OPTIONS ('format.has_header' 'true', 'format.quote_style' 'Never');
+
+# All values should have been quoted, but reading them back strips the quotes


It's hard in SLT to actually test these but I did my best...

Could do something like this:

statement ok CREATE EXTERNAL TABLE stored_quote_style_nonnumeric ( whole_file TEXT ) STORED AS CSV LOCATION 'test_files/scratch/csv_files/quote_style_nonnumeric.csv' OPTIONS ('format.has_header' 'true', 'format.delimiter' '@'); query T select * from stored_quote_style_nonnumeric; ---- 1,"hello",1.1 2,"world",2.2 3,"comma,value",3.3

Pretty much read entire file as a single column, by choosing a delimiter that doesn't appear

Oh that's a nice idea, I'll change to that!

xanderbailey · 2026-03-08T22:45:42Z

+    pub quote_style: i32,
+    /// Whether to ignore leading whitespace in string values
+    #[prost(bytes = "vec", tag = "21")]
+    pub ignore_leading_whitespace: ::prost::alloc::vec::Vec<u8>,


Following the pattern here for other bools being Vec

xanderbailey · 2026-03-08T22:45:48Z

+    pub quote_style: i32,
+    /// Whether to ignore leading whitespace in string values
+    #[prost(bytes = "vec", tag = "21")]
+    pub ignore_leading_whitespace: ::prost::alloc::vec::Vec<u8>,


Following the pattern here for other bools being Vec

xanderbailey · 2026-03-08T22:48:33Z

+                Ok(CsvQuoteStyleProto::Always) => CsvQuoteStyle::Always,
+                Ok(CsvQuoteStyleProto::NonNumeric) => CsvQuoteStyle::NonNumeric,
+                Ok(CsvQuoteStyleProto::Never) => CsvQuoteStyle::Never,
+                _ => CsvQuoteStyle::Necessary,


We don't error on:

compression: match proto.compression { 0 => CompressionTypeVariant::GZIP, 1 => CompressionTypeVariant::BZIP2, 2 => CompressionTypeVariant::XZ, 3 => CompressionTypeVariant::ZSTD, _ => CompressionTypeVariant::UNCOMPRESSED, },

So made the same true here

xanderbailey · 2026-03-24T10:05:59Z

@Jefffrey are you maybe able to take a look here?

rtyler

I have done some integration testing of this PR since it solves a problem one of my customers was having as well. I think it's good to merge, with a squashing of course 😈

I ended up backporting it to 52.4.0 which applied cleanly as well. I notice that arrow 57.2 actually introduced QuoteStyle which means this change requires arrow-csv 57.2.0 or later

rtyler · 2026-04-01T12:54:31Z

+#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Default)]
+pub enum CsvQuoteStyle {
+    /// Quote all fields
+    Always,
+    /// Only quote fields when necessary (default)
+    #[default]
+    Necessary,
+    /// Quote all non-numeric fields
+    NonNumeric,
+    /// Never quote fields
+    Never,
+}


For my own understanding is this duplicated from the arrow-csv crate in order to add more trait implementations? I am less familiar with the protocol buffer stuff that happens in Datafusion, but figure there must be a reason for the struct to be ruplicated

Yeah I wasn't 100% sure what to do here but it seemed fine to duplicate?

I believe this is in line with trend of having types here be DF versions; for example we have a DF version of parquet writer version as well:

datafusion/datafusion/common/src/parquet_config.rs

Lines 24 to 36 in cc4717a

/// Parquet writer version options for controlling the Parquet file format version

///

/// This enum validates parquet writer version values at configuration time,

/// ensuring only valid versions ("1.0" or "2.0") can be set via `SET` commands

/// or proto deserialization.

#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)]

pub enum DFParquetWriterVersion {

/// Parquet format version 1.0

#[default]

V1_0,

/// Parquet format version 2.0

V2_0,

}

Digging deeper into load failures I discovered that while arrow-csv and other Rust CSV parsers can handle some characters properly, some RDMS and C-based CSV parsers (older) cannot, without quoting. This relies on a backport of apache/datafusion#20813 to the 52.4.0 release in order to support this. It also requires arrow-csv 57.2.0 or later since that release is the first that `QuoteStyle` exists.

xanderbailey · 2026-04-14T07:27:05Z

@alamb could I bother you for a review here please?

Jefffrey · 2026-04-16T04:08:14Z

            builder = builder.with_double_quote(*v)
        }
+        let style = match value.quote_style {
+            CsvQuoteStyle::Always => QuoteStyle::Always,


nit: pull this into a From impl where CsvQuoteStyle is defined

Jefffrey · 2026-04-16T04:12:09Z

+        match s {
+            "Always" | "ALWAYS" | "always" => Ok(Self::Always),
+            "Necessary" | "NECESSARY" | "necessary" | "" => Ok(Self::Necessary),
+            "NonNumeric" | "NON_NUMERIC" | "nonnumeric" => Ok(Self::NonNumeric),
+            "Never" | "NEVER" | "never" => Ok(Self::Never),
+            _ => Err(DataFusionError::NotImplemented(format!(
+                "Unsupported CSV quote style {s}"
+            ))),
+        }


Suggested change

match s {

"Always" | "ALWAYS" | "always" => Ok(Self::Always),

"Necessary" | "NECESSARY" | "necessary" | "" => Ok(Self::Necessary),

"NonNumeric" | "NON_NUMERIC" | "nonnumeric" => Ok(Self::NonNumeric),

"Never" | "NEVER" | "never" => Ok(Self::Never),

_ => Err(DataFusionError::NotImplemented(format!(

"Unsupported CSV quote style {s}"

))),

}

match s.to_lowercase().as_str() {

"always" => Ok(Self::Always),

"necessary" | "" => Ok(Self::Necessary),

"non_numeric" | "nonnumeric" => Ok(Self::NonNumeric),

"never" => Ok(Self::Never),

_ => Err(DataFusionError::NotImplemented(format!(

"Unsupported CSV quote style {s}"

))),

}

I think it might be a better idea to be permissive like this; also not sure about that empty string parsing as the default 🤔

I'll quickly check what spark does here

Spark just has quoteAll which maps to always so I have removed the empty string.

Jefffrey · 2026-04-16T04:20:53Z

+query IT
+select * from stored_trim_leading order by id;
+----
+1 hello


Do we expect to see trailing whitespace here? Or is there an issue with the runner on how it treats trailing whitespaces 🤔

Yeah I had to look at the CSV that actually got written...

https://github.com/apache/datafusion/blob/main/datafusion/sqllogictest/src/util.rs?plain=1#L44

Let me do that

Jefffrey · 2026-04-16T04:24:21Z

+#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Default)]
+pub enum CsvQuoteStyle {
+    /// Quote all fields
+    Always,
+    /// Only quote fields when necessary (default)
+    #[default]
+    Necessary,
+    /// Quote all non-numeric fields
+    NonNumeric,
+    /// Never quote fields
+    Never,
+}


I believe this is in line with trend of having types here be DF versions; for example we have a DF version of parquet writer version as well:

datafusion/datafusion/common/src/parquet_config.rs

Lines 24 to 36 in cc4717a

/// Parquet writer version options for controlling the Parquet file format version

///

/// This enum validates parquet writer version values at configuration time,

/// ensuring only valid versions ("1.0" or "2.0") can be set via `SET` commands

/// or proto deserialization.

#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)]

pub enum DFParquetWriterVersion {

/// Parquet format version 1.0

#[default]

V1_0,

/// Parquet format version 2.0

V2_0,

}

Jefffrey · 2026-04-16T04:27:18Z

+        pub ignore_leading_whitespace: Option<bool>, default = None
+        /// Whether to ignore trailing whitespace in string values when writing CSV.
+        pub ignore_trailing_whitespace: Option<bool>, default = None
        /// Specifies whether newlines in (quoted) values are supported.


I'm wondering why we have this trend of doing Option<bool> instead of just bool 🤔

What is the default in case of None, and how is that different from just true/false (whichever it is)?

I was also curious about this but didn't want to break the pattern...

Is it some ser-deser quirk? I'll see if I can figure this out.

No I can't find a good reason for this... Would you like me to change or keep the pattern?

Perhaps its to do with hierarchy of configs and/or sql parsing 🤔

@alamb do you happen to know a reason for having Option<bool> instead of plain bool for cases where it'll end up being true or false anyway? (i.e. None doesn't represent a third state, but eventually maps to either true or false)

Add quote style and trimming to csv writier

fa967e9

github-actions bot added sqllogictest SQL Logic Tests (.slt) common Related to common crate proto Related to proto crate labels Mar 8, 2026

fmt

c45b27f

xanderbailey commented Mar 8, 2026

View reviewed changes

xanderbailey added 3 commits March 8, 2026 21:38

fixup

da2f75e

fmt

c971adf

clean things up

07616a9

xanderbailey commented Mar 8, 2026

View reviewed changes

xanderbailey added 2 commits March 8, 2026 22:58

clippy

f59eb8a

code gen

eea1681

rtyler reviewed Apr 1, 2026

View reviewed changes

Jefffrey reviewed Apr 16, 2026

View reviewed changes

xanderbailey added 3 commits April 16, 2026 07:43

comments

2316319

update tests

c94a9a3

tests

b946b6d

xanderbailey requested a review from Jefffrey April 16, 2026 10:00

Tests read a single column

308b045

	/// Parquet writer version options for controlling the Parquet file format version
	///
	/// This enum validates parquet writer version values at configuration time,
	/// ensuring only valid versions ("1.0" or "2.0") can be set via `SET` commands
	/// or proto deserialization.
	#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)]
	pub enum DFParquetWriterVersion {
	/// Parquet format version 1.0
	#[default]
	V1_0,
	/// Parquet format version 2.0
	V2_0,
	}

Conversation

xanderbailey commented Mar 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xanderbailey commented Mar 24, 2026

Uh oh!

rtyler left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xanderbailey commented Apr 14, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

xanderbailey commented Mar 8, 2026 •

edited

Loading