Repository navigation
Fix README.md image - #1
Merged
Merged
Conversation
jghoman
added a commit
that referenced
this pull request
Mar 21, 2026
pa.concat_tables(pending) raises ArrowInvalid when Arrow tables from different convert() batches have different schemas (different key sets). Add promote_options="default" to handle missing columns by filling with nulls. Applied to both the main flush path and the final flush on shutdown.
8 tasks done
jghoman
added a commit
that referenced
this pull request
Mar 21, 2026
…view (#4) * Fix concat_tables crash on heterogeneous schemas (#1) pa.concat_tables(pending) raises ArrowInvalid when Arrow tables from different convert() batches have different schemas (different key sets). Add promote_options="default" to handle missing columns by filling with nulls. Applied to both the main flush path and the final flush on shutdown. * Fix JDBC prefix in k8s manifest that breaks urlparse (#2) DUCKLAKE_METADATA_URL was set to "jdbc:postgresql://..." but ducklake.py parses it with Python's urlparse(), which doesn't understand JDBC URLs. With the jdbc: prefix, scheme becomes 'jdbc' and hostname/port/path are mangled. The docker-compose already uses the correct postgresql:// form. * Fix SQL injection via env vars in DuckDB SET statements (#3) S3 config values from environment variables were interpolated directly into SET SQL statements without validation. Add whitelist regex validation via _sanitize_setting_value() that rejects values containing quotes, semicolons, or other dangerous characters. * Validate table name against safe identifier regex at config load (#4) DUCKLAKE_TABLE was interpolated directly into SQL in ducklake.py and schema.py without validation. Add _SAFE_TABLE_NAME regex check at config load time to reject table names with unsafe characters. * Improve arrow converter: preserve int precision, optimize schema inference (#5/#7/#8/#9) - Cast integers to int64 (not float64) to preserve precision for values > 2^53 like Snowflake IDs and nanosecond timestamps. Floats still go to float64. Cross-batch type differences handled by promote_options. - Single-pass _build_schema: collect all keys and first non-null samples in one loop instead of O(keys x records) nested scan. - Use pa.array([sample]).type for type inference instead of creating a pa.Table per field. - Build all columns in one pass instead of N table.set_column() calls. * Normalize DuckDB type names to prevent schema evolution thrash (#6) DuckDB's information_schema returns "TIMESTAMP WITH TIME ZONE" while our _ARROW_TO_DUCKDB mapping uses "TIMESTAMPTZ". This mismatch caused spurious ALTER TABLE SET DATA TYPE on every flush. Add normalization when loading schema from information_schema. * Cache _ensure_table to avoid CREATE TABLE IF NOT EXISTS every flush (#10) After the first successful call, _ensure_table is a wasted metadata-DB round-trip. Cache table names in a module-level set and skip SQL on subsequent calls. * Add write retry with backoff, move lag sampling to periodic (#11/#12) - Add _write_with_retry(): 3 attempts with exponential backoff (1s, 2s) to survive transient S3/Postgres errors without killing the pod. - Move watermark offset queries from every-flush to every-60s to avoid blocking the main loop. For 512 partitions, this reduces worst-case blocking from ~42 minutes to a single 60s-gated sample. - Split committed offset tracking (always, cheap) from watermark queries (periodic, network RPC). * Fix health check for idle topics, add readiness/liveness separation (#14/#15) - Health check required recent flush, but idle topics never flush. After max_flush_age_s (600s) the pod went unhealthy and K8s killed it. Now only check flush recency if at least one flush has occurred. - Split is_healthy() into is_alive() (liveness) and is_ready() (readiness). Liveness: started + recently polling. Readiness: alive + flush ok. - Add /readyz endpoint. K8s livenessProbe uses /healthz (is_alive), readinessProbe uses /readyz (is_ready). * Fix ArrowInvalid crash on nested structs with null inner fields When _build_schema's first non-null sample for a dict field contained null inner values (e.g. {"referrer": null, "screen_width": 1920}), pa.array() inferred a null-typed struct field. Later records with actual string values for that field then crashed with ArrowInvalid. Fix: when collecting samples for type inference, prefer dict samples where all inner values are non-null. Falls back to the first non-null sample if no fully-populated sample exists. Found via docker-compose integration testing — pod 0 crashed on partitions where the first consumed batch had null inner struct fields. * Fix DuckLake ATTACH to use Postgres for metadata storage The ATTACH string was 'ducklake:host=... dbname=...' which causes DuckLake to use a local file for metadata instead of Postgres. The correct syntax requires a 'postgres:' prefix in the connection string: 'ducklake:postgres:host=... dbname=...'. Without this, metadata was ephemeral to each pod's in-memory DuckDB connection. Ref: https://ducklake.select/docs/stable/duckdb/usage/connecting
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
.claude/settings.local.jsonfrom tracking and add to.gitignoreTest plan