Skip to content

Refactor for shapes with ids - #51

Draft
jpschorr wants to merge 7 commits into
mainfrom
feat-shape-id-migration
Draft

Refactor for shapes with ids#51
jpschorr wants to merge 7 commits into
mainfrom
feat-shape-id-migration

Conversation

@jpschorr

@jpschorr jpschorr commented Aug 14, 2024

Copy link
Copy Markdown
Contributor

Draft of updates needed for partiql/partiql-lang-rust#485

Structurally, all changes are in place.

However PartiqlShapeBuilder.any_of:

    pub fn any_of<I>(&self, types: I) -> PartiqlShape
    where
        I: IntoIterator<Item = PartiqlShape>,
    {
        let any_of = AnyOf::from_iter(types);
        match any_of.types.len() {
            0 => type_dynamic!(),
            1 => {
                let AnyOf { types } = any_of;
                types.into_iter().next().unwrap()
            }
            // TODO figure out what does it mean for a Union to be nullable or not
            _ => PartiqlShape::AnyOf(any_of),
        }
    }

The AnyOf::from_iter(types) uses an IndexSet internally to deduplicate types, thus the match on any_of.types.len() could "flatten" AnyOfs that had duplicates. With the addition of IDs, this deduplication no longer happens...

A couple of tests fail due to the above.


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@jpschorr
jpschorr requested a review from am357 August 14, 2024 16:14
@jpschorr
jpschorr force-pushed the feat-shape-id-migration branch 5 times, most recently from 0791e14 to 4045500 Compare August 14, 2024 20:01
Base automatically changed from feat-finish-error-cleanup to main August 14, 2024 21:18
@jpschorr
jpschorr force-pushed the feat-shape-id-migration branch from 4045500 to 44c5f88 Compare August 19, 2024 20:19
@jpschorr
jpschorr force-pushed the feat-shape-id-migration branch 2 times, most recently from 7d07b56 to 4c37565 Compare August 20, 2024 22:09
@jpschorr
jpschorr force-pushed the feat-shape-id-migration branch from f22bffc to d4c4dd9 Compare August 22, 2024 21:41
*Issue #, if available:*
partiql/partiql-lang-rust#491

*Description of changes:*
This pull request is an attempt to complete the refactoring started in
#51

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants