Skip to content

Commit 73110cc

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

1 file changed

Lines changed: 6 additions & 0 deletions

File tree

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,4 +152,10 @@ until a human merges it. The engineer author phase reads this log (see
152152
- **Context:** PR #985 added `escape_metadata_catalog_name` using `re.sub(r"(?<!\\)_", r"\\_", name)` to escape underscores in metadata catalog names; reviewers flagged that the single-char lookbehind mis-handles backslash runs and that the "Match JDBC regardless of backslash parity" claim inverts real JDBC/`LIKE` semantics (an even-length `\\` run leaves the following `_` unescaped, so it should still be escaped).
153153
**Rule:** A single-character negative lookbehind like `(?<!\\)_` cannot decide whether an underscore is truly escaped — only backslash-run *parity* can — so don't claim SQL `LIKE`/JDBC equivalence for such a regex; if inputs may contain backslashes, count the preceding backslash run, otherwise document the backslash-free assumption as a deliberate simplification rather than as JDBC parity.
154154

155+
### 2026-10-20: learnings since 2026-10-19T17:30:56Z
156+
- **Context:** PR #990 added `enable_geospatial_support` on the kernel path; `open_session` called `_kernel_geospatial_kwargs` unconditionally and raised `NotSupportedError` whenever the installed kernel wheel didn't declare the kwarg — breaking *every* `use_kernel=True` connection against an older (still-allowed `^1.1.0`) wheel even at the default `False`, asymmetric with `max_connections`/telemetry which gate on `_kernel_session_accepts_kwarg` and silently omit.
157+
**Rule:** When forwarding an optional kwarg to the kernel `Session`, a default-off feature must fall closed by omitting the kwarg when the wheel doesn't declare it, and hard-fail only on an explicit opt-in — never let a default/unset value reject unrelated connections unless a lockstep wheel-floor bump is release-gated.
158+
- **Context:** PR #990 changed `_hive_schema_to_description` (thrift_backend.py) from pairing Thrift result columns with Arrow schema fields by a `field_dict[name]` lookup to pairing by positional index, because SQL permits duplicate column aliases for which a name lookup is ambiguous (silently mis-attributing logical types like variant/geometry/geography).
159+
**Rule:** Join result-set columns to Arrow schema fields by positional index, not by column name — duplicate aliases make name lookups ambiguous; keep name as a fallback only for malformed/short schemas and document the one-for-one ordering invariant the position pairing assumes.
160+
155161
--- *Add new entries above this line (oldest→newest); newest sections sort to the bottom.* ---

0 commit comments

Comments
 (0)