Skip to content

Commit a388e2e

Browse files
committed
fix
Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk>
1 parent 0726cb5 commit a388e2e

15 files changed

Lines changed: 386 additions & 80 deletions

File tree

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

encodings/parquet-variant/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ vortex-proto = { workspace = true, features = ["expr"] }
3434
vortex-session = { workspace = true }
3535

3636
[dev-dependencies]
37+
vortex-edition = { workspace = true }
3738
rstest = { workspace = true }
3839
tokio = { workspace = true, features = ["full"] }
3940
vortex-array = { workspace = true, features = ["_test-harness"] }

encodings/parquet-variant/src/vtable.rs

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -326,6 +326,10 @@ mod tests {
326326
use vortex_buffer::BitBuffer;
327327
use vortex_buffer::ByteBufferMut;
328328
use vortex_buffer::buffer;
329+
use vortex_edition::Edition;
330+
use vortex_edition::EditionId;
331+
use vortex_edition::EditionInclusion;
332+
use vortex_edition::EditionSessionExt;
329333
use vortex_error::VortexResult;
330334
use vortex_error::vortex_err;
331335
use vortex_file::OpenOptionsSessionExt;
@@ -388,13 +392,30 @@ mod tests {
388392
}
389393

390394
#[fixture]
391-
fn parquet_variant_file_session() -> VortexSession {
395+
fn parquet_variant_file_session() -> VortexResult<VortexSession> {
396+
const TEST_EDITION: EditionId = EditionId::new("test", 2026, 7, 0);
397+
392398
let session = vortex_array::array_session()
393399
.with::<LayoutSession>()
394400
.with::<RuntimeSession>();
395401
vortex_file::register_default_encodings(&session);
396402
session.arrays().register(ParquetVariant);
403+
let editions = session.editions();
404+
editions
405+
.declare_edition(Edition {
406+
id: TEST_EDITION,
407+
min_vortex_version: None,
408+
})
409+
.map_err(|error| vortex_err!("{error}"))?;
410+
for id in session.arrays().registry().ids() {
411+
editions
412+
.declare_inclusion(EditionInclusion::new(&id, TEST_EDITION))
413+
.map_err(|error| vortex_err!("{error}"))?;
414+
}
397415
session
416+
.enable_edition(TEST_EDITION)
417+
.map_err(|error| vortex_err!("{error}"))?;
418+
Ok(session)
398419
}
399420

400421
#[fixture]
@@ -447,9 +468,10 @@ mod tests {
447468
#[tokio::test]
448469
async fn test_file_roundtrip_typed_value_variant_with_statistics(
449470
#[from(typed_value_variant_array)] expected: VortexResult<ArrayRef>,
450-
parquet_variant_file_session: VortexSession,
471+
parquet_variant_file_session: VortexResult<VortexSession>,
451472
) -> VortexResult<()> {
452473
let expected = expected?;
474+
let parquet_variant_file_session = parquet_variant_file_session?;
453475

454476
let mut bytes = ByteBufferMut::empty();
455477
parquet_variant_file_session
@@ -474,10 +496,11 @@ mod tests {
474496
#[tokio::test]
475497
async fn test_file_roundtrip_typed_value_variant_with_zoned_strategy(
476498
#[from(typed_value_variant_array)] expected: VortexResult<ArrayRef>,
477-
parquet_variant_file_session: VortexSession,
499+
parquet_variant_file_session: VortexResult<VortexSession>,
478500
write_strategy: Arc<dyn LayoutStrategy>,
479501
) -> VortexResult<()> {
480502
let expected = expected?;
503+
let parquet_variant_file_session = parquet_variant_file_session?;
481504

482505
let mut bytes = ByteBufferMut::empty();
483506
parquet_variant_file_session

vortex-array/src/arrays/patched/vtable/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -588,7 +588,7 @@ mod tests {
588588
let session = array_session();
589589
session.arrays().register(Patched);
590590

591-
let ctx = ArrayContext::empty().with_registry(session.arrays().registry().clone());
591+
let ctx = ArrayContext::empty().with_valid_ids(session.arrays().registry().ids());
592592
let serialized = array
593593
.serialize(&ctx, &session, &SerializeOptions::default())
594594
.unwrap();

vortex-array/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -190,4 +190,4 @@ pub fn legacy_session() -> &'static VortexSession {
190190
&LEGACY_SESSION
191191
}
192192

193-
pub type ArrayContext = Context<ArrayPluginRef>;
193+
pub type ArrayContext = Context;

vortex-file/src/writer.rs

Lines changed: 15 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -15,15 +15,13 @@ use futures::future::LocalBoxFuture;
1515
use futures::future::ready;
1616
use futures::pin_mut;
1717
use futures::select;
18-
use itertools::Itertools;
1918
use vortex_array::ArrayContext;
2019
use vortex_array::ArrayRef;
2120
use vortex_array::dtype::DType;
2221
use vortex_array::dtype::FieldPath;
2322
use vortex_array::expr::stats::Stat;
2423
use vortex_array::iter::ArrayIterator;
2524
use vortex_array::iter::ArrayIteratorExt;
26-
use vortex_array::session::ArraySessionExt;
2725
use vortex_array::stats::PRUNING_STATS;
2826
use vortex_array::stream::ArrayStream;
2927
use vortex_array::stream::ArrayStreamAdapter;
@@ -60,8 +58,7 @@ use crate::segments::writer::BufferedSegmentSink;
6058
/// Configure a new writer, which can eventually be used to write an [`ArrayStream`] into a sink
6159
/// that implements [`VortexWrite`].
6260
///
63-
/// Unless overridden, the default [write strategy][crate::WriteStrategyBuilder] will be restricted
64-
/// to the encodings in the session's enabled editions.
61+
/// All write strategies are restricted to the encodings in the session's enabled editions.
6562
///
6663
/// Construct with [`WriteOptionsSessionExt::write_options`] for normal use so the writer inherits
6764
/// the session's runtime, array registry, and memory configuration.
@@ -102,7 +99,7 @@ impl VortexWriteOptions {
10299
///
103100
/// The strategy controls repartitioning, statistics layout, compression, and leaf segment
104101
/// emission. Use [`WriteStrategyBuilder`] when only a small part of the default strategy needs
105-
/// customization.
102+
/// customization. Replacing the strategy does not change the enabled-edition encoding policy.
106103
pub fn with_strategy(mut self, strategy: Arc<dyn LayoutStrategy>) -> Self {
107104
self.strategy = strategy;
108105
self
@@ -155,13 +152,11 @@ impl VortexWriteOptions {
155152
mut write: W,
156153
stream: SendableArrayStream,
157154
) -> VortexResult<WriteSummary> {
158-
// Pre-populate the array context with the registered encodings selected by the enabled
159-
// editions. This keeps the serialized ordering deterministic without advertising every
160-
// encoding the session happens to know how to read.
161-
let ctx = ArrayContext::new(self.session.enabled_encoding_ids())
162-
// The registry supplies serialization implementations; the layout strategy applies
163-
// the enabled-edition write policy before arrays reach serialization.
164-
.with_registry(self.session.arrays().registry().clone());
155+
let enabled_encoding_ids = self.session.enabled_encoding_ids();
156+
// Pre-populate the array context in deterministic order and reject any array encoding
157+
// that is not part of an enabled edition, regardless of the configured write strategy.
158+
let ctx =
159+
ArrayContext::new(enabled_encoding_ids.clone()).with_valid_ids(enabled_encoding_ids);
165160
let dtype = stream.dtype().clone();
166161

167162
let (mut ptr, eof) = SequenceId::root().split();
@@ -531,36 +526,37 @@ impl WriteSummary {
531526

532527
#[cfg(test)]
533528
mod tests {
529+
use vortex_array::ArrayContext;
534530
use vortex_array::VTable;
535531
use vortex_array::array_session;
536532
use vortex_array::arrays::Bool;
537533
use vortex_array::arrays::Primitive;
538-
use vortex_array::session::ArraySessionExt;
539534
use vortex_edition::Edition;
540535
use vortex_edition::EditionDeclaration;
541536
use vortex_edition::EditionId;
542537
use vortex_edition::EditionSession;
543538
use vortex_edition::EditionSessionExt;
544539

545540
#[test]
546-
fn initial_array_ids_are_registered_and_enabled() -> Result<(), vortex_edition::EditionError> {
541+
fn array_context_only_permits_enabled_encodings() -> Result<(), vortex_edition::EditionError> {
547542
const EDITION: EditionId = EditionId::new("test", 2026, 7, 0);
548543
static DECLARATION: EditionDeclaration = EditionDeclaration {
549544
edition: Edition {
550545
id: EDITION,
551546
min_vortex_version: None,
552547
},
553-
added: &[&"vortex.primitive", &"test.not_registered"],
548+
added: &[&"vortex.primitive"],
554549
};
555550

556551
let session = array_session().with::<EditionSession>();
557552
session.register_edition(&DECLARATION)?;
558553
session.enable_edition(EDITION)?;
559554

560-
let registry = session.arrays().registry().clone();
561-
let ids = session.enabled_encoding_ids();
562-
assert_eq!(ids, [Primitive.id()]);
563-
assert!(!ids.contains(&Bool.id()));
555+
let enabled_encoding_ids = session.enabled_encoding_ids();
556+
let ctx =
557+
ArrayContext::new(enabled_encoding_ids.clone()).with_valid_ids(enabled_encoding_ids);
558+
assert_eq!(ctx.to_ids(), [Primitive.id()]);
559+
assert!(ctx.intern(&Bool.id()).is_none());
564560
Ok(())
565561
}
566562
}

vortex-file/tests/common/mod.rs

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
// SPDX-License-Identifier: Apache-2.0
2+
// SPDX-FileCopyrightText: Copyright the Vortex contributors
3+
4+
use vortex_array::session::ArraySessionExt;
5+
use vortex_edition::Edition;
6+
use vortex_edition::EditionId;
7+
use vortex_edition::EditionInclusion;
8+
use vortex_edition::EditionSessionExt;
9+
use vortex_error::VortexExpect;
10+
use vortex_error::vortex_err;
11+
use vortex_session::VortexSession;
12+
13+
/// This is a vortex edition used for testing and shouldn't made public.
14+
const TEST_EDITION: EditionId = EditionId::new("test", 2026, 7, 0);
15+
16+
pub fn enable_all_registered_array_encodings(session: &VortexSession) {
17+
let editions = session.editions();
18+
editions
19+
.declare_edition(Edition {
20+
id: TEST_EDITION,
21+
min_vortex_version: None,
22+
})
23+
.map_err(|error| vortex_err!("{error}"))
24+
.vortex_expect("test edition is valid");
25+
for id in session.arrays().registry().ids() {
26+
editions
27+
.declare_inclusion(EditionInclusion::new(&id, TEST_EDITION))
28+
.map_err(|error| vortex_err!("{error}"))
29+
.vortex_expect("registered array encoding has one test-edition inclusion");
30+
}
31+
session
32+
.enable_edition(TEST_EDITION)
33+
.map_err(|error| vortex_err!("{error}"))
34+
.vortex_expect("test edition is registered");
35+
}

vortex-file/tests/issue_8819_footer_segment_oob.rs

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,23 +14,26 @@ use std::mem::size_of;
1414
use std::sync::LazyLock;
1515

1616
use vortex_array::IntoArray;
17-
use vortex_array::session::ArraySessionExt;
1817
use vortex_buffer::Buffer;
1918
use vortex_buffer::ByteBuffer;
2019
use vortex_buffer::ByteBufferMut;
2120
use vortex_file::OpenOptionsSessionExt;
2221
use vortex_file::WriteOptionsSessionExt;
23-
use vortex_file::WriteStrategyBuilder;
2422
use vortex_io::session::RuntimeSession;
2523
use vortex_layout::session::LayoutSession;
2624
use vortex_session::VortexSession;
2725

26+
mod common;
27+
28+
use common::enable_all_registered_array_encodings;
29+
2830
static SESSION: LazyLock<VortexSession> = LazyLock::new(|| {
2931
let session = vortex_array::array_session()
3032
.with::<LayoutSession>()
3133
.with::<RuntimeSession>();
3234

3335
vortex_file::register_default_encodings(&session);
36+
enable_all_registered_array_encodings(&session);
3437

3538
session
3639
});
@@ -40,14 +43,8 @@ async fn open_buffer_rejects_out_of_bounds_footer_segment() {
4043
// Write a valid file to obtain a well-formed footer.
4144
let mut buf = ByteBufferMut::empty();
4245
let array = Buffer::from((0i32..256).collect::<Vec<i32>>()).into_array();
43-
let allowed = SESSION.arrays().registry().ids().collect();
4446
SESSION
4547
.write_options()
46-
.with_strategy(
47-
WriteStrategyBuilder::default()
48-
.with_allow_encodings(allowed)
49-
.build(),
50-
)
5148
.write(&mut buf, array.to_array_stream())
5249
.await
5350
.expect("write");

vortex-file/tests/test_write_table.rs

Lines changed: 10 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -18,39 +18,33 @@ use vortex_array::arrays::StructArray;
1818
use vortex_array::arrays::struct_::StructArrayExt;
1919
use vortex_array::dtype::FieldNames;
2020
use vortex_array::field_path;
21-
use vortex_array::session::ArraySessionExt;
2221
use vortex_array::validity::Validity;
2322
use vortex_btrblocks::BtrBlocksCompressor;
2423
use vortex_buffer::ByteBuffer;
2524
use vortex_file::OpenOptionsSessionExt;
26-
use vortex_file::VortexWriteOptions;
2725
use vortex_file::WriteOptionsSessionExt;
28-
use vortex_file::WriteStrategyBuilder;
2926
use vortex_io::session::RuntimeSession;
3027
use vortex_layout::layouts::compressed::CompressingStrategy;
3128
use vortex_layout::layouts::flat::writer::FlatLayoutStrategy;
3229
use vortex_layout::layouts::table::TableStrategy;
3330
use vortex_layout::session::LayoutSession;
3431
use vortex_session::VortexSession;
32+
33+
mod common;
34+
35+
use common::enable_all_registered_array_encodings;
36+
3537
static SESSION: LazyLock<VortexSession> = LazyLock::new(|| {
3638
let session = vortex_array::array_session()
3739
.with::<LayoutSession>()
3840
.with::<RuntimeSession>();
3941

4042
vortex_file::register_default_encodings(&session);
43+
enable_all_registered_array_encodings(&session);
4144

4245
session
4346
});
4447

45-
fn all_registered_write_options() -> VortexWriteOptions {
46-
let allowed = SESSION.arrays().registry().ids().collect();
47-
SESSION.write_options().with_strategy(
48-
WriteStrategyBuilder::default()
49-
.with_allow_encodings(allowed)
50-
.build(),
51-
)
52-
}
53-
5448
#[tokio::test]
5549
async fn test_file_roundtrip() {
5650
let mut ctx = SESSION.create_execution_ctx();
@@ -154,7 +148,8 @@ async fn test_dict_listview_validity_roundtrip() {
154148
.into_array();
155149

156150
let mut bytes = Vec::new();
157-
all_registered_write_options()
151+
SESSION
152+
.write_options()
158153
.write(&mut bytes, data.to_array_stream())
159154
.await
160155
.expect("write should not fail with fill_null serialization error");
@@ -199,7 +194,8 @@ async fn test_write_empty_nullable_struct_column() {
199194
.into_array();
200195

201196
let mut bytes = Vec::new();
202-
all_registered_write_options()
197+
SESSION
198+
.write_options()
203199
.write(&mut bytes, data.to_array_stream())
204200
.await
205201
.expect("writing an empty nullable struct column should not panic");

0 commit comments

Comments
 (0)