HyukjinKwon commented on PR #54:
URL:
https://github.com/apache/spark-connect-rust/pull/54#issuecomment-5420244018
<!-- ai-code-review -->
## Re-review at `3559267`
Three more commits since `814cdbd`; verified. No blockers.
- **`3559267` — catalog list ops returned as lazy relations. Correct, and a
real fix.** The previous `execute_catalog_as_dataframe` eagerly executed the
op, errored with "Catalog operation returned no rows" on a legitimately empty
result, and rebuilt a `LocalRelation { data: None }` that dropped the rows and
coerced every column to `String`. The new `LogicalPlan::Catalog { catalog }` →
`RelType::Catalog` is how Spark Connect actually models catalog ops, so the
real server schema/rows flow through `.collect()` and an empty catalog yields
an empty result. Nice: this was caught by `e2e_coverage::catalog_surface` in
the live-server e2e step added for finding B — the coverage earned its keep on
the first run.
- **`b3ab882` — `uuid` + `tuple_sketch_agg_{double,integer}` now emit the
reference's trailing default args** (uuid seed → `RANDOM_SEED_FUNCS`;
`nom_entries=12`, `mode="sum"`), re-added to the golden (508→511,
byte-for-byte). This closes the out-of-scope builder-divergence follow-up I
noted. And `functions::cast(a, b)` is resolved — the golden `("cast", …)` case
passes byte-for-byte against reference (the column-column form returning the
second column is what reference emits), so my earlier "no-op stub" flag was a
false alarm.
- `763275e` — rustfmt only.
That leaves nothing outstanding from any review pass. **LGTM.**
This review was written by Isaac.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]