codeant-ai-for-open-source[bot] commented on PR #35962: URL: https://github.com/apache/superset/pull/35962#issuecomment-3608341522
## Nitpicks 🔍 <table> <tr><td>🔒 <strong>No security issues identified</strong></td></tr> <tr><td>⚡ <strong>Recommended areas for review</strong><br><br> - [ ] <a href='https://github.com/apache/superset/pull/35962/files#diff-0e7cc668906695942fd108bd1f7bc127cc630e6bd7d26e17b657c3d5468c8414R181-R183'><strong>Column Type Loss</strong></a><br>When creating zero-length arrays with `pa.null()` the resulting pyarrow columns have a null type. This can lose the original column data type information and cause downstream consumers to misinterpret column types or fail when they expect typed arrays.<br> - [ ] <a href='https://github.com/apache/superset/pull/35962/files#diff-0e7cc668906695942fd108bd1f7bc127cc630e6bd7d26e17b657c3d5468c8414R181-R183'><strong>Schema/Type Inference</strong></a><br>The added fallback only creates null-typed arrays and does not use cursor description metadata to construct a typed schema. For empty result sets, consider using cursor description and engine column specs to build a proper pa.Schema or zero-length arrays with appropriate pa.DataType to preserve types.<br> - [ ] <a href='https://github.com/apache/superset/pull/35962/files#diff-0e7cc668906695942fd108bd1f7bc127cc630e6bd7d26e17b657c3d5468c8414R181-R185'><strong>Downstream conversion risk</strong></a><br>Creating null-typed columns can change behavior of DataFrame conversion or downstream code that expects concrete types (e.g., temporal or numeric transformations). Validate conversion paths like `to_pandas` and any consumers that rely on non-null types.<br> - [ ] <a href='https://github.com/apache/superset/pull/35962/files#diff-971e3df21b39e53ffb69270dcb26e2af3d3d8b88084e559c2c240cbd20ef066bR224-R231'><strong>Schema type expectations</strong></a><br>The test checks that column names and counts are preserved but doesn't assert the PyArrow field types are the expected null placeholders. It would be useful to assert schema field types to ensure zero-length null arrays were created and metadata survives empty result sets.<br> - [ ] <a href='https://github.com/apache/superset/pull/35962/files#diff-971e3df21b39e53ffb69270dcb26e2af3d3d8b88084e559c2c240cbd20ef066bR199-R222'><strong>Fragile type assertions</strong></a><br>The test asserts exact uppercase type strings (e.g. "INT", "VARCHAR", "TIMESTAMP"). The mapping returned by BaseEngineSpec.get_datatype may vary in case or may return None/other representations depending on engine implementations or future changes, making the test brittle.<br> </td></tr> </table> -- 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]
