Bug: New buffer constructor missing empty buffer validation

The newAqlValue(velocypack::Buffer<uint8_t> const& buffer, ResourceMonitor*) constructor is missing theTRI_ASSERT(len >= 1) validation that exists in the equivalent move constructor at line 1337. If an empty buffer is passed, creating aVPackSlice frombuffer.data() would read from potentially invalid memory, causing undefined behavior. The move constructor properly validates withTRI_ASSERT(size >= 1),TRI_ASSERT(size == slice.byteSize()), andTRI_ASSERT(!slice.isExternal()), but these checks are absent from the new const-ref overload.

Bug: Missing input validation in const buffer constructor

The newAqlValue(velocypack::Buffer<uint8_t> const& buffer, ResourceMonitor*) constructor is missing the input validation assertions that the analogous move constructor has. The move constructor (lines 1336-1340) includesTRI_ASSERT(size >= 1),TRI_ASSERT(size == slice.byteSize()), andTRI_ASSERT(!slice.isExternal()). The new const reference constructor directly callsinitFromSlice without any validation. If an empty buffer is passed, this could lead to undefined behavior when creating aVPackSlice from potentially null data and callinginitFromSlice with length zero.

mot should never be anything other thanNew. I would actually prefer to remove the parameter and verify this on the caller side if appropriate.

These should remain unchanged - we still need AqlValues to be trivially copyable.

Why do we introduce these operators as part of this PR?

As previously mentioned, memory origin for our supervised values must always beNew. We can assert that here, but should otherwise not care about it. In particularsetSupervisedType should not take a MemoryOriginType parameter.

Why do we now have a default value for length?
I would rather leave the constructor that takes just a slice and also extend that one to take the ResourceMonitor. It is only forwarding to the one that takes the length, which we can still do with the ResourceMonitor. That way we can also simplify a lot of places where we currently explicitly passbyteSize.

Bug: Tests pass invalid length=0 causing assertion failure and incorrect behavior

The constructorAqlValue(slice, length, rm) at line 1399 assertsTRI_ASSERT(length >= 1), but numerous tests pass0 for length (e.g.,AqlValue a1(sliceA, 0, &resourceMonitor)). In debug builds this assertion fails. In release builds,initFromSlice doesn't properly handlelength == 0: it skips the supervised/managed path (since0 > 15 is false) and memcpys 0 bytes into the inline buffer, leaving garbage data. Theif (length > 0) check at line 1491 suggests0 was intended to mean "auto-calculate from slice.byteSize()", but this logic is missing. Tests expectVPACK_SUPERVISED_SLICE but would getVPACK_INLINE with corrupted content.

Bug: Hash and equality contract violated for cross-type comparisons

Theequal_to<AqlValue> implementation allows content-based equality betweenVPACK_SUPERVISED_SLICE andVPACK_MANAGED_SLICE/VPACK_MANAGED_STRING types, but thehash<AqlValue> uses pointer-based hashing for all these types. This violates the fundamental contract that equal objects must have the same hash. A managed slice and supervised slice with identical content will compare equal viabinaryEquals, but have different hashes since they point to different memory addresses. When used instd::unordered_set<AqlValue> (used inInputAqlItemRow::cloneToBlock), this can cause elements to not be found or duplicate entries to appear.

Bug: Private function default parameter creates unsafe API

The privateinitFromSlice function now has a defaultlength = 0 parameter, but the implementation doesn't auto-compute the length from the slice when 0 is passed. If called without an explicit length,memcpy copies 0 bytes and leaves the inline slice data uninitialized while setting the type toVPACK_INLINE. The old signature required length explicitly. All current callers pass valid lengths, but the default creates a dangerous API where future code relying on it would produce corruptedAqlValue objects containing garbage data for arrays, objects, and strings.

Bug: Hash/equality invariant violation for cross-type AqlValue comparison

Theequal_to<AqlValue> operator now allowsVPACK_SUPERVISED_SLICE to compare equal toVPACK_MANAGED_SLICE/VPACK_MANAGED_STRING based on content viabinaryEquals. However,hash<AqlValue> uses pointer identity for these types. This violates the hash/equality invariant: ifa == b, thenhash(a) == hash(b). When supervised and managed slices with identical content are used instd::unordered_set<AqlValue> (e.g., inInputAqlItemRow::cloneToBlock), lookups will fail because they hash to different buckets despite being equal.

Bug: Hash-equality contract violation for cross-type comparisons

Thestd::equal_to<AqlValue> specialization allows content-based equality betweenVPACK_SUPERVISED_SLICE andVPACK_MANAGED_SLICE/VPACK_MANAGED_STRING types, butstd::hash<AqlValue> uses pointer-based hashing for all these types. This violates the C++ standard requirement that ifa == b thenhash(a) == hash(b). The codebase usesstd::unordered_set<AqlValue> inInputAqlItemRow::cloneToBlock(), where this inconsistency can cause duplicate elements that should be deduplicated, or failed lookups for elements that exist in the set.