Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move Cast out of Storage, new common utils for cast and unary operation. #12246

Open
wants to merge 10 commits into
base: develop
Choose a base branch
from

Conversation

jdunkerley
Copy link
Member

@jdunkerley jdunkerley commented Feb 6, 2025

Pull Request Description

  • Remove cast from Storage and move to a new CastOperation.
  • Added support for casting to Boolean from a numeric (as per database convention).
  • check_cast_compatibility delegate to CastOperation to see if a supported conversion.
  • New helper class StorageIterators for building unary operations.
  • Remove the AbstractUnary classes.
  • Merged IsFinite, IsInfinite and IsNaN to single class.
  • Moved all Unary operators to use StorageIterators.
  • Will add new helper base classes as operations are built up.
class StorageIterators
  /** Iterate over source, operation's job to add to builder. Use to build Long/Double/Boolean without boxing. */
  <B extends BuilderForType<T>, S, T> ColumnStorage<T> buildOverStorage(
      ColumnStorage<S> source,
      boolean preserveNothing,
      B builder,
      BuildOperation<B, S> operation) {}

  /** Iterate over source, operation's job to add to builder. Use to build Long/Double/Boolean without boxing. */
  <S, T> ColumnStorage<T> mapOverStorage(
      ColumnStorage<S> source,
      boolean preserveNothing,
      BuilderForType<T> builder,
      MapOperation<T, S> operation) {}

Overrides with preseverNothing=true and specialised versions for (map|build)Over(Long|Double|Boolean)Storage

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

  • The documentation has been updated, if necessary.
  • Screenshots/screencasts have been attached, if there are any visual changes. For interactive or animated visual changes, a screencast is preferred.
  • All code follows the
    Scala,
    Java,
    TypeScript,
    and
    Rust
    style guides. In case you are using a language not listed above, follow the Rust style guide.
  • Unit tests have been written where possible.
  • If meaningful changes were made to logic or tests affecting Enso Cloud integration in the libraries,
    or the Snowflake database integration, a run of the Extra Tests has been scheduled.
    • If applicable, it is suggested to paste a link to a successful run of the Extra Tests.

Add validity checking to the converters.
Adjust super-mix test.
@jdunkerley jdunkerley added the CI: No changelog needed Do not require a changelog entry for this PR. label Feb 6, 2025
Update is_a_valid_parse_target.
Add test for Integer -> Boolean conversion.
Move DatePart to new design.
Move TextLength and DateTruncate to standalone.
Will build up a base class as move forward.
@jdunkerley jdunkerley changed the title Wip/jd/move cast op Move Cast out of Storage, new common utils for cast and unary operation. Feb 7, 2025
@jdunkerley jdunkerley marked this pull request as ready for review February 7, 2025 13:45
(builder, index, value) -> builder.appendLong(applyObjectRow(index, value)));
}

protected long applyObjectRow(long index, Object value) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the index argument? Struggling to find where we ever use it/

Copy link
Member Author

@jdunkerley jdunkerley Feb 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the framework seemed good to have it as would allow zip operations etc in due course.

@jdunkerley jdunkerley added the CI: Keep up to date Automatically update this PR to the latest develop. label Feb 8, 2025
@jdunkerley jdunkerley added CI: Clean build required CI runners will be cleaned before and after this PR is built. and removed CI: Keep up to date Automatically update this PR to the latest develop. labels Feb 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI: Clean build required CI runners will be cleaned before and after this PR is built. CI: No changelog needed Do not require a changelog entry for this PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants