Tentative claude fix of portal C_34 error

[bobular]

Another update: first proposed changes were using newer a FgpUtil version which hasn't yet been rolled out for this repo. The change suggested now is much simpler, but may cause memory issues. Leaving this to @ryanrdoherty and others to take care of.

(To reproduce the bug, go to any DE search on beta.plasmodb.org, open up the subsetting cell and browse the variables on the counts entities.)

Claude Sonnet 4.6 took 45 minutes and one whole context window to figure this out!
(Opus review as a comment below)

Root Cause

In lib-eda-subsetting v7.0.0 (pg-support branch), FilteredResultFactory.produceVariableDistribution() was changed from accepting a raw DataSource to a DatabaseInstance. Using dbInstance.getDataSource() routes connections through ConnectionWrapper, which calls applyFetchSize(200) on every prepareStatement(). Combined with QueryExecutor.setAutocommit() always setting autocommit=false, this activates PostgreSQL JDBC server-side cursors.

The JDBC driver then declares DECLARE C_34 CURSOR FOR <query> and fetches rows in batches of 200. For variables with ≤200 distinct values this is invisible — only one batch is needed. For SEQUENCE_READ_COUNT_SENSE in pfal3D7_GENE_EXPRESSION_RNASEQ_DATA (which has >200 distinct values), the second batch fetch arrives to find portal "C_34" no longer exists on the server.

This failure mode is explicitly documented in PreparedStatementExecutor.setAutocommit():

"cursor is closed during commit, leading to runtime error"

What is a Portal?

A portal in PostgreSQL is a named server-side cursor — an execution context holding the state of a running query, allowing rows to be fetched incrementally in batches.

When PostgreSQL JDBC uses server-side streaming (fetchSize > 0, autocommit=false), it:

1. Sends DECLARE C_34 CURSOR FOR <query> to create the portal
2. Fetches rows in batches: FETCH 200 FROM C_34
3. Continues fetching until exhausted

Portals are transaction-scoped. When the transaction commits or rolls back, all non-WITH HOLD portals are immediately destroyed. "Portal C_34 does not exist" means the cursor was declared, the first batch fetched successfully, but by the time the next batch was requested, the transaction had ended.

The Fix

FilteredResultFactory.produceVariableDistribution() in lib-eda-subsetting — replace ResultSets.openStream() with a direct SQLRunner call using QueryFlags.setFetchSize(0). This overrides ConnectionWrapper's default of 200, disabling server-side cursors for this query. Distribution queries return aggregated data (one row per distinct value), so loading all rows at once is safe.

[bobular]

I thought the fix looked a bit cut-and-paste so I asked Claude Opus to look for a more elegant solution. This would mean some changes to FgpUtil. See below

---

The research shows that ResultSets.openStream() has no overload accepting a fetchSize or QueryFlags. Internally, it always constructs a QueryFlags with just CALLER_IS_RESPONSIBLE and no fetch size override. So the current fix — dropping down to SQLRunner.executeQuery() directly — is the only way to set a custom fetch size with the existing fgputil API.

That said, there are two options for making this more elegant:

Option 1: Add an overload in FgpUtil (upstream fix)

Add a ResultSets.openStream() overload that accepts QueryFlags, so this call site could become:

return ResultSets.openStream(dbInstance.getDataSource(), sql,
    "Produce variable distribution",
    new QueryFlags()
        .setCommitAndCloseFlag(CommitAndClose.CALLER_IS_RESPONSIBLE)
        .setFetchSize(0),
    row -> Optional.of(new TwoTuple<>(
        distributionVariable.getType().convertRowValueToStringValue(row),
        row.getLong(COUNT_COLUMN_NAME))));

This keeps the simplicity of ResultSets.openStream at the call site and the change in FgpUtil would be trivial — just threading the QueryFlags through instead of constructing a default one.

Option 2: Keep the current fix as-is

The current implementation is actually the idiomatic approach given the existing API. It's explicit about what's happening and the comment explains why. The only minor cleanup would be dropping the unnecessary empty args:

return new SQLRunner(dbInstance.getDataSource(), sql, "Produce variable distribution")
    .executeQuery(
        new QueryFlags()
            .setCommitAndCloseFlag(CommitAndClose.CALLER_IS_RESPONSIBLE)
            .setFetchSize(0),
        rs -> new ResultSetStream<>(rs, row -> Optional.of(
            new TwoTuple<>(distributionVariable.getType().convertRowValueToStringValue(row),
                row.getLong(COUNT_COLUMN_NAME)))));

(Using the executeQuery(QueryFlags, ResultSetHandler) overload instead of the one with empty Object[]/Integer[] args.)

Bottom line: The current fix is correct and reasonable. If you want it cleaner at this call site, a small FgpUtil PR adding a QueryFlags overload to ResultSets.openStream would be the most elegant path. Otherwise, Option 2 (dropping the empty args) is a minor improvement you can make right now.