Skip to content

Commit b6008a0

Browse files
learn: retrospective learnings
Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com>
1 parent 1a1a53e commit b6008a0

1 file changed

Lines changed: 8 additions & 0 deletions

File tree

‎.claude/knowledge/learning-log.md‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,4 +170,12 @@ until a human merges it. The engineer author phase reads this log (see
170170
- **Context:** PR #1001 (kernel async readers) switched same-connection re-fetch and concurrent polling to call `ExecutedAsyncStatement.reader()`, a post-1.3.0 kernel API, while `pyproject.toml` still declared the floor as `databricks-sql-kernel ^1.3.0`. The reviewer flagged that a resolver picking 1.3.0 raises `NotSupportedError` on a path that previously worked via `attach_async_statement` — a pure regression for standard sessions.
171171
**Rule:** When code starts calling a newly-added API from a dependency, bump the version floor in packaging metadata (pyproject/setup) to the first release that actually ships that API; a capability requirement stated only in comments/README/PR text — not in the dependency graph — lets the resolver pick a non-capable version and regress previously-working code paths.
172172

173+
### 2026-10-30: learnings since 2026-10-29T17:26:19Z
174+
- **Context:** Run #40957684953 (GH #978) found that `ResultSet._convert_arrow_table` maps Arrow float32/float64 to pandas' nullable `Float32Dtype`/`Float64Dtype`. Those dtypes treat IEEE NaN as missing (`pd.NA`), and `to_numpy(na_value=None)` then turns it into `None`, so NaN looked the same as SQL NULL on the pandas path (the `_disable_pandas` path was correct). The fix rebuilt the NaN mask from the original Arrow column with `pyarrow.compute.is_nan(...).fill_null(False)`.
175+
**Rule:** pandas nullable dtypes (Float64Dtype/Float32Dtype) silently turn NaN into NA/None. Whenever a result conversion goes through pandas, test that NaN, NULL and ±Infinity stay distinct, and check that the output matches the non-pandas (`as_py`) path.
176+
- **Context:** Run #40957736967 (issue #981, `fetchone`/`__iter__` converting one row at a time) reproduced a performance bug in E2E without timing anything. It wrapped `ResultSet._convert_arrow_table` (and the columnar/JSON converters) with a call counter and asserted the count stays near N/arraysize instead of N. That gave a reliable red/green result: 5000 calls before the fix against a bound of about 7.
177+
**Rule:** For performance bugs, write deterministic tests that count calls to the expensive operation (patch it with a wrapper that still calls the original) and assert a bound on the count. Do not assert on wall-clock time, which is flaky on a live warehouse.
178+
- **Context:** PR #1004 bumped OSV-Scanner from v2.3.8 to v2.7.0 by find-and-replacing the version string. That also changed comments claiming "vX doesn't support per-package CVE scoping" without checking whether the claim still holds, and the reviewer flagged it. A stale claim can leave suppressions broader than they need to be.
179+
**Rule:** When bumping a tool or dependency version, never find-and-replace the version inside capability claims ("vX does not support Y"). Re-check each claim against the new version's docs, then update the claim or the config it justifies.
180+
173181
--- *Add new entries above this line (oldest→newest); newest sections sort to the bottom.* ---

0 commit comments

Comments
 (0)