core/src/main/java/io/questdb/griffin/engine/functions/bool/InLongFunctionFactory.java (1)

120-121: Avoid double load‑factor scaling when constructing DirectLongHashSet

DirectLongHashSet’s ctor already scales its capacity argument by 1 / loadFactor:

this.capacity = Math.max(Numbers.ceilPow2((int) (capacity / loadFactor)), MIN_CAPACITY);

But callers here pre-scale as well:

DirectLongHashSet inVals =
    new DirectLongHashSet((int) (argCount / DirectLongHashSet.DEFAULT_LOAD_FACTOR),
                          DirectLongHashSet.DEFAULT_LOAD_FACTOR,
                          MemoryTag.NATIVE_FUNC_RSS);

this.inSet =
    new DirectLongHashSet((int) ((valueFunctions.size() - 1) / DirectLongHashSet.DEFAULT_LOAD_FACTOR),
                          DirectLongHashSet.DEFAULT_LOAD_FACTOR,
                          MemoryTag.NATIVE_FUNC_RSS);

This effectively applies the 1 / loadFactor factor twice, significantly over-allocating off-heap memory for large IN-lists.

If the ctor’s capacity parameter is intended to be “expected element count” (which matches the name), callers should pass the raw element count and let the ctor do the scaling:

- DirectLongHashSet inVals = new DirectLongHashSet(
-         (int) (argCount / DirectLongHashSet.DEFAULT_LOAD_FACTOR),
-         DirectLongHashSet.DEFAULT_LOAD_FACTOR,
-         MemoryTag.NATIVE_FUNC_RSS);
+ DirectLongHashSet inVals = new DirectLongHashSet(
+         argCount,
+         DirectLongHashSet.DEFAULT_LOAD_FACTOR,
+         MemoryTag.NATIVE_FUNC_RSS);
...
- this.inSet = new DirectLongHashSet(
-         (int) ((valueFunctions.size() - 1) / DirectLongHashSet.DEFAULT_LOAD_FACTOR),
-         DirectLongHashSet.DEFAULT_LOAD_FACTOR,
-         MemoryTag.NATIVE_FUNC_RSS);
+ this.inSet = new DirectLongHashSet(
+         valueFunctions.size() - 1,
+         DirectLongHashSet.DEFAULT_LOAD_FACTOR,
+         MemoryTag.NATIVE_FUNC_RSS);

This keeps the load factor behavior while avoiding unnecessary off-heap over-allocation.

Also applies to: 230-231

core/src/test/java/io/questdb/test/std/DirectLongHashSetTest.java (1)

220-231: Minor: avoid fully‑qualified StringSink where it’s already imported

Here you can rely on the existing import instead of the fully qualified name:

-            io.questdb.std.str.StringSink sink = new io.questdb.std.str.StringSink();
+            StringSink sink = new StringSink();

This keeps the test code a bit cleaner and consistent with the other tests using StringSink.

core/src/main/java/io/questdb/std/DirectLongHashSet.java (1)

37-70: DirectLongHashSet implementation looks correct; only minor API/robustness nits

Algorithmically this is solid:

• Open addressing with linear probing and backward‑shift deletion (removeAt + canMoveBack) is implemented correctly.
• Zero is handled via hasZero so it doesn’t conflict with the internal 0‑as‑empty sentinel.
• size(), clear(), rehash(), and toSink() all behave consistently with that design, including coverage for sentinel values and ordering (as validated in the tests).

Two small nits to consider:

1. Clarify/lock in ctor semantics

   The ctor currently interprets its capacity argument as an expected element count and scales it by 1 / loadFactor internally. It would be good to document this explicitly in the Javadoc (and keep callers from pre‑scaling), e.g., “capacity is the expected number of elements”.

2. Optional: guard against use after close()

   Methods like add, contains, remove, and keyIndex assume memStart != 0. If someone accidentally calls them after close(), they’ll operate on addresses derived from 0. Adding a lightweight guard (e.g., if (memStart == 0) throw new IllegalStateException("closed")) would make misuse fail fast instead of doing undefined off‑heap access. This is optional but can help future refactors.

Also applies to: 83-103, 115-188, 190-257, 259-285

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Learnt from: jerrinot
Repo: questdb/questdb PR: 6413
File: core/src/test/java/io/questdb/test/cutlass/pgwire/PGJobContextTest.java:11982-12002
Timestamp: 2025-11-19T12:21:00.062Z
Learning: QuestDB Java tests use a deterministic random seed. The test utilities (e.g., io.questdb.test.tools.TestUtils and io.questdb.std.Rnd) produce reproducible sequences, so rnd_* functions (including rnd_uuid4) yield deterministic outputs across runs. Do not flag tests in core/src/test/** that assert against values produced by rnd_* as flaky due to randomness.

Learnt from: mtopolnik
Repo: questdb/questdb PR: 5997
File: core/src/main/java/io/questdb/griffin/engine/functions/array/DoubleArrayAccessFunctionFactory.java:127-131
Timestamp: 2025-07-31T08:21:17.390Z
Learning: In DoubleArrayAccessFunctionFactory.java, overflow validation for array index casting from long to int is performed in the validateIndexArgs() method rather than at each individual cast site. The validateIndexArgs() method is called early in newInstance() to validate all index arguments, making subsequent casts safe.