rusackas commented on PR #35962: URL: https://github.com/apache/superset/pull/35962#issuecomment-4910185555
Thanks @anthonyhungnguyen. I went digging on this one and I think the code hunk might be a no-op on current master... the `if not pa_data and column_names:` branch looks unreachable, since the loop above appends one array per column name (a length-0 null array for empty data), so `pa_data` is only empty when `column_names` is too. I ran your exact test scenario against unpatched master and it already returns the 3 columns with types preserved. So the regression test is really the valuable part here, and it passes without the code change. Do you have a concrete repro on current master where the columns still get dropped? If not, I'd lean toward keeping this as a test-only guard and dropping the hunk. Either way the test is worth having. Thanks! -- 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]
