CodiumAI-Agent commented on PR #9029: URL: https://github.com/apache/incubator-gluten/pull/9029#issuecomment-2729314615
## PR Code Suggestions ✨ <!-- 38c9c76 --> <table><thead><tr><td><strong>Category</strong></td><td align=left><strong>Suggestion </strong></td><td align=center><strong>Impact</strong></td></tr><tbody><tr><td rowspan=1>Possible issue</td> <td> <details><summary>Handle JSON parse error</summary> ___ **Replace the assertion with proper error handling for JSON parse failures.** [cpp-ch/local-engine/Storages/SubstraitSource/Delta/DeltaReader.cpp [54-56]](https://github.com/apache/incubator-gluten/pull/9029/files#diff-36f06b08ace618c32827d19432436597006f12fadf5db2e7cdc689903f405e71R54-R56) ```diff rapidjson::Document doc; doc.Parse(row_index_ids_encoded.c_str()); -assert(!doc.HasParseError()); +if (doc.HasParseError()) + throw DB::Exception(DB::ErrorCodes::BAD_ARGUMENTS, "Invalid JSON in row_index_ids_encoded"); ``` <details><summary>Suggestion importance[1-10]: 8</summary> __ Why: Replacing the assertion with proper error handling improves robustness by providing a clear exception on JSON parse failures, though error handling suggestions are capped at 8. </details></details></td><td align=center>Medium </td></tr><tr><td rowspan=1>General</td> <td> <details><summary>Remove duplicate drop table</summary> ___ **Remove the duplicate DROP TABLE statement to avoid redundant operations.** [backends-clickhouse/src-delta-32/test/scala/org/apache/spark/gluten/delta/GlutenDeltaParquetDeletionVectorSuite.scala [233-238]](https://github.com/apache/incubator-gluten/pull/9029/files#diff-e4849973756cd63c7ef50ccba8d828a6c16e24d448a381fd3299212979647c71R233-R238) ```diff -spark.sql(s""" - |DROP TABLE IF EXISTS lineitem_delta_parquet_upsert_dv; - |""".stripMargin) spark.sql(s""" |DROP TABLE IF EXISTS lineitem_delta_parquet_upsert_dv; |""".stripMargin) ``` <details><summary>Suggestion importance[1-10]: 5</summary> __ Why: Removing the duplicate DROP TABLE statement cleans up redundant operations without impacting functionality, offering a moderate improvement. </details></details></td><td align=center>Low </td></tr></tr></tbody></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]
