Skip to content

Commit a805d3e

Browse files
authored
Write the list layout when the enabled editions include it (#10563)
## Summary The default writer now writes the list layout whenever the session's enabled editions include `vortex.list`, so the editions are the only switch. `with_list_layout()` is removed, with no opt-out in its place. No default edition includes `vortex.list`, so default writes are unchanged. ## Changes - `WriteStrategyBuilder::from_session` turns the list layout on when the enabled editions include `vortex.list`. - Removes `WriteStrategyBuilder::with_list_layout()`. - vortex-file's test session enables every registered layout, so its list tests now write the list layout; the flat/list switch in `write_read_roundtrip_with_layout` is removed. ## API Changes - Removed `WriteStrategyBuilder::with_list_layout()` (added in #10529, unreleased). --------- Signed-off-by: Matt Katz <mhkatz97@gmail.com>
1 parent 8a1f2cb commit a805d3e

3 files changed

Lines changed: 38 additions & 93 deletions

File tree

‎vortex-file/src/strategy.rs‎

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -10,15 +10,19 @@ use vortex_array::dtype::FieldPath;
1010
use vortex_btrblocks::BtrBlocksCompressorBuilder;
1111
use vortex_btrblocks::SchemeExt;
1212
use vortex_btrblocks::schemes::integer::IntDictScheme;
13+
use vortex_edition::ComponentKind;
14+
use vortex_edition::EditionSessionExt;
1315
use vortex_error::VortexExpect;
1416
use vortex_layout::LayoutStrategy;
17+
use vortex_layout::VTable;
1518
use vortex_layout::layouts::buffered::BufferedStrategy;
1619
use vortex_layout::layouts::chunked::writer::ChunkedLayoutStrategy;
1720
use vortex_layout::layouts::collect::CollectStrategy;
1821
use vortex_layout::layouts::compressed::CompressingStrategy;
1922
use vortex_layout::layouts::compressed::CompressorPlugin;
2023
use vortex_layout::layouts::dict::writer::DictStrategy;
2124
use vortex_layout::layouts::flat::writer::FlatLayoutStrategy;
25+
use vortex_layout::layouts::list::List;
2226
use vortex_layout::layouts::list::writer::ListLayoutStrategy;
2327
use vortex_layout::layouts::repartition::RepartitionStrategy;
2428
use vortex_layout::layouts::repartition::RepartitionWriterOptions;
@@ -77,7 +81,9 @@ impl WriteStrategyBuilder {
7781
field_writers: HashMap::new(),
7882
flat_strategy: None,
7983
probe_compressor: None,
80-
use_list_layout: false,
84+
use_list_layout: session
85+
.enabled_component_ids(ComponentKind::Layout)
86+
.contains(&List.id()),
8187
}
8288
}
8389

@@ -99,19 +105,6 @@ impl WriteStrategyBuilder {
99105
self
100106
}
101107

102-
/// Enable writing list fields with [`ListLayoutStrategy`].
103-
///
104-
/// **Note**: this is an unstable and experimental layout that is expected to change.
105-
/// Using it may lead to unreadable files in the future.
106-
///
107-
/// Writing fails unless the session's enabled editions include the `vortex.list` layout.
108-
///
109-
/// [`ListLayoutStrategy`]: vortex_layout::layouts::list::writer::ListLayoutStrategy
110-
pub fn with_list_layout(mut self) -> Self {
111-
self.use_list_layout = true;
112-
self
113-
}
114-
115108
/// Override the write layout for a specific field somewhere in the nested schema tree.
116109
///
117110
/// The field path is matched after the root struct is split into columns. This is useful when a

‎vortex-file/src/tests.rs‎

Lines changed: 24 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -2010,32 +2010,14 @@ async fn test_writer_with_complex_types() -> VortexResult<()> {
20102010
Ok(())
20112011
}
20122012

2013-
/// Write `array` with list decomposition forced on (through the full compress/zone pipeline) and
2014-
/// read the whole thing back.
2013+
/// Write `array` through the full compress/zone pipeline and read the whole thing back. `SESSION`
2014+
/// enables the `vortex.list` layout, so lists are decomposed.
20152015
async fn write_read_roundtrip(array: ArrayRef) -> VortexResult<ArrayRef> {
2016-
write_read_roundtrip_with_layout(array, true).await
2017-
}
2018-
2019-
async fn write_read_roundtrip_with_layout(
2020-
array: ArrayRef,
2021-
use_list_layout: bool,
2022-
) -> VortexResult<ArrayRef> {
2023-
let strategy = crate::strategy::WriteStrategyBuilder::from_session(&SESSION)
2024-
.with_list_layout()
2025-
.build();
20262016
let mut buf = ByteBufferMut::empty();
2027-
if use_list_layout {
2028-
SESSION
2029-
.write_options()
2030-
.with_strategy(strategy)
2031-
.write(&mut buf, array.to_array_stream())
2032-
.await?;
2033-
} else {
2034-
SESSION
2035-
.write_options()
2036-
.write(&mut buf, array.to_array_stream())
2037-
.await?;
2038-
}
2017+
SESSION
2018+
.write_options()
2019+
.write(&mut buf, array.to_array_stream())
2020+
.await?;
20392021
SESSION
20402022
.open_options()
20412023
.open_buffer(buf)?
@@ -2110,30 +2092,28 @@ fn map_array_from_rows(rows: &[MapRowFixture<'_>], keys_sorted: bool) -> VortexR
21102092
Ok(builder.finish_into_map().into_array())
21112093
}
21122094

2113-
/// A struct containing a Map column crosses both the default flat writer and the list layout
2114-
/// strategy without changing map nullability, empty rows, duplicate keys, or scalar values.
2095+
/// A struct containing a Map column crosses the list layout strategy without changing map
2096+
/// nullability, empty rows, duplicate keys, or scalar values.
21152097
#[tokio::test]
21162098
#[cfg_attr(miri, ignore)]
21172099
async fn struct_with_map_column_roundtrip() -> VortexResult<()> {
2118-
for use_list_layout in [false, true] {
2119-
let maps = map_array_from_rows(
2120-
&[
2121-
Some(vec![(1, Some("one")), (2, None)]),
2122-
None,
2123-
Some(vec![]),
2124-
Some(vec![(1, Some("dup-old")), (1, Some("dup-new"))]),
2125-
],
2126-
false,
2127-
)?;
2128-
let st = StructArray::from_fields(&[
2129-
("id", buffer![10i32, 20, 30, 40].into_array()),
2130-
("attrs", maps),
2131-
])?
2132-
.into_array();
2100+
let maps = map_array_from_rows(
2101+
&[
2102+
Some(vec![(1, Some("one")), (2, None)]),
2103+
None,
2104+
Some(vec![]),
2105+
Some(vec![(1, Some("dup-old")), (1, Some("dup-new"))]),
2106+
],
2107+
false,
2108+
)?;
2109+
let st = StructArray::from_fields(&[
2110+
("id", buffer![10i32, 20, 30, 40].into_array()),
2111+
("attrs", maps),
2112+
])?
2113+
.into_array();
21332114

2134-
let result = write_read_roundtrip_with_layout(st.clone(), use_list_layout).await?;
2135-
assert_arrays_eq!(result, st, &mut SESSION.create_execution_ctx());
2136-
}
2115+
let result = write_read_roundtrip(st.clone()).await?;
2116+
assert_arrays_eq!(result, st, &mut SESSION.create_execution_ctx());
21372117

21382118
Ok(())
21392119
}

‎vortex/src/editions/tests.rs‎

Lines changed: 7 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -440,24 +440,6 @@ async fn write_with(session: &VortexSession, array: ArrayRef) -> VortexResult<By
440440
Ok(buffer)
441441
}
442442

443-
/// Write `array` with `session`, opting in to the list layout.
444-
async fn write_list_layout(
445-
session: &VortexSession,
446-
array: ArrayRef,
447-
) -> VortexResult<ByteBufferMut> {
448-
let mut buffer = ByteBufferMut::empty();
449-
session
450-
.write_options()
451-
.with_strategy(
452-
WriteStrategyBuilder::from_session(session)
453-
.with_list_layout()
454-
.build(),
455-
)
456-
.write(&mut buffer, array.to_array_stream())
457-
.await?;
458-
Ok(buffer)
459-
}
460-
461443
/// The layout encodings a written file actually contains, depth first.
462444
fn written_layout_ids(session: &VortexSession, buffer: ByteBufferMut) -> VortexResult<Vec<Id>> {
463445
let file = session.open_options().open_buffer(buffer)?;
@@ -562,10 +544,9 @@ async fn writer_restricts_layouts_to_the_enabled_editions() -> VortexResult<()>
562544
Ok(())
563545
}
564546

565-
/// The list layout is opt-in: the default writer never emits it, and opting in writes it only when
566-
/// the enabled editions permit it.
547+
/// The writer emits the list layout exactly when the enabled editions include it.
567548
#[tokio::test]
568-
async fn list_layout_is_opt_in_and_edition_gated() -> VortexResult<()> {
549+
async fn list_layout_follows_enabled_editions() -> VortexResult<()> {
569550
use crate::VortexSessionDefault;
570551

571552
let list = ListArray::try_new(
@@ -576,15 +557,10 @@ async fn list_layout_is_opt_in_and_edition_gated() -> VortexResult<()> {
576557
let array = StructArray::from_fields(&[("l", list.into_array())])?.into_array();
577558
let list_layout = Id::from("vortex.list");
578559

579-
// The default core edition does not include the list layout, so opting in fails the write.
560+
// The default core edition does not include the list layout, so lists stay flat.
580561
let session = VortexSession::default();
581-
let error = write_list_layout(&session, array.clone())
582-
.await
583-
.expect_err("the list layout is not in the default editions");
584-
assert!(
585-
error.to_string().contains("not permitted by ctx"),
586-
"unexpected error: {error}"
587-
);
562+
let written = written_layout_ids(&session, write_with(&session, array.clone()).await?)?;
563+
assert!(!written.contains(&list_layout), "{written:?}");
588564

589565
let session = VortexSession::default();
590566
session.editions().declare_inclusion(EditionInclusion::new(
@@ -594,12 +570,8 @@ async fn list_layout_is_opt_in_and_edition_gated() -> VortexResult<()> {
594570
))?;
595571
session.enable_edition(DEFAULT_PREVIEW_EDITION)?;
596572

597-
// Permitted but not requested: the default writer keeps lists flat.
598-
let written = written_layout_ids(&session, write_with(&session, array.clone()).await?)?;
599-
assert!(!written.contains(&list_layout), "{written:?}");
600-
601-
// Permitted and requested.
602-
let written = written_layout_ids(&session, write_list_layout(&session, array).await?)?;
573+
// Permitted: the default writer emits it.
574+
let written = written_layout_ids(&session, write_with(&session, array).await?)?;
603575
assert!(written.contains(&list_layout), "{written:?}");
604576
Ok(())
605577
}

0 commit comments

Comments
 (0)