Removes PartiQLValue from public API #1678
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
johnedquinn
force-pushed
the
main-remove-partiql-value
branch
from
c04cc9d to
6f526aa
Compare
Removes PartiQLValueType from public API Removes PartiQLValueExperimental from public API Removes PartiQLValue reader/writer from public API Moves public and internal APIs to textFixtures until testing can be fully migrated
Fixes conformance tests related to the lack of a VARIANT type.
johnedquinn
force-pushed
the
main-remove-partiql-value
branch
from
9f97db1 to
f70aa7a
Compare
RCHowell
previously requested changes
Dec 16, 2024
partiql-eval/src/main/kotlin/org/partiql/eval/internal/helpers/ValueUtility.kt
Show resolved
Hide resolved
partiql-eval/src/main/kotlin/org/partiql/eval/internal/operator/rel/RelOpUnpivot.kt
Outdated
Show resolved
Hide resolved
partiql-eval/src/test/kotlin/org/partiql/eval/internal/SuccessTestCase.kt
Outdated
Show resolved
Hide resolved
partiql-planner/src/main/kotlin/org/partiql/planner/internal/casts/CastTable.kt
Show resolved
Hide resolved
partiql-planner/src/main/kotlin/org/partiql/planner/internal/transforms/RexConverter.kt
Show resolved
Hide resolved
alancai98
reviewed
Dec 16, 2024
partiql-spi/src/testFixtures/java/org/partiql/spi/value/DatumUtils.java
Outdated
Show resolved
Hide resolved
partiql-spi/src/testFixtures/java/org/partiql/spi/value/DatumUtils.java
Outdated
Show resolved
Hide resolved
test/partiql-randomized-tests/src/test/kotlin/org/partiql/lang/randomized/eval/Utils.kt
Outdated
Show resolved
Hide resolved
partiql-spi/src/main/kotlin/org/partiql/spi/value/ion/IonVariant.kt
Outdated
Show resolved
Hide resolved
partiql-planner/src/main/kotlin/org/partiql/planner/internal/FnResolver.kt
Outdated
Show resolved
Hide resolved
partiql-eval/src/main/kotlin/org/partiql/eval/internal/operator/rel/RelOpUnpivot.kt
Outdated
Show resolved
Hide resolved
partiql-eval/src/main/kotlin/org/partiql/eval/internal/helpers/ValueUtility.kt
Show resolved
Hide resolved
partiql-spi/src/main/kotlin/org/partiql/spi/value/ion/IonDatumReader.kt
Outdated
Show resolved
Hide resolved
alancai98
reviewed
Dec 16, 2024
partiql-planner/src/main/kotlin/org/partiql/planner/internal/transforms/RexConverter.kt
Show resolved
Hide resolved
Adds PType static factory methods with default parameters Renames factory methods for creating Datum and PartiQLValue Adds AST-to-Plan typing logic of integers to the Ion Variant Reverts the IonDatumReader changes to focus on limited scope Adds a lowerSafe() internal utility method for reducing code size Updates some Javadocs & comments
alancai98
reviewed
Dec 24, 2024
partiql-eval/src/main/kotlin/org/partiql/eval/internal/helpers/DatumUtils.kt
Outdated
Show resolved
Hide resolved
partiql-spi/src/main/kotlin/org/partiql/spi/function/builtins/internal/Accumulator.kt
Show resolved
Hide resolved
partiql-spi/src/main/kotlin/org/partiql/spi/value/ion/IonVariant.kt
Outdated
Show resolved
Hide resolved
Updates Accumulator to check for variant on number types Updates internal docs for clarification Simplifies IonVariant conversion for BigIntegers
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1678 +/- ##
============================================
+ Coverage 73.16% 80.03% +6.87%
+ Complexity 2393 47 -2346
============================================
Files 247 19 -228
Lines 17627 506 -17121
Branches 3178 23 -3155
============================================
- Hits 12896 405 -12491
+ Misses 3854 88 -3766
+ Partials 877 13 -864
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
alancai98
approved these changes
Dec 27, 2024
johnedquinn
dismissed
RCHowell’s stale review
Received approval outside of GitHub. He is OOO.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR can be broken up into TWO main commits: the removal of PartiQLValue and the true onboarding of VARIANT as a first-class type. I couldn't merge just the first commit, since the conformance tests failed an additional 1000+ tests. So, the second one naturally followed.
Removal of PartiQLValue (1st commit)
testFixtures.testFixturesispartiql-cliPartiQLValueExperimentalSupport for VARIANT (2nd commit)
lower()whenever we find a variant value at runtime. It also adds the static type checking required for variant.Reviewer's Notices
License Information
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.