zhangshenghang commented on PR #10331:
URL: https://github.com/apache/seatunnel/pull/10331#issuecomment-3754478252

   <!-- code-pr-reviewer -->
   This PR has several blocking issues that must be addressed:
   
   **[CRITICAL] Partition column types lost**
   `HiveSourceConfig.java:420` - `appendPartitionColumnsAsString()` hardcodes 
all partition columns as `BasicType.STRING_TYPE`, ignoring original types from 
Hive Metastore. This breaks type correctness and may cause Sink type conversion 
failures at runtime.
   
   **[CRITICAL] Missing test coverage**
   - `HiveIT.java` - PR description mentions ORC tables but E2E tests only 
cover TEXT and PARQUET formats
   - `HiveIT.java:108-131`, `HiveSourceConfigEmptyFilesTest.java` - No tests 
for partitioned tables with empty partitions, despite PR claiming to fix this 
scenario
   
   **[MAJOR] Unsafe error handling**
   `HiveSourceConfig.java:220-234` - `parseFilePaths()` catches all 
IOExceptions as "directory not found", silently treating permission errors and 
other I/O failures as empty directories.
   
   **[MAJOR] Missing null checks**
   `HiveSourceConfig.java:365` - `buildRowTypeFromHiveMeta()` calls 
`table.getSd().getCols()` without null checks. If StorageDescriptor is null, 
NPE will cause connector initialization failure.
   
   `HiveSourceConfig.java:368-372` - Loop over `cols` doesn't check if 
FieldSchema objects or their methods return null, risking NPE with incomplete 
metadata.
   
   These issues should be fixed before merging to ensure correctness and test 
coverage for the new empty table fallback behavior.


-- 
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]

Reply via email to