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]

Reply via email to