Repository navigation
GH-53500: [Python][Parquet] Prevent crashes when comparing metadata with None - #55168
nleerollergh wants to merge 1 commit into
Conversation
…data with None Require non-null typed arguments in Statistics, ColumnChunkMetaData, RowGroupMetaData, ParquetSchema and ColumnSchema equals(), following the existing FileMetaData.equals() pattern, and add a regression test. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
|
Why the call crashes In Cython, a parameter declared as Statistics other is checked to be a Statistics instance, but Why With |
|
Let me know if you have any questions team! Hoping to get this merged in soon. Thanks! |



Rationale for this change
Five Parquet metadata/schema
equals()wrappers (Statistics,ColumnChunkMetaData,RowGroupMetaData,ParquetSchema,ColumnSchema) acceptNonefor their typedotherargument and then dereference a native pointer on it, which can crash the interpreter. This follows the existingFileMetaData.equals()pattern, which already declaresother not None.What changes are included in this PR?
equals()methods.__eq__already catches the resultingTypeErrorand returnsNotImplemented, soobj == NoneisFalseandobj != NoneisTrue.==,!=and direct.equals(None)for all five wrappers.Are these changes tested?
Yes. I added a regression test and ran it locally: it passes with the fix and segfaults without it (details in the comment below).
flake8andcython-lintpass on the changed files.See comment for detailed testing evidence.
Are there any user-facing changes?
obj.equals(None)now raisesTypeErrorinstead of risking a crash.obj == NonereturnsFalse.This PR contains a "Critical Fix". It fixes a bug that causes a crash (category c): calling
equals(None), or comparing with==/!=againstNone, onStatistics,ColumnChunkMetaData,RowGroupMetaData,ParquetSchemaorColumnSchemacould dereference a null native pointer and crash the Python interpreter instead of raising an error.Was AI used for this PR?
Yes, I wrote the initial implementation (the
not Nonechange to the fiveequals()methods, following the existingFileMetaData.equals()pattern) and a first version of the regression test. I then used AI (Claude Code) to review and refine it before then structuring the commit message. The AI completed the test (added the!=and.equals(None)TypeErrorassertions and renamed a variable that shadowed a builtin) and drafted this PR description.PR code and description written by:
Reviewed before submission by: