Statement classification guessed from the first word and got five forms wrong

The driver chose between "open a cursor and fetch" and "execute and
release" by checking whether the first word of the SQL was SELECT. That
misses a leading comment in any of the three Informix flavours (--,
/* */, { }), a parenthesized select, a parenthesized UNION, and a CTE.
All five are perfectly ordinary queries. All five failed with -260,
"Cursor name already in use" -- an error that describes neither the
cause nor anything the caller did. It says "cursor" because the driver
sent SQ_EXECUTE where the server was waiting to open one.

The server had been answering this question all along. statement_type is
the first field of the DESCRIBE response and parse_describe has always
put it in the metadata dict, where nothing read it.

The comment defending the heuristic said nfields couldn't distinguish
these cases because INSERT INTO t VALUES (?) also describes a column.
That was true, and it argued for the wrong conclusion: statement_type
distinguishes them exactly. Measured against all three servers -- every
SELECT form above reports 2, that INSERT reports 6, UPDATE 33, DELETE
32, CREATE 45, EXECUTE PROCEDURE 56.

The predicate is now JDBC's IfxSqli.isResultSet: type 2, or type 56 with
at least one column, since a routine may or may not return rows. That
last clause fixes EXECUTE FUNCTION as a side effect -- it used to run
down the DML path and discard the return value. It now yields it.

executemany keeps the first-word check as a cheap pre-flight reject, and
gains an authoritative one after PREPARE for the forms the first word
misses. The refusal releases the statement, so the connection survives.

Informix 12.10 has no CTEs and rejects WITH with -201 at offset 1. That
is the server declining the grammar rather than the driver routing it
wrongly, and the test skips on it.

src/informix_db/cursors.py

@@ -102,6 +102,39 @@ def _make_socket_reader(sock):
     return _SocketReader(sock)
 
 
+# Statement types from the DESCRIBE response's first field. The server
+# tells us exactly what it prepared; these are the two values that mean
+# "this will produce rows".
+_ST_SELECT = 2
+_ST_ROUTINE = 56  # EXECUTE PROCEDURE / EXECUTE FUNCTION
+
+
+def _produces_result_set(statement_type: int, ncolumns: int) -> bool:
+    """Whether a prepared statement needs a cursor opened for it.
+
+    This is JDBC's ``IfxSqli.isResultSet`` predicate, and it replaces a
+    first-word check for ``SELECT`` that got five ordinary forms wrong.
+    A leading comment of any of the three Informix flavours, a
+    parenthesized select, a parenthesized UNION, and a CTE were all
+    classified as DML, so the driver sent SQ_EXECUTE where the server
+    expected a cursor. The report back was ``-260 Cursor name already in
+    use``, which describes neither the cause nor anything the caller did.
+
+    The original comment justified the heuristic on the grounds that
+    ``nfields`` can't distinguish these, because ``INSERT INTO t VALUES
+    (?)`` also describes a column. True, and irrelevant: ``statement_type``
+    distinguishes them exactly. That INSERT reports 6 with one field; every
+    SELECT form above reports 2. The value was being parsed into the
+    describe metadata and thrown away.
+
+    ``EXECUTE PROCEDURE``/``FUNCTION`` (56) is the one type that depends on
+    the column count, since a routine may or may not return rows.
+    """
+    if statement_type == _ST_SELECT:
+        return True
+    return statement_type == _ST_ROUTINE and ncolumns > 0
+
+
 def _finalize_cursor(
     conn_ref: weakref.ReferenceType,
     state: list,
@@ -283,6 +316,9 @@ class Cursor:
         # from. Empirically the server accepts 0 here even when a real
         # ID was assigned, so this is best-effort tracking.
         self._statement_id: int = 0
+        # DESCRIBE's statement-type field. Decides whether a cursor is
+        # opened -- see _produces_result_set.
+        self._statement_type: int = 0
         # Phase 10: smart-LOB read via ``lotofile(col, path, 'client')``.
         # The server orchestrates a SQ_FILE (98) protocol where it tells
         # us to "open file X, write these bytes, close". We emulate the
@@ -384,6 +420,7 @@ class Cursor:
         self._rowcount = -1
         self._rows = []
         self._row_index = -1  # before-first-row
+        self._statement_type = 0
         self._statement_already_done = False
 
         # On a logged DB in non-autocommit mode, the server requires an
@@ -401,15 +438,9 @@ class Cursor:
         )
         self._read_describe_response()
 
-        # Branch on the SQL keyword. We can't use ``self._columns`` /
+        # Ask the server what it just prepared, rather than guessing from
-        # ``nfields`` here because a parameterized INSERT also returns
+        # the first word of the SQL.
-        # a non-empty DESCRIBE (server describes the would-be inserted
+        if _produces_result_set(self._statement_type, len(self._columns)):
-        # row's columns). The SQL-keyword heuristic is what JDBC effectively
-        # does too via its IfxStatement / IfxPreparedStatement subclassing.
-        first_word = sql.lstrip().split(None, 1)[0].upper() if sql.strip() else ""
-        is_select = first_word == "SELECT"
-
-        if is_select:
             if params:
                 self._execute_select_with_params(params)
             else:
@@ -1074,7 +1105,10 @@ class Cursor:
                     f"expected {first_len} (matching set [0])"
                 )
 
-        # Detect SELECT — not supported in executemany.
+        # Cheap pre-flight reject for the obvious case, so the common
+        # mistake costs no round-trip. The authoritative check is after
+        # PREPARE, below — this one shares the first-word heuristic's
+        # blind spots (leading comments, CTEs, parenthesized selects).
         first_word = operation.lstrip().split(None, 1)[0].upper() if operation.strip() else ""
         if first_word == "SELECT":
             raise NotSupportedError("executemany on SELECT is not supported")
@@ -1095,6 +1129,7 @@ class Cursor:
             self._rowcount = -1
             self._rows = []
             self._row_index = -1
+            self._statement_type = 0
             self._statement_already_done = False
 
             # Logged-DB transaction guard — same as execute(). Idempotent
@@ -1108,6 +1143,18 @@ class Cursor:
             )
             self._read_describe_response()
 
+            # Now the server has told us what it prepared. A result-set
+            # statement that slipped past the first-word check above --
+            # a CTE, a leading comment, a parenthesized select -- would
+            # otherwise be executed N times down the DML path, which
+            # opens no cursor and answers -260.
+            if _produces_result_set(self._statement_type, len(self._columns)):
+                self._release_after_failure()
+                raise NotSupportedError(
+                    "executemany on a statement that returns rows is not "
+                    "supported"
+                )
+
             # Phase 33: pipeline — build all BIND+EXECUTE PDUs first
             # (Python work, no I/O), then send them back-to-back, then
             # drain all responses. Eliminates the per-row round-trip
@@ -1735,6 +1782,7 @@ class Cursor:
             elif tag == MessageType.SQ_DESCRIBE:
                 self._columns, meta = parse_describe(reader)
                 self._statement_id = meta.get("statement_id", 0)
+                self._statement_type = meta.get("statement_type", 0)
                 self._description = (
                     [c.to_description_tuple() for c in self._columns] if self._columns else None
                 )

tests/test_statement_classification.py

@@ -0,0 +1,216 @@
+"""Deciding whether a statement needs a cursor, by asking rather than guessing.
+
+The driver chose between "open a cursor and fetch" and "execute and
+release" by checking whether the first word of the SQL was ``SELECT``.
+That gets five ordinary forms wrong — a leading comment in any of the
+three Informix flavours, a parenthesized select, a parenthesized UNION,
+and a CTE. All five are perfectly good queries, and all five failed with
+``-260 Cursor name already in use``, an error that describes neither the
+cause nor anything the caller did. It says "cursor" because the driver
+sent SQ_EXECUTE where the server was waiting to open one.
+
+The server had been telling us the answer the whole time.
+``statement_type`` is the first field of the DESCRIBE response, and
+``parse_describe`` has always parsed it into the metadata dict, where
+nothing read it. Every SELECT form above reports 2.
+
+The comment that justified the heuristic said ``nfields`` couldn't
+distinguish these cases, because ``INSERT INTO t VALUES (?)`` also
+describes a column. That much was true — and it argued for the wrong
+conclusion, because ``statement_type`` distinguishes them exactly. That
+INSERT reports 6.
+
+The predicate is now JDBC's ``IfxSqli.isResultSet``: type 2, or type 56
+(``EXECUTE PROCEDURE``/``FUNCTION``) with at least one column, since a
+routine may or may not return rows. That last clause fixes
+``EXECUTE FUNCTION`` as a side effect — it used to run down the DML path
+and discard its return value.
+"""
+
+from __future__ import annotations
+
+import contextlib
+
+import pytest
+
+import informix_db
+from informix_db.cursors import _produces_result_set
+from tests.conftest import ConnParams
+
+# ---------------------------------------------------------------------------
+# The predicate
+# ---------------------------------------------------------------------------
+
+
+@pytest.mark.parametrize(
+    ("statement_type", "ncolumns", "expected", "why"),
+    [
+        (2, 1, True, "SELECT"),
+        (2, 0, True, "SELECT describing no columns is still a SELECT"),
+        (6, 1, False, "INSERT ... VALUES (?) describes a column but is DML"),
+        (6, 0, False, "INSERT with literals"),
+        (32, 0, False, "DELETE"),
+        (33, 0, False, "UPDATE"),
+        (45, 0, False, "CREATE"),
+        (56, 0, False, "EXECUTE PROCEDURE returning nothing"),
+        (56, 2, True, "EXECUTE FUNCTION returning rows"),
+        (0, 0, False, "unknown type defaults to the non-cursor path"),
+    ],
+)
+def test_result_set_predicate(
+    statement_type: int, ncolumns: int, expected: bool, why: str
+) -> None:
+    assert _produces_result_set(statement_type, ncolumns) is expected, why
+
+
+# ---------------------------------------------------------------------------
+# Against a real server
+# ---------------------------------------------------------------------------
+
+
+def _connect(conn_params: ConnParams) -> informix_db.Connection:
+    return informix_db.connect(
+        host=conn_params.host,
+        port=conn_params.port,
+        user=conn_params.user,
+        password=conn_params.password,
+        database=conn_params.database,
+        server=conn_params.server,
+        connect_timeout=10.0,
+        read_timeout=25.0,
+        autocommit=True,
+    )
+
+
+_ONE = "SELECT FIRST 1 tabid FROM systables"
+
+
+@pytest.mark.integration
+@pytest.mark.parametrize(
+    ("label", "sql"),
+    [
+        pytest.param("plain", _ONE, id="plain"),
+        pytest.param("lowercase", _ONE.lower(), id="lowercase"),
+        pytest.param("leading-whitespace", f"  \n\t {_ONE}", id="whitespace"),
+        pytest.param("line-comment", f"-- pick one\n{_ONE}", id="line-comment"),
+        pytest.param("block-comment", f"/* pick one */ {_ONE}", id="block-comment"),
+        pytest.param("brace-comment", f"{{ pick one }} {_ONE}", id="brace-comment"),
+        pytest.param(
+            "cte",
+            "WITH c AS (SELECT tabid FROM systables) "
+            "SELECT FIRST 1 tabid FROM c",
+            id="cte",
+        ),
+        pytest.param("parenthesized", f"({_ONE})", id="parenthesized"),
+        pytest.param(
+            "union", f"{_ONE} UNION SELECT 99 FROM systables", id="union"
+        ),
+        pytest.param(
+            "parenthesized-union",
+            f"({_ONE}) UNION (SELECT 99 FROM systables)",
+            id="paren-union",
+        ),
+    ],
+)
+def test_every_select_form_opens_a_cursor(
+    conn_params: ConnParams, label: str, sql: str
+) -> None:
+    """The five non-``plain`` forms below the whitespace case all failed
+    with -260 under the first-word heuristic."""
+    with _connect(conn_params) as conn:
+        cur = conn.cursor()
+        try:
+            cur.execute(sql)
+        except informix_db.ProgrammingError as exc:
+            # Informix 12.10 has no CTEs and rejects WITH at offset 1.
+            # A syntax error is the server declining the grammar, which
+            # is a different thing from the driver routing it wrongly —
+            # that produced -260, not -201.
+            if getattr(exc, "sqlcode", None) == -201:
+                pytest.skip(f"server does not support this syntax: {label}")
+            raise
+        rows = cur.fetchall()
+        assert rows, f"{label}: expected rows, got none"
+        assert cur.description is not None
+
+
+@pytest.mark.integration
+def test_parameterized_insert_is_not_mistaken_for_a_query(
+    conn_params: ConnParams,
+) -> None:
+    """The case the old comment worried about, and the reason it kept the
+    heuristic: this DESCRIBEs a column. ``statement_type`` says 6."""
+    with _connect(conn_params) as conn:
+        cur = conn.cursor()
+        cur.execute("CREATE TEMP TABLE t_cls (k INT)")
+        cur.execute("INSERT INTO t_cls VALUES (?)", (7,))
+        assert cur.rowcount == 1
+        cur.execute("SELECT k FROM t_cls")
+        assert cur.fetchall() == [(7,)]
+
+
+@pytest.mark.integration
+def test_execute_function_returns_its_value(conn_params: ConnParams) -> None:
+    """Type 56 with columns. Under the first-word heuristic this ran down
+    the DML path and the return value was discarded."""
+    with _connect(conn_params) as conn:
+        cur = conn.cursor()
+        with contextlib.suppress(Exception):
+            cur.execute("DROP FUNCTION ifxdrv_dbl")
+        cur.execute(
+            "CREATE FUNCTION ifxdrv_dbl(n INT) RETURNING INT; "
+            "RETURN n * 2; END FUNCTION"
+        )
+        try:
+            cur.execute("EXECUTE FUNCTION ifxdrv_dbl(21)")
+            assert cur.fetchall() == [(42,)]
+        finally:
+            with contextlib.suppress(Exception):
+                cur.execute("DROP FUNCTION ifxdrv_dbl")
+
+
+@pytest.mark.integration
+def test_dml_still_takes_the_non_cursor_path(conn_params: ConnParams) -> None:
+    with _connect(conn_params) as conn:
+        cur = conn.cursor()
+        cur.execute("CREATE TEMP TABLE t_cls2 (k INT)")
+        cur.execute("INSERT INTO t_cls2 VALUES (1)")
+        cur.execute("UPDATE t_cls2 SET k = 2")
+        assert cur.rowcount == 1
+        cur.execute("DELETE FROM t_cls2")
+        assert cur.rowcount == 1
+        assert cur.description is None
+
+
+@pytest.mark.integration
+def test_executemany_refuses_a_query_the_first_word_missed(
+    conn_params: ConnParams,
+) -> None:
+    """The pre-flight check shares the heuristic's blind spots, so a
+    comment-prefixed SELECT reaches PREPARE. The post-DESCRIBE check
+    catches it, and the connection stays usable — the refusal releases
+    the statement. (A leading comment rather than a CTE, so this also
+    runs on 12.10, which has no CTEs.)"""
+    with _connect(conn_params) as conn:
+        cur = conn.cursor()
+        with pytest.raises(informix_db.NotSupportedError):
+            cur.executemany(
+                "/* batched? no */ SELECT FIRST 1 tabid FROM systables "
+                "WHERE tabid <> ?",
+                [(1,), (2,)],
+            )
+        cur.execute("SELECT FIRST 1 tabid FROM systables")
+        assert cur.fetchone() is not None, "refusal leaked the statement"
+
+
+@pytest.mark.integration
+def test_executemany_still_refuses_a_plain_select(
+    conn_params: ConnParams,
+) -> None:
+    with _connect(conn_params) as conn:
+        cur = conn.cursor()
+        with pytest.raises(informix_db.NotSupportedError):
+            cur.executemany(
+                "SELECT FIRST 1 tabid FROM systables WHERE tabid <> ?",
+                [(1,), (2,)],
+            )