iemejia commented on code in PR #3866:
URL: https://github.com/apache/avro/pull/3866#discussion_r3567622399
##########
lang/js/lib/schemas.js:
##########
@@ -2153,6 +2246,87 @@ function readArraySize(tap) {
return n;
}
+/**
+ * Minimum number of bytes a value of the given type can occupy on the wire.
+ *
+ * Used to bound collection block counts against the bytes actually remaining.
+ * Memoized on the type; recursive records are handled with a `seen` guard
+ * (a directly self-referential required field cannot encode in finite bytes,
+ * and self-references reached through an array/map/union already contribute at
+ * least one byte before recursing, so returning 0 on a cycle is safe).
+ */
+function getMinBytes(type, seen) {
+ if (type._minBytes !== undefined) {
+ return type._minBytes;
+ }
+ var mb;
+ if (type instanceof NullType) {
+ mb = 0;
+ } else if (type instanceof FixedType) {
+ mb = type._size;
+ } else if (type instanceof FloatType) {
+ mb = 4;
+ } else if (type instanceof DoubleType) {
+ mb = 8;
+ } else if (type instanceof LogicalType) {
+ mb = getMinBytes(type._underlyingType, seen);
+ } else if (type instanceof UnionType) {
+ // A 1-byte branch index plus the smallest branch encoding. A union with a
+ // zero-byte branch (e.g. null) still occupies the index byte, so the
+ // minimum is 1; a union without one (e.g. ['int','long']) is at least 2.
+ var branches = type._types;
+ var branchMin = branches.length ? Infinity : 0;
+ for (var k = 0, bl = branches.length; k < bl; k++) {
+ branchMin = Math.min(branchMin, getMinBytes(branches[k], seen));
+ if (branchMin === 0) {
+ break;
+ }
+ }
+ mb = 1 + branchMin;
+ } else if (type instanceof RecordType) {
+ seen = seen || [];
+ if (~seen.indexOf(type)) {
+ return 0; // Cycle; see note above.
+ }
Review Comment:
This is a deliberate conservative lower bound: for a cyclic schema whose
true minimum can't be computed without unbounded recursion, returning 0 can
only make the bytes-remaining check *more permissive* (it never falsely rejects
valid data) — matching how the other SDKs treat recursion. Computing the exact
minimum for a recursive union branch isn't decidable in general here. I've
reworded the docstring to state this explicitly rather than implying the bound
is tight.
##########
lang/js/lib/schemas.js:
##########
@@ -2153,6 +2246,87 @@ function readArraySize(tap) {
return n;
}
+/**
+ * Minimum number of bytes a value of the given type can occupy on the wire.
+ *
+ * Used to bound collection block counts against the bytes actually remaining.
+ * Memoized on the type; recursive records are handled with a `seen` guard
+ * (a directly self-referential required field cannot encode in finite bytes,
+ * and self-references reached through an array/map/union already contribute at
+ * least one byte before recursing, so returning 0 on a cycle is safe).
Review Comment:
Reworded the docstring — it now says the 0-on-cycle result is a conservative
lower bound that may under-estimate (e.g. a union whose only small branch is a
recursive record) but only ever makes the check more permissive, never
rejecting valid data.
--
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]
