SavicStefan commented on code in PR #59669:
URL: https://github.com/apache/spark/pull/59669#discussion_r4452619863
##########
common/variant/src/main/java/org/apache/spark/types/variant/VariantBuilder.java:
##########
@@ -205,6 +205,79 @@ public static Variant pickAtPaths(Variant v, PickNode
tree) {
return builder.result();
}
+ // Return v in canonical form: only referenced dictionary keys, unique and
sorted by unsigned
+ // UTF-8 bytes; object fields stored in key order; minimal integer, decimal,
and offset widths;
+ // strings of at most 63 UTF-8 bytes use short-string encoding; decimal
trailing zeros are
+ // stripped, and integral decimals become longs only when they fit.
Normalize floating-point
+ // -0.0 to 0.0 and NaNs within each type. Preserve string and key bytes,
including invalid UTF-8.
+ // Return v itself if it is already canonical.
+ public static Variant canonicalize(Variant v) {
+ if (isCanonical(v)) {
+ return v;
+ }
+ return doCanonicalize(v);
+ }
+
+ // Check whether v is already canonically encoded. Return false at the first
+ // non-canonical encoding. Malformed bytes also return false so that the
rebuild decides whether
+ // they fail, but an unreadable header may still throw. Returning false does
not establish
+ // validity.
+ static boolean isCanonical(Variant v) {
+ if (v.pos != 0) {
+ return false;
+ }
+ byte[] value = v.value;
+ byte[] metadata = v.metadata;
+ int offsetSize = getMetadataOffsetSize(metadata);
+ int numKeys = getMetadataNumKeys(metadata, offsetSize);
+ int stringStart = getMetadataStringStart(metadata, numKeys, offsetSize);
+ int lastOffset = readUnsigned(metadata, stringStart - offsetSize,
offsetSize);
+ if (lastOffset > metadata.length - stringStart) {
+ return false;
+ }
+ int canonicalOffsetSize = getIntegerSize(Math.max(lastOffset, numKeys));
+ if (metadata[0] != metadataHeader(canonicalOffsetSize)) {
+ return false;
+ }
+ int previousOffset = readUnsigned(metadata, 1 + offsetSize, offsetSize);
+ if (previousOffset != 0) {
+ return false;
+ }
+ int currentOffset = previousOffset;
+ for (int keyId = 0; keyId < numKeys; ++keyId) {
+ int nextOffset = readUnsigned(metadata, 1 + (keyId + 2) * offsetSize,
offsetSize);
+ if (currentOffset > nextOffset || nextOffset > metadata.length -
stringStart) {
Review Comment:
we need this because of the:
```scala
Arrays.compareUnsigned(
metadata, stringStart + previousOffset, stringStart +
currentOffset,
metadata, stringStart + currentOffset, stringStart + nextOffset)
```
and this `stringStart + nextOffset` where we read the metadata key bytes and
that means that if we don't make a check we will read out of bound
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
