compat/src/test/csharp/ExtraTests.cs (1)

50-72: Consider simplifying the result iteration pattern.

The first flag pattern works correctly, but the iteration could be slightly more idiomatic.

-            var first = true;
-            for (var i = 0; i < batch.BatchCommands.Count; i++)
-            {
-                if (!first)
-                {
-                    await reader.NextResultAsync();
-                }
-                first = false;
-
-                if (!await reader.ReadAsync())
+            for (var i = 0; i < batch.BatchCommands.Count; i++)
+            {
+                if (i > 0 && !await reader.NextResultAsync())
+                {
+                    throw new InvalidOperationException($"Expected result set {i} not found");
+                }
+
+                if (!await reader.ReadAsync())
                 {
                     throw new InvalidOperationException("No rows returned");
                 }

This eliminates the first flag and adds validation that NextResultAsync succeeds. That said, the current implementation is correct and this is purely a style preference.

core/src/main/java/io/questdb/cutlass/pgwire/PGPipelineEntry.java (1)

3271-3398: New bind replay helpers look sound; please confirm CTAS coverage

The new copyParameterValuesToBindVariableService plus populateBindingServiceForExec/Sync correctly:

• Clear and redefine the BindVariableService per pipeline entry.
• Walk the arena safely using msgBindParameterValueCount and per-parameter format codes.
• Respect PG_UNSPECIFIED entries (unused gaps) and null values (valueSize == -1).
• Use native types from Numbers.decodeLowInt(...) so QuestDB functions get the exact types inferred at compile time.

Two points to double‑check:

1. CTAS and other DDL that can contain expressions – populateBindingServiceForExec currently covers EXPLAIN/SELECT/PSEUDO_SELECT/INSERT/INSERT_AS_SELECT/UPDATE/ALTER but not CREATE_TABLE_AS_SELECT. If CTAS statements can reference $n bind variables in their SELECT part (which seems likely), you probably want to include CompiledQuery.CREATE_TABLE_AS_SELECT in this switch so CTAS EXEC runs with the correct bind values.

2. SYNC helper scope – populateBindingServiceForSync intentionally only handles EXPLAIN/SELECT/PSEUDO_SELECT, which matches the cases where re-compilation or cursor recreation may happen during SYNC. That looks reasonable and avoids unnecessary rebinds for pure DML/DDL.

If CTAS does use bind parameters in practice, consider extending the EXEC helper accordingly:

-        return switch (sqlType) {
+        return switch (sqlType) {
             // these query types use binding variables at the EXEC time
-            case CompiledQuery.EXPLAIN, CompiledQuery.SELECT, CompiledQuery.PSEUDO_SELECT, CompiledQuery.INSERT,
-                 CompiledQuery.INSERT_AS_SELECT, CompiledQuery.UPDATE, CompiledQuery.ALTER -> {
+            case CompiledQuery.EXPLAIN, CompiledQuery.SELECT, CompiledQuery.PSEUDO_SELECT,
+                 CompiledQuery.INSERT, CompiledQuery.INSERT_AS_SELECT,
+                 CompiledQuery.UPDATE, CompiledQuery.ALTER,
+                 CompiledQuery.CREATE_TABLE_AS_SELECT -> {
                 copyParameterValuesToBindVariableService(
                         sqlExecutionContext,
                         bindVariableCharacterStore,
                         directUtf8String,
                         binarySequenceParamsPool
                 );
                 yield true;
             }
             default -> false;
         };

Also applies to: 3444-3487

core/src/main/java/io/questdb/cutlass/pgwire/PGConnectionContext.java (1)

1390-1411: SYNC-side rebinding and error handling fix the pipelined bind bug; minor error-message nuance

The updated syncPipeline() loop looks correct and addresses the pipelined-SELECT bug:

• Before each msgSync call, you now (re)populate the BindVariableService for the current pipelineCurrentEntry when bindingServiceConfiguredFor points to a different entry and populateBindingServiceForSync(...) returns true. This ensures each entry’s SYNC phase sees its own bound parameter values even when EXECs were interleaved for multiple entries.
• After an entry is fully processed (or in error/close cases), you clear bindingServiceConfiguredFor when it was pointing at that entry, avoiding stale ownership across reused pooled entries.
• The catch (PGMessageProcessingException | SqlException e) path now:
  • Captures the textual error into the entry’s error sink.
  • Invokes msgSync(...) once to emit the error response.
  • Breaks out of the inner while (true) loop, avoiding the infinite retry behavior noted in the earlier review.

One small refinement you might consider (non-blocking):

• For SqlException (which implements FlyweightMessageContainer), using getFlyweightMessage() and getPosition() (mirroring the pattern in PGPipelineEntry.msgExecute’s catch) would produce more consistent error messages and preserve position info, instead of relying on e.getMessage() stringification.

Example sketch:

-                } catch (PGMessageProcessingException | SqlException e) {
-                    pipelineCurrentEntry.getErrorMessageSink().put(e.getMessage());
+                } catch (PGMessageProcessingException | SqlException e) {
+                    if (e instanceof FlyweightMessageContainer f) {
+                        int pos = f.getPosition();
+                        if (pos >= 0) {
+                            pipelineCurrentEntry.setErrorMessagePosition(pos);
+                        }
+                        pipelineCurrentEntry.getErrorMessageSink().put(f.getFlyweightMessage());
+                    } else {
+                        pipelineCurrentEntry.getErrorMessageSink().put(e.getMessage());
+                    }
                     pipelineCurrentEntry.msgSync(
                             sqlExecutionContext,
                             pendingWriters,
                             responseUtf8Sink
                     );
                     break;
                 }

This keeps the current control flow but improves diagnostics for SQL errors.

Also applies to: 1435-1454

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro