.gitignore (1)

38-39: Reconsider ignoring Cargo.lock; keep for reproducible Rust builds.

For Rust binaries/tools, committing Cargo.lock is recommended to ensure deterministic builds. If parquet-tools is a binary crate (likely), remove the ignore; if it’s a library, keeping it ignored is fine—please confirm intent. Also, align trailing slash style with nearby entries for consistency.

Apply this diff if you decide to keep the lockfile and normalize the path style:

-core/rust/qdbr/parquet2/parquet-tools/target/
-core/rust/qdbr/parquet2/parquet-tools/Cargo.lock
+core/rust/qdbr/parquet2/parquet-tools/target

core/src/test/java/io/questdb/test/griffin/CopyTest.java (1)

1167-1168: Make result ordering explicit to avoid future flakiness.

Relying on implicit row order + negative LIMIT can become brittle if planner/storage details change. Consider ordering by the timestamp.

Apply if it matches current semantics:

-                "select line,ts,d,description from x limit -10"
+                "select line,ts,d,description from x order by ts limit -10"

Please run the test suite to verify negative LIMIT behavior with ORDER BY remains consistent in your QuestDB version.

core/src/test/java/io/questdb/test/cairo/TableNameRegistryTest.java (2)

135-139: Also tolerate "could not remove table" in testConcurrentCreateDrop to avoid flakiness.

The same transient condition can surface here during concurrent drops.

Apply this diff:

-                                    if (!Chars.contains(e.getFlyweightMessage(), "table does not exist")
-                                            && !Chars.contains(e.getFlyweightMessage(), "could not lock")
-                                            && !Chars.contains(e.getFlyweightMessage(), "table name is reserved")) {
+                                    if (!Chars.contains(e.getFlyweightMessage(), "table does not exist")
+                                            && !Chars.contains(e.getFlyweightMessage(), "could not lock")
+                                            && !Chars.contains(e.getFlyweightMessage(), "table name is reserved")
+                                            && !Chars.contains(e.getFlyweightMessage(), "could not remove table")) {
                                         throw e;
                                     }

---

210-210: Deterministic RNG seeds: consider hoisting to named constants.

Improves readability and reuse across tests.

Example outside this hunk:

private static final long SEED1 = 3095520132513L;
private static final long SEED2 = 1758309268883L;
...
Rnd rnd = TestUtils.generateRandom(LOG, SEED1, SEED2);

core/pom.xml (1)

35-35: Enforce JDK 17, upgrade compiler plugin, and switch to to avoid build/toolchain skew

3.8.1 can compile with JDK 17 (it delegates to javac), but upgrade to a recent maven-compiler-plugin (3.10.1+; 3.11.0 shown) and use while updating the Java profile to require JDK 17.

@@
-                <artifactId>maven-compiler-plugin</artifactId>
-                <version>3.8.1</version>
+                <artifactId>maven-compiler-plugin</artifactId>
+                <version>3.11.0</version>
@@
-                    <source>${javac.compile.source}</source>
-                    <target>${javac.compile.target}</target>
+                    <release>${javac.compile.source}</release>

@@
-        <profile>
-            <id>java11+</id>
+        <profile>
+            <id>java11+</id> <!-- keep id if referenced externally -->
             <properties>
-                <jdk.version>11</jdk.version>
-                <java.enforce.version>11</java.enforce.version>
+                <jdk.version>17</jdk.version>
+                <java.enforce.version>17</java.enforce.version>
@@
-            <activation>
-                <jdk>(1.8,)</jdk>
-            </activation>
+            <activation>
+                <jdk>(16,)</jdk> <!-- require 17+ -->
+            </activation>

core/src/test/java/io/questdb/test/cairo/mv/MatViewIdenticalReplaceTest.java (2)

174-176: Avoid regex in timestamp replacement

replaceAll(".000000Z", ".000000000Z") treats . as “any char”. Use a literal replacement to prevent accidental matches and avoid regex overhead.

Apply this diff:

-    return ColumnType.isTimestampMicro(timestampType.getDriver().getTimestampType()) ? expected : expected.replaceAll(".000000Z", ".000000000Z");
+    return ColumnType.isTimestampMicro(timestampType.getDriver().getTimestampType())
+            ? expected
+            : expected.replace(".000000Z", ".000000000Z");

---

169-172: Minor: use literal replace, not replaceAll, for #TIMESTAMP

No regex needed here; using replace avoids regex semantics and is a tad faster.

-        sqlText = sqlText.toString().replaceAll("#TIMESTAMP", timestampType.getTypeName());
+        sqlText = sqlText.toString().replace("#TIMESTAMP", timestampType.getTypeName());

core/src/test/java/io/questdb/test/cairo/FullFwdPartitionFrameCursorTest.java (2)

544-545: Parallel YEAR tests: same 1972.3 suffix — apply the same de‑brittling

Mirror the constant/API approach here to keep the suite consistent and reduce future churn.

Apply the same constant:

-        testParallelIndexFailureAtRuntime(PartitionBy.YEAR, 10000000L * 30 * 12, false, "1972.3" + Files.SEPARATOR + "a.v", 2);
+        testParallelIndexFailureAtRuntime(PartitionBy.YEAR, 10000000L * 30 * 12, false, YEAR_1972_DIR + Files.SEPARATOR + "a.v", 2);
@@
-        testParallelIndexFailureAtRuntime(PartitionBy.YEAR, 10000000L * 30 * 12, false, "1972.3" + Files.SEPARATOR + "b.v", 2);
+        testParallelIndexFailureAtRuntime(PartitionBy.YEAR, 10000000L * 30 * 12, false, YEAR_1972_DIR + Files.SEPARATOR + "b.v", 2);
@@
-        testParallelIndexFailureAtRuntime(PartitionBy.YEAR, 10000000L * 30 * 12, false, "1972.3" + Files.SEPARATOR + "c.v", 2);
+        testParallelIndexFailureAtRuntime(PartitionBy.YEAR, 10000000L * 30 * 12, false, YEAR_1972_DIR + Files.SEPARATOR + "c.v", 2);

Also applies to: 549-550, 554-555

---

234-235: Centralize YEAR partition dir "1972.3" — avoid brittle name‑txn literals

Multiple tests hard-code "1972.3" (core/src/test/java/io/questdb/test/cairo/FullFwdPartitionFrameCursorTest.java:234, 239, 244, 544, 549, 554). If ".3" is a name‑txn suffix (not derivable from the timestamp), replace these literals with a single constant (e.g. private static final String YEAR_1972_DIR = "1972.3") or, preferably, derive the partition directory via the partition-name API so tests don’t break when name‑txn sequencing changes.

core/src/test/java/io/questdb/test/cutlass/line/tcp/LineTcpSenderTest.java (1)

59-59: Drop redundant non-static JUnit import and use existing static assert.

You already have import static org.junit.Assert.*;. Remove the direct org.junit.Assert import and switch the call site to assertEquals.

Apply this diff:

-import org.junit.Assert;
...
-                Assert.assertEquals(0, errorCount.get());
+                assertEquals(0, errorCount.get());

Also applies to: 1023-1023

core/src/test/java/io/questdb/test/cairo/fuzz/DedupInsertFuzzTest.java (3)

764-771: Switch expression looks good; add a diagnostic for unsupported types.

Throwing an informative error helps test triage.

Apply this diff:

-        return switch (colType) {
+        return switch (colType) {
             case ColumnType.SYMBOL -> rec.getSymA(2);
             case ColumnType.STRING -> rec.getStrA(2);
             case ColumnType.VARCHAR -> Utf8s.toString(rec.getVarcharA(2));
-            default -> throw new IllegalArgumentException();
+            default -> throw new IllegalArgumentException("Unsupported column type: " + ColumnType.nameOf(colType));
         };

---

641-645: Compute dupStep once per transaction, not per operation.

Reduces randomness noise and minor CPU churn inside the loop.

-                for (int op = 0; op < size; op++) {
-                    int dupStep = Math.max(2, transaction.operationList.size() / (1 + rnd.nextInt(10)));
+                int dupStep = Math.max(2, size / (1 + rnd.nextInt(10)));
+                for (int op = 0; op < size; op++) {

---

889-891: Typo: rename variable to commaSeparatedUpsertCols.

Improves readability.

-            String comaSeparatedUpsertCols;
+            String commaSeparatedUpsertCols;

And update its usages accordingly.

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

93-96: Clarify nullability for substring put()

Align with other overloads. If null is not allowed here, annotate parameter as @NotNull to prevent accidental NPEs downstream.

-    public NumericException put(CharSequence cs, int lo, int hi) {
+    public NumericException put(@NotNull CharSequence cs, int lo, int hi) {
         message.put(cs, lo, hi);
         return this;
     }

core/src/main/java/io/questdb/std/Numbers.java (3)

670-687: Simplify floorPow2 via Long.highestOneBit

Same semantics, clearer and branchless. JVM intrinsic on modern HotSpot.

-    public static long floorPow2(long value) {
-        if (value <= 0) {
-            return 0;
-        }
-        long v = value;
-        v |= v >>> 1;
-        v |= v >>> 2;
-        v |= v >>> 4;
-        v |= v >>> 8;
-        v |= v >>> 16;
-        v |= v >>> 32;
-        return v - (v >>> 1);
-    }
+    public static long floorPow2(long value) {
+        return value <= 0 ? 0 : Long.highestOneBit(value);
+    }

---

946-949: Empty hex substring check should use hi <= lo

The current check hi == 0 fails for non‑zero lo (e.g., lo==5, hi==5). Use hi <= lo to catch all empty slices.

-        if (hi == 0) {
+        if (hi <= lo) {
             throw NumericException.instance().put("empty hex string");
         }

Also applies to: 966-969, 982-985

---

1498-1525: Error messages in parseLongSize use “duration” instead of “size”

Nit but can mislead operators. Use consistent wording with int variant.

-                                throw NumericException.instance().put("duration overflow");
+                                throw NumericException.instance().put("size overflow");
@@
-                                throw NumericException.instance().put("duration overflow");
+                                throw NumericException.instance().put("size overflow");
@@
-                                throw NumericException.instance().put("duration overflow");
+                                throw NumericException.instance().put("size overflow");
@@
-                throw NumericException.instance().put("invalid duration format");
+                throw NumericException.instance().put("invalid size format");

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

331-364: Floor/Ceil relationship tests

These lock in the new ceilPow2 saturation at 2^62. Consider adding a small test clarifying behavior for negatives (e.g., value < 1), even if only to assert current behavior.

core/src/main/java/io/questdb/cutlass/text/ParallelCsvFileImporter.java (1)

1187-1204: Fail fast if symbol indices cannot be resolved

Both indices can be -1; add a quick guard to surface configuration mismatches early.

-                int tempTableDenseSymbolIndex = targetTableStructure.getDenseSymbolIndex(symbolColumnName);
-                int tempTableColumnIndex = targetTableStructure.getColumnIndex(symbolColumnName);
+                int tempTableDenseSymbolIndex = targetTableStructure.getDenseSymbolIndex(symbolColumnName);
+                int tempTableColumnIndex = targetTableStructure.getColumnIndex(symbolColumnName);
+                assert tempTableDenseSymbolIndex >= 0 && tempTableColumnIndex >= 0
+                        : "Unresolved symbol column in temp tables: " + symbolColumnName;

core/src/main/java/io/questdb/cutlass/text/CopyTask.java (1)

709-728: Use TableUtils helpers or COLUMN_NAME_TXN_NONE when appending txn suffix

In core/src/main/java/io/questdb/cutlass/text/CopyTask.java (PhaseUpdateSymbolKeys.run, ~lines 720–728) you do if (columnNameTxn != -1) { path.put('.').put(columnNameTxn); }. Use the same convention as TableUtils (either call TableUtils.dFile/iFile or check columnNameTxn > TableUtils.COLUMN_NAME_TXN_NONE) to keep suffix logic consistent and avoid edge-case mismatches.

core/src/test/java/io/questdb/test/cairo/SymbolMapTest.java (1)

106-108: Replace magic -1 with a named constant or helper to document semantics.

Every new SymbolMapWriter(...) call adds NOOP_COLLECTOR and a trailing -1. The sentinel is unclear and repeated across the file. Introduce a self-explanatory constant (e.g., SYMBOL_TABLE_TXN_NONE or COLUMN_INDEX_UNSET), or a local factory method newWriter(path, name, start) that hides these fixed args to improve readability and reduce copy/paste risk. Applies to all occurrences listed.

Also applies to: 128-130, 191-193, 265-267, 304-305, 328-329, 358-360, 395-397, 421-422, 444-445, 467-468, 492-494, 513-514, 536-537, 575-576, 598-599, 622-623, 694-695, 749-758, 820-822, 864-865, 925-926, 945-946, 971-972, 995-996, 1028-1029, 1072-1073, 1123-1124

core/src/main/java/io/questdb/cairo/ColumnVersionWriter.java (1)

302-305: Fix misleading comment in upsertSymbolTableTxnName().

The comment talks about storing the added-partition timestamp in columnTop, but here columnTop is always 0 for the symbol-table partition. Update the comment to reflect actual behavior.

Apply this wording tweak:

-        // When table is partitioned, use columnTop place to store the timestamp of the partition where the column added
+        // Store the column name txn for the symbol-table partition; columnTop is unused here and set to 0.

core/src/main/java/io/questdb/cutlass/line/tcp/SymbolCache.java (2)

121-123: Clear temp sink before UTF‑8→UTF‑16 conversion.

Without clearing, values may concatenate depending on Utf8s.utf8ToUtf16Unchecked behavior.

Apply this diff:

-        Utf8s.utf8ToUtf16Unchecked(value, tempSink);
+        tempSink.clear();
+        Utf8s.utf8ToUtf16Unchecked(value, tempSink);

---

131-156: Avoid relying on external immutability of pathToTableDir.

The comment admits fragility. Store a defensive copy to prevent accidental mutation by owners.

Apply this diff:

-        this.pathToTableDir = pathToTableDir;
+        // Defensive copy to decouple from owner mutations
+        this.pathToTableDir = Utf8String.newInstance(pathToTableDir);

core/src/main/java/io/questdb/cairo/ColumnVersionReader.java (1)

221-235: API for symbol-table txn with sane fallback; fix Javadoc tag.

Method and fallback to default txn are correct. Minor: use {@link #getDefaultColumnNameTxn} instead of {@see ...} to avoid Javadoc warnings.

core/src/test/java/io/questdb/test/cutlass/text/ParallelCsvFileImporterTest.java (2)

1582-1590: Typo in test name: Typ → Type.

Nit: rename to testImportTimestampTypeFormatMismatch for consistency/searchability.

-    public void testImportTimestampTypFormatMismatch() {
+    public void testImportTimestampTypeFormatMismatch() {

---

3517-3519: Public nested test interface likely unnecessary.

TextImportRunnable needn’t be public inside this test class; package‑private keeps surface area smaller.

-    public interface TextImportRunnable {
+    interface TextImportRunnable {

core/src/test/java/io/questdb/test/cutlass/line/tcp/SymbolCacheTest.java (4)

148-156: Column index vs symbol index: avoid off‑by‑one with explicit lookup.

You pass col + 1 (table col index accounting for timestamp) and col (symbol index). This is subtle and brittle. Prefer resolving both from metadata to avoid off‑by‑one when schema changes.

-                            symbolCache.of(
-                                    engine.getConfiguration(),
-                                    "col" + col,
-                                    col + 1,
-                                    col,
-                                    path,
-                                    new TestTableWriterAPI(rdr.getColumnVersionReader()),
-                                    txReader
-                            );
+                            final int tableColIndex = rdr.getMetadata().getColumnIndex("col" + col);
+                            final int symbolColOrdinal = txReader.unsafeSymbolColumnIndexOf(tableColIndex); // or compute from metadata if available
+                            symbolCache.of(
+                                    engine.getConfiguration(),
+                                    "col" + col,
+                                    tableColIndex,
+                                    symbolColOrdinal,
+                                    path,
+                                    new TestTableWriterAPI(rdr.getColumnVersionReader()),
+                                    txReader
+                            );

If unsafeSymbolColumnIndexOf(...) isn’t available, compute the ordinal by iterating metadata and counting SYMBOL columns up to tableColIndex.

---

221-227: PartitionBy mismatch with created tables.

Tables are created with PartitionBy.HOUR but TxReader.ofRO(...) is passed PartitionBy.DAY. Align these for consistency to prevent subtle test flakiness if TxReader starts validating this.

-                                TableUtils.getTimestampType(model),
-                                PartitionBy.DAY
+                                TableUtils.getTimestampType(model),
+                                PartitionBy.HOUR

Also applies to: 424-428

---

605-613: Recompute indices after column removal.

After writer.removeColumn("symCol1"), using hardcoded 0 assumes reindexing; safer to recompute the index for "symCol2" from metadata.

-                            "symCol2",
-                            0,
-                            0,
+                            "symCol2",
+                            writer.getColumnIndex("symCol2"),
+                            txReader.unsafeSymbolColumnIndexOf(writer.getColumnIndex("symCol2")),

If unsafeSymbolColumnIndexOf is unavailable, derive the ordinal as noted earlier.

---

230-238: Reduce duplication: helper to init/reinit SymbolCache.

Consider a small helper to DRY repeated cache.of(...) sequences (path building + index resolution + API construction).

+    private static void initCache(SymbolCache cache,
+                                  CairoConfiguration cfg,
+                                  Path path,
+                                  TableToken token,
+                                  String colName,
+                                  int tableColIdx,
+                                  int symOrdinal,
+                                  TableWriterAPI twa,
+                                  TxReader txReader) {
+        cache.of(cfg, colName, tableColIdx, symOrdinal, path.of(cfg.getDbRoot()).concat(token), twa, txReader);
+    }

Also applies to: 252-259, 452-461, 522-526, 605-613, 683-691, 766-774

core/src/test/java/io/questdb/test/AbstractCairoTest.java (1)

335-335: Minor typographical error in comment.

The word "adcanced" should be spelled "advanced" on Line 335.

Apply this diff to correct the typo:

-                    // intentionally calling hasNext() twice: we want to adcanced the cursor position,
+                    // intentionally calling hasNext() twice: we want to advanced the cursor position,

core/src/main/java/io/questdb/cairo/TableUtils.java (1)

123-123: Lowered MIN_INDEX_VALUE_BLOCK_SIZE: align validation and avoid magic number

Good change; to keep invariants consistent, replace the hardcoded check "index value block capacity < 2" in validateMeta() with MIN_INDEX_VALUE_BLOCK_SIZE.

-                    if (getIndexBlockCapacity(metaMem, i) < 2) {
+                    if (getIndexBlockCapacity(metaMem, i) < MIN_INDEX_VALUE_BLOCK_SIZE) {

core/src/main/java/io/questdb/cairo/MetadataCache.java (1)

303-318: Avoid nested try-with-resources on the same ColumnVersionReader

Currently you open the same AutoCloseable twice; simplify to a single TWR.

-        try (columnVersionReader) {
-            LOG.debug().$("hydrating symbol metadata [table=").$(token).$(", column=").$safe(columnName).I$();
-            // get column version
-            path.trimTo(configuration.getDbRoot().length()).concat(token);
-            final int rootLen = path.size();
-            path.concat(TableUtils.COLUMN_VERSION_FILE_NAME);
-            final long columnNameTxn;
-            final FilesFacade ff = configuration.getFilesFacade();
-            try (columnVersionReader) {
-                columnVersionReader.ofRO(ff, path.$());
-                columnVersionReader.readUnsafe();
-                columnNameTxn = columnVersionReader.getSymbolTableNameTxn(writerIndex);
-            }
+        try (columnVersionReader) {
+            LOG.debug().$("hydrating symbol metadata [table=").$(token).$(", column=").$safe(columnName).I$();
+            path.trimTo(configuration.getDbRoot().length()).concat(token);
+            final int rootLen = path.size();
+            path.concat(TableUtils.COLUMN_VERSION_FILE_NAME);
+            final FilesFacade ff = configuration.getFilesFacade();
+            columnVersionReader.ofRO(ff, path.$());
+            columnVersionReader.readUnsafe();
+            final long columnNameTxn = columnVersionReader.getSymbolTableNameTxn(writerIndex);

core/src/main/java/io/questdb/cairo/TableWriter.java (3)

5484-5553: Don’t hard-link .k/.v in table root for symbol table files

At table root, symbol tables consist of .c/.o; bitmap index .k/.v live per-partition. Attempting to link .k/.v at root is unnecessary and may be confusing. Simplify to .c/.o only.

@@
-                    linkFile(
-                            ff,
-                            keyFileName(path.trimTo(pathSize), columnName, symbolTableNameTxn),
-                            keyFileName(other.trimTo(pathSize), newName, newColumnNameTxn)
-                    );
-                    linkFile(
-                            ff,
-                            valueFileName(path.trimTo(pathSize), columnName, symbolTableNameTxn),
-                            valueFileName(other.trimTo(pathSize), newName, newColumnNameTxn)
-                    );

---

5571-5615: Capacity rebuild path: copy .o, link .c — good; add small robustness

This path correctly copies offsets (.o) to update header while preserving .c. Recommend fsync root dir after linking/copying (non-Windows) to reduce risk on crash between file ops, mirroring patterns elsewhere.

---

3591-3619: Tighten attach pinning to known column file types

attachPartitionPinColumnVersions() currently trusts any “name.ext.txn” pattern. Guard by checking ext ∈ {d,i,k,v,c,o} to avoid pinning on unrelated files that match the pattern.

@@
-            if (firstDot != -1) {
+            if (firstDot != -1) {
                 utf16Sink.clear();
                 utf16Sink.put(tmpDirectUtf8StringZ, 0, firstDot);
                 // all our column files have .d (or such) extensions
                 int columnIndex = metadata.getColumnIndexQuiet(utf16Sink);
                 if (columnIndex != -1) {
                     // not a random file, we have column by this name
                     int lastDot = Utf8s.lastIndexOfAscii(tmpDirectUtf8StringZ, '.');
                     int len = Utf8s.length(tmpDirectUtf8StringZ);
                     if (lastDot > 1 && lastDot < len - 1) {
+                        // ensure known column file extension
+                        int extStart = firstDot + 1;
+                        int extEnd = lastDot;
+                        if (!Utf8s.matchesAscii(tmpDirectUtf8StringZ, extStart, extEnd, "d")
+                                && !Utf8s.matchesAscii(tmpDirectUtf8StringZ, extStart, extEnd, "i")
+                                && !Utf8s.matchesAscii(tmpDirectUtf8StringZ, extStart, extEnd, "k")
+                                && !Utf8s.matchesAscii(tmpDirectUtf8StringZ, extStart, extEnd, "v")
+                                && !Utf8s.matchesAscii(tmpDirectUtf8StringZ, extStart, extEnd, "c")
+                                && !Utf8s.matchesAscii(tmpDirectUtf8StringZ, extStart, extEnd, "o")) {
+                            return;
+                        }
                         // we are rejecting 'abc', '.abc' and 'abc.', but accepting 'a.xxx'
                         try {
                             long nameTxn = Numbers.parseLong(tmpDirectUtf8StringZ, lastDot + 1, len);

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

core/src/main/java/io/questdb/cutlass/line/tcp/SymbolCache.java (1)

156-161: Pass the copied path field, not the external parameter, to avoid mutable-path aliasing

You copy the table dir into this.pathToTableDir, but the initial open still uses the external pathToTableDir parameter. If that object is mutable, the reader may retain a reference to a path that later changes. Use the copied field consistently.

Apply this diff:

-        symbolMapReader.of(configuration, pathToTableDir, columnName, symbolTableNameTxn, symCount);
+        symbolMapReader.of(configuration, this.pathToTableDir, columnName, symbolTableNameTxn, symCount);

core/pom.xml (1)

618-627: Rename the placeholder strings for clarity.
Both excludePattern1 values still end with java11plus, which now mismatches the profile ID (java17+). Consider renaming them to avoid future confusion.

core/src/test/java/io/questdb/test/cairo/fuzz/DedupInsertFuzzTest.java (2)

765-770: Switch expression looks good; avoid magic column index 2.

Hard-coding column index 2 is brittle. Consider a named constant or look up the index by column name once.

Example:

-    return switch (colType) {
-        case ColumnType.SYMBOL -> rec.getSymA(2);
-        case ColumnType.STRING -> rec.getStrA(2);
-        case ColumnType.VARCHAR -> Utf8s.toString(rec.getVarcharA(2));
-        default -> throw new IllegalArgumentException();
-    };
+    final int KEY_COL = 2; // 's' column
+    return switch (colType) {
+        case ColumnType.SYMBOL -> rec.getSymA(KEY_COL);
+        case ColumnType.STRING -> rec.getStrA(KEY_COL);
+        case ColumnType.VARCHAR -> Utf8s.toString(rec.getVarcharA(KEY_COL));
+        default -> throw new IllegalArgumentException("Unexpected colType=" + colType);
+    };

---

779-779: Five-boolean call to readTxnToString is error-prone; add intent.

The expanded signature increases ordering mistakes risk. Add named locals or inline comments to document each flag, or wrap in a helper.

Example:

-String partitions = readTxnToString(tt, false, true, true, false);
+// booleans: <arg1>, <arg2>, <arg3>, <arg4>, <arg5>
+String partitions = readTxnToString(tt, /*arg1=*/false, /*arg2=*/true, /*arg3=*/true, /*arg4=*/false);

Please confirm the flag order matches the new API.

Also applies to: 805-805

core/src/main/java/io/questdb/cairo/TableReader.java (1)

1709-1719: Provide actual symbolCount on renew and update when reopen isn’t needed

Passing 0 defers count refresh until a later pass. Use current txFile value and update when no reopen is required. This avoids transient undercount and extra work.

     private void renewSymbolMapReader(SymbolMapReader reader, int columnIndex) {
         if (ColumnType.isSymbol(metadata.getColumnType(columnIndex))) {
             final int writerColumnIndex = metadata.getWriterIndex(columnIndex);
             final long symbolTableNameTxn = columnVersionReader.getSymbolTableNameTxn(writerColumnIndex);
             String columnName = metadata.getColumnName(columnIndex);
-            if (!(reader instanceof SymbolMapReaderImpl symbolMapReader)) {
-                reader = new SymbolMapReaderImpl(configuration, path, columnName, symbolTableNameTxn, 0);
+            final int denseIdx = metadata.getDenseSymbolIndex(columnIndex);
+            final int symbolCount = txFile.getSymbolValueCount(denseIdx);
+            if (!(reader instanceof SymbolMapReaderImpl symbolMapReader)) {
+                reader = new SymbolMapReaderImpl(configuration, path, columnName, symbolTableNameTxn, symbolCount);
             } else {
                 // Fully reopen the symbol map reader only when necessary
-                if (symbolMapReader.needsReopen(symbolTableNameTxn)) {
-                    symbolMapReader.of(configuration, path, columnName, symbolTableNameTxn, 0);
+                if (symbolMapReader.needsReopen(symbolTableNameTxn)) {
+                    symbolMapReader.of(configuration, path, columnName, symbolTableNameTxn, symbolCount);
+                } else {
+                    symbolMapReader.updateSymbolCount(symbolCount);
                 }
             }
         } else {

core/src/main/java/io/questdb/cutlass/line/tcp/TableUpdateDetails.java (1)

616-632: Avoid stateful symbolNameTemp; pass the name locally for clarity

resolveSymbolIndexAndName(...) sets and relies on a mutable field (symbolNameTemp). Make it return the name alongside the index (or derive the name locally) to reduce hidden state and cross-call coupling.

Minimal change (derive name locally in addSymbolCache):

-                final int symIndex = resolveSymbolIndexAndName(reader.getMetadata(), colWriterIndex);
-                if (symbolNameTemp == null || symIndex < 0) {
+                final int symIndex = resolveSymbolIndexAndName(reader.getMetadata(), colWriterIndex);
+                final String symbolName =
+                        symIndex < 0 ? null : reader.getMetadata().getColumnName(reader.getMetadata().getColumnIndexQuiet(symbolNameTemp));
+                if (symbolName == null || symIndex < 0) {
                     return NOT_FOUND_LOOKUP;
                 }
...
-                symCache.of(
+                symCache.of(
                         cairoConfiguration,
-                        symbolNameTemp,
+                        symbolName,
                         colWriterIndex,
                         symIndex,
                         path,
                         writerAPI,
                         txReader,
                         columnVersionReader
                 );

Alternatively, refactor resolveSymbolIndexAndName to return a small record (index, name) and drop the field entirely.

Also applies to: 531-541

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro