From 79ee005ccaf0976029c20d24bdf8f3cf42f13aa4 Mon Sep 17 00:00:00 2001 From: "R. C. Howell" Date: Thu, 14 Dec 2023 15:15:14 -0800 Subject: [PATCH] Use Iterable for PartiQLValue collections rather than Sequence (#1305) --- CHANGELOG.md | 1 + .../partiql/cli/utils/ServiceLoaderUtil.kt | 46 ++-- .../main/kotlin/org/partiql/value/PartiQL.kt | 84 +++++++- .../kotlin/org/partiql/value/PartiQLValue.kt | 38 ++-- .../kotlin/org/partiql/value/helpers/ToIon.kt | 24 +-- .../org/partiql/value/impl/BagValueImpl.kt | 6 +- .../org/partiql/value/impl/ListValueImpl.kt | 6 +- .../org/partiql/value/impl/SexpValueImpl.kt | 6 +- .../org/partiql/value/impl/StructValueImpl.kt | 48 +++-- .../partiql/value/io/PartiQLValueIonReader.kt | 17 +- .../value/io/PartiQLValueTextWriter.kt | 18 +- .../value/util/PartiQLValueBaseVisitor.kt | 8 +- .../value/io/PartiQLValueIonSerdeTest.kt | 58 ++--- .../value/io/PartiQLValueTextWriterTest.kt | 198 +++++++----------- 14 files changed, 298 insertions(+), 260 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 100d6337f..0fc5b55c4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,6 +47,7 @@ Thank you to all who have contributed! - The new plan is fully resolved and typed. - Operators will be converted to function call. - Changes the return type of `filter_distinct` to a list if input collection is list +- Changes the `PartiQLValue` collections to implement Iterable rather than Sequence, allowing for multiple consumption. ### Deprecated diff --git a/partiql-cli/src/main/kotlin/org/partiql/cli/utils/ServiceLoaderUtil.kt b/partiql-cli/src/main/kotlin/org/partiql/cli/utils/ServiceLoaderUtil.kt index 3f1581ad3..cfa7120fd 100644 --- a/partiql-cli/src/main/kotlin/org/partiql/cli/utils/ServiceLoaderUtil.kt +++ b/partiql-cli/src/main/kotlin/org/partiql/cli/utils/ServiceLoaderUtil.kt @@ -268,28 +268,42 @@ class ServiceLoaderUtil { PartiQLValueType.INTERVAL -> TODO() PartiQLValueType.BAG -> { - (partiqlValue as? BagValue<*>)?.elements?.map { PartiQLtoExprValue(it) }?.let { newBag(it) } - ?: ExprValue.nullValue + if (partiqlValue.isNull) { + ExprValue.nullValue + } else { + newBag((partiqlValue as? BagValue<*>)!!.map { PartiQLtoExprValue(it) }) + } } PartiQLValueType.LIST -> { - (partiqlValue as? ListValue<*>)?.elements?.map { PartiQLtoExprValue(it) }?.let { newList(it) } - ?: ExprValue.nullValue + if (partiqlValue.isNull) { + ExprValue.nullValue + } else { + newList((partiqlValue as? ListValue<*>)!!.map { PartiQLtoExprValue(it) }) + } } PartiQLValueType.SEXP -> { - (partiqlValue as? SexpValue<*>)?.elements?.map { PartiQLtoExprValue(it) }?.let { newSexp(it) } - ?: ExprValue.nullValue + if (partiqlValue.isNull) { + ExprValue.nullValue + } else { + newSexp((partiqlValue as? SexpValue<*>)!!.map { PartiQLtoExprValue(it) }) + } } PartiQLValueType.STRUCT -> { - (partiqlValue as? StructValue<*>)?.fields?.map { - PartiQLtoExprValue(it.second).namedValue( - newString( - it.first + if (partiqlValue.isNull) { + ExprValue.nullValue + } else { + val entries = (partiqlValue as? StructValue<*>)!!.entries + entries.map { + PartiQLtoExprValue(it.second).namedValue( + newString( + it.first + ) ) - ) - }?.let { newStruct(it, StructOrdering.ORDERED) } ?: ExprValue.nullValue + }.let { newStruct(it, StructOrdering.ORDERED) } + } } PartiQLValueType.DECIMAL -> TODO() @@ -447,7 +461,7 @@ class ServiceLoaderUtil { PartiQLValueType.INTERVAL -> TODO() PartiQLValueType.BAG -> when (exprValue.type) { ExprValueType.NULL -> bagValue(null) - ExprValueType.BAG -> bagValue(exprValue.map { ExprToPartiQLValue(it, ExprToPartiQLValueType(it)) }.asSequence()) + ExprValueType.BAG -> bagValue(exprValue.map { ExprToPartiQLValue(it, ExprToPartiQLValueType(it)) }) else -> throw ExprToPartiQLValueTypeMismatchException( PartiQLValueType.BAG, ExprToPartiQLValueType(exprValue) ) @@ -459,7 +473,7 @@ class ServiceLoaderUtil { ExprToPartiQLValue( it, ExprToPartiQLValueType(it) ) - }.asSequence() + } ) else -> throw ExprToPartiQLValueTypeMismatchException( PartiQLValueType.LIST, ExprToPartiQLValueType(exprValue) @@ -472,7 +486,7 @@ class ServiceLoaderUtil { ExprToPartiQLValue( it, ExprToPartiQLValueType(it) ) - }.asSequence() + } ) else -> throw ExprToPartiQLValueTypeMismatchException( PartiQLValueType.SEXP, ExprToPartiQLValueType(exprValue) @@ -485,7 +499,7 @@ class ServiceLoaderUtil { Pair( it.name?.stringValue() ?: "", ExprToPartiQLValue(it, ExprToPartiQLValueType(it)) ) - }.asSequence() + } ) else -> throw ExprToPartiQLValueTypeMismatchException( PartiQLValueType.STRUCT, ExprToPartiQLValueType(exprValue) diff --git a/partiql-types/src/main/kotlin/org/partiql/value/PartiQL.kt b/partiql-types/src/main/kotlin/org/partiql/value/PartiQL.kt index adc7af49f..2f951db0f 100644 --- a/partiql-types/src/main/kotlin/org/partiql/value/PartiQL.kt +++ b/partiql-types/src/main/kotlin/org/partiql/value/PartiQL.kt @@ -38,12 +38,12 @@ import org.partiql.value.impl.Int64ValueImpl import org.partiql.value.impl.Int8ValueImpl import org.partiql.value.impl.IntValueImpl import org.partiql.value.impl.IntervalValueImpl +import org.partiql.value.impl.IterableStructValueImpl import org.partiql.value.impl.ListValueImpl import org.partiql.value.impl.MapStructValueImpl import org.partiql.value.impl.MissingValueImpl import org.partiql.value.impl.MultiMapStructValueImpl import org.partiql.value.impl.NullValueImpl -import org.partiql.value.impl.SequenceStructValueImpl import org.partiql.value.impl.SexpValueImpl import org.partiql.value.impl.StringValueImpl import org.partiql.value.impl.SymbolValueImpl @@ -344,10 +344,25 @@ public fun intervalValue( @JvmOverloads @PartiQLValueExperimental public fun bagValue( - elements: Sequence?, + elements: Iterable?, annotations: Annotations = emptyList(), ): BagValue = BagValueImpl(elements, annotations.toPersistentList()) +/** + * BAG type value. + * + * @param T + * @param elements + * @param annotations + * @return + */ +@JvmOverloads +@PartiQLValueExperimental +public fun bagValue( + vararg elements: T, + annotations: Annotations = emptyList(), +): BagValue = BagValueImpl(elements.asIterable(), annotations.toPersistentList()) + /** * LIST type value. * @@ -359,10 +374,25 @@ public fun bagValue( @JvmOverloads @PartiQLValueExperimental public fun listValue( - elements: Sequence?, + elements: Iterable?, annotations: Annotations = emptyList(), ): ListValue = ListValueImpl(elements, annotations.toPersistentList()) +/** + * LIST type value. + * + * @param T + * @param elements + * @param annotations + * @return + */ +@JvmOverloads +@PartiQLValueExperimental +public fun listValue( + vararg elements: T, + annotations: Annotations = emptyList(), +): ListValue = ListValueImpl(elements.asIterable(), annotations.toPersistentList()) + /** * SEXP type value. * @@ -374,12 +404,42 @@ public fun listValue( @JvmOverloads @PartiQLValueExperimental public fun sexpValue( - elements: Sequence?, + elements: Iterable?, annotations: Annotations = emptyList(), ): SexpValue = SexpValueImpl(elements, annotations.toPersistentList()) /** - * STRUCT type value. + * SEXP type value. + * + * @param T + * @param elements + * @param annotations + * @return + */ +@JvmOverloads +@PartiQLValueExperimental +public fun sexpValue( + vararg elements: T, + annotations: Annotations = emptyList(), +): SexpValue = SexpValueImpl(elements.asIterable(), annotations.toPersistentList()) + +/** + * Create a PartiQL struct value backed by an iterable of key-value field pairs. + * + * @param T + * @param fields + * @param annotations + * @return + */ +@JvmOverloads +@PartiQLValueExperimental +public fun structValue( + fields: Iterable>?, + annotations: Annotations = emptyList(), +): StructValue = IterableStructValueImpl(fields, annotations.toPersistentList()) + +/** + * Create a PartiQL struct value backed by an iterable of key-value field pairs. * * @param T * @param fields @@ -389,12 +449,13 @@ public fun sexpValue( @JvmOverloads @PartiQLValueExperimental public fun structValue( - fields: Sequence>?, + vararg fields: Pair, annotations: Annotations = emptyList(), -): StructValue = SequenceStructValueImpl(fields, annotations.toPersistentList()) +): StructValue = IterableStructValueImpl(fields.toList(), annotations.toPersistentList()) /** - * STRUCT type value. + * Create a PartiQL struct value backed by a multimap of keys with a list of values. This supports having multiple + * values per key, while improving lookup performance compared to using an iterable. * * @param T * @param fields @@ -403,13 +464,14 @@ public fun structValue( */ @JvmOverloads @PartiQLValueExperimental -public fun structValueWithDuplicates( +public fun structValueMultiMap( fields: Map>?, annotations: Annotations = emptyList(), ): StructValue = MultiMapStructValueImpl(fields, annotations.toPersistentList()) /** - * STRUCT type value. + * Create a PartiQL struct value backed by a map of keys with a list of values. This does not support having multiple + * values per key, but uses a Java HashMap for quicker lookup than an iterable backed StructValue. * * @param T * @param fields @@ -418,7 +480,7 @@ public fun structValueWithDuplicates( */ @JvmOverloads @PartiQLValueExperimental -public fun structValueNoDuplicates( +public fun structValueMap( fields: Map?, annotations: Annotations = emptyList(), ): StructValue = MapStructValueImpl(fields, annotations.toPersistentList()) diff --git a/partiql-types/src/main/kotlin/org/partiql/value/PartiQLValue.kt b/partiql-types/src/main/kotlin/org/partiql/value/PartiQLValue.kt index 2d9ab21d5..878cf51cd 100644 --- a/partiql-types/src/main/kotlin/org/partiql/value/PartiQLValue.kt +++ b/partiql-types/src/main/kotlin/org/partiql/value/PartiQLValue.kt @@ -65,14 +65,11 @@ public sealed interface ScalarValue : PartiQLValue { } @PartiQLValueExperimental -public sealed interface CollectionValue : PartiQLValue, Sequence { - - public val elements: Sequence? +public sealed interface CollectionValue : PartiQLValue, Iterable { override val isNull: Boolean - get() = elements == null - override fun iterator(): Iterator = elements!!.iterator() + override fun iterator(): Iterator override fun copy(annotations: Annotations): CollectionValue @@ -388,8 +385,8 @@ public abstract class BagValue : CollectionValue { if (this.isNull || other.isNull) return this.isNull == other.isNull // both not null, compare values - val lhs = this.elements!!.toList() - val rhs = other.elements!!.toList() + val lhs = this.toList() + val rhs = other.toList() // this is incorrect as it assumes ordered-ness, but we don't have a sort or hash yet return lhs == rhs } @@ -421,8 +418,8 @@ public abstract class ListValue : CollectionValue { if (this.isNull || other.isNull) return this.isNull == other.isNull // both not null, compare values - val lhs = this.elements!!.toList() - val rhs = other.elements!!.toList() + val lhs = this.toList() + val rhs = other.toList() return lhs == rhs } @@ -452,8 +449,8 @@ public abstract class SexpValue : CollectionValue { if (this.isNull || other.isNull) return this.isNull == other.isNull // both not null, compare values - val lhs = this.elements!!.toList() - val rhs = other.elements!!.toList() + val lhs = this.toList() + val rhs = other.toList() return lhs == rhs } @@ -464,16 +461,15 @@ public abstract class SexpValue : CollectionValue { } @PartiQLValueExperimental -public abstract class StructValue : PartiQLValue, Sequence> { +public abstract class StructValue : PartiQLValue { override val type: PartiQLValueType = PartiQLValueType.STRUCT - public abstract val fields: Sequence>? + public abstract val fields: Iterable - override val isNull: Boolean - get() = fields == null + public abstract val values: Iterable - override fun iterator(): Iterator> = fields!!.iterator() + public abstract val entries: Iterable> public abstract operator fun get(key: String): T? @@ -486,9 +482,7 @@ public abstract class StructValue : PartiQLValue, Sequence /** - * See equality of IonElement StructElementImpl - * - * https://github.com/amazon-ion/ion-element-kotlin/blob/master/src/com/amazon/ionelement/impl/StructElementImpl.kt + * Checks equality of struct entries, ignoring ordering. * * @param other * @return @@ -502,15 +496,15 @@ public abstract class StructValue : PartiQLValue, Sequence + lhs.entries.forEach { (key, values) -> val lGroup: Map = values.groupingBy { it }.eachCount() val rGroup: Map = rhs[key]!!.groupingBy { it }.eachCount() if (lGroup != rGroup) return false diff --git a/partiql-types/src/main/kotlin/org/partiql/value/helpers/ToIon.kt b/partiql-types/src/main/kotlin/org/partiql/value/helpers/ToIon.kt index d682b716e..e46330a1e 100644 --- a/partiql-types/src/main/kotlin/org/partiql/value/helpers/ToIon.kt +++ b/partiql-types/src/main/kotlin/org/partiql/value/helpers/ToIon.kt @@ -258,31 +258,31 @@ internal object ToIon : PartiQLValueBaseVisitor() { override fun visitInterval(v: IntervalValue, ctx: Unit): IonElement = TODO("Not Yet supported") override fun visitBag(v: BagValue<*>, ctx: Unit): IonElement = v.annotate { - when (val elements = v.elements) { - null -> ionNull(ElementType.LIST) - else -> ionListOf(elements.map { it.accept(ToIon, Unit) }.toList()) + when (v.isNull) { + true -> ionNull(ElementType.LIST) + else -> ionListOf(v.map { it.accept(ToIon, Unit) }.toList()) } }.withAnnotations(BAG_ANNOTATION) override fun visitList(v: ListValue<*>, ctx: Unit): IonElement = v.annotate { - when (val elements = v.elements) { - null -> ionNull(ElementType.LIST) - else -> ionListOf(elements.map { it.accept(ToIon, Unit) }.toList()) + when (v.isNull) { + true -> ionNull(ElementType.LIST) + else -> ionListOf(v.map { it.accept(ToIon, Unit) }.toList()) } } override fun visitSexp(v: SexpValue<*>, ctx: Unit): IonElement = v.annotate { - when (val elements = v.elements) { - null -> ionNull(ElementType.SEXP) - else -> ionSexpOf(elements.map { it.accept(ToIon, Unit) }.toList()) + when (v.isNull) { + true -> ionNull(ElementType.SEXP) + else -> ionSexpOf(v.map { it.accept(ToIon, Unit) }.toList()) } } override fun visitStruct(v: StructValue<*>, ctx: Unit): IonElement = v.annotate { - when (val fields = v.fields) { - null -> ionNull(ElementType.STRUCT) + when (v.isNull) { + true -> ionNull(ElementType.STRUCT) else -> { - val ionFields = fields.map { + val ionFields = entries.map { val fk = it.first val fv = it.second.accept(ToIon, ctx) field(fk, fv) diff --git a/partiql-types/src/main/kotlin/org/partiql/value/impl/BagValueImpl.kt b/partiql-types/src/main/kotlin/org/partiql/value/impl/BagValueImpl.kt index 98c8fbcd3..02c2d61e9 100644 --- a/partiql-types/src/main/kotlin/org/partiql/value/impl/BagValueImpl.kt +++ b/partiql-types/src/main/kotlin/org/partiql/value/impl/BagValueImpl.kt @@ -24,11 +24,13 @@ import org.partiql.value.util.PartiQLValueVisitor @OptIn(PartiQLValueExperimental::class) internal class BagValueImpl( - private val delegate: Sequence?, + private val delegate: Iterable?, override val annotations: PersistentList, ) : BagValue() { - override val elements: Sequence? = delegate + override val isNull: Boolean = delegate == null + + override fun iterator(): Iterator = delegate!!.iterator() override fun copy(annotations: Annotations) = BagValueImpl(delegate, annotations.toPersistentList()) diff --git a/partiql-types/src/main/kotlin/org/partiql/value/impl/ListValueImpl.kt b/partiql-types/src/main/kotlin/org/partiql/value/impl/ListValueImpl.kt index d7780b50b..304dadae3 100644 --- a/partiql-types/src/main/kotlin/org/partiql/value/impl/ListValueImpl.kt +++ b/partiql-types/src/main/kotlin/org/partiql/value/impl/ListValueImpl.kt @@ -24,11 +24,13 @@ import org.partiql.value.util.PartiQLValueVisitor @OptIn(PartiQLValueExperimental::class) internal class ListValueImpl( - private val delegate: Sequence?, + private val delegate: Iterable?, override val annotations: PersistentList, ) : ListValue() { - override val elements: Sequence? = delegate + override val isNull: Boolean = delegate == null + + override fun iterator(): Iterator = delegate!!.iterator() override fun copy(annotations: Annotations) = ListValueImpl(delegate, annotations.toPersistentList()) diff --git a/partiql-types/src/main/kotlin/org/partiql/value/impl/SexpValueImpl.kt b/partiql-types/src/main/kotlin/org/partiql/value/impl/SexpValueImpl.kt index 8e6838e26..c477dca3f 100644 --- a/partiql-types/src/main/kotlin/org/partiql/value/impl/SexpValueImpl.kt +++ b/partiql-types/src/main/kotlin/org/partiql/value/impl/SexpValueImpl.kt @@ -24,11 +24,13 @@ import org.partiql.value.util.PartiQLValueVisitor @OptIn(PartiQLValueExperimental::class) internal class SexpValueImpl( - private val delegate: Sequence?, + private val delegate: Iterable?, override val annotations: PersistentList, ) : SexpValue() { - override val elements: Sequence? = delegate + override val isNull: Boolean = delegate == null + + override fun iterator(): Iterator = delegate!!.iterator() override fun copy(annotations: Annotations) = SexpValueImpl(delegate, annotations.toPersistentList()) diff --git a/partiql-types/src/main/kotlin/org/partiql/value/impl/StructValueImpl.kt b/partiql-types/src/main/kotlin/org/partiql/value/impl/StructValueImpl.kt index 9e34c9c57..c36e64a14 100644 --- a/partiql-types/src/main/kotlin/org/partiql/value/impl/StructValueImpl.kt +++ b/partiql-types/src/main/kotlin/org/partiql/value/impl/StructValueImpl.kt @@ -23,19 +23,28 @@ import org.partiql.value.StructValue import org.partiql.value.util.PartiQLValueVisitor /** - * Implementation of a [StructValue] backed by a Sequence. + * Implementation of a [StructValue] backed by an iterator. * * @param T * @property delegate * @property annotations */ @OptIn(PartiQLValueExperimental::class) -internal class SequenceStructValueImpl( - private val delegate: Sequence>?, +internal class IterableStructValueImpl( + private val delegate: Iterable>?, override val annotations: PersistentList, ) : StructValue() { - override val fields: Sequence>? = delegate + override val isNull: Boolean = delegate == null + + override val fields: Iterable + get() = delegate!!.map { it.first } + + override val values: Iterable + get() = delegate!!.map { it.second } + + override val entries: Iterable> + get() = delegate!! override operator fun get(key: String): T? { if (delegate == null) { @@ -51,7 +60,7 @@ internal class SequenceStructValueImpl( return delegate.filter { it.first == key }.map { it.second }.asIterable() } - override fun copy(annotations: Annotations) = SequenceStructValueImpl(delegate, annotations.toPersistentList()) + override fun copy(annotations: Annotations) = IterableStructValueImpl(delegate, annotations.toPersistentList()) override fun withAnnotations(annotations: Annotations): StructValue = _withAnnotations(annotations) @@ -73,13 +82,14 @@ internal class MultiMapStructValueImpl( override val annotations: PersistentList, ) : StructValue() { - override val fields: Sequence>? - get() { - if (delegate == null) { - return null - } - return delegate.asSequence().map { f -> f.value.map { v -> f.key to v } }.flatten() - } + override val isNull: Boolean = delegate == null + + override val fields: Iterable = delegate!!.map { it.key } + + override val values: Iterable = delegate!!.flatMap { it.value } + + override val entries: Iterable> = + delegate!!.entries.map { f -> f.value.map { v -> f.key to v } }.flatten() override operator fun get(key: String): T? = getAll(key).firstOrNull() @@ -112,13 +122,13 @@ internal class MapStructValueImpl( override val annotations: PersistentList, ) : StructValue() { - override val fields: Sequence>? - get() { - if (delegate == null) { - return null - } - return delegate.asSequence().map { f -> f.key to f.value } - } + override val isNull: Boolean = delegate == null + + override val fields: Iterable = delegate!!.map { it.key } + + override val values: Iterable = delegate!!.map { it.value } + + override val entries: Iterable> = delegate!!.entries.map { it.key to it.value } override operator fun get(key: String): T? { if (delegate == null) { diff --git a/partiql-types/src/main/kotlin/org/partiql/value/io/PartiQLValueIonReader.kt b/partiql-types/src/main/kotlin/org/partiql/value/io/PartiQLValueIonReader.kt index 37cdf54c9..732738ffe 100644 --- a/partiql-types/src/main/kotlin/org/partiql/value/io/PartiQLValueIonReader.kt +++ b/partiql-types/src/main/kotlin/org/partiql/value/io/PartiQLValueIonReader.kt @@ -167,7 +167,7 @@ internal class PartiQLValueIonReader( } } reader.stepOut() - listValue(elements.asSequence(), annotations) + listValue(elements, annotations) } IonType.SEXP -> { @@ -179,7 +179,7 @@ internal class PartiQLValueIonReader( } } reader.stepOut() - sexpValue(elements.asSequence(), annotation) + sexpValue(elements, annotation) } IonType.STRUCT -> { @@ -192,7 +192,7 @@ internal class PartiQLValueIonReader( } } reader.stepOut() - structValue(elements.asSequence(), annotations) + structValue(elements, annotations) } IonType.DATAGRAM -> throw IllegalArgumentException("Datagram not supported") @@ -394,7 +394,7 @@ internal class PartiQLValueIonReader( } } reader.stepOut() - bagValue(elements.asSequence(), annotations.dropLast(1)) + bagValue(elements, annotations.dropLast(1)) } } PARTIQL_ANNOTATION.DATE_ANNOTATION -> throw IllegalArgumentException("DATE_ANNOTATION with List Value") @@ -412,7 +412,7 @@ internal class PartiQLValueIonReader( } } reader.stepOut() - listValue(elements.asSequence(), annotations) + listValue(elements, annotations) } } } @@ -437,7 +437,7 @@ internal class PartiQLValueIonReader( } } reader.stepOut() - sexpValue(elements.asSequence(), annotations) + sexpValue(elements, annotations) } } } @@ -525,8 +525,7 @@ internal class PartiQLValueIonReader( PARTIQL_ANNOTATION.GRAPH_ANNOTATION -> TODO("Not yet implemented") null -> { if (reader.isNullValue) { - val nullSequence: Sequence>? = null - structValue(nullSequence, annotations) + structValue(null, annotations) } else { reader.stepIn() val elements = mutableListOf>().also { elements -> @@ -536,7 +535,7 @@ internal class PartiQLValueIonReader( } } reader.stepOut() - structValue(elements.asSequence(), annotations) + structValue(elements, annotations) } } } diff --git a/partiql-types/src/main/kotlin/org/partiql/value/io/PartiQLValueTextWriter.kt b/partiql-types/src/main/kotlin/org/partiql/value/io/PartiQLValueTextWriter.kt index 75d59c06f..e0d1ca63b 100644 --- a/partiql-types/src/main/kotlin/org/partiql/value/io/PartiQLValueTextWriter.kt +++ b/partiql-types/src/main/kotlin/org/partiql/value/io/PartiQLValueTextWriter.kt @@ -199,14 +199,17 @@ public class PartiQLValueTextWriter( override fun visitSexp(v: SexpValue<*>, format: Format?) = collection(v, format, "(" to ")", " ") override fun visitStruct(v: StructValue<*>, format: Format?): String = buildString { + if (v.isNull) { + return "null" + } // null.struct - val fields = v.fields?.toList() ?: return "null" + val entries = v.entries.toList() // {} - if (fields.isEmpty() || format == null) { + if (entries.isEmpty() || format == null) { format?.let { append(it.prefix) } annotate(v, this) append("{") - val items = fields.map { + val items = entries.map { val fk = it.first val fv = it.second.accept(ToString, null) // it.toString() "$fk:$fv" @@ -219,10 +222,10 @@ public class PartiQLValueTextWriter( append(format.prefix) annotate(v, this) appendLine("{") - fields.forEachIndexed { i, e -> + entries.forEachIndexed { i, e -> val fk = e.first val fv = e.second.accept(ToString, format.nest()).trimStart() // e.toString(format) - val suffix = if (i == fields.size - 1) "" else "," + val suffix = if (i == entries.size - 1) "" else "," append(format.prefix + format.indent) append("$fk: $fv") appendLine(suffix) @@ -238,7 +241,10 @@ public class PartiQLValueTextWriter( separator: CharSequence = ",", ) = buildString { // null.bag, null.list, null.sexp - val elements = v.elements?.toList() ?: return "null" + if (v.isNull) { + return "null" + } + val elements = v.toList() // skip empty if (elements.isEmpty() || format == null) { format?.let { append(it.prefix) } diff --git a/partiql-types/src/main/kotlin/org/partiql/value/util/PartiQLValueBaseVisitor.kt b/partiql-types/src/main/kotlin/org/partiql/value/util/PartiQLValueBaseVisitor.kt index 9db4b742b..41e4bdba1 100644 --- a/partiql-types/src/main/kotlin/org/partiql/value/util/PartiQLValueBaseVisitor.kt +++ b/partiql-types/src/main/kotlin/org/partiql/value/util/PartiQLValueBaseVisitor.kt @@ -53,10 +53,14 @@ public abstract class PartiQLValueBaseVisitor : PartiQLValueVisitor public open fun defaultVisit(v: PartiQLValue, ctx: C): R { when (v) { is CollectionValue<*> -> { - v.elements?.forEach { it?.accept(this, ctx) } + if (!v.isNull) { + v.forEach { it.accept(this, ctx) } + } } is StructValue<*> -> { - v.fields?.forEach { it.second.accept(this, ctx) } + if (!v.isNull) { + v.entries.forEach { it.second.accept(this, ctx) } + } } else -> {} } diff --git a/partiql-types/src/test/kotlin/org/partiql/value/io/PartiQLValueIonSerdeTest.kt b/partiql-types/src/test/kotlin/org/partiql/value/io/PartiQLValueIonSerdeTest.kt index 621344666..a2a7f0dcc 100644 --- a/partiql-types/src/test/kotlin/org/partiql/value/io/PartiQLValueIonSerdeTest.kt +++ b/partiql-types/src/test/kotlin/org/partiql/value/io/PartiQLValueIonSerdeTest.kt @@ -360,24 +360,22 @@ class PartiQLValueIonSerdeTest { @JvmStatic fun collections() = listOf( roundTrip( - bagValue(emptySequence()), + bagValue(), ION.newEmptyList().apply { addTypeAnnotation("\$bag") }, ), roundTrip( - listValue(emptySequence()), + listValue(), ION.newEmptyList() ), roundTrip( - sexpValue(emptySequence()), + sexpValue(), ION.newEmptySexp() ), oneWayTrip( bagValue( - sequenceOf( - int32Value(1), - int32Value(2), - int32Value(3), - ) + int32Value(1), + int32Value(2), + int32Value(3), ), ION.newList( ION.newInt(1), @@ -385,20 +383,16 @@ class PartiQLValueIonSerdeTest { ION.newInt(3) ).apply { addTypeAnnotation("\$bag") }, bagValue( - sequenceOf( - intValue(BigInteger.ONE), - intValue(BigInteger.valueOf(2L)), - intValue(BigInteger.valueOf(3L)), - ) + intValue(BigInteger.ONE), + intValue(BigInteger.valueOf(2L)), + intValue(BigInteger.valueOf(3L)), ) ), roundTrip( listValue( - sequenceOf( - stringValue("a"), - stringValue("b"), - stringValue("c"), - ) + stringValue("a"), + stringValue("b"), + stringValue("c"), ), ION.newList( ION.newString("a"), @@ -408,11 +402,9 @@ class PartiQLValueIonSerdeTest { ), oneWayTrip( sexpValue( - sequenceOf( - int32Value(1), - int32Value(2), - int32Value(3), - ) + int32Value(1), + int32Value(2), + int32Value(3), ), ION.newSexp( ION.newInt(1), @@ -420,19 +412,15 @@ class PartiQLValueIonSerdeTest { ION.newInt(3) ), sexpValue( - sequenceOf( - intValue(BigInteger.ONE), - intValue(BigInteger.valueOf(2L)), - intValue(BigInteger.valueOf(3L)), - ) + intValue(BigInteger.ONE), + intValue(BigInteger.valueOf(2L)), + intValue(BigInteger.valueOf(3L)), ) ), oneWayTrip( structValue( - sequenceOf( - "a" to int32Value(1), - "b" to stringValue("x"), - ) + "a" to int32Value(1), + "b" to stringValue("x"), ), ION.newEmptyStruct() .apply { @@ -440,10 +428,8 @@ class PartiQLValueIonSerdeTest { add("b", ION.newString("x")) }, structValue( - sequenceOf( - "a" to intValue(BigInteger.ONE), - "b" to stringValue("x"), - ) + "a" to intValue(BigInteger.ONE), + "b" to stringValue("x"), ), ) ) diff --git a/partiql-types/src/test/kotlin/org/partiql/value/io/PartiQLValueTextWriterTest.kt b/partiql-types/src/test/kotlin/org/partiql/value/io/PartiQLValueTextWriterTest.kt index bf4531e6f..309d832a6 100644 --- a/partiql-types/src/test/kotlin/org/partiql/value/io/PartiQLValueTextWriterTest.kt +++ b/partiql-types/src/test/kotlin/org/partiql/value/io/PartiQLValueTextWriterTest.kt @@ -322,44 +322,38 @@ class PartiQLValueTextWriterTest { @JvmStatic fun collections() = listOf( case( - value = bagValue(emptySequence()), + value = bagValue(), expected = "<<>>", ), case( - value = listValue(emptySequence()), + value = listValue(), expected = "[]", ), case( - value = sexpValue(emptySequence()), + value = sexpValue(), expected = "()", ), case( value = bagValue( - sequenceOf( - int32Value(1), - int32Value(2), - int32Value(3), - ) + int32Value(1), + int32Value(2), + int32Value(3), ), expected = "<<1,2,3>>", ), case( value = listValue( - sequenceOf( - stringValue("a"), - stringValue("b"), - stringValue("c"), - ) + stringValue("a"), + stringValue("b"), + stringValue("c"), ), expected = "['a','b','c']", ), case( value = sexpValue( - sequenceOf( - int32Value(1), - int32Value(2), - int32Value(3), - ) + int32Value(1), + int32Value(2), + int32Value(3), ), expected = "(1 2 3)", ), @@ -385,15 +379,13 @@ class PartiQLValueTextWriterTest { @JvmStatic fun struct() = listOf( case( - value = structValue(emptySequence()), + value = structValue(), expected = "{}", ), case( value = structValue( - sequenceOf( - "a" to int32Value(1), - "b" to stringValue("x"), - ) + "a" to int32Value(1), + "b" to stringValue("x"), ), expected = "{a:1,b:'x'}", ), @@ -403,11 +395,9 @@ class PartiQLValueTextWriterTest { fun collectionsFormatted() = listOf( formatted( value = bagValue( - sequenceOf( - int32Value(1), - int32Value(2), - int32Value(3), - ) + int32Value(1), + int32Value(2), + int32Value(3), ), expected = """ |<< @@ -419,11 +409,9 @@ class PartiQLValueTextWriterTest { ), formatted( value = listValue( - sequenceOf( - stringValue("a"), - stringValue("b"), - stringValue("c"), - ) + stringValue("a"), + stringValue("b"), + stringValue("c"), ), expected = """ |[ @@ -435,11 +423,9 @@ class PartiQLValueTextWriterTest { ), formatted( value = sexpValue( - sequenceOf( - int32Value(1), - int32Value(2), - int32Value(3), - ) + int32Value(1), + int32Value(2), + int32Value(3), ), expected = """ |( @@ -454,15 +440,13 @@ class PartiQLValueTextWriterTest { @JvmStatic fun structFormatted() = listOf( formatted( - value = structValue(emptySequence()), + value = structValue(), expected = "{}", ), formatted( value = structValue( - sequenceOf( - "a" to int32Value(1), - "b" to stringValue("x"), - ) + "a" to int32Value(1), + "b" to stringValue("x"), ), expected = """ |{ @@ -477,29 +461,21 @@ class PartiQLValueTextWriterTest { fun nestedCollectionsFormatted() = listOf( formatted( value = structValue( - sequenceOf( - "bag" to bagValue( - sequenceOf( - int32Value(1), - int32Value(2), - int32Value(3), - ) - ), - "list" to listValue( - sequenceOf( - stringValue("a"), - stringValue("b"), - stringValue("c"), - ) - ), - "sexp" to sexpValue( - sequenceOf( - int32Value(1), - int32Value(2), - int32Value(3), - ) - ), - ) + "bag" to bagValue( + int32Value(1), + int32Value(2), + int32Value(3), + ), + "list" to listValue( + stringValue("a"), + stringValue("b"), + stringValue("c"), + ), + "sexp" to sexpValue( + int32Value(1), + int32Value(2), + int32Value(3), + ), ), expected = """ |{ @@ -523,28 +499,20 @@ class PartiQLValueTextWriterTest { ), formatted( value = bagValue( - sequenceOf( - listValue( - sequenceOf( - stringValue("a"), - stringValue("b"), - stringValue("c"), - ) - ), - sexpValue( - sequenceOf( - int32Value(1), - int32Value(2), - int32Value(3), - ) - ), - structValue( - sequenceOf( - "a" to int32Value(1), - "b" to stringValue("x"), - ) - ), - ) + listValue( + stringValue("a"), + stringValue("b"), + stringValue("c"), + ), + sexpValue( + int32Value(1), + int32Value(2), + int32Value(3), + ), + structValue( + "a" to int32Value(1), + "b" to stringValue("x"), + ), ), expected = """ |<< @@ -567,11 +535,9 @@ class PartiQLValueTextWriterTest { ), formatted( value = structValue( - sequenceOf( - "bag" to bagValue(emptySequence()), - "list" to listValue(emptySequence()), - "sexp" to sexpValue(emptySequence()), - ) + "bag" to bagValue(), + "list" to listValue(), + "sexp" to sexpValue(), ), expected = """ |{ @@ -583,11 +549,9 @@ class PartiQLValueTextWriterTest { ), formatted( value = bagValue( - sequenceOf( - listValue(emptySequence()), - sexpValue(emptySequence()), - structValue(emptySequence()), - ) + listValue(), + sexpValue(), + structValue(), ), expected = """ |<< @@ -690,39 +654,31 @@ class PartiQLValueTextWriterTest { // TODO TIMESTAMP // TODO INTERVAL case( - value = bagValue(emptySequence(), annotations), + value = bagValue(annotations = annotations), expected = "x::y::<<>>", ), case( - value = listValue(emptySequence(), annotations), + value = listValue(annotations = annotations), expected = "x::y::[]", ), case( - value = sexpValue(emptySequence(), annotations), + value = sexpValue(annotations = annotations), expected = "x::y::()", ), formatted( value = bagValue( - sequenceOf( - listValue( - sequenceOf( - stringValue("a", listOf("x")), - ), - listOf("list") - ), - sexpValue( - sequenceOf( - int32Value(1, listOf("y")), - ), - listOf("sexp") - ), - structValue( - sequenceOf( - "a" to int32Value(1, listOf("z")), - ), - listOf("struct") - ), - ) + listValue( + stringValue("a", listOf("x")), + annotations = listOf("list") + ), + sexpValue( + int32Value(1, listOf("y")), + annotations = listOf("sexp") + ), + structValue( + "a" to int32Value(1, listOf("z")), + annotations = listOf("struct") + ), ), expected = """ |<<